linux-kernel - Re: [PATCH net-next v13 05/13] net: ethtool: Allow passing a phy index for some commands

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, 16 Jun 2024 23:31:52 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
 davem@...emloft.net, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 thomas.petazzoni@...tlin.com, Eric Dumazet <edumazet@...gle.com>, Paolo
 Abeni <pabeni@...hat.com>, linux-arm-kernel@...ts.infradead.org, Christophe
 Leroy <christophe.leroy@...roup.eu>, Herve Codina
 <herve.codina@...tlin.com>, Florian Fainelli <f.fainelli@...il.com>, Heiner
 Kallweit <hkallweit1@...il.com>, Vladimir Oltean <vladimir.oltean@....com>,
 Köry Maincent <kory.maincent@...tlin.com>, Jesse Brandeburg
 <jesse.brandeburg@...el.com>, Marek Behún
 <kabel@...nel.org>, Piergiorgio Beruto <piergiorgio.beruto@...il.com>,
 Oleksij Rempel <o.rempel@...gutronix.de>, Nicolò Veronese
 <nicveronese@...il.com>, Simon Horman <horms@...nel.org>,
 mwojtas@...omium.org, Nathan Chancellor <nathan@...nel.org>, Antoine Tenart
 <atenart@...nel.org>
Subject: Re: [PATCH net-next v13 05/13] net: ethtool: Allow passing a phy
 index for some commands

Hi Russell,

> > It gets interesting with copper SFP. They contain a PHY, and that PHY
> > can physically disappear at any time. What i don't know is when the
> > logical representation of the PHY will disappear after the hotunplug
> > event.

I'm replying to your mail to summarize what the topology code does,
which I think is correct according to these explanations.

> 
> On a SFP module unplug, the following upstream device methods will be
> called in order:
> 1. link_down
> 2. module_stop
> 3. disconnect_phy

Patch 03 adds a phy_sfp_connect_phy() / phy_sfp_disconnect_phy() set of
helpers that new PHY drivers supporting downstream SFP busses must
specify in their sfp_upstream_ops, which will add/remove the SFP phy
to/from the topology. I realize now that I need to add some clear
documentation so that new drivers get that right.

That is because in this situation, the SFP PHY won't be attached to the
netdev (as the media-converter PHY already is), so relying on the
phy_attach / phy_detach paths won't cover these cases.

> 
> At this point, the PHY device will be removed (phy_device_remove()) and
> freed (phy_device_free()), and shortly thereafter, the MDIO bus is
> unregistered and thus destroyed.
> 
> In response to the above, phylink will, respectively for each method:
> 
> 1. disable the netdev carrier and call mac_link_down()
> 2. call phy_stop() on the attached PHY
> 3. remove the PHY from phylink, and then call phy_disconnect(),
>    disconnecting it from the netdev.
> 
> Thus, when a SFP PHY is being removed, phylib will see in order the
> following calls:
> 
> 	phy_disconnect()
> 	phy_device_remove()
> 	phy_device_free()
> 
> Provided the topology linkage is removed on phy_disconnect() which
> disassociates the PHY from the netdev, SFP PHYs should be fine.
> 

And here in that case, there's a hook in phy_detach() to remove the phy
from the topology as well, which deals with the case where phylink
deals with the sfp_upstream_ops. I think this covers all cases :)

Thanks,

Maxime