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

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

[Kuniyuki Iwashima]

Patch 1 is a prep patch to make patch 2 clean, which
adds a per-netns mutex for geneve linked lists.

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


Kuniyuki Iwashima (3):
  geneve: Unlink geneve->sock[46].hlist[46].hlist in
    __geneve_sock_release().
  geneve: Protect geneve_net and geneve_sock with per-netns mutex.
  geneve: Support per-netns netdev unregistration.

 drivers/net/geneve.c | 126 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 95 insertions(+), 31 deletions(-)

-- 
2.55.0.571.g244d577d93-goog

[PATCH v1 net-next 1/3] geneve: Unlink geneve->sock[46].hlist[46].hlist in __geneve_sock_release().

[Kuniyuki Iwashima]

Currently, geneve->sock[46].hlist[46] is unliked from
geneve_sock.vni_list in geneve_stop() and geneve_sock.refcnt is
decremented for each socket later in __geneve_sock_release().

The following patch will introduce a mutex in geneve_net to
protect geneve_sock.{refcnt,vni_list}.

However, udp_tunnel_notify_del_rx_port() must be outside of the
lock to avoid AB-BA deadlock.

To make the change cleaner, let's move hlist_del_init_rcu()
from geneve_stop() to __geneve_sock_release().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/geneve.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 542e53f9dc3e..bd4fc6dba2fa 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1052,9 +1052,30 @@ static struct geneve_sock *geneve_socket_create(struct net *net,
 	return gs;
 }
 
-static void __geneve_sock_release(struct geneve_sock *gs)
+static void __geneve_sock_release(struct geneve_dev *geneve, bool ipv6)
 {
-	if (!gs || --gs->refcnt)
+	struct geneve_dev_node *node;
+	struct geneve_sock *gs;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (ipv6) {
+		gs = rtnl_dereference(geneve->sock6);
+		rcu_assign_pointer(geneve->sock6, NULL);
+		node = &geneve->hlist6;
+	} else
+#endif
+	{
+		gs = rtnl_dereference(geneve->sock4);
+		rcu_assign_pointer(geneve->sock4, NULL);
+		node = &geneve->hlist4;
+	}
+
+	if (!gs)
+		return;
+
+	hlist_del_init_rcu(&node->hlist);
+
+	if (--gs->refcnt)
 		return;
 
 	list_del(&gs->list);
@@ -1065,19 +1086,10 @@ static void __geneve_sock_release(struct geneve_sock *gs)
 
 static void geneve_sock_release(struct geneve_dev *geneve)
 {
-	struct geneve_sock *gs4 = rtnl_dereference(geneve->sock4);
 #if IS_ENABLED(CONFIG_IPV6)
-	struct geneve_sock *gs6 = rtnl_dereference(geneve->sock6);
-
-	rcu_assign_pointer(geneve->sock6, NULL);
-#endif
-
-	rcu_assign_pointer(geneve->sock4, NULL);
-
-	__geneve_sock_release(gs4);
-#if IS_ENABLED(CONFIG_IPV6)
-	__geneve_sock_release(gs6);
+	__geneve_sock_release(geneve, true);
 #endif
+	__geneve_sock_release(geneve, false);
 }
 
 static struct geneve_sock *geneve_find_sock(struct net *net,
@@ -1187,10 +1199,6 @@ static int geneve_stop(struct net_device *dev)
 {
 	struct geneve_dev *geneve = netdev_priv(dev);
 
-	hlist_del_init_rcu(&geneve->hlist4.hlist);
-#if IS_ENABLED(CONFIG_IPV6)
-	hlist_del_init_rcu(&geneve->hlist6.hlist);
-#endif
 	geneve_sock_release(geneve);
 	return 0;
 }
-- 
2.55.0.571.g244d577d93-goog

[PATCH v1 net-next 2/3] geneve: Protect geneve_net and geneve_sock with per-netns mutex.

[Kuniyuki Iwashima]

struct geneve_dev.net is the netns where the backend geneve
socket resides.

struct geneve_dev is linked to the geneve_net.geneve_list of
the socket's netns.

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

Moreover, a socket can be shared by multiple geneve devices in
different netns, and geneve_open() and geneve_stop() modify
geneve_sock.vni_list and geneve_net.sock_list.

Thus, once RTNL is removed, the three lists can be modified
concurrently from different netns due to device removal and
link-up/down.

Let's protect them with per-netns mutex.

geneve_newlink() is still protected by rtnl_net_lock()s, so
acquiring gn->lock twice in geneve_find_dev() and
geneve_configure() is not a problem.

Note that udp_tunnel_notify_add_rx_port() is moved outside of
the mutex, otherwise gn->lock -> utn->lock ordering would trigger
AB-BA deadlock in geneve_offload_rx_ports(), which acquires
gn->lock under utn->lock.  Even without gn->lock, geneve_sock_add()
and geneve_offload_rx_ports() are still serialised with (per-netns)
RTNL, so there is no race.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/geneve.c | 82 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 68 insertions(+), 14 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index bd4fc6dba2fa..f456a85dca77 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -69,8 +69,8 @@ struct geneve_skb_cb {
 /* per-network namespace private data for this module */
 struct geneve_net {
 	struct list_head	geneve_list;
-	/* sock_list is protected by rtnl lock */
 	struct list_head	sock_list;
+	struct mutex		lock;
 };
 
 static unsigned int geneve_net_id;
@@ -1035,9 +1035,6 @@ static struct geneve_sock *geneve_socket_create(struct net *net,
 	for (h = 0; h < VNI_HASH_SIZE; ++h)
 		INIT_HLIST_HEAD(&gs->vni_list[h]);
 
-	/* Initialize the geneve udp offloads structure */
-	udp_tunnel_notify_add_rx_port(sk, UDP_TUNNEL_TYPE_GENEVE);
-
 	/* Mark socket as an encapsulation socket */
 	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
 	tunnel_cfg.sk_user_data = gs;
@@ -1056,6 +1053,7 @@ static void __geneve_sock_release(struct geneve_dev *geneve, bool ipv6)
 {
 	struct geneve_dev_node *node;
 	struct geneve_sock *gs;
+	struct geneve_net *gn;
 
 #if IS_ENABLED(CONFIG_IPV6)
 	if (ipv6) {
@@ -1073,12 +1071,19 @@ static void __geneve_sock_release(struct geneve_dev *geneve, bool ipv6)
 	if (!gs)
 		return;
 
+	gn = net_generic(sock_net(gs->sk), geneve_net_id);
+	mutex_lock(&gn->lock);
+
 	hlist_del_init_rcu(&node->hlist);
 
-	if (--gs->refcnt)
+	if (--gs->refcnt) {
+		mutex_unlock(&gn->lock);
 		return;
+	}
 
 	list_del(&gs->list);
+	mutex_unlock(&gn->lock);
+
 	udp_tunnel_notify_del_rx_port(gs->sk, UDP_TUNNEL_TYPE_GENEVE);
 	udp_tunnel_sock_release(gs->sk);
 	kfree_rcu(gs, rcu);
@@ -1135,22 +1140,31 @@ static int geneve_sock_add(struct geneve_dev *geneve,
 	struct net *net = geneve->net;
 	struct geneve_dev_node *node;
 	struct geneve_sock *gs;
+	struct geneve_net *gn;
+	bool created = false;
 	__u8 vni[3];
+	int ret = 0;
 	__u32 hash;
 
+	gn = net_generic(net, geneve_net_id);
+	mutex_lock(&gn->lock);
+
 	gs = geneve_find_sock(net, geneve, cfg, ipv6);
 	if (gs) {
 		gs->refcnt++;
-		goto out;
-	}
+	} else {
+		gs = geneve_socket_create(net, geneve, cfg, ipv6);
+		if (IS_ERR(gs)) {
+			ret = PTR_ERR(gs);
+			goto out;
+		}
 
-	gs = geneve_socket_create(net, geneve, cfg, ipv6);
-	if (IS_ERR(gs))
-		return PTR_ERR(gs);
+		created = true;
+	}
 
-out:
 	gs->collect_md = cfg->collect_md;
 	gs->gro_hint = cfg->gro_hint;
+
 #if IS_ENABLED(CONFIG_IPV6)
 	if (ipv6) {
 		rcu_assign_pointer(geneve->sock6, gs);
@@ -1166,7 +1180,16 @@ static int geneve_sock_add(struct geneve_dev *geneve,
 	tunnel_id_to_vni(cfg->info.key.tun_id, vni);
 	hash = geneve_net_vni_hash(vni);
 	hlist_add_head_rcu(&node->hlist, &gs->vni_list[hash]);
-	return 0;
+
+out:
+	mutex_unlock(&gn->lock);
+
+	if (created) {
+		/* Initialize the geneve udp offloads structure */
+		udp_tunnel_notify_add_rx_port(gs->sk, UDP_TUNNEL_TYPE_GENEVE);
+	}
+
+	return ret;
 }
 
 static int geneve_open(struct net_device *dev)
@@ -1743,6 +1766,8 @@ static void geneve_offload_rx_ports(struct net_device *dev, bool push)
 
 	ASSERT_RTNL();
 
+	mutex_lock(&gn->lock);
+
 	list_for_each_entry(gs, &gn->sock_list, list) {
 		if (push) {
 			udp_tunnel_push_rx_port(dev, gs->sk,
@@ -1752,6 +1777,8 @@ static void geneve_offload_rx_ports(struct net_device *dev, bool push)
 						UDP_TUNNEL_TYPE_GENEVE);
 		}
 	}
+
+	mutex_unlock(&gn->lock);
 }
 
 static struct geneve_config *geneve_config_alloc(const struct geneve_config *src)
@@ -1968,6 +1995,9 @@ static struct geneve_dev *geneve_find_dev(struct geneve_net *gn,
 
 	*tun_on_same_port = false;
 	*tun_collect_md = false;
+
+	mutex_lock(&gn->lock);
+
 	list_for_each_entry(geneve, &gn->geneve_list, next) {
 		const struct geneve_config *gcfg = rtnl_dereference(geneve->cfg);
 
@@ -1982,6 +2012,9 @@ static struct geneve_dev *geneve_find_dev(struct geneve_net *gn,
 		    !memcmp(&info->key.u, &gcfg->info.key.u, sizeof(info->key.u)))
 			t = geneve;
 	}
+
+	mutex_unlock(&gn->lock);
+
 	return t;
 }
 
@@ -2076,7 +2109,10 @@ static int geneve_configure(struct net *net, struct net_device *dev,
 		return err;
 	}
 
+	mutex_lock(&gn->lock);
 	list_add(&geneve->next, &gn->geneve_list);
+	mutex_unlock(&gn->lock);
+
 	return 0;
 }
 
@@ -2466,7 +2502,7 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
 	return err;
 }
 
-static void geneve_dellink(struct net_device *dev, struct list_head *head)
+static void __geneve_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct geneve_dev *geneve = netdev_priv(dev);
 
@@ -2474,6 +2510,18 @@ static void geneve_dellink(struct net_device *dev, struct list_head *head)
 	unregister_netdevice_queue(dev, head);
 }
 
+static void geneve_dellink(struct net_device *dev, struct list_head *head)
+{
+	struct geneve_dev *geneve = netdev_priv(dev);
+	struct geneve_net *gn;
+
+	gn = net_generic(geneve->net, geneve_net_id);
+
+	mutex_lock(&gn->lock);
+	__geneve_dellink(dev, head);
+	mutex_unlock(&gn->lock);
+}
+
 static size_t geneve_get_size(const struct net_device *dev)
 {
 	return nla_total_size(sizeof(__u32)) +	/* IFLA_GENEVE_ID */
@@ -2692,6 +2740,8 @@ static __net_init int geneve_init_net(struct net *net)
 
 	INIT_LIST_HEAD(&gn->geneve_list);
 	INIT_LIST_HEAD(&gn->sock_list);
+	mutex_init(&gn->lock);
+
 	return 0;
 }
 
@@ -2701,8 +2751,12 @@ static void __net_exit geneve_exit_rtnl_net(struct net *net,
 	struct geneve_net *gn = net_generic(net, geneve_net_id);
 	struct geneve_dev *geneve, *next;
 
+	mutex_lock(&gn->lock);
+
 	list_for_each_entry_safe(geneve, next, &gn->geneve_list, next)
-		geneve_dellink(geneve->dev, dev_to_kill);
+		__geneve_dellink(geneve->dev, dev_to_kill);
+
+	mutex_unlock(&gn->lock);
 }
 
 static void __net_exit geneve_exit_net(struct net *net)
-- 
2.55.0.571.g244d577d93-goog

[PATCH v1 net-next 3/3] geneve: Support per-netns netdev unregistration.

[Kuniyuki Iwashima]

geneve_exit_rtnl_net() iterates geneve 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 geneve_exit_rtnl_net() queues a cross-netns geneve
device, geneve_dellink() can be called concurrently for it.
In such a case, __rtnl_net_unlock() will perform the unregistration.

Note that geneve uses register_pernet_subsys() instead of _device(),
so default_device_exit_batch() guarantees that the async per-netns
works are flushed before ->exit().

Tested:

1. Create geneve device across two netns.

  # ip netns add ns1
  # ip netns add ns2
  # ip -n ns1 link add geneve0 link-netns ns2 type geneve external

2. Run bpftrace to check that geneve_uninit() is called between
   ->exit_rtnl() and ->exit().

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

3. Remove the netns where the geneve socket resides

  # ip netns del ns2

Now, we can see geneve0 is unregistered by per-netns work
instead of cleanup_net() and it finishes before ->exit() to
avoid WARN_ON_ONCE(!list_empty(&gn->sock_list)) there.

  PID: 571
          geneve_exit_rtnl_net+5
          ops_undo_list+702
          cleanup_net+1122
          process_scheduled_works+2538
  ...
  PID: 1047 | DEV: geneve0
          geneve_uninit+5
          unregister_netdevice_many_notify+7129
          unregister_netdevice_many_net+1050
          rtnl_net_work_func+136
          process_scheduled_works+2538
  ...
  PID: 571
          geneve_exit_net+5
          ops_undo_list+1064
          cleanup_net+1122
          process_scheduled_works+2538
  ...

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

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f456a85dca77..a6a8978e3b81 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -2502,12 +2502,13 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
 	return err;
 }
 
-static void __geneve_dellink(struct net_device *dev, struct list_head *head)
+static void __geneve_dellink(struct net *net, struct net_device *dev,
+			     struct list_head *head)
 {
 	struct geneve_dev *geneve = netdev_priv(dev);
 
-	list_del(&geneve->next);
-	unregister_netdevice_queue(dev, head);
+	list_del_init(&geneve->next);
+	unregister_netdevice_queue_net(net, dev, head);
 }
 
 static void geneve_dellink(struct net_device *dev, struct list_head *head)
@@ -2518,7 +2519,8 @@ static void geneve_dellink(struct net_device *dev, struct list_head *head)
 	gn = net_generic(geneve->net, geneve_net_id);
 
 	mutex_lock(&gn->lock);
-	__geneve_dellink(dev, head);
+	if (!list_empty(&geneve->next))
+		__geneve_dellink(dev_net(dev), dev, head);
 	mutex_unlock(&gn->lock);
 }
 
@@ -2754,7 +2756,7 @@ static void __net_exit geneve_exit_rtnl_net(struct net *net,
 	mutex_lock(&gn->lock);
 
 	list_for_each_entry_safe(geneve, next, &gn->geneve_list, next)
-		__geneve_dellink(geneve->dev, dev_to_kill);
+		__geneve_dellink(net, geneve->dev, dev_to_kill);
 
 	mutex_unlock(&gn->lock);
 }
-- 
2.55.0.571.g244d577d93-goog