Re: [nft PATCH] tests: shell: Fix sets/reset_command_0 for current kernels

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

Hi Phil,

Picking up on this because I still see:

W: [FAILED]     331/389 testcases/sets/reset_command_0

here, maybe you can merge this change now? 6.5.x -stable will also
enter EoL in one more.

More comments below regarding your open questions.

On Tue, Nov 07, 2023 at 11:38:18AM +0100, Phil Sutter wrote:
> On Thu, Nov 02, 2023 at 09:32:30PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Nov 02, 2023 at 06:06:34PM +0100, Phil Sutter wrote:
> > > On Thu, Nov 02, 2023 at 04:29:34PM +0100, Thomas Haller wrote:
> > > > On Thu, 2023-11-02 at 16:03 +0100, Phil Sutter wrote:
> > > > >  
> > > > > +# Note: Element expiry is no longer reset since kernel commit
> > > > > 4c90bba60c26
> > > > > +# ("netfilter: nf_tables: do not refresh timeout when resetting
> > > > > element"),
> > > > > +# the respective parts of the test have therefore been commented
> > > > > out.
> > > > 
> > > > Hi Phil,
> > > > 
> > > > do you expect that the old behavior ever comes back?
> > > 
> > > A recent nfbz comment[1] from Pablo made me doubt the decision is final,
> > > though I may have misread it.
> > 
> > I hesitate on changing --stateless behaviour, but I don't find a
> > usecase for this option all alone unless it is combined with --terse,
> > to store an initial ruleset skeleton with no elements and no states.
> > Sets with timeout likely contain elements that get dynamically added
> > either via control plane or packet path based on some heuristics.
> 
> Unrelated to the expires vs. reset question, I wonder if one should
> treat set elements with timeout as state themselves. If one leaves the
> ruleset alone for long enough, they all will eventually vanish. So one
> may argue the ruleset in its stateless form does not have elements in a
> set with defined timeout.

The only usecase I can find for --stateless is diff'ing outputs
between two delta in time, to see what new elements are added and what
are gone. So I inclined to leave --stateless as is now.

> > > > Why keep the old checks (commented out)? Maybe drop them? We can get it
> > > > from git history.
> > > 
> > > Should the change be permanent, one should change the tests to assert
> > > the opposite, namely that expires values are unaffected by the reset.
> > 
> > I think it is fine as it is now in the kernel. I have posted patches
> > to allow to update element timeouts via transaction, which looks more
> > flexible and run through the transaction path. As for counter and
> > quota, users likely only want to either: 1) restore a previous state
> > (after reboot) or 2) dump-and-reset counters for stats collection
> > (e.g. fetch counters at the end of the day).
> 
> I still doubt there's a use-case to do (1) or (2) in sets with
> temporary elements.

For the reboot case, restoring temporary elements (which were added
via datapath) might make sense to me.

But there are limitations: connlimit is one of them because the
internal state of this datastructure gets losts between reboots.