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

On Sun, Jun 16, 2024 at 06:02:31PM +0200, Maxime Chevallier wrote:

Hello Jakub,

On Thu, 13 Jun 2024 18:26:13 -0700
Jakub Kicinski [off-list ref] wrote:

quoted

On Fri,  7 Jun 2024 09:18:18 +0200 Maxime Chevallier wrote:

quoted

+		if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
+			struct nlattr *phy_id;
+
+			phy_id = tb[ETHTOOL_A_HEADER_PHY_INDEX];
+			phydev = phy_link_topo_get_phy(dev,
+						       nla_get_u32(phy_id));  

Sorry for potentially repeating question (please put the answer in the
commit message) - are phys guaranteed not to disappear, even if the
netdev gets closed? this has no rtnl protection

I'll answer here so that people can correct me if I'm wrong, but I'll
also add it in the commit logs as well (and possibly with some fixes
depending on how this discussion goes)

While a PHY can be attached to/detached from a netdevice at open/close,
the phy_device itself will keep on living, as its lifetime is tied to
the underlying mdio_device (however phy_attach/detach take a ref on the
phy_device, preventing it from vanishing while it's attached to a
netdev)

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.

	Andrew