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

Powered by blists - more mailing lists