On Thu, Mar 10, 2022 at 12:31:15PM +0100, Phil Sutter wrote:
> Hi,
>
> On Thu, Jan 20, 2022 at 01:04:02AM +0100, Pablo Neira Ayuso wrote:
> [...]
> > diff --git a/src/desc.c b/src/desc.c
> > index f73e74c2c7d3..c8b3195db850 100644
> > --- a/src/desc.c
> > +++ b/src/desc.c
> [...]
> > +static int nftnl_set_desc_build_dtype(struct nftnl_udata_buf *udbuf,
> > + const struct nftnl_set_desc *dset)
> > +{
> > + struct nftnl_udata *nest;
> > + int i, err;
> > +
> > + switch (dset->type) {
> > + case NFTNL_DESC_SET_TYPEOF:
> > + nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_KEY);
> > + for (i = 0; i < dset->key.num_type; i++) {
> > + err = __nftnl_udata_set_dtype_build(udbuf, dset->key.dtype[i], i);
> > + if (err < 0)
> > + return err;
> > + }
> > + nftnl_udata_nest_end(udbuf, nest);
> > + break;
> > + case NFTNL_DESC_SET_DATATYPE:
> > + nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_DATA);
> > + for (i = 0; i < dset->data.num_type; i++) {
> > + err = __nftnl_udata_set_dtype_build(udbuf, dset->data.dtype[i], i);
> > + if (err < 0)
> > + return err;
> > + }
> > + nftnl_udata_nest_end(udbuf, nest);
> > + break;
> > + case NFTNL_DESC_SET_UNSPEC:
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int __nftnl_set_desc_build_typeof(struct nftnl_udata_buf *udbuf,
> > + const struct nftnl_expr_desc *dexpr,
> > + uint8_t attr_type)
> > +{
> > + struct nftnl_udata *nest;
> > + int err;
> > +
> > + nest = nftnl_udata_nest_start(udbuf, attr_type);
> > + err = nftnl_expr_desc_build(udbuf, dexpr);
> > + nftnl_udata_nest_end(udbuf, nest);
> > +
> > + return err;
> > +}
> > +
> > +static int nftnl_set_desc_build_typeof(struct nftnl_udata_buf *udbuf,
> > + const struct nftnl_set_desc *dset)
> > +{
> > + struct nftnl_udata *nest;
> > + int i;
> > +
> > + switch (dset->type) {
> > + case NFTNL_DESC_SET_TYPEOF:
> > + nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_KEY_TYPEOF);
> > + for (i = 0; i < dset->key.num_typeof; i++)
> > + __nftnl_set_desc_build_typeof(udbuf, dset->key.expr[i], i);
> > +
> > + nftnl_udata_nest_end(udbuf, nest);
> > + break;
> > + case NFTNL_DESC_SET_DATATYPE:
> > + nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_DATA_TYPEOF);
> > + for (i = 0; i < dset->key.num_typeof; i++)
> > + __nftnl_set_desc_build_typeof(udbuf, dset->data.expr[i], i);
> > +
> > + nftnl_udata_nest_end(udbuf, nest);
> > + break;
> > + case NFTNL_DESC_SET_UNSPEC:
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +EXPORT_SYMBOL(nftnl_set_desc_build_udata);
> > +int nftnl_set_desc_build_udata(struct nftnl_udata_buf *udbuf,
> > + const struct nftnl_set_desc *dset)
> > +{
> > + if (!nftnl_udata_put_u32(udbuf, NFTNL_UDATA_SET_FLAGS, dset->flags))
> > + return -1;
> > +
> > + switch (dset->type) {
> > + case NFTNL_DESC_SET_DATATYPE:
> > + return nftnl_set_desc_build_dtype(udbuf, dset);
> > + case NFTNL_DESC_SET_TYPEOF:
> > + return nftnl_set_desc_build_typeof(udbuf, dset);
> > + case NFTNL_DESC_SET_UNSPEC:
> > + return -1;
> > + }
> > +
> > + if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_SET_COMMENT, dset->comment))
> > + return -1;
> > +
> > + return -1;
> > +}
>
> This is odd: Depending on dset->type, nftnl_set_desc_build_udata() calls
> either nftnl_set_desc_build_dtype() or nftnl_set_desc_build_typeof().
That's indeed incorrect, nftnl_set_desc_build_typeof() should be:
static int nftnl_set_desc_build_typeof(struct nftnl_udata_buf *udbuf,
const struct nftnl_set_desc *dset)
{
struct nftnl_udata *nest;
int i;
nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_KEY_TYPEOF);
for (i = 0; i < dset->key.num_typeof; i++)
__nftnl_set_desc_build_typeof(udbuf, dset->key.expr[i], i);
nftnl_udata_nest_end(udbuf, nest);
return 0;
}
The idea is: A set can either use datatype or typeof to define the
elements that it stores. Then, a concatenation is possible.
> Yet both check dset->type again. This looks like a mix-up of set/map key
> and data definitions and typeof vs. "regular" definition styles.
This patchset was a sketch PoC, I should have label it more explicit
as such.
