From: Claudio Jeker Subject: bgpd: rework the interface to rde_generate_updates To: tech@openbsd.org Date: Mon, 1 Dec 2025 11:26:54 +0100 Rework the interface to rde_generate_updates() and up_genrate_XYZ() to pass the old prefix as just the path_id_tx identifier. Only up_generate_addpath_all() actually uses this information and there this is enough to find the affected prefix in the adj-rib-out. Also adjust the order of operation in up_generate_addpath_all() so the prefix is not first removed and readded for the case where a prefix is simply updated. Rework the code in prefix_evaluate_nexthop() to be much more like prefix_evaluate(), it should be possible to factor out common code at some point. In peer_add() ensure that path_id_tx can't be 0. Since 0 has now a special meaning. This is done so we can introduce a queue around rde_generate_updates() and break up the big pipeline of input processing incl. decision process and output processing. -- :wq Claudio Index: rde.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v diff -u -p -r1.669 rde.c --- rde.c 20 Nov 2025 14:04:36 -0000 1.669 +++ rde.c 1 Dec 2025 10:10:30 -0000 @@ -4223,7 +4223,7 @@ rde_softreconfig_out(struct rib_entry *r /* no valid path for prefix */ return; - rde_generate_updates(re, NULL, NULL, EVAL_RECONF); + rde_generate_updates(re, NULL, 0, EVAL_RECONF); } static void Index: rde.h =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v diff -u -p -r1.324 rde.h --- rde.h 20 Nov 2025 14:04:36 -0000 1.324 +++ rde.h 1 Dec 2025 10:10:30 -0000 @@ -394,7 +394,7 @@ struct filter_head *peer_apply_out_filte struct filter_head *); void rde_generate_updates(struct rib_entry *, struct prefix *, - struct prefix *, enum eval_mode); + uint32_t, enum eval_mode); void peer_up(struct rde_peer *, struct session_up *); void peer_down(struct rde_peer *); @@ -774,7 +774,7 @@ prefix_adjout_nexthop(struct prefix_adjo void up_generate_updates(struct rde_peer *, struct rib_entry *); void up_generate_addpath(struct rde_peer *, struct rib_entry *); void up_generate_addpath_all(struct rde_peer *, struct rib_entry *, - struct prefix *, struct prefix *); + struct prefix *, uint32_t); 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 *, Index: rde_decide.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_decide.c,v diff -u -p -r1.105 rde_decide.c --- rde_decide.c 20 Nov 2025 10:47:36 -0000 1.105 +++ rde_decide.c 1 Dec 2025 10:10:30 -0000 @@ -533,6 +533,7 @@ prefix_evaluate(struct rib_entry *re, st { struct prefix *newbest, *oldbest; struct rib *rib; + uint32_t old_pathid_tx = 0; rib = re_rib(re); if (rib->flags & F_RIB_NOEVALUATE) { @@ -547,8 +548,10 @@ prefix_evaluate(struct rib_entry *re, st } oldbest = prefix_best(re); - if (old != NULL) + if (old != NULL) { prefix_remove(old, re); + old_pathid_tx = old->path_id_tx; + } if (new != NULL) prefix_insert(new, NULL, re); newbest = prefix_best(re); @@ -565,7 +568,7 @@ prefix_evaluate(struct rib_entry *re, st */ if ((rib->flags & F_RIB_NOFIB) == 0) rde_send_kroute(rib, newbest, oldbest); - rde_generate_updates(re, new, old, EVAL_DEFAULT); + rde_generate_updates(re, new, old_pathid_tx, EVAL_DEFAULT); return; } @@ -578,7 +581,7 @@ prefix_evaluate(struct rib_entry *re, st if (new != NULL && !prefix_eligible(new)) new = NULL; if (new != NULL || old != NULL) - rde_generate_updates(re, new, old, EVAL_ALL); + rde_generate_updates(re, new, old_pathid_tx, EVAL_ALL); } } @@ -589,6 +592,7 @@ prefix_evaluate_nexthop(struct prefix *p struct rib_entry *re = prefix_re(p); struct prefix *newbest, *oldbest, *new, *old; struct rib *rib; + uint32_t old_pathid_tx; /* Skip non local-RIBs or RIBs that are flagged as noeval. */ rib = re_rib(re); @@ -617,26 +621,26 @@ prefix_evaluate_nexthop(struct prefix *p * Re-evaluate the prefix by removing the prefix then updating the * nexthop state and reinserting the prefix again. */ - old = p; oldbest = prefix_best(re); - prefix_remove(p, re); + + old = p; + prefix_remove(old, re); + old_pathid_tx = old->path_id_tx; if (state == NEXTHOP_REACH) p->nhflags |= NEXTHOP_VALID; else p->nhflags &= ~NEXTHOP_VALID; - prefix_insert(p, NULL, re); - newbest = prefix_best(re); new = p; - if (!prefix_eligible(new)) - new = NULL; + prefix_insert(new, NULL, re); + newbest = prefix_best(re); /* * If the active prefix changed or the active prefix was removed * and added again then generate an update. */ - if (oldbest != newbest || newbest == p) { + if (oldbest != newbest || newbest == old) { /* * Send update withdrawing oldbest and adding newbest * but remember that newbest may be NULL aka ineligible. @@ -644,7 +648,7 @@ prefix_evaluate_nexthop(struct prefix *p */ if ((rib->flags & F_RIB_NOFIB) == 0) rde_send_kroute(rib, newbest, oldbest); - rde_generate_updates(re, new, old, EVAL_DEFAULT); + rde_generate_updates(re, new, old_pathid_tx, EVAL_DEFAULT); return; } @@ -653,6 +657,9 @@ prefix_evaluate_nexthop(struct prefix *p * to be passed on (not only a change of the best prefix). * rde_generate_updates() will then take care of distribution. */ - if (rde_evaluate_all()) - rde_generate_updates(re, new, old, EVAL_ALL); + if (rde_evaluate_all()) { + if (!prefix_eligible(new)) + new = NULL; + rde_generate_updates(re, new, old_pathid_tx, EVAL_ALL); + } } Index: rde_peer.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v diff -u -p -r1.56 rde_peer.c --- rde_peer.c 20 Nov 2025 13:46:22 -0000 1.56 +++ rde_peer.c 1 Dec 2025 10:10:30 -0000 @@ -187,7 +187,8 @@ peer_add(uint32_t id, struct peer_config conflict = 0; peer->path_id_tx = arc4random() << 1; RB_FOREACH(p, peer_tree, &peertable) { - if (p->path_id_tx == peer->path_id_tx) { + if (peer->path_id_tx == 0 || + p->path_id_tx == peer->path_id_tx) { conflict = 1; break; } @@ -240,8 +241,7 @@ RB_GENERATE(peer_tree, rde_peer, entry, static void peer_generate_update(struct rde_peer *peer, struct rib_entry *re, - struct prefix *newpath, struct prefix *oldpath, - enum eval_mode mode) + struct prefix *newpath, uint32_t old_pathid_tx, enum eval_mode mode) { uint8_t aid; @@ -271,7 +271,8 @@ peer_generate_update(struct rde_peer *pe /* handle peers with add-path */ if (peer_has_add_path(peer, aid, CAPA_AP_SEND)) { if (peer->eval.mode == ADDPATH_EVAL_ALL) - up_generate_addpath_all(peer, re, newpath, oldpath); + up_generate_addpath_all(peer, re, newpath, + old_pathid_tx); else up_generate_addpath(peer, re); return; @@ -285,12 +286,12 @@ peer_generate_update(struct rde_peer *pe void rde_generate_updates(struct rib_entry *re, struct prefix *newpath, - struct prefix *oldpath, enum eval_mode mode) + uint32_t old_pathid_tx, enum eval_mode mode) { struct rde_peer *peer; RB_FOREACH(peer, peer_tree, &peertable) - peer_generate_update(peer, re, newpath, oldpath, mode); + peer_generate_update(peer, re, newpath, old_pathid_tx, mode); } /* @@ -573,7 +574,7 @@ peer_dump_upcall(struct rib_entry *re, v /* no eligible prefix, not even for 'evaluate all' */ return; - peer_generate_update(peer, re, NULL, NULL, 0); + peer_generate_update(peer, re, NULL, 0, EVAL_DEFAULT); } static void Index: rde_rib.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v diff -u -p -r1.281 rde_rib.c --- rde_rib.c 20 Nov 2025 13:46:22 -0000 1.281 +++ rde_rib.c 1 Dec 2025 10:10:30 -0000 @@ -983,6 +983,7 @@ prefix_flowspec_update(struct rde_peer * struct rde_community *comm, *ncomm; struct rib_entry *re; struct prefix *new, *old; + uint32_t old_pathid_tx = 0; re = rib_get(&flowrib, pte); if (re == NULL) @@ -1004,7 +1005,9 @@ prefix_flowspec_update(struct rde_peer * NULL, 0, 0); TAILQ_INSERT_HEAD(&re->prefix_h, new, rib_l); - rde_generate_updates(re, new, old, EVAL_DEFAULT); + if (old != NULL) + old_pathid_tx = old->path_id_tx; + rde_generate_updates(re, new, old_pathid_tx, EVAL_DEFAULT); if (old != NULL) { TAILQ_REMOVE(&re->prefix_h, old, rib_l); @@ -1030,7 +1033,7 @@ prefix_flowspec_withdraw(struct rde_peer p = prefix_bypeer(re, peer, 0); if (p == NULL) return 0; - rde_generate_updates(re, NULL, p, EVAL_DEFAULT); + rde_generate_updates(re, NULL, p->path_id_tx, EVAL_DEFAULT); TAILQ_REMOVE(&re->prefix_h, p, rib_l); prefix_unlink(p); prefix_free(p); Index: rde_update.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v diff -u -p -r1.183 rde_update.c --- rde_update.c 20 Nov 2025 14:04:36 -0000 1.183 +++ rde_update.c 1 Dec 2025 10:10:30 -0000 @@ -344,7 +344,7 @@ up_generate_addpath(struct rde_peer *pee */ void up_generate_addpath_all(struct rde_peer *peer, struct rib_entry *re, - struct prefix *new, struct prefix *old) + struct prefix *new, uint32_t old_pathid_tx) { struct prefix_adjout *p; @@ -352,7 +352,7 @@ up_generate_addpath_all(struct rde_peer * If old and new are NULL then re-insert all prefixes from re, * use up_generate_addpath() for that. */ - if (old == NULL && new == NULL) { + if (old_pathid_tx == 0 && new == NULL) { up_generate_addpath(peer, re); return; } @@ -362,17 +362,14 @@ up_generate_addpath_all(struct rde_peer new = NULL; } - if (old != NULL) { - /* withdraw old path */ - p = prefix_adjout_get(peer, old->path_id_tx, old->pt); - if (p != NULL) - prefix_adjout_withdraw(p); - } - if (new != NULL) { /* add new path */ switch (up_process_prefix(peer, new, (void *)-1)) { case UP_OK: + /* don't remove old if an existing prefix was updated */ + if (old_pathid_tx == new->path_id_tx) + old_pathid_tx = 0; + break; case UP_FILTERED: case UP_EXCLUDED: break; @@ -380,6 +377,13 @@ up_generate_addpath_all(struct rde_peer /* just give up */ return; } + } + + if (old_pathid_tx != 0) { + /* withdraw old path */ + p = prefix_adjout_get(peer, old_pathid_tx, re->prefix); + if (p != NULL) + prefix_adjout_withdraw(p); } }