From: Claudio Jeker Subject: bgpd: more churn, split PREFIX flags To: tech@openbsd.org Date: Thu, 20 Nov 2025 13:57:28 +0100 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;