netdev - Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode

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] [day] [month] [year] [list]

Message-ID: <e34a57bf-af88-c691-e4f0-015555643fd3@mellanox.com>
Date:   Mon, 25 Feb 2019 15:30:13 +0000
From:   Tariq Toukan <tariqt@...lanox.com>
To:     Andrew Lunn <andrew@...n.ch>, Michal Kubecek <mkubecek@...e.cz>
CC:     Tariq Toukan <tariqt@...lanox.com>,
        "John W. Linville" <linville@...driver.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Aya Levin <ayal@...lanox.com>
Subject: Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per
 lane) link mode



On 2/24/2019 9:40 PM, Andrew Lunn wrote:
>>> This is getting less friendly all the time, and it was never very
>>> friendly to start with. We have the strings which represent these link
>>> modes in the table used for dumping caps. How about allowing the user
>>> to list a comma separate list of modes.
>>>
>>> ethtool -s lan42 advertise 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full
>>
>> In my preliminary netlink patchset, I'm adding support for e.g.
>>
>>    ethtool -s eth0 advertise 100baseT/Half off 1000baseT/Full on
>>
>> I'm not sure what would be more useful, if switching individual modes or
>> listing the whole set. After all, we can also support both. But I fully
>> agree that the numeric bitmaps are already too inconvenient.
> 
> Hi Michal
> 
> So are you doing a read/modify/write? In that case, off/on makes
> sense. For a pure write, i don't see the need for off/on.
> 
> I've not had to use this much, so i don't know how it is typically
> used. When i have used it, it is because i've got an SFP which can do
> 1G and 2.5G, but the peer can only do 1G. I've needed to remove the
> 2.5G in order to get link. So in that case, read/modify/write with an
> off would make sense.
> 
>      Andrew
> 
>> And there is another problem:
>>
>> +		else if (speed_wanted == SPEED_20000 &&
>> +			 duplex_wanted == DUPLEX_FULL)
>> +			adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
>>
>> The test is for SPEED_20000 but then 200G modes are added.
> 
> Oh, yes. Easy to miss. Maybe we should consider adding aliases,
> #define SPEED_200G SPEED_200000, and

Totally agree. This will help.


> 
> #define ETHTOOL_LINK_MODE_200GbaseCR4_Full_BIT ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT

In case you do it please consider adding an underscore after the speed, 
so it becomes
ETHTOOL_LINK_MODE_200G_baseCR4_Full_BIT.
I think it's more convenient for the human eye.