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

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Aug 06, 2026 at 01:08:30PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> 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/589d243b-3d88-4138-9786-1bbb4347e79d@app.fastmail.com/