Download raw body.
bgpd: adjust adjout_prefix_dump code for the reaper
On Thu, May 21, 2026 at 05:19:41PM +0200, Claudio Jeker wrote:
> Right now adjout_prefix_dump uses peer_get() to provide a peer pointer to
> the callback. This makes the peer reaper unhappy since we want to remove
> the peer from the lookup table before calling the peer_reaper walker.
>
> So in other words the peer_reaper walker is currently not working at all
> and so another peer may start with a bad Adj-RIB-Out which is hopefully
> all cleaned up by the peer_dump call. The full effect of this bug is not
> quite clear.
>
> First of all the adjout_prefix_dump context should just use the adjout_bid
> and stop using the peer->conf.id. Doing that trickles some additional
> challenges since peer can no longer be used.
>
> - The callback drop the struct rde_peer argument (but get a currently unused
> uint32_t bid argument).
> - adjout_prefix_first() also needs to switch to using the adjout bitmask
> id and drop the peer argument.
> - also change adjout_prefix_next() just to be in sync with
> adjout_prefix_first(). It is not strictly needed but it makes the most
> sense.
> - Most callbacks already pass the peer as ctx_arg pointer around so little
> changes are needed there.
> - Once again another big surgery is needed in rde_dump_ctx_new()
> - The rde_dump_adjout_upcall() now needs to use peer_get() to resolve the
> peer. That's not great, maybe passing the peer pointer in rde_dump_ctx
> would work, but that's for later.
>
> If the peer has no adj-rib-out then exit adjout_prefix_dump_new() early by
> calling its done callback. Because of that rde_dump_ctx_new() needs to
> enqueue the rde_dump_ctx before calling adjout_prefix_dump_new().
>
> I hope I covered all the crazy in this diff.
Doing the early exit in adjout_prefix_dump_new() by calling the done
callback is a can of worms. Most code does not expect the callback to be
run that early. Instead of trying to fix all the mess caused by that move
the check into adjout_prefix_dump_r() were the peer_get was done before.
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.703 rde.c
--- rde.c 21 May 2026 15:20:27 -0000 1.703
+++ rde.c 21 May 2026 21:05:39 -0000
@@ -3207,10 +3207,14 @@ rde_dump_upcall(struct rib_entry *re, vo
}
static void
-rde_dump_adjout_upcall(struct rde_peer *peer, struct pt_entry *pte,
- struct adjout_prefix *p, void *ptr)
+rde_dump_adjout_upcall(struct pt_entry *pte, struct adjout_prefix *p,
+ uint32_t bid, void *ptr)
{
struct rde_dump_ctx *ctx = ptr;
+ struct rde_peer *peer;
+
+ if ((peer = peer_get(ctx->peerid)) == NULL)
+ return;
rde_dump_adjout_filter(peer, pte, p, &ctx->req);
}
@@ -3342,8 +3346,13 @@ rde_dump_ctx_new(struct ctl_show_rib_req
do {
struct pt_entry *pte;
+ uint32_t bid = peer->adjout_bid;
int found;
+ ctx->peerid = peer->conf.id;
+ if (bid == 0)
+ continue;
+
if (req->flags & F_SHORTER) {
for (plen = 0; plen <= req->prefixlen;
plen++) {
@@ -3353,12 +3362,12 @@ rde_dump_ctx_new(struct ctl_show_rib_req
continue;
/* dump all matching paths */
for (p = adjout_prefix_first(
- peer, pte);
+ pte, bid);
p != NULL;
- p = adjout_prefix_next(
- peer, pte, p)) {
+ p = adjout_prefix_next(pte,
+ bid, p)) {
rde_dump_adjout_upcall(
- peer, pte, p, ctx);
+ pte, p, bid, ctx);
}
}
continue;
@@ -3374,12 +3383,12 @@ rde_dump_ctx_new(struct ctl_show_rib_req
do {
/* dump all matching paths */
found = 0;
- for (p = adjout_prefix_first(peer, pte);
+ for (p = adjout_prefix_first(pte, bid);
p != NULL;
- p = adjout_prefix_next(peer, pte,
+ p = adjout_prefix_next(pte, bid,
p)) {
- rde_dump_adjout_upcall(peer,
- pte, p, ctx);
+ rde_dump_adjout_upcall(pte, p,
+ bid, ctx);
found = 1;
}
plen = pte->prefixlen - 1;
@@ -3396,7 +3405,7 @@ rde_dump_ctx_new(struct ctl_show_rib_req
}
} while (!found && pte != NULL);
} while ((peer = peer_match(&req->neighbor,
- peer->conf.id)));
+ ctx->peerid)) != NULL);
imsg_compose(ibuf_se_ctl, IMSG_CTL_END, 0, ctx->req.pid,
-1, NULL, 0);
@@ -3649,9 +3658,10 @@ rde_evaluate_all(void)
/* flush Adj-RIB-Out by withdrawing all prefixes */
static void
-rde_up_flush_upcall(struct rde_peer *peer, struct pt_entry *pte,
- struct adjout_prefix *p, void *ptr)
+rde_up_flush_upcall(struct pt_entry *pte, struct adjout_prefix *p,
+ uint32_t bid, void *ptr)
{
+ struct rde_peer *peer = ptr;
adjout_prefix_withdraw(peer, pte, p);
}
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.351 rde.h
--- rde.h 21 May 2026 15:20:27 -0000 1.351
+++ rde.h 21 May 2026 18:27:08 -0000
@@ -369,8 +369,8 @@ struct rib_context {
struct pt_entry *ctx_pt;
uint32_t ctx_id;
void (*ctx_rib_call)(struct rib_entry *, void *);
- void (*ctx_prefix_call)(struct rde_peer *,
- struct pt_entry *, struct adjout_prefix *, void *);
+ void (*ctx_prefix_call)(struct pt_entry *,
+ struct adjout_prefix *, uint32_t, void *);
void (*ctx_done)(void *, uint8_t);
int (*ctx_throttle)(void *);
void *ctx_arg;
@@ -767,10 +767,9 @@ int nexthop_unref(struct nexthop *);
void adjout_init(void);
struct adjout_prefix *adjout_prefix_get(struct rde_peer *, uint32_t,
struct pt_entry *);
-struct adjout_prefix *adjout_prefix_first(struct rde_peer *,
- struct pt_entry *);
-struct adjout_prefix *adjout_prefix_next(struct rde_peer *,
- struct pt_entry *, struct adjout_prefix *);
+struct adjout_prefix *adjout_prefix_first(struct pt_entry *, uint32_t);
+struct adjout_prefix *adjout_prefix_next(struct pt_entry *, uint32_t,
+ struct adjout_prefix *);
void adjout_prefix_update(struct adjout_prefix *, struct rde_peer *,
struct filterstate *, struct pt_entry *, uint32_t, int);
@@ -781,13 +780,13 @@ void adjout_prefix_dump_cleanup(struct
void adjout_prefix_dump_r(struct rib_context *);
int adjout_prefix_dump_new(struct rde_peer *, uint8_t,
unsigned int, void *,
- void (*)(struct rde_peer *, struct pt_entry *,
- struct adjout_prefix *, void *),
+ void (*)(struct pt_entry *, struct adjout_prefix *,
+ uint32_t, void *),
void (*)(void *, uint8_t), int (*)(void *));
int adjout_prefix_dump_subtree(struct rde_peer *,
struct bgpd_addr *, uint8_t, unsigned int, void *,
- void (*)(struct rde_peer *, struct pt_entry *,
- struct adjout_prefix *, void *),
+ void (*)(struct pt_entry *, struct adjout_prefix *,
+ uint32_t, void *),
void (*)(void *, uint8_t), int (*)(void *));
void adjout_peer_init(struct rde_peer *);
void adjout_peer_flush_pending(struct rde_peer *);
Index: rde_adjout.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
diff -u -p -r1.19 rde_adjout.c
--- rde_adjout.c 20 May 2026 18:33:21 -0000 1.19
+++ rde_adjout.c 21 May 2026 20:41:05 -0000
@@ -479,21 +479,15 @@ adjout_prefix_with_attrs(struct pt_entry
* Returns NULL if not found.
*/
struct adjout_prefix *
-adjout_prefix_first(struct rde_peer *peer, struct pt_entry *pte)
+adjout_prefix_first(struct pt_entry *pte, uint32_t bid)
{
struct adjout_prefix *p;
uint32_t i;
- int has_add_path = 0;
-
- if (peer_has_add_path(peer, pte->aid, CAPA_AP_SEND))
- has_add_path = 1;
for (i = 0; i < pte->adjoutlen; i++) {
p = &pte->adjout[i];
- if (bitmap_test(&p->peermap, peer->adjout_bid))
+ if (bitmap_test(&p->peermap, bid))
return p;
- if (!has_add_path && p->path_id_tx != 0)
- return NULL;
}
return NULL;
@@ -503,22 +497,19 @@ adjout_prefix_first(struct rde_peer *pee
* Return next prefix for peer after last.
*/
struct adjout_prefix *
-adjout_prefix_next(struct rde_peer *peer, struct pt_entry *pte,
+adjout_prefix_next(struct pt_entry *pte, uint32_t bid,
struct adjout_prefix *last)
{
struct adjout_prefix *p;
uint32_t i;
- if (!peer_has_add_path(peer, pte->aid, CAPA_AP_SEND))
- return NULL;
-
i = adjout_prefix_index(pte, last);
for (; i < pte->adjoutlen; i++)
if (pte->adjout[i].path_id_tx != last->path_id_tx)
break;
for (; i < pte->adjoutlen; i++) {
p = &pte->adjout[i];
- if (bitmap_test(&p->peermap, peer->adjout_bid))
+ if (bitmap_test(&p->peermap, bid))
return p;
}
@@ -599,10 +590,6 @@ static struct pt_entry *
prefix_restart(struct rib_context *ctx)
{
struct pt_entry *pte = NULL;
- struct rde_peer *peer;
-
- if ((peer = peer_get(ctx->ctx_id)) == NULL)
- return NULL;
/* be careful when this is the last reference to pte */
if (ctx->ctx_pt != NULL) {
@@ -627,10 +614,10 @@ adjout_prefix_dump_r(struct rib_context
{
struct pt_entry *pte, *next;
struct adjout_prefix *p;
- struct rde_peer *peer;
unsigned int i;
- if ((peer = peer_get(ctx->ctx_id)) == NULL)
+ /* no adjout_bid -> no adj-rib-out */
+ if (ctx->ctx_id == 0)
goto done;
if (ctx->ctx_pt == NULL && ctx->ctx_subtree.aid == AID_UNSPEC)
@@ -656,13 +643,13 @@ adjout_prefix_dump_r(struct rib_context
ctx->ctx_pt = pt_ref(pte);
return;
}
- p = adjout_prefix_first(peer, pte);
+ p = adjout_prefix_first(pte, ctx->ctx_id);
if (p == NULL)
continue;
- ctx->ctx_prefix_call(peer, pte, p, ctx->ctx_arg);
+ ctx->ctx_prefix_call(pte, p, ctx->ctx_id, ctx->ctx_arg);
}
-done:
+ done:
if (ctx->ctx_done)
ctx->ctx_done(ctx->ctx_arg, ctx->ctx_aid);
LIST_REMOVE(ctx, entry);
@@ -672,8 +659,7 @@ done:
int
adjout_prefix_dump_new(struct rde_peer *peer, uint8_t aid,
unsigned int count, void *arg,
- void (*upcall)(struct rde_peer *, struct pt_entry *,
- struct adjout_prefix *, void *),
+ void (*upcall)(struct pt_entry *, struct adjout_prefix *, uint32_t, void *),
void (*done)(void *, uint8_t),
int (*throttle)(void *))
{
@@ -681,7 +667,7 @@ adjout_prefix_dump_new(struct rde_peer *
if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
return -1;
- ctx->ctx_id = peer->conf.id;
+ ctx->ctx_id = peer->adjout_bid;
ctx->ctx_aid = aid;
ctx->ctx_count = count;
ctx->ctx_arg = arg;
@@ -701,8 +687,7 @@ adjout_prefix_dump_new(struct rde_peer *
int
adjout_prefix_dump_subtree(struct rde_peer *peer, struct bgpd_addr *subtree,
uint8_t subtreelen, unsigned int count, void *arg,
- void (*upcall)(struct rde_peer *, struct pt_entry *,
- struct adjout_prefix *, void *),
+ void (*upcall)(struct pt_entry *, struct adjout_prefix *, uint32_t, void *),
void (*done)(void *, uint8_t),
int (*throttle)(void *))
{
@@ -710,7 +695,7 @@ adjout_prefix_dump_subtree(struct rde_pe
if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
return -1;
- ctx->ctx_id = peer->conf.id;
+ ctx->ctx_id = peer->adjout_bid;
ctx->ctx_aid = subtree->aid;
ctx->ctx_count = count;
ctx->ctx_arg = arg;
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.74 rde_peer.c
--- rde_peer.c 21 May 2026 15:20:27 -0000 1.74
+++ rde_peer.c 21 May 2026 18:27:08 -0000
@@ -513,9 +513,11 @@ peer_down(struct rde_peer *peer)
* RIB walker callback for peer_delete / the reaper.
*/
static void
-peer_reaper_upcall(struct rde_peer *peer, struct pt_entry *pte,
- struct adjout_prefix *p, void *ptr)
+peer_reaper_upcall(struct pt_entry *pte, struct adjout_prefix *p,
+ uint32_t bid, void *ptr)
{
+ struct rde_peer *peer = ptr;
+
adjout_prefix_withdraw(peer, pte, p);
}
@@ -617,9 +619,11 @@ peer_stale(struct rde_peer *peer, uint8_
* Enqueue a prefix onto the update queue so it can be sent out.
*/
static void
-peer_blast_upcall(struct rde_peer *peer, struct pt_entry *pte,
- struct adjout_prefix *p, void *ptr)
+peer_blast_upcall(struct pt_entry *pte, struct adjout_prefix *p,
+ uint32_t bid, void *ptr)
{
+ struct rde_peer *peer = ptr;
+
pend_prefix_add(peer, p->attrs, pte, p->path_id_tx);
}
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
diff -u -p -r1.296 rde_rib.c
--- rde_rib.c 21 May 2026 15:20:27 -0000 1.296
+++ rde_rib.c 21 May 2026 21:13:17 -0000
@@ -507,7 +507,7 @@ rib_dump_abort(uint16_t id)
struct rib_context *ctx, *next;
LIST_FOREACH_SAFE(ctx, &rib_dumps, entry, next) {
- if (id != ctx->ctx_id)
+ if (ctx->ctx_re != NULL && id != ctx->ctx_id)
continue;
rib_dump_free(ctx);
}
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
diff -u -p -r1.196 rde_update.c
--- rde_update.c 20 May 2026 18:33:21 -0000 1.196
+++ rde_update.c 21 May 2026 09:30:15 -0000
@@ -226,7 +226,7 @@ up_generate_updates(struct rde_peer *pee
struct prefix *new;
struct adjout_prefix *p;
- p = adjout_prefix_first(peer, re->prefix);
+ p = adjout_prefix_first(re->prefix, peer->adjout_bid);
new = prefix_best(re);
while (new != NULL) {
@@ -271,8 +271,9 @@ up_generate_addpath(struct rde_peer *pee
unsigned int pidx = 0, i;
/* collect all current paths */
- head = adjout_prefix_first(peer, re->prefix);
- for (p = head; p != NULL; p = adjout_prefix_next(peer, re->prefix, p)) {
+ head = adjout_prefix_first(re->prefix, peer->adjout_bid);
+ for (p = head; p != NULL;
+ p = adjout_prefix_next(re->prefix, peer->adjout_bid, p)) {
addpath_prefix_list[pidx++] = p->path_id_tx;
if (pidx >= nitems(addpath_prefix_list))
fatalx("too many addpath paths to select from");
@@ -450,7 +451,7 @@ up_generate_default(struct rde_peer *pee
pte = pt_get(&addr, 0);
if (pte == NULL)
pte = pt_add(&addr, 0);
- p = adjout_prefix_first(peer, pte);
+ p = adjout_prefix_first(pte, peer->adjout_bid);
adjout_prefix_update(p, peer, &state, pte, 0, 1);
rde_filterstate_clean(&state);
bgpd: adjust adjout_prefix_dump code for the reaper