Re: update element timeout support [was 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:
> > Right, I think that will work.
> > For rbtree, sync gc is kept in place, elements are not zapped,
> > they get tagged as DEAD, including the end element.
> > 
> > Then from commit, do full scan and remove any and all elements
> > that are flagged as DEAD or have expired.
> 
> Sounds good.
> 
> Would you follow this approach to fix the existing issue with the
> rbtree on-demand GC in nf.git?

Actually, I don't see why its needed. With your proposal
to make the "is_expired" check during transaction consistently based on
a fixed tstamp, expiry during transaction becomes impossible.
So we can keep immediate rb_erase around.

I suggest to take my proposal to erase, signal -EAGAIN to caller,
then have caller retry.  Apply this to nf.git as a bug fix.

Then, I can take my patches that are mixed into the gc rework;
split those up, and we take the "no more async rbtree gc" for nf-next.

Do you still spot a problem if we retain the on-insert node erase?

To give some numbers (async gc disabled):

Insert 20k ranges into rbtree (takes ~4minutes).
Wait until all have expired.
Insert a single range: takes 250ms (entire tree has to be purged).

Don't think it will be any faster with dead-bit approach,
we simply move cost to later in the transaction.

The only nf.git "advantage" is that we typically won't have
to zap the entire tree during transaction, but thats due to
async gc and I'd rather remove it.

What do you think?