Re: [PATCH net-next v5] rtnetlink: add new RTM_GETSTATS message to dump link stats

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2016-04-20 at 14:48 +0200, Jiri Benc wrote:
> On Wed, 20 Apr 2016 09:32:20 +0200, Johannes Berg wrote:
> > 
> > 2) Use the new attribute flag with some required attribute for
> >    existing commands, so that older kernel will not find the
> > required
> >    attribute and will reject the operation entirely.
> >    May or may not fall back to trying the operation again without
> > the
> >    flag.
> This is basically what I submitted half a year ago. See:
> http://thread.gmane.org/gmane.linux.network/382850
> 

That looks like a *huge* patchset though - whereas my proposal really
required only what Emmanuel sent in this thread. It did make some
assumptions, for example that any attribute lower than the "maxtype"
argument to nla_parse() was understood. [1]

Looks like you have this on a per-message basis. I thought it was
better on an attribute basis because that's really where the issue is.

You can still detect it with the per-attribute flag approach as I
described in (2) - if, for your lwtunnel example, you could specify the
flag on the RTA_ENCAP attribute, without which no lwtunnel can be
created (if I understand the code correctly.)

johannes



[1] for example, if I have three attributes:
enum attrs {__unused, A, B, C};

and the policy

policy = {
	[A] = { .type = NLA_U32 },
	[C] = { .type = NLA_U8 },
}

and then do

nla_parse(tb, 3, msg, msg_len, &policy)

it would assume that "B" is valid. Since this policy is equivalent to
the policy with
	[B] = { .type = NLA_BINARY }

(minimum length 0) we could also reject anything that has type=len=0 in
the policy, if the NLA_F_NET_MUST_PARSE flag is set in the nla_type.

This would likely be the right approach for most netlink families,
since they usually don't have holes that they actually care about -
I've yet to see any attribute that's not specified at all in the policy
but used anyway, normally you want some level of checking, and indicate
that by using { .type = NLA_BINARY } - but other things are possible.

johannes