[PATCH net-next 05/11] can: vxcan: support per-netns device unregistration.

Marc Kleine-Budde <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-can
Message-ID <[email protected]>
From: Kuniyuki Iwashima <[email protected]>

Currently, vxcan_dellink() unregisters both local and peer devices
synchronously under RTNL.

Once RTNL is removed, it can be called concurrently from different
netns.

Let's use xchg() and unregister_netdevice_queue_net() to support
per-netns device unregistration.

This way, each device is queued for destruction only once by
the winner of the race.

Note that the extra netdev_hold() ensures that @peer obtained by
the first xchg() is not freed during the subsequent access to
netdev_priv(peer).  The 2nd xchg() overwrites @dev to balance
the refcount.

Tested:

1. Create two vxcan pairs (vxcan1-2, vxcan3-4) between two netns
   (ns1 & ns2).

  # ip netns add ns1
  # ip netns add ns2
  # ip -n ns1 link add vxcan1 type vxcan peer vxcan2 netns ns2
  # ip -n ns1 link add vxcan3 type vxcan peer vxcan4 netns ns2

2. Run bpftrace to check if the same process does NOT
   unregister the paired vxcan devices

  # bpftrace -e '#include <linux/netdevice.h>
  kprobe:free_netdev {
      $dev = (struct net_device *)arg0;
      printf("PID: %d | DEV: %s%s\n", pid, $dev->name, kstack());
  }'

3. Remove vxcan2 in ns2 and check bpftrace output

  # ip -n ns2 link del vxcan2

  PID: 1524 | DEV: vxcan2
          free_netdev+5
          netdev_run_todo+4798
          rtnl_dellink+1507
          rtnetlink_rcv_msg+1791
          netlink_rcv_skb+504
  ...
  PID: 453 | DEV: vxcan1
          free_netdev+5
          netdev_run_todo+4798
          process_scheduled_works+2538
          worker_thread+1906
          kthread+806
          ret_from_fork+805
          ret_from_fork_asm+17

4. Remove ns2 (thus vxcan4) and check bpftrace output

  # ip netns del ns2

  PID: 12 | DEV: vxcan4
          free_netdev+5
          netdev_run_todo+4798
          default_device_exit_batch+2271
          ops_undo_list+993
          cleanup_net+1122
          process_scheduled_works+2538
          worker_thread+1906
          kthread+806
          ret_from_fork+805
          ret_from_fork_asm+17
  ...
  PID: 462 | DEV: vxcan3
          free_netdev+5
          netdev_run_todo+4798
          process_scheduled_works+2538
          worker_thread+1906
          kthread+806
          ret_from_fork+805
          ret_from_fork_asm+17

Signed-off-by: Kuniyuki Iwashima <[email protected]>
Link: https://patch.msgid.link/[email protected]
[mkl: fix indention struct vxcan_priv::peer_tracker]
Signed-off-by: Marc Kleine-Budde <[email protected]>
---
 drivers/net/can/vxcan.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index e882250180ef..9e2e25d02471 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -33,6 +33,7 @@ MODULE_ALIAS_RTNL_LINK(DRV_NAME);
 
 struct vxcan_priv {
 	struct net_device __rcu	*peer;
+	netdevice_tracker peer_tracker;
 };
 
 static netdev_tx_t vxcan_xmit(struct sk_buff *oskb, struct net_device *dev)
@@ -268,9 +269,11 @@ static int vxcan_newlink(struct net_device *dev,
 	/* cross link the device pair */
 	priv = netdev_priv(dev);
 	rcu_assign_pointer(priv->peer, peer);
+	netdev_hold(peer, &priv->peer_tracker, GFP_KERNEL);
 
 	priv = netdev_priv(peer);
 	rcu_assign_pointer(priv->peer, dev);
+	netdev_hold(dev, &priv->peer_tracker, GFP_KERNEL);
 
 	return 0;
 
@@ -281,24 +284,25 @@ static int vxcan_newlink(struct net_device *dev,
 
 static void vxcan_dellink(struct net_device *dev, struct list_head *head)
 {
+	netdevice_tracker *peer_tracker;
 	struct vxcan_priv *priv;
 	struct net_device *peer;
 
 	priv = netdev_priv(dev);
-	peer = rtnl_dereference(priv->peer);
+	peer_tracker = &priv->peer_tracker;
+	peer = unrcu_pointer(xchg(&priv->peer, NULL));
+	if (!peer)
+		return;
 
-	/* Note : dellink() is called from default_device_exit_batch(),
-	 * before a rcu_synchronize() point. The devices are guaranteed
-	 * not being freed before one RCU grace period.
-	 */
-	RCU_INIT_POINTER(priv->peer, NULL);
 	unregister_netdevice_queue(dev, head);
 
-	if (peer) {
-		priv = netdev_priv(peer);
-		RCU_INIT_POINTER(priv->peer, NULL);
-		unregister_netdevice_queue(peer, head);
-	}
+	priv = netdev_priv(peer);
+	dev = unrcu_pointer(xchg(&priv->peer, NULL));
+	if (dev)
+		unregister_netdevice_queue_net(dev_net(dev), peer, head);
+
+	netdev_put(peer, peer_tracker);
+	netdev_put(dev, &priv->peer_tracker);
 }
 
 static const struct nla_policy vxcan_policy[VXCAN_INFO_MAX + 1] = {
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.