From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: rework the force_update logic in adjout
To: tech@openbsd.org
Date: Wed, 15 Jul 2026 12:02:39 +0200

On initial connect the system needs to sync the adj-rib-out of a peer with
the Loc-RIB. This happens via peer_dump() and in this case the system
needs to push out all updates (but can drop any withdraw).

The other user of peer_dump() is during config reloads where the RIB of a
peer is altered. In that case the adj-rib-out is first flushed
(withdrawing all paths) and then refilled via peer_dump(). Again in that
case all updates should be pushed out and there should be no withdraw at
all since the table was just flushed.

To do this a force_update flag was introduced but actually it is better to
use the mode and introduce EVAL_SYNC for this case.
If mode == EVAL_SYNC adjout_prefix_update needs to force the update and
adjout_prefix_withdraw can drop the withdraw.

While there also remove the NOTYET bits in peer_generate_update() and
replace them with a simple hack inside up_generate_addpath_all()
to just call up_generate_addpath().

This is another stepping stone to bring back the add-path send all
optimisation.
-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.708 rde.c
--- rde.c	1 Jul 2026 09:53:47 -0000	1.708
+++ rde.c	15 Jul 2026 09:43:03 -0000
@@ -3671,7 +3671,7 @@ rde_up_flush_upcall(struct pt_entry *pte
 {
 	struct rde_peer *peer = ptr;
 
-	adjout_prefix_withdraw(peer, pte, p);
+	adjout_prefix_withdraw(peer, pte, p, 0);
 }
 
 int
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.357 rde.h
--- rde.h	13 Jul 2026 12:27:34 -0000	1.357
+++ rde.h	15 Jul 2026 09:32:29 -0000
@@ -359,6 +359,7 @@ struct filterstate {
 
 enum eval_mode {
 	EVAL_NONE,
+	EVAL_SYNC,
 	EVAL_REEVAL,
 	EVAL_DEFAULT,
 	EVAL_ALL,
@@ -777,7 +778,7 @@ struct adjout_prefix	*adjout_prefix_next
 void		 adjout_prefix_update(struct adjout_prefix *, struct rde_peer *,
 		    struct filterstate *, struct pt_entry *, uint32_t, int);
 void		 adjout_prefix_withdraw(struct rde_peer *, struct pt_entry *,
-		    struct adjout_prefix *);
+		    struct adjout_prefix *, int);
 void		 adjout_prefix_reaper(struct rde_peer *);
 void		 adjout_prefix_dump_cleanup(struct rib_context *);
 void		 adjout_prefix_dump_r(struct rib_context *);
@@ -808,9 +809,12 @@ void		 pend_prefix_stats(struct ch_stats
 void		 adjout_attr_stats(struct ch_stats *);
 
 /* rde_update.c */
-void	 up_generate_updates(struct rde_peer *, struct rib_entry *, int);
-void	 up_generate_addpath(struct rde_peer *, struct rib_entry *, int);
-void	 up_generate_addpath_all(struct rde_peer *, struct rib_entry *, int);
+void	 up_generate_updates(struct rde_peer *, struct rib_entry *,
+	    enum eval_mode);
+void	 up_generate_addpath(struct rde_peer *, struct rib_entry *,
+	    enum eval_mode);
+void	 up_generate_addpath_all(struct rde_peer *, struct rib_entry *,
+	    enum eval_mode);
 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 *, uint8_t);
Index: rde_adjout.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
diff -u -p -r1.21 rde_adjout.c
--- rde_adjout.c	13 Jul 2026 12:27:34 -0000	1.21
+++ rde_adjout.c	15 Jul 2026 09:32:29 -0000
@@ -516,11 +516,12 @@ adjout_prefix_next(struct pt_entry *pte,
 
 /*
  * Put a prefix from the Adj-RIB-Out onto the update queue.
+ * If force_send is set, ensure an UPDATE is sent in any case to the peer.
  */
 void
 adjout_prefix_update(struct adjout_prefix *p, struct rde_peer *peer,
     struct filterstate *state, struct pt_entry *pte, uint32_t path_id_tx,
-    int force_update)
+    int force_send)
 {
 	struct adjout_attr *attrs;
 
@@ -542,7 +543,7 @@ adjout_prefix_update(struct adjout_prefi
 		    attrs->communities) &&
 		    path_equal(&state->aspath, attrs->aspath)) {
 			/* nothing changed */
-			if (force_update && peer_is_up(peer))
+			if (force_send && peer_is_up(peer))
 				pend_prefix_add(peer, attrs, pte, path_id_tx);
 			return;
 		}
@@ -563,15 +564,16 @@ adjout_prefix_update(struct adjout_prefi
 /*
  * Withdraw a prefix from the Adj-RIB-Out, this unlinks the aspath but leaves
  * the prefix in the RIB linked to the peer withdraw list.
+ * If skip_send is set there is no need to send a withdraw to the peer.
  */
 void
 adjout_prefix_withdraw(struct rde_peer *peer, struct pt_entry *pte,
-    struct adjout_prefix *p)
+    struct adjout_prefix *p, int skip_send)
 {
 	if (bitmap_test(&p->peermap, peer->adjout_bid) == 0)
 		fatalx("%s: king bula is unhappy", __func__);
 
-	if (peer_is_up(peer))
+	if (peer_is_up(peer) && !skip_send)
 		pend_prefix_add(peer, NULL, pte, p->path_id_tx);
 
 	adjout_prefix_unlink(p, pte, peer);
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.79 rde_peer.c
--- rde_peer.c	13 Jul 2026 12:27:34 -0000	1.79
+++ rde_peer.c	15 Jul 2026 09:36:22 -0000
@@ -244,7 +244,7 @@ RB_GENERATE(peer_tree, rde_peer, entry, 
 
 static void
 peer_generate_update(struct rde_peer *peer, struct rib_entry *re,
-    enum eval_mode mode, int force_update)
+    enum eval_mode mode)
 {
 	uint8_t		 aid;
 
@@ -269,26 +269,17 @@ peer_generate_update(struct rde_peer *pe
 
 	/* handle peers with add-path */
 	if (peer_has_add_path(peer, aid, CAPA_AP_SEND)) {
-#if NOTYET
-		/* XXX skip addpath all optimisation for now until the queue
-		 * is extended to allow this mode to work again.
-		 * This mode is not very common and so taking the slow path
-		 * here like for all other addpath send modes is not that
-		 * big of an issue right now.
-		 */
-		if (peer->eval.mode == ADDPATH_EVAL_ALL) {
-			up_generate_addpath_all(peer, re, force_update);
-			return;
-		}
-#endif
-		up_generate_addpath(peer, re, force_update);
+		if (peer->eval.mode == ADDPATH_EVAL_ALL)
+			up_generate_addpath_all(peer, re, mode);
+		else
+			up_generate_addpath(peer, re, mode);
 		return;
 	}
 
 	/* skip regular peers if the best path didn't change */
 	if (mode == EVAL_ALL && (peer->flags & PEERFLAG_EVALUATE_ALL) == 0)
 		return;
-	up_generate_updates(peer, re, force_update);
+	up_generate_updates(peer, re, mode);
 }
 
 /*
@@ -298,8 +289,8 @@ peer_generate_update(struct rde_peer *pe
  *	EVAL_ALL is sent for any other update (needed for peers with
  *	addpath or evaluate all set).
  *	EVAL_REEVAL is used by config reloads (a full RIB refresh is needed)
- *	and for single peer RIB dumps but those call peer_generate_update()
- *	directly.
+ *	EVAL_SYNC is used for single peer RIB dumps but those call
+ *	peer_generate_update() directly.
  * peer, newpath and old_pathid_tx are ignored if mode is EVAL_REEVAL.
  * In the other cases either newpath or old_pathid_tx are valid but not
  * both at the same time.
@@ -317,13 +308,14 @@ rde_enqueue_updates(struct rib_entry *re
 		RB_FOREACH(p, peer_tree, &peertable) {
 			if (p->reconf_out == 0)
 				continue;
-			peer_generate_update(p, re, EVAL_REEVAL, 0);
+			peer_generate_update(p, re, mode);
 		}
 		adjout_prefix_collect(re->prefix);
 		return;
 	case EVAL_DEFAULT:
 	case EVAL_ALL:
 		break;
+	case EVAL_SYNC:
 	case EVAL_NONE:
 		fatalx("bad eval mode in %s", __func__);
 	}
@@ -354,7 +346,7 @@ peer_process_updates(struct rde_peer *pe
 	peer->stats.rib_entry_count--;
 
 	RB_FOREACH(p, peer_tree, &peertable)
-		peer_generate_update(p, re, re->pq_mode, 0);
+		peer_generate_update(p, re, re->pq_mode);
 
 	adjout_prefix_collect(re->prefix);
 	rib_pq_dequeue(re);
@@ -505,7 +497,7 @@ peer_reaper_upcall(struct pt_entry *pte,
 {
 	struct rde_peer		*peer = ptr;
 
-	adjout_prefix_withdraw(peer, pte, p);
+	adjout_prefix_withdraw(peer, pte, p, 1);
 }
 
 /*
@@ -656,7 +648,7 @@ peer_dump_upcall(struct rib_entry *re, v
 		/* no eligible prefix, not even for 'evaluate all' */
 		return;
 
-	peer_generate_update(peer, re, EVAL_REEVAL, 1);
+	peer_generate_update(peer, re, EVAL_SYNC);
 	adjout_prefix_collect(re->prefix);
 }
 
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
diff -u -p -r1.200 rde_update.c
--- rde_update.c	13 Jul 2026 12:27:34 -0000	1.200
+++ rde_update.c	15 Jul 2026 10:00:42 -0000
@@ -159,7 +159,7 @@ up_enforce_open_policy(struct rde_peer *
  */
 static enum up_state
 up_process_prefix(struct rde_peer *peer, struct prefix *new,
-    struct adjout_prefix *p, int force_update)
+    struct adjout_prefix *p, enum eval_mode mode)
 {
 	struct filterstate state;
 	struct bgpd_addr addr;
@@ -202,7 +202,7 @@ up_process_prefix(struct rde_peer *peer,
 
 	up_prep_adjout(peer, &state, new->pt->aid);
 	adjout_prefix_update(p, peer, &state, new->pt, new_path_id_tx,
-	    force_update);
+	    mode == EVAL_SYNC);
 	rde_filterstate_clean(&state);
 
 	/* max prefix checker outbound */
@@ -221,7 +221,7 @@ up_process_prefix(struct rde_peer *peer,
 
 void
 up_generate_updates(struct rde_peer *peer, struct rib_entry *re,
-    int force_update)
+    enum eval_mode mode)
 {
 	struct prefix		*new;
 	struct adjout_prefix	*p;
@@ -230,7 +230,7 @@ up_generate_updates(struct rde_peer *pee
 
 	new = prefix_best(re);
 	while (new != NULL) {
-		switch (up_process_prefix(peer, new, p, force_update)) {
+		switch (up_process_prefix(peer, new, p, mode)) {
 		case UP_OK:
 		case UP_ERR_LIMIT:
 			return;
@@ -249,7 +249,7 @@ up_generate_updates(struct rde_peer *pee
 done:
 	/* withdraw prefix */
 	if (p != NULL)
-		adjout_prefix_withdraw(peer, re->prefix, p);
+		adjout_prefix_withdraw(peer, re->prefix, p, mode == EVAL_SYNC);
 }
 
 /*
@@ -261,7 +261,7 @@ done:
  */
 void
 up_generate_addpath(struct rde_peer *peer, struct rib_entry *re,
-    int force_update)
+    enum eval_mode mode)
 {
 	struct prefix		*new;
 	struct adjout_prefix	*head, *p;
@@ -322,8 +322,7 @@ up_generate_addpath(struct rde_peer *pee
 		if (extra != 0 && extrapaths >= peer->eval.extrapaths)
 			break;
 
-		switch (up_process_prefix(peer, new, (void *)-1,
-		    force_update)) {
+		switch (up_process_prefix(peer, new, (void *)-1, mode)) {
 		case UP_OK:
 			maxpaths++;
 			extrapaths += extra;
@@ -354,7 +353,8 @@ up_generate_addpath(struct rde_peer *pee
 			p = adjout_prefix_get(peer, addpath_prefix_list[i],
 			    re->prefix);
 			if (p != NULL)
-				adjout_prefix_withdraw(peer, re->prefix, p);
+				adjout_prefix_withdraw(peer, re->prefix, p,
+				    mode == EVAL_SYNC);
 		}
 	}
 }
@@ -365,7 +365,7 @@ up_generate_addpath(struct rde_peer *pee
  */
 void
 up_generate_addpath_all(struct rde_peer *peer, struct rib_entry *re,
-    int force_update)
+    enum eval_mode mode)
 {
 #if 0
 	/* XXX needs to be rewritten */
@@ -376,7 +376,7 @@ up_generate_addpath_all(struct rde_peer 
 	 * use up_generate_addpath() for that.
 	 */
 	if (old_pathid_tx == 0 && new == NULL) {
-		up_generate_addpath(peer, re, force_update);
+		up_generate_addpath(peer, re, mode);
 		return;
 	}
 
@@ -387,8 +387,7 @@ up_generate_addpath_all(struct rde_peer 
 
 	if (new != NULL) {
 		/* add new path */
-		switch (up_process_prefix(peer, new, (void *)-1,
-		    force_update)) {
+		switch (up_process_prefix(peer, new, (void *)-1, mode)) {
 		case UP_OK:
 			/* don't remove old if an existing prefix was updated */
 			if (old_pathid_tx == new->path_id_tx)
@@ -407,8 +406,11 @@ up_generate_addpath_all(struct rde_peer 
 		/* withdraw old path */
 		p = adjout_prefix_get(peer, old_pathid_tx, re->prefix);
 		if (p != NULL)
-			adjout_prefix_withdraw(peer, re->prefix, p);
+			adjout_prefix_withdraw(peer, re->prefix, p,
+			    mode == EVAL_SYNC);
 	}
+#else
+	up_generate_addpath(peer, re, mode);
 #endif
 }