netdev - Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed

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] [day] [month] [year] [list]

Message-ID: <5de812f3-1553-667f-95ab-46c1619ebcff@gmail.com>
Date:   Thu, 12 Jul 2018 22:01:34 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and
 phy_speed_up

On 12.07.2018 21:53, Florian Fainelli wrote:
> 
> 
> On 07/12/2018 12:25 PM, Florian Fainelli wrote:
>>
>>
>> On 07/12/2018 12:10 PM, Heiner Kallweit wrote:
>>> On 12.07.2018 21:09, Andrew Lunn wrote:
>>>>> Like r8169 also tg3 driver doesn't wait for the speed-down-renegotiation
>>>>> to finish. Therefore, even though I share Andrew's concerns, there seem
>>>>> to be chips where it's safe to not wait for the renegotiation to finish
>>>>> (e.g. because device is in PCI D3 already and can't generate an interrupt).
>>>>> Having said that I'd keep the sync parameter for phy_speed_down so that
>>>>> the driver can decide.
>>>>
>>>> Hi Heiner
>>>>
>>>> Please put a big fat comment about the dangers of sync=false in the
>>>> function header. We want people to known it is dangerous by default,
>>>> and should only be used in special conditions, when it is known to be
>>>> safe.
>>>> 	Andrew
>>>>
>>> OK ..
>>
>> What part do you find dangerous? Magic Packets are UDP packets and they
>> are not routed (unless specifically taken care of) so there is already
>> some "lossy" behavior involved with waking-up an Ethernet MAC, I don't
>> think that is too bad to retry several times until the link comes up.
> 
> I see the concern with the comment from v2, and indeed you could get an
> interrupt signaling the PHY auto-negotiated the link before or at the
> time we are suspending causing potentially an early wake-up. Not that
> this should be a problem though since there is usually a point of not
> return past which you can't do early wake-up anyway.
> 
I think we should leave the comment in for the moment so that people
think twice about the described scenario. If we should find out that
the issue can't be triggered on all platforms then we still can remove
the comment.