Message ID | 20201120175757.8063-1-phil@nwl.cc |
---|---|
State | Changes Requested |
Delegated to: | Pablo Neira |
Headers | show |
Series | [iptables] tests: shell: Stabilize nft-only/0009-needless-bitwise_0 | expand |
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.
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")}' +)
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(-)