From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: bgpd: rework rde out filters
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Fri, 13 Feb 2026 13:39:23 +0100

On Fri, Feb 13, 2026 at 12:22:07AM +0100, Theo Buehler wrote:
> On Thu, Feb 12, 2026 at 04:40:10PM +0100, Claudio Jeker wrote:
> > The out filters are currently very expensive because at 700 peers
> > rde_filter_out() is chasing memory all over the place and spends lot of
> > time waiting for RAM access into pretty fat objects.
> > 
> > Do the same thing as was done for filter_sets. Use a rde_filter struct
> > that uses an array of match rules and on top of this use a hash table
> > and refcnt to dedup equal filters.
> > 
> > This diff reduces the initial load of my test IXP RS setup from 25min to
> > around 18min. So this change is a significant speedup on busy systems.
> > 
> > I want to apply the same for inbound filters but that requires a fair
> > amount of reshuffling which will take some time.
> > 
> 
> ok tb
> 
> Two very minor comments below.
> 
> [...]
> 
> > 
> > Index: usr.sbin/bgpd/rde_filter.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
> > diff -u -p -r1.144 rde_filter.c
> > --- usr.sbin/bgpd/rde_filter.c	11 Feb 2026 12:25:57 -0000	1.144
> > +++ usr.sbin/bgpd/rde_filter.c	11 Feb 2026 22:03:30 -0000
> > @@ -56,6 +56,20 @@ struct rde_filter_set {
> >  	struct rde_filter_set_elm	set[0];
> >  };
> >  
> > +struct rde_filter_rule {
> > +	struct filter_match		match;
> > +	struct rde_filter_set		*rde_set;
> > +	enum filter_actions		action;
> > +	uint8_t				quick;
> 
> Nit: we usually indent with an extra space when there's no *

Yes I know. I'm not so fuzzed about this extra space. It often just
adds noise in later changes. Now there is precedence in the same file so I
followed style.
 
> [...]
> 
> > +static int
> > +rde_filtertable_equal(const struct rde_filter *arf,
> > +    const struct rde_filter *brf)
> > +{
> > +	if (arf->len != brf->len)
> > +		return 0;
> > +	if (memcmp(arf->rules, brf->rules,
> > +	    arf->len * sizeof(arf->rules[0])) != 0)
> > +		return 0;
> > +	return 1;
> 
> Nit: I'd write this as
> 
> 	return memcmp(...) == 0;
 
Sure can do. I think any resonably competent compiler will produce the
same code for both versions.

-- 
:wq Claudio