netdev - Re: [PATCH net-next] net: phy: improve link partner capability detection

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: <6a86f31f-dda8-77d9-aace-9ad1ad770a28@gmail.com>
Date:   Fri, 5 Apr 2019 22:04:22 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: phy: improve link partner capability
 detection

On 05.04.2019 21:48, Andrew Lunn wrote:
> On Fri, Apr 05, 2019 at 09:23:13PM +0200, Heiner Kallweit wrote:
>> genphy_read_status() so far checks phydev->supported, not the actual
>> PHY capabilities. This can make a difference if the supported speeds
>> have been limited by of_set_phy_supported() or phy_set_max_speed().
>>
>> It seems that this issue only affects the link partner advertisements
>> as displayed by ethtool. Also this patch wouldn't apply to older
>> kernels because linkmode bitmaps have been introduced recently.
>> Therefore net-next.
> 
> Hi Heiner
> 
Hi Andrew,

> Are you saying, if the local PHY/MAC does not support 1000Base-T, we
> should not check if the peer is advertising 1000Base-T? That seems
> wrong. We should report everything it offers. The fact we cannot make
> use of 1000Base-T should not matter, and resolving of the auto-get
> should do the right thing when presented with modes it cannot use.
> 
Link partner gigabit advertisement is reported in register MII_STAT1000.
If we have a 100MBit PHY w/o this register, then we will never know
whether the link partner supports gigabit.
I would have to check the 802.3 specs, but it could even be that the
gigabit autoneg is a separate step in the autoneg process. If this
separate step isn't supported by a 100MBit PHY, then this PHY per se
has no chance to detect whether link partner is gigabit-capable.

> What do we do in the c45 case? The peer can do 2.5G, 5G and
> 10G, but the local can only do 2.5G? Do we just report the peers 2.5G
> and skip the others? Or do we report them all?
> 
We report everything the chip reports. But similar to what I wrote
about C22 and gigabit, what the chip reports may depend on which
autoneg steps it does. Again I would have to check the standard whether
e.g. NBase-T (2.5Gbps, 5Gbps) uses a separate autoneg step.
If we don't advertise at least of these modes, then the PHY may decide
to skip this autoneg step, and in the end we won't know whether link
partner supports 2.5Gbps/5Gbps.

>     Andrew
> 
Heiner