From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix for bgpctl show rib out <IP>
To: tech@openbsd.org
Date: Wed, 20 May 2026 16:38:46 +0200

On Wed, May 20, 2026 at 04:22:43PM +0200, Claudio Jeker wrote:
> On Wed, May 20, 2026 at 11:50:05AM +0200, Theo Buehler wrote:
> > On Wed, May 20, 2026 at 11:44:18AM +0200, Claudio Jeker wrote:
> > > There is an error when it comes to handling
> > > 	bgpctl show rib out <IP>
> > > queries. If the IP to lookup up is covered by a not exported route but
> > > instead by a less specific one the lookup fails.
> > > 
> > > For example in this setup where the /26 route is filtered:
> > > bgpctl show rib 
> > > flags  vs destination          gateway          lpref   med aspath origin
> > > AI*>  N-? 192.0.2.0/24         0.0.0.0           100     0 i
> > > AI*>  N-? 192.0.2.0/26         0.0.0.0           100     0 i
> > > bgpctl show rib out
> > > flags  vs destination          gateway          lpref   med aspath origin
> > > AI*   N-? 192.0.2.0/24         10.83.0.138       100     0 i
> > > bgpctl show rib out 192.0.2.0/24
> > > flags  vs destination          gateway          lpref   med aspath origin
> > > AI*   N-? 192.0.2.0/24         10.83.0.138       100     0 i
> > > 
> > > but
> > > bgpctl show rib out 192.0.2.1
> > > flags  vs destination          gateway          lpref   med aspath origin
> > > 
> > > The last command fails even though there is a route for 192.0.2.1 it just
> > > hides behind 192.0.2.0/26.
> > > 
> > > The diff below fixes this. In the pt_lookup() case one needs to walk up
> > > all possible prefixes to see if a covering route is actually exported.
> > > 
> > > Not super fond of this diff since it adds yet another loop into a nested
> > > mess. This should all be refactored but I'm not doing that right now.
> > 
> > Yeah, well... Diff makes sense and if it works that's better than leaving
> > it broken.
> > 
> > ok tb
> 
> I realized that I sent out and committed an older version of what I
> wanted. Here is a fixup for that.
> 
> The loop needs to be done a bit differently to properly terminate in all
> cases. In the not-found case where req->prefixlen != hostplen we currently
> end up spinning forever. There is a similar issue if pte->prefixlen = 0
> and not-found.
> 
> In both cases the do { } while (!found && pte != NULL); keeps spinning.
> The fix for this is below. Force pte to NULL before doing the extra
> lookup. Because of that set plen early and use a while loop.

							Can't say I'm
							surprised. It's
							very difficult
							to read this
							kind of code. It
							does look a bit
							better in this
							second version.

							ok tb


> 
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> diff -u -p -r1.701 rde.c
> --- rde.c	20 May 2026 09:56:56 -0000	1.701
> +++ rde.c	20 May 2026 14:05:58 -0000
> @@ -3382,14 +3382,16 @@ rde_dump_ctx_new(struct ctl_show_rib_req
>  						    pte, p, ctx);
>  						found = 1;
>  					}
> +					plen = pte->prefixlen - 1;
> +					pte = NULL;
>  					if (!found &&
>  					    req->prefixlen == hostplen) {
> -						for (plen = pte->prefixlen - 1;
> -						    plen >= 0; plen--) {
> +						while (plen >= 0) {
>  							pte = pt_get(
>  							    &req->prefix, plen);
>  							if (pte != NULL)
>  								break;
> +							plen--;
>  						}
>  					}
>  				} while (!found && pte != NULL);