Download raw body.
bgpd: more churn, split PREFIX flags
On Thu, Nov 20, 2025 at 01:57:28PM +0100, Claudio Jeker wrote:
> 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.
Reads fine. One question below which you can fix (or not) before commit.
ok tb
>
> --
> :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;
Maybe I'm missing something, but should this not do
p->flags &= ~PREFIX_ADJOUT_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;
>
bgpd: more churn, split PREFIX flags