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  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]
Date:   Wed, 13 May 2020 09:14:24 +0200
From:   Julien Beraud <>
To:     Russell King - ARM Linux admin <>
Cc:     Florian Fainelli <>,
        "" <>,
        Andrew Lunn <>,
        Heiner Kallweit <>,
        Antoine Tenart <>
Subject: Re: net: phylink: supported modes set to 0 with genphy sfp module

On 12/05/2020 14:28, Russell King - ARM Linux admin wrote:
> On Tue, May 12, 2020 at 11:28:40AM +0200, Julien Beraud wrote:
>> On 11/05/2020 21:06, Florian Fainelli wrote:
>>> On 5/11/2020 11:29 AM, Russell King - ARM Linux admin wrote:
>>>> On Mon, May 11, 2020 at 05:45:02PM +0200, Julien Beraud wrote:
>>>>> Following commit:
>>>>> commit 52c956003a9d5bcae1f445f9dfd42b624adb6e87
>>>>> Author: Russell King <>
>>>>> Date:   Wed Dec 11 10:56:45 2019 +0000
>>>>>       net: phylink: delay MAC configuration for copper SFP modules
>>>>> In function phylink_sfp_connect_phy, phylink_sfp_config is called before
>>>>> phylink_attach_phy.
>>>>> In the case of a genphy, the "supported" field of the phy_device is filled
>>>>> by:
>>>>> phylink_attach_phy->phy_attach_direct->phy_probe->genphy_read_abilities.
>>>>> It means that:
>>>>> ret = phylink_sfp_config(pl, mode, phy->supported, phy->advertising);
>>>>> will have phy->supported with no bits set, and then the first call to
>>>>> phylink_validate in phylink_sfp_config will return an error:
>>>>> return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
>>>>> this results in putting the sfp driver in "failed" state.
>>>> Which PHY is this?
>> The phy seems to be Marvell 88E1111, so the simple solution is to just add the driver for this PHY to my config.
>> That said, if for some reason someone plugs a module for which no phy driver is found the issue will happen again.
> Right, please use the specific PHY driver for modules that contain the
> 88E1111 to avoid any surprises, otherwise things can end up being
> misconfigured - for example, the PHY using 1000base-X and the host
> using SGMII, which may work or may lead to problems.
I'll do that, thanks.

>>> Using the generic PHY with a copper SFP module does not sound like a
>>> great idea because without a specialized PHY driver (that is, not the
>>> Generic PHY driver) there is not usually much that can happen.
>> Thanks for the info. I don't have an advice on whether it is a good idea to use a copper sfp without a specialized driver,
>> but before commit 52c956003a9d5bcae1f445f9dfd42b624adb6e87, it used to work and I could at least get a network connection.
>> Moreover, this commit didn't explicitely intend to forbid this behavior and the error is not explicit either.
>> If phylink+sfp still supports using genphy as a fallback, It may be good to fix the current behavior.
>> If not, maybe adding an explicit warning or error would be preferrable.
> The commit is designed to increase the range of modules that can be
> used, especially when the module supports higher rates than the MAC.
> The downside is that we _must_ know the PHYs capabilities before
> attaching to it, so that we can choose an appropriate interface to
> _attach_ to it with.  It's a chicken and egg problem.
> For this to work reliably in cases such as those you've identified,
> I need phylib to always give me that information earlier than it
> currently seems to for the genphy fallback - but the problem is the
> genphy fallback only happens after attaching to it.  So again,
> another chicken and egg problem.
> Subsituting the SFP modules capabilities seems like a workaround,
> but those are also a guess from poor information in the SFP EEPROM.
> It is what we were doing before however...

Thank you very much for the explanations.
I'll try this patch for now.


Powered by blists - more mailing lists