Re: [PATCH nft,v2 5/7] cache: consolidate reset command

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

On Thu, Sep 26, 2024 at 12:47:30AM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Mon, Aug 26, 2024 at 10:54:53AM +0200, Pablo Neira Ayuso wrote:
> > Reset command does not utilize the cache infrastructure.
> 
> This commit changes audit log output for some reason. At least I see
> tools/testing/selftests/net/netfilter/nft_audit.sh failing and git
> bisect pointed at it. The relevant kselftest output is:
> 
> # testing for cmd: nft reset rules ... FAIL
> #  table=t1 family=2 entries=3 op=nft_reset_rule
> #  table=t2 family=2 entries=3 op=nft_reset_rule
> #  table=t2 family=2 entries=3 op=nft_reset_rule
> # -table=t2 family=2 entries=180 op=nft_reset_rule
> # +table=t2 family=2 entries=186 op=nft_reset_rule
> #  table=t2 family=2 entries=188 op=nft_reset_rule
> # -table=t2 family=2 entries=135 op=nft_reset_rule
> # +table=t2 family=2 entries=129 op=nft_reset_rule
> 
> I don't know why entries value changes and whether it is expected or
> not. Could you perhaps have a look?

Before my patch, there is a single dump request to the kernel:

  rule_cache_dump table (null) chain (null) rule_handle 0 dump 1 reset 1

the skbuff already contains 6 entries for t1/c1 and t1/c2, which why
180 entries of t2 fit into the skbuff is delivered to userspace:

  table=t2 family=2 entries=180 op=nft_reset_rule

(it seems 186 rule entries can fit into the skbuff).

after my patch, there is one for each table:

  rule_cache_dump table t1 chain (null) rule_handle 0 dump 1 reset 1
  rule_cache_dump table t2 chain (null) rule_handle 0 dump 1 reset 1

the skbuff is empty when dumping t2, so we can fit 6 more entries:

table=t2 family=2 entries=186 op=nft_reset_rule

If you take the number, before patch:

180+188+135=503

after patch:

186+188+129=503

show that is the same number of entries. Behaviour is correct.

I don't know how to fix this test without removing the check for
'reset rules', because it will break between different nftables
versions (due to the different strategies to dump all rules vs. dump
rules per table).

And I don't think it makes sense to revert my userspace update just to
make this test happy.