netdev - Re: [PATCH net-next 0/2] net: phy: add generic ndo_do_ioctl handler phy_do

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]

Date:   Sun, 19 Jan 2020 20:19:03 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>,
        Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/2] net: phy: add generic ndo_do_ioctl handler
 phy_do_ioctl

On 19.01.2020 19:50, Andrew Lunn wrote:
>> Also it seems we don't consider situations like runtime PM yet.
>> Then the MDIO bus may not be accessible, but ndev is running
>> and PHY is attached.
> 
> I don't think this can happen. If the device is running, the MDIO bus
> has to work, or phylib is broken.
> 

netif_running() checks flag __LINK_STATE_START. Once the network has
been brought up, this flag is set. Also the comment in the code says:
"Test if the device has been brought up."
A driver has no means to clear this flag when runtime-suspending.
netif_device_detach() however clears flag __LINK_STATE_PRESENT.
I think it's not completely consistent if a device can be running
that is not present. IMO naming of netif_running() is misleading.
It better should be named netif_if_up(), or similar.

So far very few network drivers use runtime PM, this may be the
reason why such questions didn't come up before.

Speaking for r8169:
If interface is up and cable detached, then it runtime-suspends
and goes into PCI D3 (chip and MDIO bus not accessible).
But ndev is "running" and PHY is attached.

> I have had issue with the FEC, runtime PM and MDIO, but that was when
> the interface was not running, but an Ethernet switch was using the
> MDIO bus. MDIO transactions would time out, until i made the MDIO
> operations PM aware.
> 
> But in general, we should keep the running test, just to avoid
> breakage of assumptions we don't know about.
> 
> 	 Andrew
> 

Heiner