Re: [PATCH nf 1/2] netfilter: nft_set_rbtree: move sync GC from insert path to set->ops->commit

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> - read spin lock is required for the sync GC to make sure this does
>   not zap entries that are being used from the datapath.

It needs to grab the write spinlock for each rb_erase, plus
the seqcount increase to make sure that parallel lookup doesn't
miss an element in the tree.

> - the full GC batch could be used to amortize the memory allocation
>   (not only two slots as it happens now, I am recycling an existing
>    function).

Yes.

> - ENOMEM on GC sync commit path could be an issue. It is too late to
>   fail. The tree would start collecting expired elements that might
>   duplicate existing, triggering bogus mismatches. In this path the
>   commit_mutex is held, and this set backend does not support for
>   lockless read,

It does.  If lockless doesn't return a match it falls back to readlock.

>   it might be possible to simply grab the spinlock
>   in write mode and release entries inmediately, without requiring the
>   sync GC batch infrastructure that pipapo is using.

Is there evidence that the on-demand GC is a problem?
It only searches in the relevant subtree, it should rarely, if ever,
encounter any expired element.