From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix error handling PREFIX_FLAG_FILTERED
To: tech@openbsd.org
Date: Mon, 4 May 2026 22:41:02 +0200

On Mon, May 04, 2026 at 10:37:12PM +0200, Claudio Jeker wrote:
> 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?

Yes, that's what I meant. I'm ok with either version.

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