[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201019122643.GC11282@nanopsycho.orion>
Date: Mon, 19 Oct 2020 14:26:43 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Michal Kubecek <mkubecek@...e.cz>
Cc: Danielle Ratson <danieller@...dia.com>,
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
Mon, Oct 19, 2020 at 01:04:22PM CEST, mkubecek@...e.cz wrote:
>On Mon, Oct 19, 2020 at 07:19:34AM +0000, Danielle Ratson wrote:
>> > -----Original Message-----
>> > From: Andrew Lunn <andrew@...n.ch>
>> > Sent: Saturday, October 17, 2020 1:16 AM
>> >
>> > I'm not sure i fully understand all these different link modes, but
>> > i thought these 5 are all 100G using 2 lanes? So why cannot the user
>> > simply do
>> >
>> > ethtool -s swp1 advertise 100000baseKR2/Full
>> >
>> > and the driver can figure out it needs to use two lanes at 50G?
>> >
>> > Andrew
>>
>> Hi Andrew,
>>
>> Thanks for the feedback.
>>
>> I guess you mean " ethtool -s swp1 advertise 100000baseKR2/Full on".
>>
>> First, the idea might work but only for auto negotiation mode, whereas
>> the lanes parameter is a solution for both.
>>
>> Second, the command as you have suggested it, wouldn't change anything
>> in the current situation as I checked. We can disable all the others
>> and leave only the one we want but the command doesn't filter the
>> other link modes but it just turns the mentioned link modes up if they
>> aren't. However, the lanes parameter is a selector, which make it much
>> more user friendly in my opinion.
>
>It would be quite easy to extend the ethtool command line parser to
>allow also
>
> ethtool -s <dev> advertise <mode> ...
>
>in addition to already supported
>
> ethtool -s <dev> advertise <mask>
> ethtool -s <dev> advertise <mask>/<mask>
> ethtool -s { <mode> { on | off } } ...
>
>Parser converting simple list of values into a maskless bitset is
>already there so it would be only matter of checking if there are at
>least two arguments and second is "on" or "off" and using corresponding
>parser. I think it would be useful independently of this series.
Understood. So basically you will pass some selectors on cmdline and the
uapi would stay intact.
How do you imagine the specific lane number selection should look like
on the cmdline?
>
>> Also, we can't turn only one of them up. But you have to set for
>> example:
>>
>> $ ethtool -s swp1 advertise 100000baseKR2/Full on 100000baseSR2/Full on 100000baseCR2/Full on 100000baseLR2_ER2_FR2/Full on 100000baseDR2/Full on
>>
>> Am I missing something?
>
>IIUC Jakub's concern is rather about real life need for such selectors,
>i.e. how realistic is "I want a(ny) 100Gb/s mode with two lanes" as an
>actual user need; if it wouldn't be mostly (or only) used as a quick way
>to distinguish between two supported 100Gb/s modes.
>
>IMHO if we go this way, we should consider going all the way, i.e. allow
>also selecting by the remaining part of the mode ("media type", e.g.
>"LR", not sure what the official name is) and, more important, get full
>information about link mode in use from driver (we only get speed and
>duplex at the moment). But that would require changes in the
>get_linksettings() interface and drivers.
>
>Michal
Powered by blists - more mailing lists