Re: [RFC nf] netfilter: nf_tables: nft_set_rbtree: invalidate greater range element on removal

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Main agenda here is to not just fix the spurious failure but to
> > get rid of the async gc worker.
> 
> I would like to move this sync GC collection from insert() path, it is
> sloppy and zapping entries that we hold references to as in this case.
> I would like to move to use the .commit phase just like pipapo.

I can experiment with this next week.

I already have a patch that converts async to sync gc similar to
pipapo but it currently keeps the limited on-demand cycle too.

> The only solution I can see right now is to maintain two copies of the
> rbtree, just like pipapo, then use the .commit phase, I started
> sketching this updates.

I would like to avoid this, see below.

> Meanwhile setting rbe_ge and rbe_le to NULL if the element that is
> referenced is removed makes sense to me.

Great, I will submit this patch formally with a slightly updated
commit message.

> The current GC sync inlined in insert() is also making it hard to
> support for timeout refresh (element update command) without
> reintroducing the _BUSY bit, which is something I would like to skip.

Ugh, yes, no busy bit please.

> Then, there is another possibility that is to provide a hint to
> userspace to use pipapo instead rbtree, via _GENMSG, but there is a
> need to update pipapo to allow for singleton sets (with no
> concatenation), which requires a oneliner in the kernel.
>
> The rbtree set backend is the corner that holds more technical debt
> IMO.

I'm all in favor of getting rid of rbtree where possible.
So we can keep it in-tree with 'acceptable' shortcomings (= no crashes)
but userspace would no longer use it.