public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Russell King <linux@armlinux.org.uk>
Cc: Wei Fang <wei.fang@nxp.com>, netdev@vger.kernel.org
Subject: Re: Disappearance of network PHYs
Date: Wed, 11 Mar 2026 16:55:48 +0100	[thread overview]
Message-ID: <eb6edd0d-36b6-416a-bf74-676421f12f2d@bootlin.com> (raw)
In-Reply-To: <20260311153421.u454m3e4blkstymt@skbuf>

Hi Vladimir,

On 11/03/2026 16:34, Vladimir Oltean wrote:
> Hi,
> 
> This is a follow-up from this thread:
> https://lore.kernel.org/netdev/PAXPR04MB8510CB77C95D4D044FE62C64889BA@PAXPR04MB8510.eurprd04.prod.outlook.com/
> 
> I picked up from where Russell, Maxime and Wei left the discussion more
> than 1 month ago, and quickly prototyped patches that would mechanically
> remove the kernel crashes caused by invalid phydev dereferences from the
> MAC driver of phylib/phylink, when the phydev goes away.
> 
> (far from perfect, may still have deadlocks or lockdep splats) PoC patches at:
> https://github.com/vladimiroltean/linux/tree/phy-remove
> 
> I ran out of steam because I'm not really sure what we want given what's
> possible, and I don't want the effort/discussion to completely die away,
> so I'm asking the PHY maintainers and other interested people for advice,
> while explaining what I found to be (not) possible.

Thanks for keeping this discussion alive :)

> 
> Mechanically, what happens now in my branch, with a disappeared phydev but
> there's nothing that can crash the kernel, is that the netdev carrier state is
> in holdover mode. Meaning: if the link was up before, it still is up; if it was
> down, it still is down. Furthermore, traffic still works if it worked
> before. This is because the PHY is not an active component to the data
> path (I am excluding things such as PHY timestamping), and I've patched the
> state machine to preserve the last state and do no further work.
> 
> However, if I stop and start again a disappeared phydev, phy_start()
> leaves it in the PHY_DOWN state, to avoid some WARN_ON()s. This is
> admittedly inconsistent.
> 
> This serves as an illustration of the most complicated part of surviving
> the loss of one of your providers - what to do afterwards? The MAC driver
> may have done stateful stuff with the PHY prior to it going away.
> The netdev->phydev pointer persists, but even if the phydev later comes
> back - it's no longer the same phydev and those operations need to be
> repeated. But how to repeat those operations, when
> (1) no one kept track of them
> (2) the netdev->phydev that the MAC is holding on to is not the same as
>     the new phydev that gets created on rebind
> 
> So even if the netdev->phydev pointer lingers on, it is effectively junk and in
> everyone's best interest that the MAC driver gets rid of it ASAP. And then do
> what?
> 
> There are 2 distinct cases to think about:
> 1. MAC driver connects to the PHY at ndo_open() and disconnects at ndo_stop().
>    I can see something like a forced admin down from the kernel (somehow).
> 2. MAC driver connects to the PHY at probe() and disconnects at remove().
>    I don't see how these can survive the loss of netdev->phydev in a meaningful
>    way (meaning: have a way to recover when it comes back).
> 
> Actually DSA belongs to case 2, which complicates the discussion, since it is
> one of the reasons we don't consider device links as good.
> 
> However, I must point this out. Device links provide a very reasonable and clean
> answer to what should the MAC driver do when its PHY goes away. Unbinding the
> MAC makes sure that none of its internal assumptions about PHY state will be
> violated when the PHY later binds back, and it doesn't require complex tracking
> either. It also scales to multiple (and different kinds of) providers, which
> can also go away, in much the same way.
> 
> Sure, I don't like the side effects of that answer when applied to DSA either,
> but maybe that's something we can work on, while not fully rejecting it.
> 
> Some ideas, mostly listed as conversation starters:
> - Modify DSA to connect to the PHY at ndo_open() time.
> - Modify DSA to register a separate struct device (with generic DSA port driver)
>   for each port. Link the net_device parent device with this port device.
>   The PHY device link unbinds only the port device, which can be later rebound
>   via sysfs. Solution gets repeated for whatever other switchdev/multi-port
>   NIC driver is written that uses external providers. We modify
>   Documentation/networking/switchdev.rst to make driver authors aware of
>   the problem.
> - Create an optional notifier chain that the PHY is going away, which the MAC
>   monitors and informs phylib that it does. Drivers that don't inform phylib
>   get unbound via the device link mechanism. Those who monitor the notifier
>   don't get unbound.
> - A combination of the above

One other thing could be to rely on phylink to handle that ? It's the
one part of the net stack that already handles PHY devices suddenly
disappearing, with the SFP case. It has all the logic in place to
maintain the netdev's phydev pointer without the MAC driver having to
deal with that.

And for MAC drivers that don't use phylink, we could consider falling
back to the fw_devlink approach that was proposed, i.e. big hammer
solution that has nasty side-effects but that doesn't crash the kernel
ang gives user a chance to recover, provided the side-effects in
question didn't kick them out of their HW ?

Maxime


  reply	other threads:[~2026-03-11 15:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 15:34 Disappearance of network PHYs Vladimir Oltean
2026-03-11 15:55 ` Maxime Chevallier [this message]
2026-03-11 18:17   ` Andrew Lunn
2026-03-11 20:34     ` Vladimir Oltean
2026-03-11 21:18   ` Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eb6edd0d-36b6-416a-bf74-676421f12f2d@bootlin.com \
    --to=maxime.chevallier@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=hkallweit1@gmail.com \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=vladimir.oltean@nxp.com \
    --cc=wei.fang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox