Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: better error logs for UPDATE attribute list errors
To:
tech@openbsd.org
Date:
Wed, 18 Feb 2026 14:48:32 +0100

Download raw body.

Thread
  • Claudio Jeker:

    bgpd: better error logs for UPDATE attribute list errors

I see ERR_UPD_ATTRLIST notifications on one particular system with many
sessions and it is a pain to find the cause for this.

While most UPDATE errors include the attribute data that caused the
failure and we added ways to dump this data into the log.
ERR_UPD_ATTRLIST does not and so it is hard to track down why this error
happens. Lets add some extra logging to better track this.

-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.685 rde.c
--- rde.c	17 Feb 2026 10:51:43 -0000	1.685
+++ rde.c	18 Feb 2026 13:44:01 -0000
@@ -2039,27 +2039,27 @@ rde_attr_parse(struct ibuf *buf, struct 
 	ibuf_from_ibuf(&attrbuf, buf);
 	if (ibuf_get_n8(&attrbuf, &flags) == -1 ||
 	    ibuf_get_n8(&attrbuf, &type) == -1)
-		goto bad_list;
+		goto bad_ibuf;
 
 	if (flags & ATTR_EXTLEN) {
 		uint16_t attr_len;
 		if (ibuf_get_n16(&attrbuf, &attr_len) == -1)
-			goto bad_list;
+			goto bad_ibuf;
 		alen = attr_len;
 		hlen = 4;
 	} else {
 		uint8_t attr_len;
 		if (ibuf_get_n8(&attrbuf, &attr_len) == -1)
-			goto bad_list;
+			goto bad_ibuf;
 		alen = attr_len;
 		hlen = 3;
 	}
 
 	if (ibuf_truncate(&attrbuf, alen) == -1)
-		goto bad_list;
+		goto bad_ibuf;
 	/* consume the attribute in buf before moving forward */
 	if (ibuf_skip(buf, hlen + alen) == -1)
-		goto bad_list;
+		goto bad_ibuf;
 
 	switch (type) {
 	case ATTR_UNDEF:
@@ -2135,7 +2135,6 @@ rde_attr_parse(struct ibuf *buf, struct 
 			goto bad_len;
 		if (a->flags & F_ATTR_NEXTHOP)
 			goto bad_list;
-		a->flags |= F_ATTR_NEXTHOP;
 
 		memset(&nexthop, 0, sizeof(nexthop));
 		nexthop.aid = AID_INET;
@@ -2151,6 +2150,7 @@ rde_attr_parse(struct ibuf *buf, struct 
 			    &attrbuf);
 			return (-1);
 		}
+		a->flags |= F_ATTR_NEXTHOP;
 		nexthop_unref(state->nexthop);	/* just to be sure */
 		state->nexthop = nexthop_get(&nexthop);
 		break;
@@ -2205,7 +2205,7 @@ rde_attr_parse(struct ibuf *buf, struct 
 			u_char	t[8];
 			t[0] = t[1] = 0;
 			if (ibuf_get(&attrbuf, &t[2], 6) == -1)
-				goto bad_list;
+				goto bad_ibuf;
 			if (memcmp(t, &zero, sizeof(uint32_t)) == 0) {
 				/* As per RFC7606 use "attribute discard". */
 				log_peer_warnx(&peer->conf, "bad AGGREGATOR, "
@@ -2407,6 +2407,11 @@ rde_attr_parse(struct ibuf *buf, struct 
 	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRFLAGS, &attrbuf);
 	return (-1);
  bad_list:
+	log_peer_warnx(&peer->conf, "bad path, list error for type %d", type);
+	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL);
+	return (-1);
+ bad_ibuf:
+	log_peer_warn(&peer->conf, "bad path, header parse error");
 	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL);
 	return (-1);
 }