Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: kill some adj-rib-out flags
To:
tech@openbsd.org
Date:
Thu, 11 Dec 2025 17:29:20 +0100

Download raw body.

Thread
PREFIX_ADJOUT_FLAG_DEAD is no longer needed and can be replaced with
a check that the attrs pointer is NULL. Refactor the code now a bit
since the logic got a bit simpler.

This leaves PREFIX_ADJOUT_FLAG_STALE and PREFIX_ADJOUT_FLAG_LOCKED.
PREFIX_ADJOUT_FLAG_STALE will die sooner than later and in a subsequent
step PREFIX_ADJOUT_FLAG_LOCKED will follow the dodo.

-- 
:wq Claudio

Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.329 rde.h
--- rde.h	10 Dec 2025 12:36:51 -0000	1.329
+++ rde.h	11 Dec 2025 16:24:28 -0000
@@ -324,9 +324,7 @@ struct adjout_prefix {
 	uint32_t			 path_id_tx;
 	uint8_t			 	 flags;
 };
-#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_STALE	0x01	/* stale entry (for addpath) */
 #define	PREFIX_ADJOUT_FLAG_LOCKED	0x20	/* locked by rib walker */
 
 struct pend_attr {
Index: rde_adjout.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
diff -u -p -r1.9 rde_adjout.c
--- rde_adjout.c	10 Dec 2025 12:36:51 -0000	1.9
+++ rde_adjout.c	11 Dec 2025 16:24:28 -0000
@@ -412,7 +412,7 @@ prefix_is_locked(struct adjout_prefix *p
 static inline int
 prefix_is_dead(struct adjout_prefix *p)
 {
-	return (p->flags & PREFIX_ADJOUT_FLAG_DEAD) != 0;
+	return p->attrs == NULL;
 }
 
 static void	 adjout_prefix_link(struct adjout_prefix *, struct rde_peer *,
@@ -545,16 +545,13 @@ adjout_prefix_update(struct adjout_prefi
 	if (p == NULL) {
 		p = adjout_prefix_alloc();
 		/* initially mark DEAD so code below is skipped */
-		p->flags |= PREFIX_ADJOUT_FLAG_DEAD;
 
 		p->pt = pt_ref(pte);
 		p->path_id_tx = path_id_tx;
 
 		if (RB_INSERT(prefix_index, &peer->adj_rib_out, p) != NULL)
 			fatalx("%s: RB index invariant violated", __func__);
-	}
-
-	if ((p->flags & PREFIX_ADJOUT_FLAG_DEAD) == 0) {
+	} else {
 		/*
 		 * XXX for now treat a different path_id_tx like different
 		 * attributes and force out an update. It is unclear how
@@ -577,8 +574,8 @@ adjout_prefix_update(struct adjout_prefi
 		peer->stats.prefix_out_cnt--;
 	}
 
-	/* nothing needs to be done for PREFIX_ADJOUT_FLAG_DEAD and STALE */
-	p->flags &= ~PREFIX_ADJOUT_FLAG_MASK;
+	/* clear PREFIX_ADJOUT_FLAG_STALE for up_generate_addpath() */
+	p->flags &= ~PREFIX_ADJOUT_FLAG_STALE;
 
 	/* update path_id_tx now that the prefix is unlinked */
 	if (p->path_id_tx != path_id_tx) {
@@ -594,11 +591,8 @@ adjout_prefix_update(struct adjout_prefi
 	adjout_prefix_link(p, peer, attrs, p->pt, p->path_id_tx);
 	peer->stats.prefix_out_cnt++;
 
-	if (p->flags & PREFIX_ADJOUT_FLAG_MASK)
-		fatalx("%s: bad flags %x", __func__, p->flags);
-	if (peer_is_up(peer)) {
+	if (peer_is_up(peer))
 		pend_prefix_add(peer, p->attrs, p->pt, p->path_id_tx);
-	}
 }
 
 /*
@@ -618,18 +612,15 @@ void
 adjout_prefix_destroy(struct rde_peer *peer, struct adjout_prefix *p)
 {
 	/* unlink prefix if it was linked (not dead) */
-	if ((p->flags & PREFIX_ADJOUT_FLAG_DEAD) == 0) {
+	if (!prefix_is_dead(p)) {
 		adjout_prefix_unlink(p, peer);
 		peer->stats.prefix_out_cnt--;
 	}
 
-	/* nothing needs to be done for PREFIX_ADJOUT_FLAG_DEAD and STALE */
-	p->flags &= ~PREFIX_ADJOUT_FLAG_MASK;
+	/* clear PREFIX_ADJOUT_FLAG_STALE just in case */
+	p->flags &= ~PREFIX_ADJOUT_FLAG_STALE;
 
-	if (prefix_is_locked(p)) {
-		/* mark prefix dead but leave it for prefix_restart */
-		p->flags |= PREFIX_ADJOUT_FLAG_DEAD;
-	} else {
+	if (!prefix_is_locked(p)) {
 		RB_REMOVE(prefix_index, &peer->adj_rib_out, p);
 		/* remove the last prefix reference before free */
 		pt_unref(p->pt);