netdev - Re: [PATCH net-next] net: phy: add GBit master / slave error 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: <a2e47fd0-f9a1-b8bd-c84d-6940d75dd453@gmail.com>
Date:   Wed, 18 Jul 2018 02:22:19 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: phy: add GBit master / slave error
 detection



On 07/17/2018 11:14 PM, Heiner Kallweit wrote:
> Certain PHY's have issues when operating in GBit slave mode and can
> be forced to master mode. Examples are RTL8211C, also the Micrel PHY
> driver has a DT setting to force master mode.
> If two such chips are link partners the autonegotiation will fail.
> Standard defines a self-clearing on read, latched-high bit to
> indicate this error. Check this bit to inform the user.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/phy/phy_device.c | 5 +++++
>  include/uapi/linux/mii.h     | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index b9f5f40a..249c6f75 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1551,6 +1551,11 @@ int genphy_read_status(struct phy_device *phydev)
>  			if (lpagb < 0)
>  				return lpagb;
>  
> +			if (lpagb & LPA_1000MSFAIL) {
> +				phydev_err(phydev, "Master/Slave resolution failed, maybe conflicting manual settings?\n");
> +				return -ENOLINK;
> +			}
> +

You should also be able to read whether Master/Slave was configured as
automatic or manual, and possibly issue a different message when
automatic/forced is specified?

AFAIR there was a patch a while ago from Mellanox guys that was possibly
extending the link notification with an error cause, this sounds like
something that could be useful to report to user space somehow to help
troubleshoot link down events.

Thanks for your improvements to PHYLIB.
-- 
Florian