[libnftnl,0/3] add description infrastructure

Hi Phil,

This is my proposal to address the snprintf data printing depending on
the arch. The idea is to add description objects that can be used to
build the userdata area as well as to parse the userdata to create the
description object.

This is revisiting 6e48df5329ea ("src: add "typeof" build/parse/print
support") in nftables which adds build and parse userdata callbacks to
expression in libnftables. My proposal is to move this to libnftnl.

This allows to consolidate codebase to address two different usecases:
- you can pass the description object to the snprintf function.
- it provides helpers to build and to parse the userdata area. The
  userdata TLV attributes do not need to be exposed through
  libnftnl/udata.h anymore, instead users can just rely on these helper
  functions.

The userdata area has been extended with new attributes, but it is still
incomplete since it does not allow to represent a concatenation.

Note that this will also allow us to deprecate NFTA_SET_DATA_TYPE at
some point (this netlink attribute represents a concatenation using 6
bits of the 32-bit integer, hence limiting concatenations to 5
components at this stage).

I'm afraid we'll also have to keep the existing userdata attributes
added by 6e48df5329ea in nftables for a little while (at least the
parser functions), so nftables does not break on updates since my
userdata TLV coming in this patchset for libnftnl is different than the
one available at 6e48df5329ea.

Please also note that nftables needs to be updated to use this
infrastructure.

My proposal follows a longer route but it will allow to addressing a
number of existing shortcomings in the set infrastrcture.

This is compile-tested only.

Pablo Neira Ayuso (3):
  desc: add expression description
  desc: add datatype description
  desc: add set description

 include/Makefile.am          |   1 +
 include/desc.h               |  50 +++
 include/expr_ops.h           |  11 +
 include/internal.h           |   1 +
 include/libnftnl/Makefile.am |   1 +
 include/libnftnl/desc.h      | 107 +++++++
 include/libnftnl/udata.h     |  18 +-
 src/Makefile.am              |   1 +
 src/desc.c                   | 598 +++++++++++++++++++++++++++++++++++
 src/expr/payload.c           |  81 +++++
 src/expr_ops.c               |  13 +
 11 files changed, 875 insertions(+), 7 deletions(-)
 create mode 100644 include/desc.h
 create mode 100644 include/libnftnl/desc.h
 create mode 100644 src/desc.c

Hi Pablo,

On Thu, Jan 20, 2022 at 01:03:59AM +0100, Pablo Neira Ayuso wrote:
> This is my proposal to address the snprintf data printing depending on
> the arch. The idea is to add description objects that can be used to
> build the userdata area as well as to parse the userdata to create the
> description object.

I tried to integrate this into nftables, but failed to understand how
this all is supposed to come together: In nftables, concat is treated
like any other expression. Your series seems to require special
treatment? At least there are separate "desc" data structures for each.
It seems like one can't just replace build_udata callbacks to populate
an nftnl_expr_desc object?

Cheers, Phil

On Thu, Mar 10, 2022 at 12:35:57PM +0100, Phil Sutter wrote:
> Hi Pablo,
> 
> On Thu, Jan 20, 2022 at 01:03:59AM +0100, Pablo Neira Ayuso wrote:
> > This is my proposal to address the snprintf data printing depending on
> > the arch. The idea is to add description objects that can be used to
> > build the userdata area as well as to parse the userdata to create the
> > description object.
> 
> I tried to integrate this into nftables, but failed to understand how
> this all is supposed to come together: In nftables, concat is treated
> like any other expression. Your series seems to require special
> treatment?

The idea is that you build the nftnl description object either from
the set typeof expression or the set datatype (depending on how the
user has defined the set).

> At least there are separate "desc" data structures for each.
> It seems like one can't just replace build_udata callbacks to populate
> an nftnl_expr_desc object?

You can use the description object in two ways:

- build_udata is called when setting the libnftnl set udata
  field, to build it.

- you pass the description object to snprintf.

The existing code to build the userdata TLV that resides in nftables
should go away and use this new infrastructure, I'm basically moving
to libnftnl the existing nftables code to build the set userdata area.

Hi Pablo,

On Thu, Jan 20, 2022 at 01:03:59AM +0100, Pablo Neira Ayuso wrote:
> This is my proposal to address the snprintf data printing depending on
> the arch. The idea is to add description objects that can be used to
> build the userdata area as well as to parse the userdata to create the
> description object.
> 
> This is revisiting 6e48df5329ea ("src: add "typeof" build/parse/print
> support") in nftables which adds build and parse userdata callbacks to
> expression in libnftables. My proposal is to move this to libnftnl.

Looking at your PoC again, I assume it was meant for use by applications
to create and populate an nftnl_set_desc object and serialize it into
nftnl_set's userdata using nftnl_set_desc_build_udata(). Since the
information is needed within libnftnl though, the whole API does not
make sense anymore and nftnl_set_desc must be serialized by libnftnl
itself. This in turn means one may just integrate the data structure
into nftnl_set's 'desc' field directly and extend nftnl_set_set_data()
to allow populating the new fields, plus
nftnl_set_desc_add_{expr,datatype}() I guess.

Am I on the right track there?

Maybe it's quicker for me to add the missing bits to my stuff instead of
adjusting it to your series after making it work for the intended
purpose. Especially since I'm not quite sure what goal we're trying to
achieve.

Cheers, Phil

On Wed, Apr 06, 2022 at 01:06:13PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Thu, Jan 20, 2022 at 01:03:59AM +0100, Pablo Neira Ayuso wrote:
> > This is my proposal to address the snprintf data printing depending on
> > the arch. The idea is to add description objects that can be used to
> > build the userdata area as well as to parse the userdata to create the
> > description object.
> > 
> > This is revisiting 6e48df5329ea ("src: add "typeof" build/parse/print
> > support") in nftables which adds build and parse userdata callbacks to
> > expression in libnftables. My proposal is to move this to libnftnl.
> 
> Looking at your PoC again, I assume it was meant for use by applications
> to create and populate an nftnl_set_desc object and serialize it into
> nftnl_set's userdata using nftnl_set_desc_build_udata(). Since the
> information is needed within libnftnl though, the whole API does not
> make sense anymore and nftnl_set_desc must be serialized by libnftnl
> itself. This in turn means one may just integrate the data structure
> into nftnl_set's 'desc' field directly and extend nftnl_set_set_data()
> to allow populating the new fields, plus
> nftnl_set_desc_add_{expr,datatype}() I guess.
> 
> Am I on the right track there?
> 
> Maybe it's quicker for me to add the missing bits to my stuff instead of
> adjusting it to your series after making it work for the intended
> purpose. Especially since I'm not quite sure what goal we're trying to
> achieve.

Goal is to consolidate code. Move the existing code in nftables to
libnftnl so there is a desc object that can be use to build the
userdata and to all assist the libnftnl print functions.

This will take a bit of work.