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]

Date:   Mon, 23 Nov 2020 09:47:53 +0000
From:   Danielle Ratson <danieller@...dia.com>
To:     Michal Kubecek <mkubecek@...e.cz>
CC:     Jiri Pirko <jiri@...nulli.us>, Andrew Lunn <andrew@...n.ch>,
        "Jakub Kicinski" <kuba@...nel.org>,
        Ido Schimmel <idosch@...sch.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...dia.com>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        mlxsw <mlxsw@...dia.com>, Ido Schimmel <idosch@...dia.com>,
        "johannes@...solutions.net" <johannes@...solutions.net>
Subject: RE: [PATCH net-next 1/6] ethtool: Extend link modes settings uAPI
 with lanes



> -----Original Message-----
> From: Michal Kubecek <mkubecek@...e.cz>
> Sent: Thursday, October 22, 2020 7:28 PM
> To: Danielle Ratson <danieller@...dia.com>
> Cc: Jiri Pirko <jiri@...nulli.us>; Andrew Lunn <andrew@...n.ch>; Jakub Kicinski <kuba@...nel.org>; Ido Schimmel
> <idosch@...sch.org>; netdev@...r.kernel.org; davem@...emloft.net; Jiri Pirko <jiri@...dia.com>; f.fainelli@...il.com; mlxsw
> <mlxsw@...dia.com>; Ido Schimmel <idosch@...dia.com>; johannes@...solutions.net
> Subject: Re: [PATCH net-next 1/6] ethtool: Extend link modes settings uAPI with lanes
> 
> On Thu, Oct 22, 2020 at 06:15:48AM +0000, Danielle Ratson wrote:
> > > -----Original Message-----
> > > From: Michal Kubecek <mkubecek@...e.cz>
> > > Sent: Wednesday, October 21, 2020 11:48 AM
> > >
> > > Ah, right, it does. But as you extend struct ethtool_link_ksettings
> > > and drivers will need to be updated to provide this information,
> > > wouldn't it be more useful to let the driver provide link mode in
> > > use instead (and derive number of lanes from it)?
> >
> > This is the way it is done with the speed parameter, so I have aligned
> > it to it. Why the lanes should be done differently comparing to the
> > speed?
> 
> Speed and duplex have worked this way since ages and the interface was probably introduced back in times when combination of
> speed and duplex was sufficient to identify the link mode. This is no longer the case and even adding number of lanes wouldn't make
> the combination unique. So if we are going to extend the interface now and update drivers to provide extra information, I believe it
> would be more useful to provide full information.
> 
> Michal

Hi Michal,

What do you think of passing the link modes you have suggested as a bitmask, similar to "supported", that contains only one positive bit?
Something like that:

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index afae2beacbc3..dd946c88daa3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -127,6 +127,7 @@ struct ethtool_link_ksettings {
                __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
                __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
                __ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
+               __ETHTOOL_DECLARE_LINK_MODE_MASK(chosen);
        } link_modes;
        u32     lanes;
 };

Do you have perhaps a better suggestion?

And the speed and duplex parameters should be removed from being passed like as well, right?

Thanks,
Danielle