Re: [PATCH net] net: txgbe: fix RTNL assertion warning when remove module

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Apr 07, 2026 at 02:27:34PM +0800, Jiawen Wu wrote:
> On Wed, Apr 1, 2026 2:22 PM, Russell King (Oracle) wrote:
> > On Wed, Apr 01, 2026 at 10:16:34AM +0800, Jiawen Wu wrote:
> > > On Tue, Mar 31, 2026 9:08 PM, Russell King (Oracle) wrote:
> > > > On Tue, Mar 31, 2026 at 03:11:07PM +0800, Jiawen Wu wrote:
> > > > > For the copper NIC with external PHY, the driver called
> > > > > phylink_connect_phy() during probe and phylink_disconnect_phy() during
> > > > > remove. It caused an RTNL assertion warning in phylink_disconnect_phy()
> > > > > upon module remove.
> > > > >
> > > > > To fix this, move the phylink connect/disconnect PHY to ndo_open/close.
> > > >
> > > > Wouldn't it be simpler to just wrap the phylink_disconnect_phy() in the
> > > > remove function with rtnl_lock()..rtnl_unlock() ?
> > >
> > > This is also a solution. But I think it would be nice to unify with other drivers
> > > that call the functions in ndo_open/close.
> > 
> > Both approaches are equally valid. Some network drivers attach the PHY
> > at probe time (and thus can return -EPROBE_DEFER if the PHY is specified
> > but not present). Others attach in .ndo_open which can only fail in this
> > circumstance with no retry without userspace manually implementing that.
> > 
> > There are other advantages and disadvantages to each approach.
> 
> Hi,
> 
> So is it still recommended that add rtnl_lock()...rtnl_unlock() instead of moving it?

The reaosn phylink_disconnect_phy() requires the RTNL lock is because it
_can_ be called while the netdev is published, and the RTNL lock
protects the networking core from the PHY being removed from the netdev,
preventing ethtool ops into the PHY driver from running concurrently
with the PHY's disconnection and potential later destruction.

Offering two APIs, one which requires the lock to provide that
protection and one which doesn't would over-complicate the phylink code
and make reviews way more difficult, as we'd now have to spot the
wrong function being used in the wrong code path.

It's simpler for drivers that want to connect and disconnect the PHY
at probe/remove time for them to just take the RTNL lock briefly over
the call to phylink_disconnect_phy().

There is no "recommendation" for connecting and disconnecting the
PHY at probe/remove time vs ndo_open/ndo_release. That's entirely up
to the driver author. As I've already said, there are advantages and
disadvantages of either way and that's a matter for the driver author
to consider and select the most appropriate choice for their driver.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!