On Thu, Sep 26, 2024 at 04:34:46PM +0200, Pablo Neira Ayuso wrote: > 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. Thanks for investigating! > 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. Maybe a valid measure is to sum up entries there to make sure the total remains the same (just like you did above). I'll have a look at making the test tolerant to both variants. Thanks, Phil
