Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: bgpd: simplify / fix path_id_tx assignment in peer_add
To:
tech@openbsd.org
Date:
Thu, 7 May 2026 11:06:20 +0200

Download raw body.

Thread
On Thu, May 07, 2026 at 10:51:30AM +0200, Claudio Jeker wrote:
> The logic in peer_add() to handle 0 path_id_tx values is a bit convoluted
> and theoretically the first peer could end up with a 0 value since the
> RB_FOREACH block is skipped.
> 
> Let's write this in a way that is more obvious.
> -- 
> :wq Claudio
> 
> Index: rde_peer.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
> diff -u -p -r1.70 rde_peer.c
> --- rde_peer.c	2 Mar 2026 12:08:30 -0000	1.70
> +++ rde_peer.c	6 May 2026 08:37:00 -0000
> @@ -192,11 +192,11 @@ peer_add(uint32_t id, struct peer_config
>  	do {
>  		struct rde_peer *p;
>  
> +		while ((peer->path_id_tx = arc4random() << 1) == 0)
> +			;

I thought the party line was to use 'continue;'. style(9) was changed
to that effect about 10 years ago.

either way is ok with me.

>  		conflict = 0;
> -		peer->path_id_tx = arc4random() << 1;
>  		RB_FOREACH(p, peer_tree, &peertable) {
> -			if (peer->path_id_tx == 0 ||
> -			    p->path_id_tx == peer->path_id_tx) {
> +			if (p->path_id_tx == peer->path_id_tx) {
>  				conflict = 1;
>  				break;
>  			}
>