netdev - Re: [PATCH] net: usb: lan78xx: Enforce a minimum interrupt polling period

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: <c20b01c9-0412-482d-b82e-c1bf1c7ef4ef@gmx.de>
Date: Mon, 14 Apr 2025 17:31:36 +0200
From: Fiona Klute <fiona.klute@....de>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, Thangaraj Samynathan <Thangaraj.S@...rochip.com>,
 Rengarajan Sundararajan <Rengarajan.S@...rochip.com>,
 UNGLinuxDriver@...rochip.com, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel-list@...pberrypi.com,
 stable@...r.kernel.org
Subject: Re: [PATCH] net: usb: lan78xx: Enforce a minimum interrupt polling
 period

Am 10.04.25 um 16:43 schrieb Andrew Lunn:
>>> Ah, O.K. This tells me the PHY is a lan88xx. And there is a workaround
>>> involved for an issue in this PHY. Often PHYs are driven by polling
>>> for status changes once per second. Not all PHYs/boards support
>>> interrupts. It could be this workaround has only been tested with
>>> polling, not interrupts, and so is broken when interrupts are used.
>>>
>>> As a quick hack test, in lan78xx_phy_init()
>>>
>>> 	/* if phyirq is not set, use polling mode in phylib */
>>> 	if (dev->domain_data.phyirq > 0)
>>> 		phydev->irq = dev->domain_data.phyirq;
>>> 	else
>>> 		phydev->irq = PHY_POLL;
>>>
>>> Hard code phydev->irq to PHY_POLL, so interrupts are not used.
>>>
>>> See if you can reproduce the issue when interrupts are not used.
>> It took a while, but I'm fairly confident now that the workaround works,
>> I've had over 1000 boots on the hardware in question and didn't see the
>> bug. Someone going by upsampled reported the same in the issue on Github
>> [1], and pointed out that people working with some Nvidia board and a
>> LAN7800 USB device came to the same conclusion a while ago [2].
>>
>> That leaves me with the question, what does that mean going forward?
>> Would it make sense to add a quirk to unconditionally force polling on
>> lan88xx, at least until/unless the interrupt handling can be fixed?
> 
> I don't think you need a quirk:
> 
> static struct phy_driver microchip_phy_driver[] = {
> {
>          .phy_id         = 0x0007c132,
>          /* This mask (0xfffffff2) is to differentiate from
>           * LAN8742 (phy_id 0x0007c130 and 0x0007c131)
>           * and allows future phy_id revisions.
>           */
>          .phy_id_mask    = 0xfffffff2,
>          .name           = "Microchip LAN88xx",
> 
>          /* PHY_GBIT_FEATURES */
> 
>          .probe          = lan88xx_probe,
>          .remove         = lan88xx_remove,
> 
>          .config_init    = lan88xx_config_init,
>          .config_aneg    = lan88xx_config_aneg,
>          .link_change_notify = lan88xx_link_change_notify,
> 
>          .config_intr    = lan88xx_phy_config_intr,
>          .handle_interrupt = lan88xx_handle_interrupt,
> 
> Just remove .config_intr and .handle_interrupt. If these are not
> provided, phylib will poll, even if an interrupt number has been
> passed. And since these functions are not shared with any other PHY,
> you can remove them.
> 
> Please write a good commit message, we want it clear why they where
> removed, to try to prevent somebody putting them back again.
> 
> And please aim this for net, not net-next:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> The change will then get back ported to stable kernels.

I just sent the new patch, I hope I got that right, thank you! Test got 
to about 650 successful boots by now. :-)

Best regards,
Fiona