From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix accounting errors for pending updates
To: tech@openbsd.org
Date: Thu, 13 Mar 2025 16:30:36 +0100

On Thu, Mar 13, 2025 at 04:22:58PM +0100, Claudio Jeker wrote:
> With the introduction of the adj-rib-out cache the pending update count
> could go off (wrap around 0).
> 
> There are two issues:
> - In peer_blast_upcall() we need to increase pending_update for every
>   prefix we insert.
> - In prefix_adjout_flush_pending() the EoR marker needs special handling.
>   The EoR marker is not counted in pending_update but when unlinking it
>   we need to free the marker. It will be inserted again when we blast
>   out the table.

All this makes sense.

> With these two diffs the pending updates is now correct in my tests.
> The pending withdraws does not have this issue since the accouting already
> happens in prefix_adjout_destroy().

Yes, that looks right, too.

ok tb

> -- 
> :wq Claudio
> 
> Index: rde_peer.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
> diff -u -p -r1.47 rde_peer.c
> --- rde_peer.c	20 Feb 2025 19:47:31 -0000	1.47
> +++ rde_peer.c	12 Mar 2025 13:33:01 -0000
> @@ -523,14 +523,18 @@ peer_stale(struct rde_peer *peer, uint8_
>  static void
>  peer_blast_upcall(struct prefix *p, void *ptr)
>  {
> +	struct rde_peer		*peer;
> +
>  	if (p->flags & PREFIX_FLAG_DEAD) {
>  		/* ignore dead prefixes, they will go away soon */
>  	} else if ((p->flags & PREFIX_FLAG_MASK) == 0) {
> +		peer = prefix_peer(p);
>  		/* put entries on the update queue if not already on a queue */
>  		p->flags |= PREFIX_FLAG_UPDATE;
> -		if (RB_INSERT(prefix_tree, &prefix_peer(p)->updates[p->pt->aid],
> +		if (RB_INSERT(prefix_tree, &peer->updates[p->pt->aid],
>  		    p) != NULL)
>  			fatalx("%s: RB tree invariant violated", __func__);
> +		peer->stats.pending_update++;
>  	}
>  }
>  
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> diff -u -p -r1.267 rde_rib.c
> --- rde_rib.c	14 Jan 2025 12:24:23 -0000	1.267
> +++ rde_rib.c	12 Mar 2025 16:21:28 -0000
> @@ -1430,7 +1430,11 @@ prefix_adjout_flush_pending(struct rde_p
>  		RB_FOREACH_SAFE(p, prefix_tree, &peer->updates[aid], np) {
>  			p->flags &= ~PREFIX_FLAG_UPDATE;
>  			RB_REMOVE(prefix_tree, &peer->updates[aid], p);
> -			peer->stats.pending_update--;
> +			if (p->flags & PREFIX_FLAG_EOR) {
> +				prefix_adjout_destroy(p);
> +			} else {
> +				peer->stats.pending_update--;
> +			}
>  		}
>  	}
>  }
>