Re: [PATCH libnftnl 1/5] expr: add and use incomplete tag

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Oct 07, 2024 at 11:49:34AM +0200, Florian Westphal wrote:
> > Extend netlink dump decoder functions to set
> > expr->incomplete marker if there are unrecognized attributes
> > set in the kernel dump.
> >
> > This can be used by frontend tools to provide a warning to the user
> > that the rule dump might be incomplete.
> 
> This is to handle old binary and new kernel scenario, correct?

Yes, old binary is listing, newer binary added something old binary
can't understand.

> I think it is hard to know if this attribute is fundamental to rise a
> warning from libnftnl. It could be just an new attribute that can be
> ignored by userspace or not?

Yes, we can't know if its something harmless or not.

> I think libnftables (higher layer) knows
> better what to do in this case, if such new attribute is required or
> not.

Well, libnftables can't know that either.  libnfntl saw an netlink
attribute that it doesn't know about.

What that attibute is doing, if its harmless or important, we cannot
know.

> > diff --git a/src/expr/bitwise.c b/src/expr/bitwise.c
> > index e99131a090ed..46346712e462 100644
> > --- a/src/expr/bitwise.c
> > +++ b/src/expr/bitwise.c
> > @@ -97,9 +97,6 @@ static int nftnl_expr_bitwise_cb(const struct nlattr *attr, void *data)
> >  	const struct nlattr **tb = data;
> >  	int type = mnl_attr_get_type(attr);
> 
> Why not simplify with:
> 
> 	if (mnl_attr_type_valid(attr, NFTA_BITWISE_MAX) < 0) {
> 		tb[NFTA_BITWISE_UNSPEC] = attr;
> 		return MNL_CB_OK;
>         }

That would work too. I don't really get mnl_attr_type_valid().

All of the callbacks have a switch statement, so anything not handled
is 'unknown'.
But if you prefer the mnl_attr_type_valid() use then I can rewrite it.