Re: [PATCH nf,v2 2/2] netfilter: nf_tables: call set ops .commit when building new ruleset

Pablo Neira Ayuso <[email protected]>
Newsgroups gmane.comp.security.firewalls.netfilter.devel
Message-ID <anSe0eK9dl1d2J7-@chamomile>
On Thu, Aug 06, 2026 at 01:08:30PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <[email protected]> wrote:
> > The rbtree set only builds the b-search array after the new ruleset has
> > been exposed through set ops .commit.
> > 
> > This is currently needed by pipapo because it purges the elements from
> > the clone after the transactions are handled, therefore, pipapo still
> > needs the delayed set ops .commit call after the transaction handling.
> > 
> > Allow the rbtree to call .commit before the transaction handling which
> > purges the stale elements from the frontend rbtree datastructure.
> > 
> > Update rbtree .commit to skip deactivated and expired elements when
> > building the new b-search array.
> 
> I'm not following, sorry.  What's the difference between pipapo and rbtree?

pipapo needs to call nft_setelem_remove() to purge the deactivated
elements from priv->clone, then the new version of the pipapo lookup
table datastructure can be published.

The rbtree does not need to wait to call nft_setelem_remove() since it
updates the rbtree frontend datastructure that is only exposed to
control plane.

> static void nft_pipapo_commit(struct nft_set *set)
> {
> [..]
>         if (time_after_eq(jiffies, priv->last_gc + nft_set_gc_interval(set)))
>                 pipapo_gc_scan(set, priv->clone);
> 
>         old = rcu_replace_pointer(priv->match, priv->clone,
>                                   nft_pipapo_transaction_mutex_held(set));
> 
> [..] after this, new incarnation is live.
> 
> static void nft_rbtree_commit(struct nft_set *set)
> {
> [..]
>         if (time_after_eq(jiffies, priv->last_gc + nft_set_gc_interval(set)))
>                 nft_rbtree_gc_scan(set);
> 
> [ ... build the new blob ... ]
> 
> err_out:
>         priv->array_next->num_intervals = num_intervals;
>         old = rcu_replace_pointer(priv->array, priv->array_next,
>                                   lockdep_is_held(&nft_pernet(read_pnet(&set->net))->commit_mutex));
> 
> 
> [..] after this, new incarnation is live.
> 
> What is the problem? These two functions do the same thing, no?

Apparently yes, but...

> Why must the blob be rebuilt before stale node purge in rbtree case?

... there is a gap between the ruleset blob is built and published and
the set .commit interface is called to publish the new version of the
rbtree/pipapo datastructure.

See:
https://lore.kernel.org/netfilter-devel/[email protected]/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.