netdev - Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters

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: <20210128202632.iqixlvdfey6sh7fe@lion.mk-sys.cz>
Date:   Thu, 28 Jan 2021 21:26:32 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Danielle Ratson <danieller@...dia.com>
Cc:     Edwin Peer <edwin.peer@...adcom.com>,
        netdev <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...dia.com>,
        Andrew Lunn <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        mlxsw <mlxsw@...dia.com>, Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of
 speed and duplex parameters

On Wed, Jan 27, 2021 at 01:22:02PM +0000, Danielle Ratson wrote:
> > -----Original Message-----
> > From: Edwin Peer <edwin.peer@...adcom.com>
> > Sent: Tuesday, January 26, 2021 7:14 PM
> > To: Danielle Ratson <danieller@...dia.com>
> > Cc: netdev <netdev@...r.kernel.org>; David S . Miller <davem@...emloft.net>; Jakub Kicinski <kuba@...nel.org>; Jiri Pirko
> > <jiri@...dia.com>; Andrew Lunn <andrew@...n.ch>; f.fainelli@...il.com; Michal Kubecek <mkubecek@...e.cz>; mlxsw
> > <mlxsw@...dia.com>; Ido Schimmel <idosch@...dia.com>
> > Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
> > 
> > For one thing, it's cleaner if the driver API is symmetric. The
> > proposed solution sets attributes in terms of speeds and lanes,
> > etc., but it gets them in terms of a compound link_info. But, this
> > asymmetry aside, if link_mode may eventually become R/W at the
> > driver API, as you suggest, then it is more appropriate to guard it
> > with a capability bit, as has been done for lanes, rather than use
> > the -1 special value to indicate that the driver did not set it.
> > 
> > Regards,
> > Edwin Peer
> 
> This patchset adds lanes parameter, not link_mode. The link_mode
> addition was added as a read-only parameter for the reasons we
> mentioned, and I am not sure that implementing the symmetric side is
> relevant for this patchset.
> 
> Michal, do you think we will use the Write side of the link_mode
> parameter?

It makes sense, IMHO. Unless we also add "media" (or whatever name would
be most appropriate) parameter, we cannot in general fully determine the
link mode by speed, duplex and lanes. And using "advertise" to select
a specific mode with disabled autonegotiation would be rather confusing,
I believe. (At the moment, ethtool does not even support syntax like
"advertise <mode>" but it will be easy to support
"advertise <mode>... [--]" and I think we should extend the syntax to
support it, regardless of what we choose.) So if we want to allow user
to pick a specific link node by name or bit mask (or rather index?),
I would prefer using a separate parameter.

>            And if so, do you think it is relevant for this specific
> patchset?

I don't see an obvious problem with leaving that part for later so
I would say it's up to you.

Michal