netdev - Re: [PATCH net v1] net: stmmac: Disable PCS Link and AN interrupt when PCS AN is disabled

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: <89f188d2-2d4e-43bf-98f3-aae7e9d68cab@quicinc.com>
Date: Mon, 21 Oct 2024 08:09:53 -0700
From: "Abhishek Chauhan (ABC)" <quic_abchauha@...cinc.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>,
        Andrew Lunn
	<andrew@...n.ch>, Serge Semin <fancer.lancer@...il.com>
CC: Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu
	<joabreu@...opsys.com>,
        "David S . Miller" <davem@...emloft.net>,
        "Eric
 Dumazet" <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>, <netdev@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-arm-kernel@...ts.infradead.org>,
        Andrew Halaney <ahalaney@...hat.com>, Simon Horman <horms@...nel.org>,
        Jon Hunter <jonathanh@...dia.com>, <kernel@...cinc.com>
Subject: Re: [PATCH net v1] net: stmmac: Disable PCS Link and AN interrupt
 when PCS AN is disabled



On 10/21/2024 2:32 AM, Russell King (Oracle) wrote:
> On Mon, Oct 21, 2024 at 10:20:24AM +0100, Russell King (Oracle) wrote:
>> On Sat, Oct 19, 2024 at 04:45:16AM +0200, Andrew Lunn wrote:
>>> On Fri, Oct 18, 2024 at 03:24:07PM -0700, Abhishek Chauhan wrote:
>>>> Currently we disable PCS ANE when the link speed is 2.5Gbps.
>>>> mac_link_up callback internally calls the fix_mac_speed which internally
>>>> calls stmmac_pcs_ctrl_ane to disable the ANE for 2.5Gbps.
>>>>
>>>> We observed that the CPU utilization is pretty high. That is because
>>>> we saw that the PCS interrupt status line for Link and AN always remain
>>>> asserted. Since we are disabling the PCS ANE for 2.5Gbps it makes sense
>>>> to also disable the PCS link status and AN complete in the interrupt
>>>> enable register.
>>>>
>>>> Interrupt storm Issue:-
>>>> [   25.465754][    C2] stmmac_pcs: Link Down
>>>> [   25.469888][    C2] stmmac_pcs: Link Down
>>>> [   25.474030][    C2] stmmac_pcs: Link Down
>>>> [   25.478164][    C2] stmmac_pcs: Link Down
>>>> [   25.482305][    C2] stmmac_pcs: Link Down
>>>
>>> I don't know this code, so i cannot really comment if not enabling the
>>> interrupt is the correct fix or not. But generally an interrupt storm
>>> like this is cause because you are not acknowledging the interrupt
>>> correctly to clear its status. So rather than not enabling it, maybe
>>> you should check what is the correct way to clear the interrupt once
>>> it happens?
>>
>> stmmac PCS support is total crap and shouldn't be used, or stmmac
>> should not be using phylink. It's one or the other. Blame Serge for
>> this mess.
> 
> Seriously, we could've had this fixed had the patch set I was working
> on that fixed stmmac's _bad_ _conversion_ to phylink progressed to the
> point of being merged.
> 
> The whole stmmac PCS support is broken, bypassing phylink.
> 
> This series also contained bug fixes for stuff like this interrupt
> storm after Serge tested it. However, Serge wanted to turn my series
> into his maze of indirect function pointers approach that I disagreed
> with, and he wouldn't change his mind on that, so I deleted the series.
> 
> As I keep saying - either stmmac uses phylink *properly* and gets its
> PCS hacks sorted out, or it does not use phylink *at* *all*. It's one
> or the other.
> 
> I am not going to patch stmmac for any future phylink changes, and if
> it breaks, then I'll just say "oh that's a shame, not my problem."
> Blame Serge for that. I've had it with the pile of crap that is
> stmmac.
> 
Thanks Andrew and Russell for you review comments. 

Adding Serge here. 

Lets take a step back and see how i can help here to make sure 
we can get things merged and the discussion proceeds. 

Serge please help if can here. Thanks!