[PATCH net-next] net: free altname using an RCU callback

[PATCH net-next] net: free altname using an RCU callback

[Jakub Kicinski]

We had to add another synchronize_rcu() in recent fix.
Bite the bullet and add an rcu_head to netdev_name_node,
free from RCU.

Note that name_node does not hold any reference on dev
to which it points, but there must be a synchronize_rcu()
on device removal path, so we should be fine.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: daniel@iogearbox.net
CC: jiri@resnulli.us
CC: lucien.xin@gmail.com
CC: johannes.berg@intel.com
---
 net/core/dev.c | 27 ++++++++++++++++-----------
 net/core/dev.h |  1 +
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index cb2dab0feee0..b53b9c94de40 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -341,13 +341,22 @@ int netdev_name_node_alt_create(struct net_device *dev, const char *name)
 	return 0;
 }
 
-static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
+static void netdev_name_node_alt_free(struct rcu_head *head)
 {
-	list_del(&name_node->list);
+	struct netdev_name_node *name_node =
+		container_of(head, struct netdev_name_node, rcu);
+
 	kfree(name_node->name);
 	netdev_name_node_free(name_node);
 }
 
+static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
+{
+	netdev_name_node_del(name_node);
+	list_del(&name_node->list);
+	call_rcu(&name_node->rcu, netdev_name_node_alt_free);
+}
+
 int netdev_name_node_alt_destroy(struct net_device *dev, const char *name)
 {
 	struct netdev_name_node *name_node;
@@ -362,10 +371,7 @@ int netdev_name_node_alt_destroy(struct net_device *dev, const char *name)
 	if (name_node == dev->name_node || name_node->dev != dev)
 		return -EINVAL;
 
-	netdev_name_node_del(name_node);
-	synchronize_rcu();
 	__netdev_name_node_alt_destroy(name_node);
-
 	return 0;
 }
 
@@ -373,8 +379,10 @@ static void netdev_name_node_alt_flush(struct net_device *dev)
 {
 	struct netdev_name_node *name_node, *tmp;
 
-	list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
-		__netdev_name_node_alt_destroy(name_node);
+	list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list) {
+		list_del(&name_node->list);
+		netdev_name_node_alt_free(&name_node->rcu);
+	}
 }
 
 /* Device list insertion */
@@ -11576,11 +11584,8 @@ static void __net_exit default_device_exit_net(struct net *net)
 			snprintf(fb_name, IFNAMSIZ, "dev%%d");
 
 		netdev_for_each_altname_safe(dev, name_node, tmp)
-			if (netdev_name_in_use(&init_net, name_node->name)) {
-				netdev_name_node_del(name_node);
-				synchronize_rcu();
+			if (netdev_name_in_use(&init_net, name_node->name))
 				__netdev_name_node_alt_destroy(name_node);
-			}
 
 		err = dev_change_net_namespace(dev, &init_net, fb_name);
 		if (err) {
diff --git a/net/core/dev.h b/net/core/dev.h
index 7480b4c84298..a43dfe3de50e 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -56,6 +56,7 @@ struct netdev_name_node {
 	struct list_head list;
 	struct net_device *dev;
 	const char *name;
+	struct rcu_head rcu;
 };
 
 int netdev_get_name(struct net *net, char *name, int ifindex);
-- 
2.43.0

Re: [PATCH net-next] net: free altname using an RCU callback

[Jiri Pirko]

Fri, Jan 26, 2024 at 09:14:49PM CET, kuba@kernel.org wrote:
>We had to add another synchronize_rcu() in recent fix.
>Bite the bullet and add an rcu_head to netdev_name_node,
>free from RCU.
>
>Note that name_node does not hold any reference on dev
>to which it points, but there must be a synchronize_rcu()
>on device removal path, so we should be fine.
>
>Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Looks good to me.

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

Re: [PATCH net-next] net: free altname using an RCU callback

[Eric Dumazet]

On Fri, Jan 26, 2024 at 9:14 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> We had to add another synchronize_rcu() in recent fix.
> Bite the bullet and add an rcu_head to netdev_name_node,
> free from RCU.
>
> Note that name_node does not hold any reference on dev
> to which it points, but there must be a synchronize_rcu()
> on device removal path, so we should be fine.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net-next] net: free altname using an RCU callback

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 26 Jan 2024 12:14:49 -0800 you wrote:
> We had to add another synchronize_rcu() in recent fix.
> Bite the bullet and add an rcu_head to netdev_name_node,
> free from RCU.
> 
> Note that name_node does not hold any reference on dev
> to which it points, but there must be a synchronize_rcu()
> on device removal path, so we should be fine.
> 
> [...]

Here is the summary with links:
  - [net-next] net: free altname using an RCU callback
    https://git.kernel.org/netdev/net-next/c/723de3ebef03

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html