[PATCH v1 net-next 0/2] pfcp: Support per-netns device unregistration.

[PATCH v1 net-next 0/2] pfcp: Support per-netns device unregistration.

[Kuniyuki Iwashima]

Patch 1 adds a per-netns mutex for pfcp_net.pfcp_dev_list.

Patch 2 supports per-netns netdev unreg by using
unregister_netdevice_queue_net().


Kuniyuki Iwashima (2):
  pfcp: Protect pfcp_net.pfcp_dev_list with mutex.
  pfcp: Support per-netns netdev unregistration.

 drivers/net/pfcp.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

-- 
2.55.0.571.g244d577d93-goog

[PATCH v1 net-next 1/2] pfcp: Protect pfcp_net.pfcp_dev_list with mutex.

[Kuniyuki Iwashima]

struct pfcp_dev.net is the netns where the backend pfcp socket
resides.

struct pfcp_dev is linked to the pfcp_net.pfcp_dev_list of
the socket's netns.

During netns dismantle or module unload, pfcp_net_exit_rtnl()
iterates the list and queues devices for destruction regardless
of the devices' netns.

Thus, once RTNL is removed, the list can be modified concurrently
from different netns due to device removal.

Let's protect it with per-netns mutex.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/pfcp.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pfcp.c b/drivers/net/pfcp.c
index d8e4d60f5834..45f9c5d3cb20 100644
--- a/drivers/net/pfcp.c
+++ b/drivers/net/pfcp.c
@@ -29,6 +29,7 @@ static unsigned int pfcp_net_id __read_mostly;
 
 struct pfcp_net {
 	struct list_head	pfcp_dev_list;
+	struct mutex		lock;
 };
 
 static void
@@ -209,7 +210,10 @@ static int pfcp_newlink(struct net_device *dev,
 	}
 
 	pn = net_generic(link_net, pfcp_net_id);
+
+	mutex_lock(&pn->lock);
 	list_add(&pfcp->list, &pn->pfcp_dev_list);
+	mutex_unlock(&pn->lock);
 
 	netdev_dbg(dev, "registered new PFCP interface\n");
 
@@ -223,7 +227,7 @@ static int pfcp_newlink(struct net_device *dev,
 	return err;
 }
 
-static void pfcp_dellink(struct net_device *dev, struct list_head *head)
+static void __pfcp_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct pfcp_dev *pfcp = netdev_priv(dev);
 
@@ -231,6 +235,18 @@ static void pfcp_dellink(struct net_device *dev, struct list_head *head)
 	unregister_netdevice_queue(dev, head);
 }
 
+static void pfcp_dellink(struct net_device *dev, struct list_head *head)
+{
+	struct pfcp_dev *pfcp = netdev_priv(dev);
+	struct pfcp_net *pn;
+
+	pn = net_generic(pfcp->net, pfcp_net_id);
+
+	mutex_lock(&pn->lock);
+	__pfcp_dellink(dev, head);
+	mutex_unlock(&pn->lock);
+}
+
 static struct rtnl_link_ops pfcp_link_ops __read_mostly = {
 	.kind		= "pfcp",
 	.priv_size	= sizeof(struct pfcp_dev),
@@ -244,6 +260,8 @@ static int __net_init pfcp_net_init(struct net *net)
 	struct pfcp_net *pn = net_generic(net, pfcp_net_id);
 
 	INIT_LIST_HEAD(&pn->pfcp_dev_list);
+	mutex_init(&pn->lock);
+
 	return 0;
 }
 
@@ -253,8 +271,12 @@ static void __net_exit pfcp_net_exit_rtnl(struct net *net,
 	struct pfcp_net *pn = net_generic(net, pfcp_net_id);
 	struct pfcp_dev *pfcp, *pfcp_next;
 
+	mutex_lock(&pn->lock);
+
 	list_for_each_entry_safe(pfcp, pfcp_next, &pn->pfcp_dev_list, list)
-		pfcp_dellink(pfcp->dev, dev_to_kill);
+		__pfcp_dellink(pfcp->dev, dev_to_kill);
+
+	mutex_unlock(&pn->lock);
 }
 
 static struct pernet_operations pfcp_net_ops = {
-- 
2.55.0.571.g244d577d93-goog

[PATCH v1 net-next 2/2] pfcp: Support per-netns netdev unregistration.

[Kuniyuki Iwashima]

pfcp_net_exit_rtnl() iterates pfcp devices whose sockets
are in the dying netns and queues them for destruction.

So the devices may reside in different netns.

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

list_del() is changed to list_del_init() to avoid queueing the
same device twice.

Even after pfcp_net_exit_rtnl() queues a cross-netns pfcp device,
pfcp_dellink() could be called concurrently for it (once RTNL is
removed).  In such a case, __rtnl_net_unlock() will perform the
unregistration.

We can see pfcp0 below is unregistered by the per-netns work
instead of cleanup_net().

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

  # ip netns add ns1
  # ip netns add ns2
  # ip -n ns1 link add pfcp0 link-netns ns2 type pfcp
  # ip netns del ns2

  PID: 12
          pfcp_net_exit_rtnl+5
          ops_undo_list+702
          cleanup_net+1122
          process_scheduled_works+2538
  ...
  PID: 462 | DEV: pfcp0
          pfcp_dev_uninit+5
          unregister_netdevice_many_notify+7129
          unregister_netdevice_many_net+1050
          rtnl_net_work_func+136
          process_scheduled_works+2538

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/pfcp.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pfcp.c b/drivers/net/pfcp.c
index 45f9c5d3cb20..e1cca779d2ec 100644
--- a/drivers/net/pfcp.c
+++ b/drivers/net/pfcp.c
@@ -227,12 +227,13 @@ static int pfcp_newlink(struct net_device *dev,
 	return err;
 }
 
-static void __pfcp_dellink(struct net_device *dev, struct list_head *head)
+static void __pfcp_dellink(struct net *net, struct net_device *dev,
+			   struct list_head *head)
 {
 	struct pfcp_dev *pfcp = netdev_priv(dev);
 
-	list_del(&pfcp->list);
-	unregister_netdevice_queue(dev, head);
+	list_del_init(&pfcp->list);
+	unregister_netdevice_queue_net(net, dev, head);
 }
 
 static void pfcp_dellink(struct net_device *dev, struct list_head *head)
@@ -243,7 +244,8 @@ static void pfcp_dellink(struct net_device *dev, struct list_head *head)
 	pn = net_generic(pfcp->net, pfcp_net_id);
 
 	mutex_lock(&pn->lock);
-	__pfcp_dellink(dev, head);
+	if (!list_empty(&pfcp->list))
+		__pfcp_dellink(dev_net(dev), dev, head);
 	mutex_unlock(&pn->lock);
 }
 
@@ -274,7 +276,7 @@ static void __net_exit pfcp_net_exit_rtnl(struct net *net,
 	mutex_lock(&pn->lock);
 
 	list_for_each_entry_safe(pfcp, pfcp_next, &pn->pfcp_dev_list, list)
-		__pfcp_dellink(pfcp->dev, dev_to_kill);
+		__pfcp_dellink(net, pfcp->dev, dev_to_kill);
 
 	mutex_unlock(&pn->lock);
 }
-- 
2.55.0.571.g244d577d93-goog