Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: adjust peer_dump to properly blast updates out
To:
tech@openbsd.org
Date:
Wed, 20 May 2026 17:57:58 +0200

Download raw body.

Thread
I replaced the peer_blast() call in peer_dump_done() with calling
peer_blast_done() instead. The idea was that peer_dump() already does all
the pend_prefix_add() calls and so peer_blast() just redoes all the work.

This is true for most cases but there is the case where peer_up() calls
peer_dump() with a pre-populated (cached) Adj-RIB-Out. In that case it is
possible that peer_dump() via peer_generate_update and up_generate_updates
does not really alter the Adj-RIB-Out entry and in that case
adjout_prefix_update() will simply skip sending an update out.

I don't want to walk the RIB twice for peer_up() calls so instead add a
force_update flag to adjout_prefix_update() and all callers upwards.
This way peer_dump_upcall() can call peer_generate_update() with
force_update set to true which ensures that all prefixes call
pend_prefix_add().

I switched up_generate_default() to always send one update (since it is
just one) so this code can now also use peer_blast_done() directly.

The good thing is the explanation for this is a lot more complicated than
the diff itself.
-- 
:wq Claudio

Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.349 rde.h
--- rde.h	13 May 2026 18:50:09 -0000	1.349
+++ rde.h	20 May 2026 12:26:39 -0000
@@ -773,7 +773,7 @@ struct adjout_prefix	*adjout_prefix_next
 			    struct pt_entry *, struct adjout_prefix *);
 
 void		 adjout_prefix_update(struct adjout_prefix *, struct rde_peer *,
-		    struct filterstate *, struct pt_entry *, uint32_t);
+		    struct filterstate *, struct pt_entry *, uint32_t, int);
 void		 adjout_prefix_withdraw(struct rde_peer *, struct pt_entry *,
 		    struct adjout_prefix *);
 void		 adjout_prefix_reaper(struct rde_peer *);
@@ -805,15 +805,14 @@ 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 *);
-void		 up_generate_addpath(struct rde_peer *, struct rib_entry *);
-void		 up_generate_addpath_all(struct rde_peer *, struct rib_entry *,
-		    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 *,
-		    uint8_t);
-void		 up_dump_update(struct imsgbuf *, struct rde_peer *, uint8_t);
+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 *,
+	    struct prefix *, uint32_t, int);
+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);
+void	 up_dump_update(struct imsgbuf *, struct rde_peer *, uint8_t);
 
 /* rde_aspa.c */
 void		 aspa_validation(struct rde_aspa *, struct aspath *,
Index: rde_adjout.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
diff -u -p -r1.18 rde_adjout.c
--- rde_adjout.c	8 May 2026 12:03:50 -0000	1.18
+++ rde_adjout.c	20 May 2026 10:56:32 -0000
@@ -530,7 +530,8 @@ adjout_prefix_next(struct rde_peer *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)
+    struct filterstate *state, struct pt_entry *pte, uint32_t path_id_tx,
+    int force_update)
 {
 	struct adjout_attr *attrs;
 
@@ -552,6 +553,8 @@ adjout_prefix_update(struct adjout_prefi
 		    attrs->communities) &&
 		    path_equal(&state->aspath, attrs->aspath)) {
 			/* nothing changed */
+			if (force_update && peer_is_up(peer))
+				pend_prefix_add(peer, attrs, pte, path_id_tx);
 			return;
 		}
 
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.72 rde_peer.c
--- rde_peer.c	8 May 2026 12:03:50 -0000	1.72
+++ rde_peer.c	20 May 2026 15:08:42 -0000
@@ -250,7 +250,8 @@ RB_GENERATE(peer_tree, rde_peer, entry, 
 
 static void
 peer_generate_update(struct rde_peer *peer, struct rib_entry *re,
-    struct prefix *newpath, uint32_t old_pathid_tx, enum eval_mode mode)
+    struct prefix *newpath, uint32_t old_pathid_tx, enum eval_mode mode,
+    int force_update)
 {
 	uint8_t		 aid;
 
@@ -284,18 +285,18 @@ peer_generate_update(struct rde_peer *pe
 		 */
 		if (peer->eval.mode == ADDPATH_EVAL_ALL) {
 			up_generate_addpath_all(peer, re, newpath,
-			    old_pathid_tx);
+			    old_pathid_tx, force_update);
 			return;
 		}
 #endif
-		up_generate_addpath(peer, re);
+		up_generate_addpath(peer, re, force_update);
 		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);
+	up_generate_updates(peer, re, force_update);
 }
 
 void
@@ -310,7 +311,7 @@ rde_generate_updates(struct rib_entry *r
 		RB_FOREACH(peer, peer_tree, &peertable) {
 			if (peer->reconf_out == 0)
 				continue;
-			peer_generate_update(peer, re, NULL, 0, EVAL_RECONF);
+			peer_generate_update(peer, re, NULL, 0, EVAL_RECONF, 0);
 		}
 		return;
 	case EVAL_DEFAULT:
@@ -367,7 +368,7 @@ peer_process_updates(struct rde_peer *pe
 	mode = re->pq_mode;
 
 	RB_FOREACH(p, peer_tree, &peertable)
-		peer_generate_update(p, re, NULL, 0, mode);
+		peer_generate_update(p, re, NULL, 0, mode, 0);
 
 	rib_dequeue(re);
 }
@@ -664,7 +665,7 @@ peer_dump_upcall(struct rib_entry *re, v
 		/* no eligible prefix, not even for 'evaluate all' */
 		return;
 
-	peer_generate_update(peer, re, NULL, 0, EVAL_DEFAULT);
+	peer_generate_update(peer, re, NULL, 0, EVAL_DEFAULT, 1);
 }
 
 static void
@@ -688,7 +689,7 @@ peer_dump(struct rde_peer *peer, uint8_t
 		peer_blast(peer, aid);
 	} else if (peer->export_type == EXPORT_DEFAULT_ROUTE) {
 		up_generate_default(peer, aid);
-		peer_blast(peer, aid);
+		peer_blast_done(peer, aid);
 	} else if (aid == AID_FLOWSPECv4 || aid == AID_FLOWSPECv6) {
 		prefix_flowspec_dump(aid, peer, peer_dump_upcall,
 		    peer_dump_done);
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
diff -u -p -r1.195 rde_update.c
--- rde_update.c	19 May 2026 11:25:57 -0000	1.195
+++ rde_update.c	20 May 2026 12:28:32 -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)
+    struct adjout_prefix *p, int force_update)
 {
 	struct filterstate state;
 	struct bgpd_addr addr;
@@ -201,7 +201,8 @@ up_process_prefix(struct rde_peer *peer,
 	}
 
 	up_prep_adjout(peer, &state, new->pt->aid);
-	adjout_prefix_update(p, peer, &state, new->pt, path_id_tx);
+	adjout_prefix_update(p, peer, &state, new->pt, path_id_tx,
+	    force_update);
 	rde_filterstate_clean(&state);
 
 	/* max prefix checker outbound */
@@ -219,7 +220,8 @@ up_process_prefix(struct rde_peer *peer,
 }
 
 void
-up_generate_updates(struct rde_peer *peer, struct rib_entry *re)
+up_generate_updates(struct rde_peer *peer, struct rib_entry *re,
+    int force_update)
 {
 	struct prefix		*new;
 	struct adjout_prefix	*p;
@@ -228,7 +230,7 @@ up_generate_updates(struct rde_peer *pee
 
 	new = prefix_best(re);
 	while (new != NULL) {
-		switch (up_process_prefix(peer, new, p)) {
+		switch (up_process_prefix(peer, new, p, force_update)) {
 		case UP_OK:
 		case UP_ERR_LIMIT:
 			return;
@@ -258,7 +260,8 @@ done:
  * less churn is needed.
  */
 void
-up_generate_addpath(struct rde_peer *peer, struct rib_entry *re)
+up_generate_addpath(struct rde_peer *peer, struct rib_entry *re,
+    int force_update)
 {
 	struct prefix		*new;
 	struct adjout_prefix	*head, *p;
@@ -318,7 +321,8 @@ up_generate_addpath(struct rde_peer *pee
 		if (extra != 0 && extrapaths >= peer->eval.extrapaths)
 			break;
 
-		switch (up_process_prefix(peer, new, (void *)-1)) {
+		switch (up_process_prefix(peer, new, (void *)-1,
+		    force_update)) {
 		case UP_OK:
 			maxpaths++;
 			extrapaths += extra;
@@ -360,7 +364,7 @@ up_generate_addpath(struct rde_peer *pee
  */
 void
 up_generate_addpath_all(struct rde_peer *peer, struct rib_entry *re,
-    struct prefix *new, uint32_t old_pathid_tx)
+    struct prefix *new, uint32_t old_pathid_tx, int force_update)
 {
 	struct adjout_prefix	*p;
 
@@ -369,7 +373,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);
+		up_generate_addpath(peer, re, force_update);
 		return;
 	}
 
@@ -380,7 +384,8 @@ up_generate_addpath_all(struct rde_peer 
 
 	if (new != NULL) {
 		/* add new path */
-		switch (up_process_prefix(peer, new, (void *)-1)) {
+		switch (up_process_prefix(peer, new, (void *)-1,
+		    force_update)) {
 		case UP_OK:
 			/* don't remove old if an existing prefix was updated */
 			if (old_pathid_tx == new->path_id_tx)
@@ -403,7 +408,7 @@ up_generate_addpath_all(struct rde_peer 
 	}
 }
 
-/* send a default route to the specified peer */
+/* send a default route to the specified peer, always force the update out */
 void
 up_generate_default(struct rde_peer *peer, uint8_t aid)
 {
@@ -446,7 +451,7 @@ up_generate_default(struct rde_peer *pee
 	if (pte == NULL)
 		pte = pt_add(&addr, 0);
 	p = adjout_prefix_first(peer, pte);
-	adjout_prefix_update(p, peer, &state, pte, 0);
+	adjout_prefix_update(p, peer, &state, pte, 0, 1);
 	rde_filterstate_clean(&state);
 
 	/* max prefix checker outbound */