Re: [PATCH nf] nft_set_rbtree: Move clauses for expired nodes, last active node as leaf

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

On Tue, Jun 14, 2022 at 11:58:14AM +0200, Stefano Brivio wrote:
> On Mon, 6 Jun 2022 11:01:21 +0200
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
[...]
> > That sounds an incremental fix, I prefer this too.
> 
> ...finally posted now.

Thanks.

[...]
> > I don't see how we can obsolete "activate" operation, though, the
> > existing approach works at set element granularity.
> 
> Yes, and that's what I'm arguing against: it would be more natural, in
> a transaction, to have a single commit operation for all the elements
> at hand -- otherwise it's not so much of a transaction.
> 
> To the user it's atomic (minus bugs) because we have tricks to ensure
> it, but to the set back-ends it's absolutely not. I think we have this
> kind of situation:
> 
> 
> nft            <->     core       <->   set back-end    <->    storage
>                 |                  |                     |
> 
> hash:   transaction commit    element commit       element commit
> 
> rbtree: transaction commit    element commit       element commit
>                                                    ^ problematic to the
>                                                    point we're
>                                                    considering a
>                                                    transaction approach
> 
> pipapo: transaction commit    element commit       transaction commit
> 
> The single advantage I see of the current approach is that with the
> hash back-ends we don't need two copies of the hash table, but that
> also has the downside of the nft_set_elem_active(&he->ext, genmask)
> check in the lookup function, which should be, in relative terms, even
> more expensive than it is in the pipapo implementation, given that hash
> back-ends are (in most cases) faster.

There is also runtime set updates from packet path. In that case, we
cannot keep a copy of the data structure that is being updated from
the control plane while the packet path is also adding/deleting
entries from it.