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

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

[Jiawen Wu]

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.

 ------------[ cut here ]------------
 RTNL: assertion failed at drivers/net/phy/phylink.c (2351)
 WARNING: drivers/net/phy/phylink.c:2351 at
phylink_disconnect_phy+0xd8/0xf0 [phylink], CPU#0: rmmod/4464
 Modules linked in: ...
 CPU: 0 UID: 0 PID: 4464 Comm: rmmod Kdump: loaded Not tainted 7.0.0-rc4+
 Hardware name: Micro-Star International Co., Ltd. MS-7E16/X670E GAMING
PLUS WIFI (MS-7E16), BIOS 1.90 12/31/2024
 RIP: 0010:phylink_disconnect_phy+0xe4/0xf0 [phylink]
 Code: 5b 41 5c 41 5d 41 5e 41 5f 5d 31 c0 31 d2 31 f6 31 ff e9 3a 38 8f e7
48 8d 3d 48 87 e2 ff ba 2f 09 00 00 48 c7 c6 c1 22 24 c0 <67> 48 0f b9 3a
e9 34 ff ff ff 66 90 90 90 90 90 90 90 90 90 90 90
 RSP: 0018:ffffce7288363ac0 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: ffff89654b2a1a00 RCX: 0000000000000000
 RDX: 000000000000092f RSI: ffffffffc02422c1 RDI: ffffffffc0239020
 RBP: ffffce7288363ae8 R08: 0000000000000000 R09: 0000000000000000
 R10: 0000000000000000 R11: 0000000000000000 R12: ffff8964c4022000
 R13: ffff89654fce3028 R14: ffff89654ebb4000 R15: ffffffffc0226348
 FS:  0000795e80d93780(0000) GS:ffff896c52857000(0000)
knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00005b528b592000 CR3: 0000000170d0f000 CR4: 0000000000f50ef0
 PKRU: 55555554
 Call Trace:
  <TASK>
  txgbe_remove_phy+0xbb/0xd0 [txgbe]
  txgbe_remove+0x4c/0xb0 [txgbe]
  pci_device_remove+0x41/0xb0
  device_remove+0x43/0x80
  device_release_driver_internal+0x206/0x270
  driver_detach+0x4a/0xa0
  bus_remove_driver+0x83/0x120
  driver_unregister+0x2f/0x60
  pci_unregister_driver+0x40/0x90
  txgbe_driver_exit+0x10/0x850 [txgbe]
  __do_sys_delete_module.isra.0+0x1c3/0x2f0
  __x64_sys_delete_module+0x12/0x20
  x64_sys_call+0x20c3/0x2390
  do_syscall_64+0x11c/0x1500
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? do_syscall_64+0x15a/0x1500
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? do_fault+0x312/0x580
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? __handle_mm_fault+0x9d5/0x1040
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? count_memcg_events+0x101/0x1d0
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? handle_mm_fault+0x1e8/0x2f0
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? do_user_addr_fault+0x2f8/0x820
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? irqentry_exit+0xb2/0x600
  ? srso_alias_return_thunk+0x5/0xfbef5
  ? exc_page_fault+0x92/0x1c0
  entry_SYSCALL_64_after_hwframe+0x76/0x7e

Fixes: 02b2a6f91b90 ("net: txgbe: support copper NIC with external PHY")
Cc: stable@vger.kernel.org
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/txgbe/txgbe_main.c |  8 ++++++++
 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c  | 11 -----------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index 0de051450a82..b4f95b3188dd 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -474,6 +474,12 @@ static int txgbe_open(struct net_device *netdev)
 	if (err)
 		goto err_free_irq;
 
+	if (wx->phydev) {
+		err = phylink_connect_phy(wx->phylink, wx->phydev);
+		if (err)
+			goto err_free_irq;
+	}
+
 	wx_ptp_init(wx);
 
 	txgbe_up_complete(wx);
@@ -527,6 +533,8 @@ static int txgbe_close(struct net_device *netdev)
 	wx_free_irq(wx);
 	txgbe_free_misc_irq(wx->priv);
 	wx_free_resources(wx);
+	if (wx->phydev)
+		phylink_disconnect_phy(wx->phylink);
 	txgbe_fdir_filter_exit(wx);
 	wx_control_hw(wx, false);
 
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 8ea7aa07ae4e..7268a0c101f3 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -294,16 +294,6 @@ static int txgbe_phylink_init(struct txgbe *txgbe)
 	if (IS_ERR(phylink))
 		return PTR_ERR(phylink);
 
-	if (wx->phydev) {
-		int ret;
-
-		ret = phylink_connect_phy(phylink, wx->phydev);
-		if (ret) {
-			phylink_destroy(phylink);
-			return ret;
-		}
-	}
-
 	wx->phylink = phylink;
 
 	return 0;
@@ -657,7 +647,6 @@ void txgbe_remove_phy(struct txgbe *txgbe)
 		return;
 	case wx_mac_sp:
 		if (txgbe->wx->media_type == wx_media_copper) {
-			phylink_disconnect_phy(txgbe->wx->phylink);
 			phylink_destroy(txgbe->wx->phylink);
 			return;
 		}
-- 
2.48.1

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

[Andrew Lunn]

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.
> 
>  ------------[ cut here ]------------
>  RTNL: assertion failed at drivers/net/phy/phylink.c (2351)
>  WARNING: drivers/net/phy/phylink.c:2351 at
> phylink_disconnect_phy+0xd8/0xf0 [phylink], CPU#0: rmmod/4464
>  Modules linked in: ...
>  CPU: 0 UID: 0 PID: 4464 Comm: rmmod Kdump: loaded Not tainted 7.0.0-rc4+
>  Hardware name: Micro-Star International Co., Ltd. MS-7E16/X670E GAMING
> PLUS WIFI (MS-7E16), BIOS 1.90 12/31/2024
>  RIP: 0010:phylink_disconnect_phy+0xe4/0xf0 [phylink]
>  Code: 5b 41 5c 41 5d 41 5e 41 5f 5d 31 c0 31 d2 31 f6 31 ff e9 3a 38 8f e7
> 48 8d 3d 48 87 e2 ff ba 2f 09 00 00 48 c7 c6 c1 22 24 c0 <67> 48 0f b9 3a
> e9 34 ff ff ff 66 90 90 90 90 90 90 90 90 90 90 90
>  RSP: 0018:ffffce7288363ac0 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: ffff89654b2a1a00 RCX: 0000000000000000
>  RDX: 000000000000092f RSI: ffffffffc02422c1 RDI: ffffffffc0239020
>  RBP: ffffce7288363ae8 R08: 0000000000000000 R09: 0000000000000000
>  R10: 0000000000000000 R11: 0000000000000000 R12: ffff8964c4022000
>  R13: ffff89654fce3028 R14: ffff89654ebb4000 R15: ffffffffc0226348
>  FS:  0000795e80d93780(0000) GS:ffff896c52857000(0000)
> knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00005b528b592000 CR3: 0000000170d0f000 CR4: 0000000000f50ef0
>  PKRU: 55555554
>  Call Trace:
>   <TASK>
>   txgbe_remove_phy+0xbb/0xd0 [txgbe]
>   txgbe_remove+0x4c/0xb0 [txgbe]
>   pci_device_remove+0x41/0xb0
>   device_remove+0x43/0x80
>   device_release_driver_internal+0x206/0x270
>   driver_detach+0x4a/0xa0
>   bus_remove_driver+0x83/0x120
>   driver_unregister+0x2f/0x60
>   pci_unregister_driver+0x40/0x90
>   txgbe_driver_exit+0x10/0x850 [txgbe]
>   __do_sys_delete_module.isra.0+0x1c3/0x2f0
>   __x64_sys_delete_module+0x12/0x20
>   x64_sys_call+0x20c3/0x2390
>   do_syscall_64+0x11c/0x1500
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? do_syscall_64+0x15a/0x1500
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? do_fault+0x312/0x580
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? __handle_mm_fault+0x9d5/0x1040
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? count_memcg_events+0x101/0x1d0
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? handle_mm_fault+0x1e8/0x2f0
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? do_user_addr_fault+0x2f8/0x820
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? irqentry_exit+0xb2/0x600
>   ? srso_alias_return_thunk+0x5/0xfbef5
>   ? exc_page_fault+0x92/0x1c0
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
> 
> Fixes: 02b2a6f91b90 ("net: txgbe: support copper NIC with external PHY")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Russell King (Oracle)]

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() ?

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

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

[Jiawen Wu]

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.

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

[Russell King (Oracle)]

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.

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

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

[Jiawen Wu]

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?

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

[Russell King (Oracle)]

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!

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

[Jiawen Wu]

On Tue, Apr 7, 2026 3:41 PM, Russell King (Oracle) wrote:
> 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.

OK. I'll change it at v2 patch.