From: Theo Buehler Subject: Re: bgpd: rework the force_update logic in adjout To: tech@openbsd.org Date: Wed, 15 Jul 2026 13:25:49 +0200 On Wed, Jul 15, 2026 at 12:02:39PM +0200, Claudio Jeker wrote: > On initial connect the system needs to sync the adj-rib-out of a peer with > the Loc-RIB. This happens via peer_dump() and in this case the system > needs to push out all updates (but can drop any withdraw). > > The other user of peer_dump() is during config reloads where the RIB of a > peer is altered. In that case the adj-rib-out is first flushed > (withdrawing all paths) and then refilled via peer_dump(). Again in that > case all updates should be pushed out and there should be no withdraw at > all since the table was just flushed. > > To do this a force_update flag was introduced but actually it is better to > use the mode and introduce EVAL_SYNC for this case. > If mode == EVAL_SYNC adjout_prefix_update needs to force the update and > adjout_prefix_withdraw can drop the withdraw. I like this. > While there also remove the NOTYET bits in peer_generate_update() and > replace them with a simple hack inside up_generate_addpath_all() > to just call up_generate_addpath(). > > This is another stepping stone to bring back the add-path send all > optimisation. ok tb > -- > :wq Claudio > > Index: rde.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v > diff -u -p -r1.708 rde.c > --- rde.c 1 Jul 2026 09:53:47 -0000 1.708 > +++ rde.c 15 Jul 2026 09:43:03 -0000 > @@ -3671,7 +3671,7 @@ rde_up_flush_upcall(struct pt_entry *pte > { > struct rde_peer *peer = ptr; > > - adjout_prefix_withdraw(peer, pte, p); > + adjout_prefix_withdraw(peer, pte, p, 0); > } > > int > Index: rde.h > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v > diff -u -p -r1.357 rde.h > --- rde.h 13 Jul 2026 12:27:34 -0000 1.357 > +++ rde.h 15 Jul 2026 09:32:29 -0000 > @@ -359,6 +359,7 @@ struct filterstate { > > enum eval_mode { > EVAL_NONE, > + EVAL_SYNC, > EVAL_REEVAL, > EVAL_DEFAULT, > EVAL_ALL, > @@ -777,7 +778,7 @@ struct adjout_prefix *adjout_prefix_next > void adjout_prefix_update(struct adjout_prefix *, struct rde_peer *, > struct filterstate *, struct pt_entry *, uint32_t, int); > void adjout_prefix_withdraw(struct rde_peer *, struct pt_entry *, > - struct adjout_prefix *); > + struct adjout_prefix *, int); > void adjout_prefix_reaper(struct rde_peer *); > void adjout_prefix_dump_cleanup(struct rib_context *); > void adjout_prefix_dump_r(struct rib_context *); > @@ -808,9 +809,12 @@ 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 *, int); > -void up_generate_addpath(struct rde_peer *, struct rib_entry *, int); > -void up_generate_addpath_all(struct rde_peer *, struct rib_entry *, int); > +void up_generate_updates(struct rde_peer *, struct rib_entry *, > + enum eval_mode); > +void up_generate_addpath(struct rde_peer *, struct rib_entry *, > + enum eval_mode); > +void up_generate_addpath_all(struct rde_peer *, struct rib_entry *, > + enum eval_mode); > 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); > Index: rde_adjout.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v > diff -u -p -r1.21 rde_adjout.c > --- rde_adjout.c 13 Jul 2026 12:27:34 -0000 1.21 > +++ rde_adjout.c 15 Jul 2026 09:32:29 -0000 > @@ -516,11 +516,12 @@ adjout_prefix_next(struct pt_entry *pte, > > /* > * Put a prefix from the Adj-RIB-Out onto the update queue. > + * If force_send is set, ensure an UPDATE is sent in any case to the 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, > - int force_update) > + int force_send) > { > struct adjout_attr *attrs; > > @@ -542,7 +543,7 @@ adjout_prefix_update(struct adjout_prefi > attrs->communities) && > path_equal(&state->aspath, attrs->aspath)) { > /* nothing changed */ > - if (force_update && peer_is_up(peer)) > + if (force_send && peer_is_up(peer)) > pend_prefix_add(peer, attrs, pte, path_id_tx); > return; > } > @@ -563,15 +564,16 @@ adjout_prefix_update(struct adjout_prefi > /* > * Withdraw a prefix from the Adj-RIB-Out, this unlinks the aspath but leaves > * the prefix in the RIB linked to the peer withdraw list. > + * If skip_send is set there is no need to send a withdraw to the peer. > */ > void > adjout_prefix_withdraw(struct rde_peer *peer, struct pt_entry *pte, > - struct adjout_prefix *p) > + struct adjout_prefix *p, int skip_send) > { > if (bitmap_test(&p->peermap, peer->adjout_bid) == 0) > fatalx("%s: king bula is unhappy", __func__); > > - if (peer_is_up(peer)) > + if (peer_is_up(peer) && !skip_send) > pend_prefix_add(peer, NULL, pte, p->path_id_tx); > > adjout_prefix_unlink(p, pte, peer); > Index: rde_peer.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v > diff -u -p -r1.79 rde_peer.c > --- rde_peer.c 13 Jul 2026 12:27:34 -0000 1.79 > +++ rde_peer.c 15 Jul 2026 09:36:22 -0000 > @@ -244,7 +244,7 @@ RB_GENERATE(peer_tree, rde_peer, entry, > > static void > peer_generate_update(struct rde_peer *peer, struct rib_entry *re, > - enum eval_mode mode, int force_update) > + enum eval_mode mode) > { > uint8_t aid; > > @@ -269,26 +269,17 @@ peer_generate_update(struct rde_peer *pe > > /* handle peers with add-path */ > if (peer_has_add_path(peer, aid, CAPA_AP_SEND)) { > -#if NOTYET > - /* XXX skip addpath all optimisation for now until the queue > - * is extended to allow this mode to work again. > - * This mode is not very common and so taking the slow path > - * here like for all other addpath send modes is not that > - * big of an issue right now. > - */ > - if (peer->eval.mode == ADDPATH_EVAL_ALL) { > - up_generate_addpath_all(peer, re, force_update); > - return; > - } > -#endif > - up_generate_addpath(peer, re, force_update); > + if (peer->eval.mode == ADDPATH_EVAL_ALL) > + up_generate_addpath_all(peer, re, mode); > + else > + up_generate_addpath(peer, re, mode); > 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, force_update); > + up_generate_updates(peer, re, mode); > } > > /* > @@ -298,8 +289,8 @@ peer_generate_update(struct rde_peer *pe > * EVAL_ALL is sent for any other update (needed for peers with > * addpath or evaluate all set). > * EVAL_REEVAL is used by config reloads (a full RIB refresh is needed) > - * and for single peer RIB dumps but those call peer_generate_update() > - * directly. > + * EVAL_SYNC is used for single peer RIB dumps but those call > + * peer_generate_update() directly. > * peer, newpath and old_pathid_tx are ignored if mode is EVAL_REEVAL. > * In the other cases either newpath or old_pathid_tx are valid but not > * both at the same time. > @@ -317,13 +308,14 @@ rde_enqueue_updates(struct rib_entry *re > RB_FOREACH(p, peer_tree, &peertable) { > if (p->reconf_out == 0) > continue; > - peer_generate_update(p, re, EVAL_REEVAL, 0); > + peer_generate_update(p, re, mode); > } > adjout_prefix_collect(re->prefix); > return; > case EVAL_DEFAULT: > case EVAL_ALL: > break; > + case EVAL_SYNC: > case EVAL_NONE: > fatalx("bad eval mode in %s", __func__); > } > @@ -354,7 +346,7 @@ peer_process_updates(struct rde_peer *pe > peer->stats.rib_entry_count--; > > RB_FOREACH(p, peer_tree, &peertable) > - peer_generate_update(p, re, re->pq_mode, 0); > + peer_generate_update(p, re, re->pq_mode); > > adjout_prefix_collect(re->prefix); > rib_pq_dequeue(re); > @@ -505,7 +497,7 @@ peer_reaper_upcall(struct pt_entry *pte, > { > struct rde_peer *peer = ptr; > > - adjout_prefix_withdraw(peer, pte, p); > + adjout_prefix_withdraw(peer, pte, p, 1); > } > > /* > @@ -656,7 +648,7 @@ peer_dump_upcall(struct rib_entry *re, v > /* no eligible prefix, not even for 'evaluate all' */ > return; > > - peer_generate_update(peer, re, EVAL_REEVAL, 1); > + peer_generate_update(peer, re, EVAL_SYNC); > adjout_prefix_collect(re->prefix); > } > > Index: rde_update.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v > diff -u -p -r1.200 rde_update.c > --- rde_update.c 13 Jul 2026 12:27:34 -0000 1.200 > +++ rde_update.c 15 Jul 2026 10:00:42 -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, int force_update) > + struct adjout_prefix *p, enum eval_mode mode) > { > struct filterstate state; > struct bgpd_addr addr; > @@ -202,7 +202,7 @@ up_process_prefix(struct rde_peer *peer, > > up_prep_adjout(peer, &state, new->pt->aid); > adjout_prefix_update(p, peer, &state, new->pt, new_path_id_tx, > - force_update); > + mode == EVAL_SYNC); > rde_filterstate_clean(&state); > > /* max prefix checker outbound */ > @@ -221,7 +221,7 @@ up_process_prefix(struct rde_peer *peer, > > void > up_generate_updates(struct rde_peer *peer, struct rib_entry *re, > - int force_update) > + enum eval_mode mode) > { > struct prefix *new; > struct adjout_prefix *p; > @@ -230,7 +230,7 @@ up_generate_updates(struct rde_peer *pee > > new = prefix_best(re); > while (new != NULL) { > - switch (up_process_prefix(peer, new, p, force_update)) { > + switch (up_process_prefix(peer, new, p, mode)) { > case UP_OK: > case UP_ERR_LIMIT: > return; > @@ -249,7 +249,7 @@ up_generate_updates(struct rde_peer *pee > done: > /* withdraw prefix */ > if (p != NULL) > - adjout_prefix_withdraw(peer, re->prefix, p); > + adjout_prefix_withdraw(peer, re->prefix, p, mode == EVAL_SYNC); > } > > /* > @@ -261,7 +261,7 @@ done: > */ > void > up_generate_addpath(struct rde_peer *peer, struct rib_entry *re, > - int force_update) > + enum eval_mode mode) > { > struct prefix *new; > struct adjout_prefix *head, *p; > @@ -322,8 +322,7 @@ up_generate_addpath(struct rde_peer *pee > if (extra != 0 && extrapaths >= peer->eval.extrapaths) > break; > > - switch (up_process_prefix(peer, new, (void *)-1, > - force_update)) { > + switch (up_process_prefix(peer, new, (void *)-1, mode)) { > case UP_OK: > maxpaths++; > extrapaths += extra; > @@ -354,7 +353,8 @@ up_generate_addpath(struct rde_peer *pee > p = adjout_prefix_get(peer, addpath_prefix_list[i], > re->prefix); > if (p != NULL) > - adjout_prefix_withdraw(peer, re->prefix, p); > + adjout_prefix_withdraw(peer, re->prefix, p, > + mode == EVAL_SYNC); > } > } > } > @@ -365,7 +365,7 @@ up_generate_addpath(struct rde_peer *pee > */ > void > up_generate_addpath_all(struct rde_peer *peer, struct rib_entry *re, > - int force_update) > + enum eval_mode mode) > { > #if 0 > /* XXX needs to be rewritten */ > @@ -376,7 +376,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, force_update); > + up_generate_addpath(peer, re, mode); > return; > } > > @@ -387,8 +387,7 @@ up_generate_addpath_all(struct rde_peer > > if (new != NULL) { > /* add new path */ > - switch (up_process_prefix(peer, new, (void *)-1, > - force_update)) { > + switch (up_process_prefix(peer, new, (void *)-1, mode)) { > case UP_OK: > /* don't remove old if an existing prefix was updated */ > if (old_pathid_tx == new->path_id_tx) > @@ -407,8 +406,11 @@ up_generate_addpath_all(struct rde_peer > /* withdraw old path */ > p = adjout_prefix_get(peer, old_pathid_tx, re->prefix); > if (p != NULL) > - adjout_prefix_withdraw(peer, re->prefix, p); > + adjout_prefix_withdraw(peer, re->prefix, p, > + mode == EVAL_SYNC); > } > +#else > + up_generate_addpath(peer, re, mode); > #endif > } > >