[iptables] tests: shell: Stabilize nft-only/0009-needless-bitwise_0

Netlink debug output varies depending on host's endianness and therefore
the test fails on Big Endian machines. Since for the sake of asserting
no needless bitwise expressions in output the actual data values are not
relevant, simply crop the output to just the expression names.

Fixes: 81a2e12851283 ("tests/shell: Add test for bitwise avoidance fixes")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../nft-only/0009-needless-bitwise_0          | 413 +++++++++---------
 1 file changed, 208 insertions(+), 205 deletions(-)

On Fri, Nov 20, 2020 at 06:57:57PM +0100, Phil Sutter wrote:
> Netlink debug output varies depending on host's endianness and therefore
> the test fails on Big Endian machines. Since for the sake of asserting
> no needless bitwise expressions in output the actual data values are not
> relevant, simply crop the output to just the expression names.

Probably we can fix this in libnftnl before we apply patches like this
to nft as well?

> Fixes: 81a2e12851283 ("tests/shell: Add test for bitwise avoidance fixes")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  .../nft-only/0009-needless-bitwise_0          | 413 +++++++++---------
>  1 file changed, 208 insertions(+), 205 deletions(-)
> 
> diff --git a/iptables/tests/shell/testcases/nft-only/0009-needless-bitwise_0 b/iptables/tests/shell/testcases/nft-only/0009-needless-bitwise_0
> index 41d765e537312..0254208bcf69f 100755
> --- a/iptables/tests/shell/testcases/nft-only/0009-needless-bitwise_0
> +++ b/iptables/tests/shell/testcases/nft-only/0009-needless-bitwise_0
> @@ -1,4 +1,4 @@
> -#!/bin/bash -x
> +#!/bin/bash
>  
>  [[ $XT_MULTI == *xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
>  set -e
> @@ -53,287 +53,290 @@ ff:00:00:00:00:00
>  ) | $XT_MULTI ebtables-restore
>  
>  EXPECT="ip filter OUTPUT 4
> -  [ payload load 4b @ network header + 16 => reg 1 ]
> -  [ cmp eq reg 1 0x0302010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip filter OUTPUT 5 4
> -  [ payload load 4b @ network header + 16 => reg 1 ]
> -  [ cmp eq reg 1 0x0302010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip filter OUTPUT 6 5
> -  [ payload load 4b @ network header + 16 => reg 1 ]
> -  [ bitwise reg 1 = ( reg 1 & 0xfcffffff ) ^ 0x00000000 ]
> -  [ cmp eq reg 1 0x0002010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ bitwise ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip filter OUTPUT 7 6
> -  [ payload load 3b @ network header + 16 => reg 1 ]
> -  [ cmp eq reg 1 0x0002010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip filter OUTPUT 8 7
> -  [ payload load 2b @ network header + 16 => reg 1 ]
> -  [ cmp eq reg 1 0x0000010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip filter OUTPUT 9 8
> -  [ payload load 1b @ network header + 16 => reg 1 ]
> -  [ cmp eq reg 1 0x0000000a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip filter OUTPUT 10 9
> -  [ counter pkts 0 bytes 0 ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 4
> -  [ payload load 16b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x06050403 0x0a090807 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 5 4
> -  [ payload load 16b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x06050403 0x0a090807 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 6 5
> -  [ payload load 16b @ network header + 24 => reg 1 ]
> -  [ bitwise reg 1 = ( reg 1 & 0xffffffff 0xffffffff 0xffffffff 0xf0ffffff ) ^ 0x00000000 0x00000000 0x00000000 0x00000000 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x06050403 0x00090807 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ bitwise ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 7 6
> -  [ payload load 15b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x06050403 0x00090807 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 8 7
> -  [ payload load 14b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x06050403 0x00000807 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 9 8
> -  [ payload load 11b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x00050403 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 10 9
> -  [ payload load 10b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee 0x00000403 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 11 10
> -  [ payload load 8b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x020100ee ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 12 11
> -  [ payload load 6b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0xffc0edfe 0x000000ee ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 13 12
> -  [ payload load 2b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x0000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 14 13
> -  [ payload load 1b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x000000fe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  ip6 filter OUTPUT 15 14
> -  [ counter pkts 0 bytes 0 ]
> +  [ counter ]
>  
>  arp filter OUTPUT 3
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 4b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x0302010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 4 3
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 4b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x0302010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 5 4
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 4b @ network header + 24 => reg 1 ]
> -  [ bitwise reg 1 = ( reg 1 & 0xfcffffff ) ^ 0x00000000 ]
> -  [ cmp eq reg 1 0x0002010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ bitwise ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 6 5
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 3b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x0002010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 7 6
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 2b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x0000010a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 8 7
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 1b @ network header + 24 => reg 1 ]
> -  [ cmp eq reg 1 0x0000000a ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 9 8
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 10 9
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 6b @ network header + 18 => reg 1 ]
> -  [ cmp eq reg 1 0xc000edfe 0x0000eeff ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 11 10
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 6b @ network header + 18 => reg 1 ]
> -  [ bitwise reg 1 = ( reg 1 & 0xffffffff 0x0000f0ff ) ^ 0x00000000 0x00000000 ]
> -  [ cmp eq reg 1 0xc000edfe 0x0000e0ff ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ bitwise ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 12 11
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 5b @ network header + 18 => reg 1 ]
> -  [ cmp eq reg 1 0xc000edfe 0x000000ff ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 13 12
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 4b @ network header + 18 => reg 1 ]
> -  [ cmp eq reg 1 0xc000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 14 13
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 3b @ network header + 18 => reg 1 ]
> -  [ cmp eq reg 1 0x0000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 15 14
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 2b @ network header + 18 => reg 1 ]
> -  [ cmp eq reg 1 0x0000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  arp filter OUTPUT 16 15
> -  [ payload load 2b @ network header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x00000100 ]
> -  [ payload load 1b @ network header + 4 => reg 1 ]
> -  [ cmp eq reg 1 0x00000006 ]
> -  [ payload load 1b @ network header + 5 => reg 1 ]
> -  [ cmp eq reg 1 0x00000004 ]
> -  [ payload load 1b @ network header + 18 => reg 1 ]
> -  [ cmp eq reg 1 0x000000fe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 4
> -  [ payload load 6b @ link header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0xc000edfe 0x0000eeff ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 5 4
> -  [ payload load 6b @ link header + 0 => reg 1 ]
> -  [ bitwise reg 1 = ( reg 1 & 0xffffffff 0x0000f0ff ) ^ 0x00000000 0x00000000 ]
> -  [ cmp eq reg 1 0xc000edfe 0x0000e0ff ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ bitwise ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 6 5
> -  [ payload load 5b @ link header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0xc000edfe 0x000000ff ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 7 6
> -  [ payload load 4b @ link header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0xc000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 8 7
> -  [ payload load 3b @ link header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x0000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 9 8
> -  [ payload load 2b @ link header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x0000edfe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  
>  bridge filter OUTPUT 10 9
> -  [ payload load 1b @ link header + 0 => reg 1 ]
> -  [ cmp eq reg 1 0x000000fe ]
> -  [ counter pkts 0 bytes 0 ]
> +  [ payload ]
> +  [ cmp ]
> +  [ counter ]
>  "
>  
> -diff -u -Z <(echo "$EXPECT") <(nft --debug=netlink list ruleset | awk '/^table/{exit} {print}')
> +diff -u -Z <(echo "$EXPECT") <(nft --debug=netlink list ruleset | awk '
> +	/^table/{exit}
> +	{print gensub(/\[ ([^ ]+) .* ]/,"[ \\1 ]", "g")}'
> +)
> -- 
> 2.28.0
>

Hi,

On Fri, Nov 20, 2020 at 07:50:00PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Nov 20, 2020 at 06:57:57PM +0100, Phil Sutter wrote:
> > Netlink debug output varies depending on host's endianness and therefore
> > the test fails on Big Endian machines. Since for the sake of asserting
> > no needless bitwise expressions in output the actual data values are not
> > relevant, simply crop the output to just the expression names.
> 
> Probably we can fix this in libnftnl before we apply patches like this
> to nft as well?

You're right, ignoring the problems in nft testsuite is pretty
inconsistent. OTOH this is the first test that breaks iptables testsuite
on Big Endian while nft testsuite is entirely broken. ;)

I had a look at libnftnl and it seems like even kernel support is needed
to carry the endianness info from input to output. IMHO data should be
in a consistent format in netlink messages, but I fear we can't change
this anymore. I tried to print the data byte-by-byte, but we obviously
still get problems with any data in host byte order. Do you see an
easier way to fix this than adding extra info to all expressions
containing data?

Thanks, Phil

On Fri, Nov 20, 2020 at 08:37:23PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Fri, Nov 20, 2020 at 07:50:00PM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Nov 20, 2020 at 06:57:57PM +0100, Phil Sutter wrote:
> > > Netlink debug output varies depending on host's endianness and therefore
> > > the test fails on Big Endian machines. Since for the sake of asserting
> > > no needless bitwise expressions in output the actual data values are not
> > > relevant, simply crop the output to just the expression names.
> > 
> > Probably we can fix this in libnftnl before we apply patches like this
> > to nft as well?
> 
> You're right, ignoring the problems in nft testsuite is pretty
> inconsistent. OTOH this is the first test that breaks iptables testsuite
> on Big Endian while nft testsuite is entirely broken. ;)

Do you think we can fix this from the testsuite site? It would require
to replicate payload files. The snprintf printing is used for
debugging only at this stage. That would fix nft and this specific case.

> I had a look at libnftnl and it seems like even kernel support is needed
> to carry the endianness info from input to output. IMHO data should be
> in a consistent format in netlink messages, but I fear we can't change
> this anymore. I tried to print the data byte-by-byte, but we obviously
> still get problems with any data in host byte order. Do you see an
> easier way to fix this than adding extra info to all expressions
> containing data?

Probably we can make assumptions based on context, such as payload
expression always express things in network byte order, and annotate
that such register stores something in network byteorder. For meta,
assume host byte order. Unless there is an explicit byteorder
expression.

Hi Pablo,

On Sat, Nov 21, 2020 at 01:11:54PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Nov 20, 2020 at 08:37:23PM +0100, Phil Sutter wrote:
> > Hi,
> > 
> > On Fri, Nov 20, 2020 at 07:50:00PM +0100, Pablo Neira Ayuso wrote:
> > > On Fri, Nov 20, 2020 at 06:57:57PM +0100, Phil Sutter wrote:
> > > > Netlink debug output varies depending on host's endianness and therefore
> > > > the test fails on Big Endian machines. Since for the sake of asserting
> > > > no needless bitwise expressions in output the actual data values are not
> > > > relevant, simply crop the output to just the expression names.
> > > 
> > > Probably we can fix this in libnftnl before we apply patches like this
> > > to nft as well?
> > 
> > You're right, ignoring the problems in nft testsuite is pretty
> > inconsistent. OTOH this is the first test that breaks iptables testsuite
> > on Big Endian while nft testsuite is entirely broken. ;)
> 
> Do you think we can fix this from the testsuite site? It would require
> to replicate payload files. The snprintf printing is used for
> debugging only at this stage. That would fix nft and this specific case.
> 
> > I had a look at libnftnl and it seems like even kernel support is needed
> > to carry the endianness info from input to output. IMHO data should be
> > in a consistent format in netlink messages, but I fear we can't change
> > this anymore. I tried to print the data byte-by-byte, but we obviously
> > still get problems with any data in host byte order. Do you see an
> > easier way to fix this than adding extra info to all expressions
> > containing data?
> 
> Probably we can make assumptions based on context, such as payload
> expression always express things in network byte order, and annotate
> that such register stores something in network byteorder. For meta,
> assume host byte order. Unless there is an explicit byteorder
> expression.

I like this simple approach, but it's not easy to implement: libnftnl
doesn't know about other expressions, so 'cmp' for instance doesn't know
which expression stored data in reg 1 and therefore can't deduce the
likely endianness of its stored data.

Any idea how to solve that?

Thanks, Phil

On Mon, Nov 23, 2020 at 01:13:21AM +0100, Phil Sutter wrote:
> Hi Pablo,
> 
> On Sat, Nov 21, 2020 at 01:11:54PM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Nov 20, 2020 at 08:37:23PM +0100, Phil Sutter wrote:
> > > Hi,
> > > 
> > > On Fri, Nov 20, 2020 at 07:50:00PM +0100, Pablo Neira Ayuso wrote:
> > > > On Fri, Nov 20, 2020 at 06:57:57PM +0100, Phil Sutter wrote:
> > > > > Netlink debug output varies depending on host's endianness and therefore
> > > > > the test fails on Big Endian machines. Since for the sake of asserting
> > > > > no needless bitwise expressions in output the actual data values are not
> > > > > relevant, simply crop the output to just the expression names.
> > > > 
> > > > Probably we can fix this in libnftnl before we apply patches like this
> > > > to nft as well?
> > > 
> > > You're right, ignoring the problems in nft testsuite is pretty
> > > inconsistent. OTOH this is the first test that breaks iptables testsuite
> > > on Big Endian while nft testsuite is entirely broken. ;)
> > 
> > Do you think we can fix this from the testsuite site? It would require
> > to replicate payload files. The snprintf printing is used for
> > debugging only at this stage. That would fix nft and this specific case.
> > 
> > > I had a look at libnftnl and it seems like even kernel support is needed
> > > to carry the endianness info from input to output. IMHO data should be
> > > in a consistent format in netlink messages, but I fear we can't change
> > > this anymore. I tried to print the data byte-by-byte, but we obviously
> > > still get problems with any data in host byte order. Do you see an
> > > easier way to fix this than adding extra info to all expressions
> > > containing data?
> > 
> > Probably we can make assumptions based on context, such as payload
> > expression always express things in network byte order, and annotate
> > that such register stores something in network byteorder. For meta,
> > assume host byte order. Unless there is an explicit byteorder
> > expression.
> 
> I like this simple approach, but it's not easy to implement: libnftnl
> doesn't know about other expressions, so 'cmp' for instance doesn't know
> which expression stored data in reg 1 and therefore can't deduce the
> likely endianness of its stored data.
> 
> Any idea how to solve that?

Probably tracking endianess like this? See attached patch. Then,
default to network byteorder in all cases when printing.

We know the key endianess for meta, ct, ... this allows to annotate
this information on the register.

For "meta mark set 10", the immediate expression is used which comes
_before_ the "meta mark" that provides the endianness information,
this still needs to be fixed somehow.

Note also that this patch sets host byteorder for all ct keys, which
is not correct. We should set the endianess based on the key.

We'll have to fix nft tests after this too.