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: <8ccaeb57-3150-65ce-2310-3aac52f446c5@gmail.com>
Date:   Thu, 19 Jul 2018 07:54:31 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Florian Fainelli <f.fainelli@...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 18.07.2018 11:22, Florian Fainelli wrote:
> 
> 
> 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?
> 
Indeed, this could be done, based on the local automatic/manual setting.

> 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.
> 
Do you by chance have a reference to this patch? There's heavy development
on the Mellanox drivers with a lot of patches.

> Thanks for your improvements to PHYLIB.
>