netdev - Re: [PATCH net-next] net: sfp: add some quirks for FreeTel direct attach modules

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: <20200508152844.GV1551@shell.armlinux.org.uk>
Date:   Fri, 8 May 2020 16:28:44 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Marek Behún <marek.behun@....cz>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: sfp: add some quirks for FreeTel direct
 attach modules

On Thu, May 07, 2020 at 03:21:35PM +0200, Marek Behún wrote:
> FreeTel P.C30.2 and P.C30.3 may fail to report anything useful from
> their EEPROM. They report correct nominal bitrate of 10300 MBd, but do
> not report sfp_ct_passive nor sfp_ct_active in their ERPROM.
> 
> These modules can also operate at 1000baseX and 2500baseX.
> 
> Signed-off-by: Marek Behún <marek.behun@....cz>
> Cc: Russell King <rmk+kernel@...linux.org.uk>
> ---
>  drivers/net/phy/sfp-bus.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> index 6900c68260e0..f021709bedcc 100644
> --- a/drivers/net/phy/sfp-bus.c
> +++ b/drivers/net/phy/sfp-bus.c
> @@ -44,6 +44,14 @@ static void sfp_quirk_2500basex(const struct sfp_eeprom_id *id,
>  	phylink_set(modes, 2500baseX_Full);
>  }
>  
> +static void sfp_quirk_direct_attach_10g(const struct sfp_eeprom_id *id,
> +					unsigned long *modes)
> +{
> +	phylink_set(modes, 10000baseCR_Full);
> +	phylink_set(modes, 2500baseX_Full);
> +	phylink_set(modes, 1000baseX_Full);
> +}
> +
>  static const struct sfp_quirk sfp_quirks[] = {
>  	{
>  		// Alcatel Lucent G-010S-P can operate at 2500base-X, but
> @@ -63,6 +71,18 @@ static const struct sfp_quirk sfp_quirks[] = {
>  		.vendor = "HUAWEI",
>  		.part = "MA5671A",
>  		.modes = sfp_quirk_2500basex,
> +	}, {
> +		// FreeTel P.C30.2 is a SFP+ direct attach that can operate at
> +		// at 1000baseX, 2500baseX and 10000baseCR, but may report none
> +		// of these in their EEPROM
> +		.vendor = "FreeTel",
> +		.part = "P.C30.2",
> +		.modes = sfp_quirk_direct_attach_10g,
> +	}, {
> +		// same as previous
> +		.vendor = "FreeTel",
> +		.part = "P.C30.3",
> +		.modes = sfp_quirk_direct_attach_10g,

Looking at the EEPROM capabilities, it seems that these modules give
either:

Transceiver codes     : 0x01 0x00 0x00 0x00 0x00 0x04 0x80 0x00 0x00
Transceiver type      : Infiniband: 1X Copper Passive
Transceiver type      : Passive Cable
Transceiver type      : FC: Twin Axial Pair (TW)
Encoding              : 0x06 (64B/66B)
BR, Nominal           : 10300MBd
Passive Cu cmplnce.   : 0x01 (SFF-8431 appendix E) [SFF-8472 rev10.4 only]
BR margin, max        : 0%
BR margin, min        : 0%

or:

Transceiver codes     : 0x00 0x00 0x00 0x00 0x00 0x04 0x80 0x00 0x00
Transceiver type      : Passive Cable
Transceiver type      : FC: Twin Axial Pair (TW)
Encoding              : 0x06 (64B/66B)
BR, Nominal           : 10300MBd
Passive Cu cmplnce.   : 0x01 (SFF-8431 appendix E) [SFF-8472 rev10.4 only]
BR margin, max        : 0%
BR margin, min        : 0%

These give ethtool capability mask of 000,00000600,0000e040, which
is:

	2500baseX (bit 15)
	1000baseX (bit 41)
	10000baseCR (bit 42)

10000baseCR, 2500baseX and 1000baseX comes from:

        if ((id->base.sfp_ct_passive || id->base.sfp_ct_active) && br_nom) {
                /* This may look odd, but some manufacturers use 12000MBd */
                if (br_min <= 12000 && br_max >= 10300)
                        phylink_set(modes, 10000baseCR_Full);
                if (br_min <= 3200 && br_max >= 3100)
                        phylink_set(modes, 2500baseX_Full);
                if (br_min <= 1300 && br_max >= 1200)
                        phylink_set(modes, 1000baseX_Full);

since id->base.sfp_ct_passive is true, and br_nom = br_max = 10300 and
br_min = 0.

10000baseCR will also come from:

        if (id->base.sfp_ct_passive) {
                if (id->base.passive.sff8431_app_e)
                        phylink_set(modes, 10000baseCR_Full);
        }

You claimed in your patch description that sfp_ct_passive is not set,
but the EEPROM dumps contain:

	Transceiver type      : Passive Cable

which is correctly parsed by the kernel.

So, I'm rather confused, and I don't see why this patch is needed.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up