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: <20190224164751.GG26626@lunn.ch>
Date:   Sun, 24 Feb 2019 17:47:51 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Tariq Toukan <tariqt@...lanox.com>
Cc:     "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

On Sun, Feb 24, 2019 at 05:08:21PM +0200, Tariq Toukan wrote:
> From: Aya Levin <ayal@...lanox.com>
> index 5a26cff5fb33..64ce0711ad5f 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -650,6 +650,11 @@ lB	l	lB.
>  0x400000000	50000baseCR2 Full
>  0x800000000	50000baseKR2 Full
>  0x10000000000	50000baseSR2 Full
> +0x10000000000000	50000baseKR Full
> +0x20000000000000	50000baseSR Full
> +0x40000000000000	50000baseCR Full
> +0x80000000000000	50000baseLR_ER_FR Full
> +0x100000000000000	50000baseDR Full
>  0x8000000	56000baseKR4 Full
>  0x10000000	56000baseCR4 Full
>  0x20000000	56000baseSR4 Full
> @@ -658,6 +663,16 @@ lB	l	lB.
>  0x2000000000	100000baseSR4 Full
>  0x4000000000	100000baseCR4 Full
>  0x8000000000	100000baseLR4_ER4 Full
> +0x200000000000000	100000baseKR2 Full
> +0x400000000000000	100000baseSR2 Full
> +0x800000000000000	100000baseCR2 Full
> +0x1000000000000000	100000baseLR2_ER2_FR2 Full
> +0x2000000000000000	100000baseDR2 Full
> +0x4000000000000000	200000baseKR4 Full
> +0x8000000000000000	200000baseSR4 Full
> +0x10000000000000000	200000baseLR4_ER4_FR4 Full
> +0x20000000000000000	200000baseDR4 Full
> +0x40000000000000000	200000baseCR4 Full

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

	Andrew
> +			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;

Maybe i'm wrong, but this looks odd.

enum ethtool_link_mode_bit_indices {
        ETHTOOL_LINK_MODE_10baseT_Half_BIT      = 0,
        ETHTOOL_LINK_MODE_10baseT_Full_BIT      = 1,
        ETHTOOL_LINK_MODE_100baseT_Half_BIT     = 2,
        ETHTOOL_LINK_MODE_100baseT_Full_BIT     = 3,
        ETHTOOL_LINK_MODE_1000baseT_Half_BIT    = 4,
        ETHTOOL_LINK_MODE_1000baseT_Full_BIT    = 5,

These are numbers, not bitmasks, so & them together does not look
correct.

	Andrew