Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller <thaller@redhat.com>]

On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hi Thomas,
> > 
> > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > 
> > > Hi,
> > > 
> > > Could you provide more details?
> > > 
> > > For example,
> > > 
> > >     make -j && ./tests/shell/run-tests.sh
> > > tests/shell/testcases/include/0007glob_double_0 -x
> > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > 
> > # cat [...]/ruleset-diff.json
> > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15
> > 13:27:20.272084254 +0100
> > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-
> > include-0007glob_double_0.1/ruleset-after.json      2023-11-16
> > 17:06:18.332535411 +0100
> > @@ -1 +1 @@
> > -{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip",
> > "name": "y", "handle": 2}}]}
> > +{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip",
> > "name": "y", "handle": 159}}]}
> > 
> > It seems that handles are a problem in this diff.
> 
> Are you running tests with -s option?
> 
> In that case, modules are removed after each test.
> 
> I suspect its because we can then hit -EAGAIN mid-transaction
> because module is missing (again), then replay logic does its
> thing.
> 
> But the handle generator isn't transaction aware,
> so it has advanced vs. the aborted partial transaction.

> I'm not sure what to do here.

a combination of:

a) make an effort, that kernel behavior is consistent and reproducible.
Stable output seems important to me, and the automatic loading of a
kernel module should not make a difference. This is IMO a bug.

b) let `nft -j list ruleset` honor (the lack of) `--handle` option and
not print those handles. That bugfix would change behavior, so maybe
instead add a "--no-handle" option for `nft -j` dumps. 

c) sanitize the output with the sed command (my other mail).



This also means, that the .json-nft dumps won't work, if you run
without `unshare`. IMO, the mode without unshare should not be
supported anymore. But if it's deemed important, then it requires b) or
c) or detect the case and skip the diffs with .json-nft.


Thomas