Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: fix accounting errors for pending updates
To:
tech@openbsd.org
Date:
Thu, 13 Mar 2025 16:22:58 +0100

Download raw body.

Thread
With the introduction of the adj-rib-out cache the pending update count
could go off (wrap around 0).

There are two issues:
- In peer_blast_upcall() we need to increase pending_update for every
  prefix we insert.
- In prefix_adjout_flush_pending() the EoR marker needs special handling.
  The EoR marker is not counted in pending_update but when unlinking it
  we need to free the marker. It will be inserted again when we blast
  out the table.

With these two diffs the pending updates is now correct in my tests.
The pending withdraws does not have this issue since the accouting already
happens in prefix_adjout_destroy().
-- 
:wq Claudio

Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.47 rde_peer.c
--- rde_peer.c	20 Feb 2025 19:47:31 -0000	1.47
+++ rde_peer.c	12 Mar 2025 13:33:01 -0000
@@ -523,14 +523,18 @@ peer_stale(struct rde_peer *peer, uint8_
 static void
 peer_blast_upcall(struct prefix *p, void *ptr)
 {
+	struct rde_peer		*peer;
+
 	if (p->flags & PREFIX_FLAG_DEAD) {
 		/* ignore dead prefixes, they will go away soon */
 	} else if ((p->flags & PREFIX_FLAG_MASK) == 0) {
+		peer = prefix_peer(p);
 		/* put entries on the update queue if not already on a queue */
 		p->flags |= PREFIX_FLAG_UPDATE;
-		if (RB_INSERT(prefix_tree, &prefix_peer(p)->updates[p->pt->aid],
+		if (RB_INSERT(prefix_tree, &peer->updates[p->pt->aid],
 		    p) != NULL)
 			fatalx("%s: RB tree invariant violated", __func__);
+		peer->stats.pending_update++;
 	}
 }
 
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
diff -u -p -r1.267 rde_rib.c
--- rde_rib.c	14 Jan 2025 12:24:23 -0000	1.267
+++ rde_rib.c	12 Mar 2025 16:21:28 -0000
@@ -1430,7 +1430,11 @@ prefix_adjout_flush_pending(struct rde_p
 		RB_FOREACH_SAFE(p, prefix_tree, &peer->updates[aid], np) {
 			p->flags &= ~PREFIX_FLAG_UPDATE;
 			RB_REMOVE(prefix_tree, &peer->updates[aid], p);
-			peer->stats.pending_update--;
+			if (p->flags & PREFIX_FLAG_EOR) {
+				prefix_adjout_destroy(p);
+			} else {
+				peer->stats.pending_update--;
+			}
 		}
 	}
 }