Re: [libnftnl PATCH 3/7] set: Introduce NFTNL_SET_DESC_CONCAT_DATA

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Tue, Nov 30, 2021 at 02:46:58PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 24, 2021 at 06:22:38PM +0100, Phil Sutter wrote:
> > Analogous to NFTNL_SET_DESC_CONCAT, introduce a data structure
> > describing individual data lengths of elements' concatenated data
> > fields.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  include/libnftnl/set.h | 1 +
> >  include/set.h          | 2 ++
> >  src/set.c              | 8 ++++++++
> >  3 files changed, 11 insertions(+)
> > 
> > diff --git a/include/libnftnl/set.h b/include/libnftnl/set.h
> > index 1ffb6c415260d..958bbc9065f67 100644
> > --- a/include/libnftnl/set.h
> > +++ b/include/libnftnl/set.h
> > @@ -33,6 +33,7 @@ enum nftnl_set_attr {
> >  	NFTNL_SET_EXPR,
> >  	NFTNL_SET_EXPRESSIONS,
> >  	NFTNL_SET_DESC_BYTEORDER,
> > +	NFTNL_SET_DESC_CONCAT_DATA,
> 
> This information is already encoded in NFTNL_SET_DATA_TYPE, the
> datatypes that are defined in libnftables have an explicit byteorder
> and length.

We don't define data types in libnftnl, merely expressions and (with
your patch) those define what byteorder the source/destination registers
are supposed to be.

> For concatenation, this information is stored in 6 bits (see
> TYPE_BITS). By parsing the NFTNL_SET_DATA_TYPE field you can extract
> both types (and byteorders) of the set definition.

For this to work, I would have to duplicate nftables' enum datatypes and
in addition to that add an array defining each type's byteorder. I had
considered this once, but didn't like the amount of duplication.

> For the typeof case, where a generic datatype such as integer is used,
> this information is stored in the SET_USERDATA area.

This does not work for concatenated elements, right? At least I see e.g.
NFTNL_UDATA_SET_KEYBYTEORDER being set to set->key->byteorder, so that's
just a single value, no?

> This update for libnftnl is adding a third way to describe the
> datatypes in the set, right?

Well, it extends the logic around NFTNL_SET_DESC_CONCAT to non-interval
sets and to maps (adding the same data for the target part).

Then there is the new NFTNL_SET_DESC_BYTEORDER which defines the
byteorder of each part of the key (and value in maps).

Cheers, Phil