Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: rework the interface to rde_generate_updates
To:
tech@openbsd.org
Date:
Mon, 1 Dec 2025 11:26:54 +0100

Download raw body.

Thread
Rework the interface to rde_generate_updates() and up_genrate_XYZ()
to pass the old prefix as just the path_id_tx identifier.

Only up_generate_addpath_all() actually uses this information and there
this is enough to find the affected prefix in the adj-rib-out.
Also adjust the order of operation in up_generate_addpath_all() so the
prefix is not first removed and readded for the case where a prefix
is simply updated.

Rework the code in prefix_evaluate_nexthop() to be much more like
prefix_evaluate(), it should be possible to factor out common code at
some point.

In peer_add() ensure that path_id_tx can't be 0. Since 0 has now a special
meaning.

This is done so we can introduce a queue around rde_generate_updates() and
break up the big pipeline of input processing incl. decision process and
output processing.

-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.669 rde.c
--- rde.c	20 Nov 2025 14:04:36 -0000	1.669
+++ rde.c	1 Dec 2025 10:10:30 -0000
@@ -4223,7 +4223,7 @@ rde_softreconfig_out(struct rib_entry *r
 		/* no valid path for prefix */
 		return;
 
-	rde_generate_updates(re, NULL, NULL, EVAL_RECONF);
+	rde_generate_updates(re, NULL, 0, EVAL_RECONF);
 }
 
 static void
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.324 rde.h
--- rde.h	20 Nov 2025 14:04:36 -0000	1.324
+++ rde.h	1 Dec 2025 10:10:30 -0000
@@ -394,7 +394,7 @@ struct filter_head	*peer_apply_out_filte
 			    struct filter_head *);
 
 void		 rde_generate_updates(struct rib_entry *, struct prefix *,
-		    struct prefix *, enum eval_mode);
+		    uint32_t, enum eval_mode);
 
 void		 peer_up(struct rde_peer *, struct session_up *);
 void		 peer_down(struct rde_peer *);
@@ -774,7 +774,7 @@ prefix_adjout_nexthop(struct prefix_adjo
 void		 up_generate_updates(struct rde_peer *, struct rib_entry *);
 void		 up_generate_addpath(struct rde_peer *, struct rib_entry *);
 void		 up_generate_addpath_all(struct rde_peer *, struct rib_entry *,
-		    struct prefix *, struct prefix *);
+		    struct prefix *, uint32_t);
 void		 up_generate_default(struct rde_peer *, uint8_t);
 int		 up_is_eor(struct rde_peer *, uint8_t);
 void		 up_dump_withdraws(struct imsgbuf *, struct rde_peer *,
Index: rde_decide.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_decide.c,v
diff -u -p -r1.105 rde_decide.c
--- rde_decide.c	20 Nov 2025 10:47:36 -0000	1.105
+++ rde_decide.c	1 Dec 2025 10:10:30 -0000
@@ -533,6 +533,7 @@ prefix_evaluate(struct rib_entry *re, st
 {
 	struct prefix	*newbest, *oldbest;
 	struct rib	*rib;
+	uint32_t	 old_pathid_tx = 0;
 
 	rib = re_rib(re);
 	if (rib->flags & F_RIB_NOEVALUATE) {
@@ -547,8 +548,10 @@ prefix_evaluate(struct rib_entry *re, st
 	}
 
 	oldbest = prefix_best(re);
-	if (old != NULL)
+	if (old != NULL) {
 		prefix_remove(old, re);
+		old_pathid_tx = old->path_id_tx;
+	}
 	if (new != NULL)
 		prefix_insert(new, NULL, re);
 	newbest = prefix_best(re);
@@ -565,7 +568,7 @@ prefix_evaluate(struct rib_entry *re, st
 		 */
 		if ((rib->flags & F_RIB_NOFIB) == 0)
 			rde_send_kroute(rib, newbest, oldbest);
-		rde_generate_updates(re, new, old, EVAL_DEFAULT);
+		rde_generate_updates(re, new, old_pathid_tx, EVAL_DEFAULT);
 		return;
 	}
 
@@ -578,7 +581,7 @@ prefix_evaluate(struct rib_entry *re, st
 		if (new != NULL && !prefix_eligible(new))
 			new = NULL;
 		if (new != NULL || old != NULL)
-			rde_generate_updates(re, new, old, EVAL_ALL);
+			rde_generate_updates(re, new, old_pathid_tx, EVAL_ALL);
 	}
 }
 
@@ -589,6 +592,7 @@ prefix_evaluate_nexthop(struct prefix *p
 	struct rib_entry *re = prefix_re(p);
 	struct prefix	*newbest, *oldbest, *new, *old;
 	struct rib	*rib;
+	uint32_t	 old_pathid_tx;
 
 	/* Skip non local-RIBs or RIBs that are flagged as noeval. */
 	rib = re_rib(re);
@@ -617,26 +621,26 @@ prefix_evaluate_nexthop(struct prefix *p
 	 * Re-evaluate the prefix by removing the prefix then updating the
 	 * nexthop state and reinserting the prefix again.
 	 */
-	old = p;
 	oldbest = prefix_best(re);
-	prefix_remove(p, re);
+
+	old = p;
+	prefix_remove(old, re);
+	old_pathid_tx = old->path_id_tx;
 
 	if (state == NEXTHOP_REACH)
 		p->nhflags |= NEXTHOP_VALID;
 	else
 		p->nhflags &= ~NEXTHOP_VALID;
 
-	prefix_insert(p, NULL, re);
-	newbest = prefix_best(re);
 	new = p;
-	if (!prefix_eligible(new))
-		new = NULL;
+	prefix_insert(new, NULL, re);
+	newbest = prefix_best(re);
 
 	/*
 	 * If the active prefix changed or the active prefix was removed
 	 * and added again then generate an update.
 	 */
-	if (oldbest != newbest || newbest == p) {
+	if (oldbest != newbest || newbest == old) {
 		/*
 		 * Send update withdrawing oldbest and adding newbest
 		 * but remember that newbest may be NULL aka ineligible.
@@ -644,7 +648,7 @@ prefix_evaluate_nexthop(struct prefix *p
 		 */
 		if ((rib->flags & F_RIB_NOFIB) == 0)
 			rde_send_kroute(rib, newbest, oldbest);
-		rde_generate_updates(re, new, old, EVAL_DEFAULT);
+		rde_generate_updates(re, new, old_pathid_tx, EVAL_DEFAULT);
 		return;
 	}
 
@@ -653,6 +657,9 @@ prefix_evaluate_nexthop(struct prefix *p
 	 * to be passed on (not only a change of the best prefix).
 	 * rde_generate_updates() will then take care of distribution.
 	 */
-	if (rde_evaluate_all())
-		rde_generate_updates(re, new, old, EVAL_ALL);
+	if (rde_evaluate_all()) {
+		if (!prefix_eligible(new))
+			new = NULL;
+		rde_generate_updates(re, new, old_pathid_tx, EVAL_ALL);
+	}
 }
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.56 rde_peer.c
--- rde_peer.c	20 Nov 2025 13:46:22 -0000	1.56
+++ rde_peer.c	1 Dec 2025 10:10:30 -0000
@@ -187,7 +187,8 @@ peer_add(uint32_t id, struct peer_config
 		conflict = 0;
 		peer->path_id_tx = arc4random() << 1;
 		RB_FOREACH(p, peer_tree, &peertable) {
-			if (p->path_id_tx == peer->path_id_tx) {
+			if (peer->path_id_tx == 0 ||
+			    p->path_id_tx == peer->path_id_tx) {
 				conflict = 1;
 				break;
 			}
@@ -240,8 +241,7 @@ RB_GENERATE(peer_tree, rde_peer, entry, 
 
 static void
 peer_generate_update(struct rde_peer *peer, struct rib_entry *re,
-    struct prefix *newpath, struct prefix *oldpath,
-    enum eval_mode mode)
+    struct prefix *newpath, uint32_t old_pathid_tx, enum eval_mode mode)
 {
 	uint8_t		 aid;
 
@@ -271,7 +271,8 @@ peer_generate_update(struct rde_peer *pe
 	/* handle peers with add-path */
 	if (peer_has_add_path(peer, aid, CAPA_AP_SEND)) {
 		if (peer->eval.mode == ADDPATH_EVAL_ALL)
-			up_generate_addpath_all(peer, re, newpath, oldpath);
+			up_generate_addpath_all(peer, re, newpath,
+			    old_pathid_tx);
 		else
 			up_generate_addpath(peer, re);
 		return;
@@ -285,12 +286,12 @@ peer_generate_update(struct rde_peer *pe
 
 void
 rde_generate_updates(struct rib_entry *re, struct prefix *newpath,
-    struct prefix *oldpath, enum eval_mode mode)
+    uint32_t old_pathid_tx, enum eval_mode mode)
 {
 	struct rde_peer	*peer;
 
 	RB_FOREACH(peer, peer_tree, &peertable)
-		peer_generate_update(peer, re, newpath, oldpath, mode);
+		peer_generate_update(peer, re, newpath, old_pathid_tx, mode);
 }
 
 /*
@@ -573,7 +574,7 @@ peer_dump_upcall(struct rib_entry *re, v
 		/* no eligible prefix, not even for 'evaluate all' */
 		return;
 
-	peer_generate_update(peer, re, NULL, NULL, 0);
+	peer_generate_update(peer, re, NULL, 0, EVAL_DEFAULT);
 }
 
 static void
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
diff -u -p -r1.281 rde_rib.c
--- rde_rib.c	20 Nov 2025 13:46:22 -0000	1.281
+++ rde_rib.c	1 Dec 2025 10:10:30 -0000
@@ -983,6 +983,7 @@ prefix_flowspec_update(struct rde_peer *
 	struct rde_community *comm, *ncomm;
 	struct rib_entry *re;
 	struct prefix *new, *old;
+	uint32_t old_pathid_tx = 0;
 
 	re = rib_get(&flowrib, pte);
 	if (re == NULL)
@@ -1004,7 +1005,9 @@ prefix_flowspec_update(struct rde_peer *
 	    NULL, 0, 0);
 	TAILQ_INSERT_HEAD(&re->prefix_h, new, rib_l);
 
-	rde_generate_updates(re, new, old, EVAL_DEFAULT);
+	if (old != NULL)
+		old_pathid_tx = old->path_id_tx;
+	rde_generate_updates(re, new, old_pathid_tx, EVAL_DEFAULT);
 
 	if (old != NULL) {
 		TAILQ_REMOVE(&re->prefix_h, old, rib_l);
@@ -1030,7 +1033,7 @@ prefix_flowspec_withdraw(struct rde_peer
 	p = prefix_bypeer(re, peer, 0);
 	if (p == NULL)
 		return 0;
-	rde_generate_updates(re, NULL, p, EVAL_DEFAULT);
+	rde_generate_updates(re, NULL, p->path_id_tx, EVAL_DEFAULT);
 	TAILQ_REMOVE(&re->prefix_h, p, rib_l);
 	prefix_unlink(p);
 	prefix_free(p);
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
diff -u -p -r1.183 rde_update.c
--- rde_update.c	20 Nov 2025 14:04:36 -0000	1.183
+++ rde_update.c	1 Dec 2025 10:10:30 -0000
@@ -344,7 +344,7 @@ up_generate_addpath(struct rde_peer *pee
  */
 void
 up_generate_addpath_all(struct rde_peer *peer, struct rib_entry *re,
-    struct prefix *new, struct prefix *old)
+    struct prefix *new, uint32_t old_pathid_tx)
 {
 	struct prefix_adjout	*p;
 
@@ -352,7 +352,7 @@ up_generate_addpath_all(struct rde_peer 
 	 * If old and new are NULL then re-insert all prefixes from re,
 	 * use up_generate_addpath() for that.
 	 */
-	if (old == NULL && new == NULL) {
+	if (old_pathid_tx == 0 && new == NULL) {
 		up_generate_addpath(peer, re);
 		return;
 	}
@@ -362,17 +362,14 @@ up_generate_addpath_all(struct rde_peer 
 		new = NULL;
 	}
 
-	if (old != NULL) {
-		/* withdraw old path */
-		p = prefix_adjout_get(peer, old->path_id_tx, old->pt);
-		if (p != NULL)
-			prefix_adjout_withdraw(p);
-	}
-
 	if (new != NULL) {
 		/* add new path */
 		switch (up_process_prefix(peer, new, (void *)-1)) {
 		case UP_OK:
+			/* don't remove old if an existing prefix was updated */
+			if (old_pathid_tx == new->path_id_tx)
+				old_pathid_tx = 0;
+			break;
 		case UP_FILTERED:
 		case UP_EXCLUDED:
 			break;
@@ -380,6 +377,13 @@ up_generate_addpath_all(struct rde_peer 
 			/* just give up */
 			return;
 		}
+	}
+
+	if (old_pathid_tx != 0) {
+		/* withdraw old path */
+		p = prefix_adjout_get(peer, old_pathid_tx, re->prefix);
+		if (p != NULL)
+			prefix_adjout_withdraw(p);
 	}
 }