From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: bgpd: fix error handling PREFIX_FLAG_FILTERED
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Mon, 4 May 2026 22:37:12 +0200

On Mon, May 04, 2026 at 09:30:36PM +0200, Theo Buehler wrote:
> On Mon, May 04, 2026 at 09:15:11PM +0200, Claudio Jeker wrote:
> > With the introduction of 'rde rib Loc-RIB include filtered' it is possible
> > that prefixes change prefix_eligible() outcome without a change of
> > attributes or nexthop. This happens when new filters are installed.
> >  
> > In prefix_update() this takes a shortcut path but that path is missing a
> > call to prefix_evaluate() to ensure that if the filtered flag changes an
> > update is forced.
> > 
> > Below is my try to fix this. If the filtered state changed, toggle the
> > PREFIX_FLAG_FILTERED bit and then call prefix_evaluate(). If the filtered
> > flag is in sync with filtered then this can be skipped.
> 
> That makes sense.
> 
> ok
> 
> > I used p_filtered to be 0 or 1 and check against !filtered (to ensure that
> > this value is also 0 or 1). I hope that's not too much magic...
> 
> The magic seems fine. I wouldn't mind normalizing filtered = !!filtered;
> at the top, so the check could become if (p_filtered != filtered), which
> I find more readable, but we tend to avoid !! in our tree for some reason.
 
Like this?

-- 
:wq Claudio

Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
diff -u -p -r1.290 rde_rib.c
--- rde_rib.c	17 Mar 2026 09:29:29 -0000	1.290
+++ rde_rib.c	4 May 2026 20:34:58 -0000
@@ -862,6 +862,8 @@ prefix_update(struct rib *rib, struct rd
 	struct rde_community	*comm, *ncomm = &state->communities;
 	struct prefix		*p;
 
+	filtered = !!filtered;	/* normalize value for later */
+
 	/*
 	 * First try to find a prefix in the specified RIB.
 	 */
@@ -872,13 +874,22 @@ prefix_update(struct rib *rib, struct rd
 		    prefix_nhflags(p) == state->nhflags &&
 		    communities_equal(ncomm, prefix_communities(p)) &&
 		    path_equal(nasp, prefix_aspath(p))) {
+			int p_filtered;
+
 			/* no change, update last change */
 			p->lastchange = getmonotime();
 			p->validation_state = state->vstate;
-			if (filtered)
-				p->flags |= PREFIX_FLAG_FILTERED;
-			else
-				p->flags &= ~PREFIX_FLAG_FILTERED;
+			p_filtered = (p->flags & PREFIX_FLAG_FILTERED) != 0;
+			/* check if filtered flag changed */
+			if (p_filtered != filtered) {
+				struct rib_entry	*re;
+
+				/* toggle filtered flag */
+				p->flags ^= PREFIX_FLAG_FILTERED;
+				/* make route decision */
+				re = rib_get_addr(rib, prefix, prefixlen);
+				prefix_evaluate(re, p, p);
+			}
 			return (0);
 		}
 	}