On Sat, Jan 31, 2026 at 12:41:20PM -0800, Jakub Kicinski wrote:

On Fri, 30 Jan 2026 13:34:07 +0800 Kevin Hao wrote:

quoted

--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c

What's your plan for fixing drivers/net/ethernet/ti/cpsw.c ?
My preference would be to post both of the fixes at once,

Sure, I will include the fix for cpsw.c in the next version.

I think this version is quite close, just a couple of nit picks
below..

quoted

@@ -248,15 +248,25 @@ static int cpsw_purge_all_mc(struct net_device *ndev, const u8 *addr, int num)
 	return 0;
 }
 
-static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
+static void cpsw_ndo_set_rx_mode_work(struct work_struct *work)
 {
-	struct cpsw_priv *priv = netdev_priv(ndev);
+	struct cpsw_priv *priv = container_of(work, struct cpsw_priv, rx_mode_work);
 	struct cpsw_common *cpsw = priv->cpsw;
+	struct net_device *ndev = priv->ndev;
 
+	rtnl_lock();
+	if (!netif_running(ndev)) {
+		rtnl_unlock();
+		return;

since the "undo" logic is getting complex you should use a goto.
Replace the unlock and the return; here with:

		goto unlock_rtnl;

quoted

+	}
+
+	netif_addr_lock_bh(ndev);
 	if (ndev->flags & IFF_PROMISC) {
 		/* Enable promiscuous mode */
 		cpsw_set_promiscious(ndev, true);
 		cpsw_ale_set_allmulti(cpsw->ale, IFF_ALLMULTI, priv->emac_port);
+		netif_addr_unlock_bh(ndev);
+		rtnl_unlock();

		goto unlock_addr;

quoted

 		return;
 	}
 

@@ -270,6 +280,15 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
 	/* add/remove mcast address either for real netdev or for vlan */
 	__hw_addr_ref_sync_dev(&ndev->mc, ndev, cpsw_add_mc_addr,
 			       cpsw_del_mc_addr);

And place a labels here:

unlock_addr:

quoted

+	netif_addr_unlock_bh(ndev);

unlock_rtnl:

Will do. Thanks.

quoted

+	rtnl_unlock();
+}

quoted

 	for (i = 0; i < cpsw->data.slaves; i++) {
-		if (!cpsw->slaves[i].ndev)
+		ndev = cpsw->slaves[i].ndev;
+		if (!ndev)
 			continue;
 
-		unregister_netdev(cpsw->slaves[i].ndev);
+		priv = netdev_priv(ndev);
+		disable_work_sync(&priv->rx_mode_work);
+		unregister_netdev(ndev);

I understand that this is safe but I think that more logical ordering
would be to shut things down _after_ object is unregistered.

I'm a bit confused—are you suggesting that we move disable_work_sync() after
unregister_netdev()? If that's the case, the scheduled cpsw_ndo_set_rx_mode_work()
could potentially run after the network device has been unregistered, leading to
a use-after-free issue. Or am I misunderstanding something here?

Thanks,
Kevin