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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201012130842.5f498631@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 12 Oct 2020 13:08:42 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     Ido Schimmel <idosch@...sch.org>, netdev@...r.kernel.org,
        davem@...emloft.net, jiri@...dia.com, danieller@...dia.com,
        andrew@...n.ch, f.fainelli@...il.com, mkubecek@...e.cz,
        mlxsw@...dia.com, Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH net-next 1/6] ethtool: Extend link modes settings uAPI
 with lanes

On Mon, 12 Oct 2020 21:10:53 +0200 Johannes Berg wrote:
> 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 :-)

Yes, you're right. Sorry for the noise, one day I'll learn to read..