On Tue, Oct 08, 2024 at 02:17:02PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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 see.
> > 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.
Indeed.
> What that attibute is doing, if its harmless or important, we cannot
> know.
Yes, this is an old binary, it does not 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().
mnl_attr_type_valid() can go away if all switch() are audited, yes, it
is just defensive.
> 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.
There is also _PAD attributes that maybe trigger default case.
