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.