From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix peer match logic
To: tech@openbsd.org
Date: Mon, 28 Oct 2024 14:00:05 +0100

On Mon, Oct 28, 2024 at 01:35:16PM +0100, Claudio Jeker wrote:
> I noticed that the "no such peer" logic is actually incorrect.
> We want to send an error when there was no match but only if there are
> neighbors configured. So the check for RB_EMPTY() needs to be reversed.

Right. ok tb

> Another option is to remove the RB_EMPTY() checks all together. Then
> 'bgpctl show' would print 'no such neighbor' when no neighbor is
> configured. Do we care about that?

No real opinion. I generally don't like this kind of noise. If it came
with a significant simplification I'd say go for it, but here it doesn't
really make much of a difference.

> -- 
> :wq Claudio
> 
> Index: control.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
> diff -u -p -r1.120 control.c
> --- control.c	1 Oct 2024 18:31:10 -0000	1.120
> +++ control.c	28 Oct 2024 12:07:40 -0000
> @@ -352,7 +352,7 @@ control_dispatch_msg(struct pollfd *pfd,
>  					}
>  				}
>  			}
> -			if (!matched && RB_EMPTY(peers)) {
> +			if (!matched && !RB_EMPTY(peers)) {
>  				control_result(c, CTL_RES_NOSUCHPEER);
>  			} else if (!neighbor.show_timers) {
>  				imsg_ctl_rde_msg(IMSG_CTL_END, 0, pid);
> @@ -473,7 +473,7 @@ control_dispatch_msg(struct pollfd *pfd,
>  			RB_FOREACH(p, peer_head, peers)
>  				if (peer_matched(p, &ribreq.neighbor))
>  					break;
> -			if (p == NULL && RB_EMPTY(peers)) {
> +			if (p == NULL && !RB_EMPTY(peers)) {
>  				control_result(c, CTL_RES_NOSUCHPEER);
>  				break;
>  			}
>