Re: [PATCH net-next 1/6] ethtool: Extend link modes settings uAPI with lanes

[Johannes Berg <johannes@sipsolutions.net>]

Hi,

Sorry, somehow didn't see this until now.

> > +/* Lanes, 1, 2, 4 or 8. */
> > +#define ETHTOOL_LANES_1			1
> > +#define ETHTOOL_LANES_2			2
> > +#define ETHTOOL_LANES_4			4
> > +#define ETHTOOL_LANES_8			8
> 
> Not an extremely useful set of defines, not sure Michal would agree.
> 
> > +#define ETHTOOL_LANES_UNKNOWN		0
> >  struct link_mode_info {
> >  	int				speed;
> > +	int				lanes;
> 
> why signed?
> 
> >  	u8				duplex;
> >  };
> > @@ -274,16 +277,17 @@ const struct nla_policy ethnl_linkmodes_set_policy[] = {
> >  	[ETHTOOL_A_LINKMODES_SPEED]		= { .type = NLA_U32 },
> >  	[ETHTOOL_A_LINKMODES_DUPLEX]		= { .type = NLA_U8 },
> >  	[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]	= { .type = NLA_U8 },
> > +	[ETHTOOL_A_LINKMODES_LANES]		= { .type = NLA_U32 },
> 
> NLA_POLICY_VALIDATE_FN(), not sure why the types for this
> validation_type are limited.. Johannes, just an abundance of caution?

So let me see if I got this right - you're saying you'd like to use
NLA_POLICY_VALIDATE_FN() for an NLA_U32, to validate against the _LANES
being 1, 2, 4 or 8?

First of all, you _can_, no? I mean, it's limited by

#define NLA_ENSURE_NO_VALIDATION_PTR(tp)                \
        (__NLA_ENSURE(tp != NLA_BITFIELD32 &&           \
                      tp != NLA_REJECT &&               \
                      tp != NLA_NESTED &&               \
                      tp != NLA_NESTED_ARRAY) + tp)

and the reason is sort of encoded in that - the types listed here
already use the pointer *regardless of the validation_type*, so you
can't have a pointer to the function in the same union.

But not sure I understood :-)

johannes