netdev - Re: [PATCH net v2] net: phy: phy_ethtool_ksettings_set: Don't discard phy_start

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: <63e5522a-f420-28c4-dd60-ce317dbbdfe0@linutronix.de>
Date:   Mon, 8 Nov 2021 17:32:23 +0100
From:   Bastian Germann <bage@...utronix.de>
To:     Andrew Lunn <andrew@...n.ch>,
        Benedikt Spranger <b.spranger@...utronix.de>
Cc:     "Russell King (Oracle)" <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>, davem@...emloft.net,
        netdev@...r.kernel.org, Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH net v2] net: phy: phy_ethtool_ksettings_set: Don't discard
 phy_start_aneg's return

Am 08.11.21 um 17:09 schrieb Andrew Lunn:
> On Mon, Nov 08, 2021 at 04:06:53PM +0100, Benedikt Spranger wrote:
>> On Mon, 8 Nov 2021 14:25:48 +0000
>> "Russell King (Oracle)" <linux@...linux.org.uk> wrote:
>>
>>> On Mon, Nov 08, 2021 at 03:18:34PM +0100, bage@...utronix.de wrote:
>>>> From: Bastian Germann <bage@...utronix.de>
>>>>
>>>> Take the return of phy_start_aneg into account so that ethtool will
>>>> handle negotiation errors and not silently accept invalid input.
>>>
>>> I don't think this description is accurate. If we get to call
>>> phy_start_aneg() with invalid input, then something has already
>>> gone wrong.
>> The MDI/MDIX/auto-MDIX settings are not checked before calling
>> phy_start_aneg(). If the PHY supports forcing MDI and auto-MDIX, but
>> not forcing MDIX _phy_start_aneg() returns a failure, which is silently
>> ignored.
> 
> Does the broadcom driver currently do this, or is this the new
> functionality you are adding?
> 
> It actually seems odd that auto and MDI is supported, but not MDIX?  I
> would suggest checking with Florian about that. Which particular
> broadcom PHY is it?

It is BCM53125. Currently, you can set "mdix auto|off|on" which does not take any effect.
The chip will do what is its default depending on copper autonegotiation.

I am adding support for setting "mdix auto|off". I want the thing to error on "mdix on".
Where would I add that check?