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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 5 Apr 2019 22:27:29 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Heiner Kallweit <hkallweit1@...il.com>
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 Fri, Apr 05, 2019 at 10:04:22PM +0200, Heiner Kallweit wrote:
> 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.

Hi Heiner

There seems to be two uses cases:

A Fast MAC connected to a Fast PHY. In that case, MII_STAT1000
probably does not exist, so we should not read it. Hopefully .features
says BASIC, or if we ask the PHY what is it, it reports it is a Fast
PHY.

A Fast MAC connected to a Giga PHY. The MAC driver will of used
phy_set_max_speed() to indicate its limits. In that case, MII_STAT1000
does exist and we should report what the peer is advertising.

      Andrew