Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: more churn, split PREFIX flags
To:
tech@openbsd.org
Date:
Thu, 20 Nov 2025 13:57:28 +0100

Download raw body.

Thread
Split up the PREFIX flags, split the name space and use PREFIX_ADJOUT
for those that only affect that struct.
In the process retire PREFIX_FLAG_ADJOUT.

-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.667 rde.c
--- rde.c	20 Nov 2025 10:47:36 -0000	1.667
+++ rde.c	20 Nov 2025 12:55:49 -0000
@@ -3142,9 +3142,7 @@ rde_dump_adjout_upcall(struct prefix_adj
 {
 	struct rde_dump_ctx	*ctx = ptr;
 
-	if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
-		fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
-	if (p->flags & PREFIX_FLAG_WITHDRAW)
+	if (p->flags & PREFIX_ADJOUT_FLAG_WITHDRAW)
 		return;
 	rde_dump_adjout_filter(p, &ctx->req);
 }
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.322 rde.h
--- rde.h	20 Nov 2025 10:47:36 -0000	1.322
+++ rde.h	20 Nov 2025 12:55:49 -0000
@@ -283,16 +283,9 @@ struct prefix {
 	uint8_t			 nhflags;
 	int8_t			 dmetric;	/* decision metric */
 };
-#define	PREFIX_FLAG_WITHDRAW	0x0001	/* enqueued on withdraw queue */
-#define	PREFIX_FLAG_UPDATE	0x0002	/* enqueued on update queue */
-#define	PREFIX_FLAG_DEAD	0x0004	/* locked but removed */
-#define	PREFIX_FLAG_STALE	0x0008	/* stale entry (for addpath) */
-#define	PREFIX_FLAG_MASK	0x000f	/* mask for the prefix types */
-#define	PREFIX_FLAG_ADJOUT	0x0010	/* prefix is in the adj-out rib */
-#define	PREFIX_FLAG_EOR		0x0020	/* prefix is EoR */
-#define	PREFIX_NEXTHOP_LINKED	0x0040	/* prefix is linked onto nexthop list */
-#define	PREFIX_FLAG_LOCKED	0x0080	/* locked by rib walker */
-#define	PREFIX_FLAG_FILTERED	0x0100	/* prefix is filtered (ineligible) */
+
+#define	PREFIX_NEXTHOP_LINKED	0x01	/* prefix is linked onto nexthop list */
+#define	PREFIX_FLAG_FILTERED	0x04	/* prefix is filtered (ineligible) */
 
 #define	PREFIX_DMETRIC_NONE	0
 #define	PREFIX_DMETRIC_INVALID	1
@@ -319,11 +312,18 @@ struct prefix_adjout {
 	monotime_t			 lastchange;
 	uint32_t			 path_id;
 	uint32_t			 path_id_tx;
-	uint16_t			 flags;
+	uint8_t			 	 flags;
 	uint8_t				 validation_state;
 	uint8_t				 nhflags;
 	int8_t				 dmetric;	/* decision metric */
 };
+#define	PREFIX_ADJOUT_FLAG_WITHDRAW	0x01	/* enqueued on withdraw queue */
+#define	PREFIX_ADJOUT_FLAG_UPDATE	0x02	/* enqueued on update queue */
+#define	PREFIX_ADJOUT_FLAG_DEAD		0x04	/* locked but removed */
+#define	PREFIX_ADJOUT_FLAG_STALE	0x08	/* stale entry (for addpath) */
+#define	PREFIX_ADJOUT_FLAG_MASK		0x0f	/* mask for the prefix types */
+#define	PREFIX_ADJOUT_FLAG_EOR		0x10	/* prefix is EoR */
+#define	PREFIX_ADJOUT_FLAG_LOCKED	0x20	/* locked by rib walker */
 
 struct filterstate {
 	struct rde_aspath	 aspath;
Index: rde_adjout.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
diff -u -p -r1.4 rde_adjout.c
--- rde_adjout.c	20 Nov 2025 10:47:36 -0000	1.4
+++ rde_adjout.c	20 Nov 2025 12:55:49 -0000
@@ -33,31 +33,31 @@
 static inline struct prefix_adjout *
 prefix_adjout_lock(struct prefix_adjout *p)
 {
-	if (p->flags & PREFIX_FLAG_LOCKED)
+	if (p->flags & PREFIX_ADJOUT_FLAG_LOCKED)
 		fatalx("%s: locking locked prefix", __func__);
-	p->flags |= PREFIX_FLAG_LOCKED;
+	p->flags |= PREFIX_ADJOUT_FLAG_LOCKED;
 	return p;
 }
 
 static inline struct prefix_adjout *
 prefix_adjout_unlock(struct prefix_adjout *p)
 {
-	if ((p->flags & PREFIX_FLAG_LOCKED) == 0)
+	if ((p->flags & PREFIX_ADJOUT_FLAG_LOCKED) == 0)
 		fatalx("%s: unlocking unlocked prefix", __func__);
-	p->flags &= ~PREFIX_FLAG_LOCKED;
+	p->flags &= ~PREFIX_ADJOUT_FLAG_LOCKED;
 	return p;
 }
 
 static inline int
 prefix_is_locked(struct prefix_adjout *p)
 {
-	return (p->flags & PREFIX_FLAG_LOCKED) != 0;
+	return (p->flags & PREFIX_ADJOUT_FLAG_LOCKED) != 0;
 }
 
 static inline int
 prefix_is_dead(struct prefix_adjout *p)
 {
-	return (p->flags & PREFIX_FLAG_DEAD) != 0;
+	return (p->flags & PREFIX_ADJOUT_FLAG_DEAD) != 0;
 }
 
 static void	 prefix_adjout_link(struct prefix_adjout *, struct pt_entry *,
@@ -88,10 +88,11 @@ prefix_index_cmp(struct prefix_adjout *a
 static inline int
 prefix_cmp(struct prefix_adjout *a, struct prefix_adjout *b)
 {
-	if ((a->flags & PREFIX_FLAG_EOR) != (b->flags & PREFIX_FLAG_EOR))
-		return (a->flags & PREFIX_FLAG_EOR) ? 1 : -1;
+	if ((a->flags & PREFIX_ADJOUT_FLAG_EOR) !=
+	    (b->flags & PREFIX_ADJOUT_FLAG_EOR))
+		return (a->flags & PREFIX_ADJOUT_FLAG_EOR) ? 1 : -1;
 	/* if EOR marker no need to check the rest */
-	if (a->flags & PREFIX_FLAG_EOR)
+	if (a->flags & PREFIX_ADJOUT_FLAG_EOR)
 		return 0;
 
 	if (a->aspath != b->aspath)
@@ -209,7 +210,7 @@ prefix_add_eor(struct rde_peer *peer, ui
 	struct prefix_adjout *p;
 
 	p = prefix_adjout_alloc();
-	p->flags = PREFIX_FLAG_ADJOUT | PREFIX_FLAG_UPDATE | PREFIX_FLAG_EOR;
+	p->flags = PREFIX_ADJOUT_FLAG_UPDATE | PREFIX_ADJOUT_FLAG_EOR;
 	if (RB_INSERT(prefix_tree, &peer->updates[aid], p) != NULL)
 		/* no need to add if EoR marker already present */
 		prefix_adjout_free(p);
@@ -229,7 +230,7 @@ prefix_adjout_update(struct prefix_adjou
 	if (p == NULL) {
 		p = prefix_adjout_alloc();
 		/* initially mark DEAD so code below is skipped */
-		p->flags |= PREFIX_FLAG_ADJOUT | PREFIX_FLAG_DEAD;
+		p->flags |= PREFIX_ADJOUT_FLAG_DEAD;
 
 		p->pt = pt_ref(pte);
 		p->peer = peer;
@@ -239,9 +240,8 @@ prefix_adjout_update(struct prefix_adjou
 			fatalx("%s: RB index invariant violated", __func__);
 	}
 
-	if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
-		fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
-	if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) {
+	if ((p->flags & (PREFIX_ADJOUT_FLAG_WITHDRAW |
+	    PREFIX_ADJOUT_FLAG_DEAD)) == 0) {
 		/*
 		 * XXX for now treat a different path_id_tx like different
 		 * attributes and force out an update. It is unclear how
@@ -257,12 +257,11 @@ prefix_adjout_update(struct prefix_adjou
 			/* nothing changed */
 			p->validation_state = state->vstate;
 			p->lastchange = getmonotime();
-			p->flags &= ~PREFIX_FLAG_STALE;
 			return;
 		}
 
 		/* if pending update unhook it before it is unlinked */
-		if (p->flags & PREFIX_FLAG_UPDATE) {
+		if (p->flags & PREFIX_ADJOUT_FLAG_UPDATE) {
 			RB_REMOVE(prefix_tree, &peer->updates[pte->aid], p);
 			peer->stats.pending_update--;
 		}
@@ -271,13 +270,13 @@ prefix_adjout_update(struct prefix_adjou
 		prefix_adjout_unlink(p);
 		peer->stats.prefix_out_cnt--;
 	}
-	if (p->flags & PREFIX_FLAG_WITHDRAW) {
+	if (p->flags & PREFIX_ADJOUT_FLAG_WITHDRAW) {
 		RB_REMOVE(prefix_tree, &peer->withdraws[pte->aid], p);
 		peer->stats.pending_withdraw--;
 	}
 
-	/* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */
-	p->flags &= ~PREFIX_FLAG_MASK;
+	/* nothing needs to be done for PREFIX_ADJOUT_FLAG_DEAD and STALE */
+	p->flags &= ~PREFIX_ADJOUT_FLAG_MASK;
 
 	/* update path_id_tx now that the prefix is unlinked */
 	if (p->path_id_tx != path_id_tx) {
@@ -299,10 +298,10 @@ prefix_adjout_update(struct prefix_adjou
 	    state->nexthop, state->nhflags, state->vstate);
 	peer->stats.prefix_out_cnt++;
 
-	if (p->flags & PREFIX_FLAG_MASK)
+	if (p->flags & PREFIX_ADJOUT_FLAG_MASK)
 		fatalx("%s: bad flags %x", __func__, p->flags);
 	if (peer_is_up(peer)) {
-		p->flags |= PREFIX_FLAG_UPDATE;
+		p->flags |= PREFIX_ADJOUT_FLAG_UPDATE;
 		if (RB_INSERT(prefix_tree, &peer->updates[pte->aid], p) != NULL)
 			fatalx("%s: RB tree invariant violated", __func__);
 		peer->stats.pending_update++;
@@ -318,39 +317,37 @@ prefix_adjout_withdraw(struct prefix_adj
 {
 	struct rde_peer *peer = prefix_adjout_peer(p);
 
-	if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
-		fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
-
 	/* already a withdraw, shortcut */
-	if (p->flags & PREFIX_FLAG_WITHDRAW) {
+	if (p->flags & PREFIX_ADJOUT_FLAG_WITHDRAW) {
 		p->lastchange = getmonotime();
-		p->flags &= ~PREFIX_FLAG_STALE;
+		p->flags &= ~PREFIX_ADJOUT_FLAG_STALE;
 		return;
 	}
 	/* pending update just got withdrawn */
-	if (p->flags & PREFIX_FLAG_UPDATE) {
+	if (p->flags & PREFIX_ADJOUT_FLAG_UPDATE) {
 		RB_REMOVE(prefix_tree, &peer->updates[p->pt->aid], p);
 		peer->stats.pending_update--;
 	}
 	/* unlink prefix if it was linked (not a withdraw or dead) */
-	if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) {
+	if ((p->flags & (PREFIX_ADJOUT_FLAG_WITHDRAW |
+	    PREFIX_ADJOUT_FLAG_DEAD)) == 0) {
 		prefix_adjout_unlink(p);
 		peer->stats.prefix_out_cnt--;
 	}
 
-	/* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */
-	p->flags &= ~PREFIX_FLAG_MASK;
+	/* nothing needs to be done for PREFIX_ADJOUT_FLAG_DEAD and STALE */
+	p->flags &= ~PREFIX_ADJOUT_FLAG_MASK;
 	p->lastchange = getmonotime();
 
 	if (peer_is_up(peer)) {
-		p->flags |= PREFIX_FLAG_WITHDRAW;
+		p->flags |= PREFIX_ADJOUT_FLAG_WITHDRAW;
 		if (RB_INSERT(prefix_tree, &peer->withdraws[p->pt->aid],
 		    p) != NULL)
 			fatalx("%s: RB tree invariant violated", __func__);
 		peer->stats.pending_withdraw++;
 	} else {
 		/* mark prefix dead to skip unlink on destroy */
-		p->flags |= PREFIX_FLAG_DEAD;
+		p->flags |= PREFIX_ADJOUT_FLAG_DEAD;
 		prefix_adjout_destroy(p);
 	}
 }
@@ -360,35 +357,33 @@ prefix_adjout_destroy(struct prefix_adjo
 {
 	struct rde_peer *peer = prefix_adjout_peer(p);
 
-	if ((p->flags & PREFIX_FLAG_ADJOUT) == 0)
-		fatalx("%s: prefix without PREFIX_FLAG_ADJOUT hit", __func__);
-
-	if (p->flags & PREFIX_FLAG_EOR) {
+	if (p->flags & PREFIX_ADJOUT_FLAG_EOR) {
 		/* EOR marker is not linked in the index */
 		prefix_adjout_free(p);
 		return;
 	}
 
-	if (p->flags & PREFIX_FLAG_WITHDRAW) {
+	if (p->flags & PREFIX_ADJOUT_FLAG_WITHDRAW) {
 		RB_REMOVE(prefix_tree, &peer->withdraws[p->pt->aid], p);
 		peer->stats.pending_withdraw--;
 	}
-	if (p->flags & PREFIX_FLAG_UPDATE) {
+	if (p->flags & PREFIX_ADJOUT_FLAG_UPDATE) {
 		RB_REMOVE(prefix_tree, &peer->updates[p->pt->aid], p);
 		peer->stats.pending_update--;
 	}
 	/* unlink prefix if it was linked (not a withdraw or dead) */
-	if ((p->flags & (PREFIX_FLAG_WITHDRAW | PREFIX_FLAG_DEAD)) == 0) {
+	if ((p->flags & (PREFIX_ADJOUT_FLAG_WITHDRAW |
+	    PREFIX_ADJOUT_FLAG_DEAD)) == 0) {
 		prefix_adjout_unlink(p);
 		peer->stats.prefix_out_cnt--;
 	}
 
-	/* nothing needs to be done for PREFIX_FLAG_DEAD and STALE */
-	p->flags &= ~PREFIX_FLAG_MASK;
+	/* nothing needs to be done for PREFIX_ADJOUT_FLAG_DEAD and STALE */
+	p->flags &= ~PREFIX_ADJOUT_FLAG_MASK;
 
 	if (prefix_is_locked(p)) {
 		/* mark prefix dead but leave it for prefix_restart */
-		p->flags |= PREFIX_FLAG_DEAD;
+		p->flags |= PREFIX_ADJOUT_FLAG_DEAD;
 	} else {
 		RB_REMOVE(prefix_index, &peer->adj_rib_out, p);
 		/* remove the last prefix reference before free */
@@ -408,9 +403,9 @@ prefix_adjout_flush_pending(struct rde_p
 			prefix_adjout_destroy(p);
 		}
 		RB_FOREACH_SAFE(p, prefix_tree, &peer->updates[aid], np) {
-			p->flags &= ~PREFIX_FLAG_UPDATE;
+			p->flags &= ~PREFIX_ADJOUT_FLAG_UPDATE;
 			RB_REMOVE(prefix_tree, &peer->updates[aid], p);
-			if (p->flags & PREFIX_FLAG_EOR) {
+			if (p->flags & PREFIX_ADJOUT_FLAG_EOR) {
 				prefix_adjout_destroy(p);
 			} else {
 				peer->stats.pending_update--;
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.55 rde_peer.c
--- rde_peer.c	20 Nov 2025 10:47:36 -0000	1.55
+++ rde_peer.c	20 Nov 2025 12:55:49 -0000
@@ -520,10 +520,10 @@ peer_blast_upcall(struct prefix_adjout *
 {
 	struct rde_peer		*peer;
 
-	if ((p->flags & PREFIX_FLAG_MASK) == 0) {
+	if ((p->flags & PREFIX_ADJOUT_FLAG_MASK) == 0) {
 		peer = prefix_adjout_peer(p);
 		/* put entries on the update queue if not already on a queue */
-		p->flags |= PREFIX_FLAG_UPDATE;
+		p->flags |= PREFIX_ADJOUT_FLAG_UPDATE;
 		if (RB_INSERT(prefix_tree, &peer->updates[p->pt->aid],
 		    p) != NULL)
 			fatalx("%s: RB tree invariant violated", __func__);
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
diff -u -p -r1.280 rde_rib.c
--- rde_rib.c	20 Nov 2025 10:47:36 -0000	1.280
+++ rde_rib.c	20 Nov 2025 12:55:49 -0000
@@ -913,9 +913,6 @@ prefix_move(struct prefix *p, struct rde
 {
 	struct prefix		*np;
 
-	if (p->flags & PREFIX_FLAG_ADJOUT)
-		fatalx("%s: prefix with PREFIX_FLAG_ADJOUT hit", __func__);
-
 	if (peer != prefix_peer(p))
 		fatalx("prefix_move: cross peer move");
 
@@ -963,8 +960,6 @@ prefix_withdraw(struct rib *rib, struct 
 	if (p == NULL)		/* Got a dummy withdrawn request. */
 		return (0);
 
-	if (p->flags & PREFIX_FLAG_ADJOUT)
-		fatalx("%s: prefix with PREFIX_FLAG_ADJOUT hit", __func__);
 	asp = prefix_aspath(p);
 	if (asp && asp->pftableid)
 		/* only prefixes in the local RIB were pushed into pf */
@@ -1319,12 +1314,6 @@ void
 nexthop_link(struct prefix *p)
 {
 	p->nhflags &= ~NEXTHOP_VALID;
-
-	if (p->flags & PREFIX_FLAG_ADJOUT) {
-		/* All nexthops are valid in Adj-RIB-Out */
-		p->nhflags |= NEXTHOP_VALID;
-		return;
-	}
 
 	/* no need to link prefixes in RIBs that have no decision process */
 	if (re_rib(prefix_re(p))->flags & F_RIB_NOEVALUATE)
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
diff -u -p -r1.181 rde_update.c
--- rde_update.c	20 Nov 2025 10:47:36 -0000	1.181
+++ rde_update.c	20 Nov 2025 12:55:49 -0000
@@ -266,7 +266,7 @@ up_generate_addpath(struct rde_peer *pee
 
 	/* mark all paths as stale */
 	for (p = head; p != NULL; p = prefix_adjout_next(peer, p))
-		p->flags |= PREFIX_FLAG_STALE;
+		p->flags |= PREFIX_ADJOUT_FLAG_STALE;
 
 	/* update paths */
 	new = prefix_best(re);
@@ -333,7 +333,7 @@ up_generate_addpath(struct rde_peer *pee
 
 	/* withdraw stale paths */
 	for (p = head; p != NULL; p = prefix_adjout_next(peer, p)) {
-		if (p->flags & PREFIX_FLAG_STALE)
+		if (p->flags & PREFIX_ADJOUT_FLAG_STALE)
 			prefix_adjout_withdraw(p);
 	}
 }
@@ -797,13 +797,13 @@ up_is_eor(struct rde_peer *peer, uint8_t
 	struct prefix_adjout *p;
 
 	p = RB_MIN(prefix_tree, &peer->updates[aid]);
-	if (p != NULL && (p->flags & PREFIX_FLAG_EOR)) {
+	if (p != NULL && (p->flags & PREFIX_ADJOUT_FLAG_EOR)) {
 		/*
 		 * Need to remove eor from update tree because
 		 * prefix_adjout_destroy() can't handle that.
 		 */
 		RB_REMOVE(prefix_tree, &peer->updates[aid], p);
-		p->flags &= ~PREFIX_FLAG_UPDATE;
+		p->flags &= ~PREFIX_ADJOUT_FLAG_UPDATE;
 		prefix_adjout_destroy(p);
 		return 1;
 	}
@@ -824,7 +824,7 @@ up_prefix_free(struct prefix_tree *prefi
 	} else {
 		/* prefix still in Adj-RIB-Out, keep it */
 		RB_REMOVE(prefix_tree, prefix_head, p);
-		p->flags &= ~PREFIX_FLAG_UPDATE;
+		p->flags &= ~PREFIX_ADJOUT_FLAG_UPDATE;
 		peer->stats.pending_update--;
 		peer->stats.prefix_sent_update++;
 	}
@@ -856,7 +856,7 @@ up_dump_prefix(struct ibuf *buf, struct 
 		    np->communities != p->communities ||
 		    np->nexthop != p->nexthop ||
 		    np->nhflags != p->nhflags ||
-		    (np->flags & PREFIX_FLAG_EOR))
+		    (np->flags & PREFIX_ADJOUT_FLAG_EOR))
 			done = 1;
 
 		rv = 0;