Re: [PATCH v2 net-next 08/14] veth: Support per-netns device unregistration.
From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2026-07-06 16:44:07
Replying to Sashiko review, On Thu, Jul 2, 2026 at 5:10 PM Kuniyuki Iwashima [off-list ref] wrote: [...]
quoted hunk ↗ jump to hunk
@@ -1901,15 +1902,17 @@ static int veth_newlink(struct net_device *dev, priv = netdev_priv(dev); rcu_assign_pointer(priv->peer, peer); + netdev_hold(peer, &priv->peer_tracker, GFP_KERNEL);
---8<--- Because the devices are already registered and visible to the system at this point, could a concurrent ip link del trigger veth_dellink() right after rcu_assign_pointer() publishes priv->peer, but before netdev_hold() is called? ---8<--- No, ->newlink() and ->dellink() are serialised with the same rtnl_net_lock().
quoted hunk ↗ jump to hunk
err = veth_init_queues(dev, tb); if (err) goto err_queues; priv = netdev_priv(peer); rcu_assign_pointer(priv->peer, dev); + netdev_hold(dev, &priv->peer_tracker, GFP_KERNEL); err = veth_init_queues(peer, tb); if (err) - goto err_queues; + goto err_peer_queues; veth_disable_gro(dev); /* update XDP supported features */@@ -1918,7 +1921,11 @@ static int veth_newlink(struct net_device *dev, return 0; +err_peer_queues: + netdev_put(dev, &priv->peer_tracker); + priv = netdev_priv(dev); err_queues: + netdev_put(peer, &priv->peer_tracker); unregister_netdevice(dev);
---8<--- If veth_init_queues() fails, does this error path safely handle a concurrent veth_dellink()? ---8<--- This depends on the same wrong assumption, so no, ->newlink() holds multiple rtnl_net_lock() so that the concurrent ->dellink() never occurs.
quoted hunk ↗ jump to hunk
err_register_dev: /* nothing to do */@@ -1933,24 +1940,25 @@ static int veth_newlink(struct net_device *dev, static void veth_dellink(struct net_device *dev, struct list_head *head) { - struct veth_priv *priv; + netdevice_tracker *peer_tracker; struct net_device *peer; + struct veth_priv *priv; priv = netdev_priv(dev); - peer = rtnl_dereference(priv->peer); + peer_tracker = &priv->peer_tracker; + peer = unrcu_pointer(xchg(&priv->peer, NULL));
---8<--- Does changing this to use xchg() introduce a data race with read paths? Functions like veth_set_xdp_features() still use rtnl_dereference(), ... Might this need READ_ONCE() or rcu_dereference() to safely handle the concurrent xchg() writes? ---8<--- Yes, but with another series before removing RTNL. As mentioned in the cover letter, the device unreg path is the only target of this series, and we need more changes in other paths.