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

Message-ID: <20190224194015.GM26626@lunn.ch>
Date:   Sun, 24 Feb 2019 20:40:15 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     Tariq Toukan <tariqt@...lanox.com>,
        "John W. Linville" <linville@...driver.com>,
        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

> > 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

#define ETHTOOL_LINK_MODE_200GbaseCR4_Full_BIT ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT

	Andrew