Re: [PATCH libftnl,RFC] src: add infrastructure to infer byteorder from keys

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Dec 03, 2020 at 10:46:51PM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Thu, Dec 03, 2020 at 05:22:17PM +0100, Phil Sutter wrote:
> [...]
> > On Thu, Nov 26, 2020 at 11:48:50AM +0100, Pablo Neira Ayuso wrote:
> > > This patch adds a new .byteorder callback to expressions to allow infer
> > > the data byteorder that is placed in registers. Given that keys have a
> > > fixed datatype, this patch tracks register operations to obtain the data
> > > byteorder. This new infrastructure is internal and it is only used by
> > > the nftnl_rule_snprintf() function to make it portable regardless the
> > > endianess.
> > > 
> > > A few examples after this patch running on x86_64:
> > > 
> > > netdev
> > >   [ meta load protocol => reg 1 ]
> > >   [ cmp eq reg 1 0x00000008 ]
> > >   [ immediate reg 1 0x01020304 ]
> > >   [ payload write reg 1 => 4b @ network header + 12 csum_type 1 csum_off 10 csum_flags 0x1 ]
> > > 
> > > root@salvia:/home/pablo/devel/scm/git-netfilter/libnftnl# nft --debug=netlink add rule netdev x z ip saddr 1.2.3.4
> > > netdev
> > >   [ meta load protocol => reg 1 ]
> > >   [ cmp eq reg 1 0x00000008 ]
> > >   [ payload load 4b @ network header + 12 => reg 1 ]
> > >   [ cmp eq reg 1 0x01020304 ]
> > > 
> > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > ---
> > > Hi Phil,
> > > 
> > > This patch is incomplete. Many expressions are still missing the byteorder.
> > > This is adding minimal infrastructure to "delinearize" expression for printing
> > > on the debug information.
> > > 
> > > The set infrastructure is also missing, this requires to move the TYPE_
> > > definitions to libnftnl (this is part of existing technical debt) and
> > > add minimal code to "delinearize" the set element again from snprintf
> > > based in the NFTNL_SET_DATATYPE / userdata information of the set
> > > definition.
> > 
> > Thanks for this initial implementation, I think it's a good start and I
> > would like to complete it.
> 
> Thanks.
> 
> > Currently I'm running into roadblocks with anonymous sets, though (I
> > didn't even test named ones yet). The anonymous ones are what I hit
> > first when trying to fix tests/py/ payload files.
> >
> > The simple example is:
> > | nft --debug=netlink add rule ip t c ip saddr { 10.0.0.1, 1.2.3.4 }
> > 
> > I tried to extract NFTNL_UDATA_SET_KEYBYTEORDER and
> > NFTNL_UDATA_SET_DATABYTEORDER from set's udata in
> > nftnl_set_snprintf_default() but those are not present. Also set's
> > 'key_type' and 'data_type' fields are zero, probably because the set
> > doesn't have a formal definition.
> > 
> > I added some debug printing to nftnl_rule_snprintf_default() and
> > apparently debug output prints the set content before it is called,
> > therefore I can't use your infrastructure to deduce the set elements'
> > byteorder from the lookup expression's sreg.
> > 
> > Any ideas how this could be solved?
> 
> netlink_get_setelem() calls netlink_dump_set() to display the debug
> information. There the nls object key_type and data_type are not set.
> The set object that was obtained from the evaluation phase is already
> in place, it contains the key_type and data_type. You have to use it
> to set the missing bits in nls accordingly.

Actually, it's mnl_nft_setelem_add() that calls netlink_dump_set() in
this case. The set object is already around too, so it can be used to
set the nls accordingly.