From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: bgpd: fix for bgpctl show rib out <IP>
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Wed, 20 May 2026 16:22:43 +0200

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.

-- 
: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);