Make tuns and vlans devices work per-net.

+int register_pernet_gen_device(int *id, struct pernet_operations *ops) +{
+ int error;
+ mutex_lock(&net_mutex);
+again:
+ error = ida_get_new_above(&net_generic_ids, 1, id);
+ if (error) {
+ if (error == -EAGAIN) {
+ ida_pre_get(&net_generic_ids, GFP_KERNEL);
+ goto again;
+ }

To make some per-net generic pointers, we need some way to address them, i.e. - IDs. This is simple IDA-based IDs generator for pernet subsystems. They will be used in the next patches.

Since it will be used by devices only (tun and vlan), I make it resemble the register_pernet_device functionality.

The new ids is stored in the *id pointer _before_ calling the init callback to make this id available in this callback.

Signed-off-by Pavel Emelyanov <[email protected]>

---
 include/net/net_namespace.h |    2 ++
 net/core/net_namespace.c    |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

include/net/net_namespace.h +2 -0

index 0ab62ed..6971fdb 100644
@@ -181,6 +181,8 @@ extern int register_pernet_subsys(struct pernet_operations *);
 extern void unregister_pernet_subsys(struct pernet_operations *);
 extern int register_pernet_device(struct pernet_operations *);
 extern void unregister_pernet_device(struct pernet_operations *);
+extern int register_pernet_gen_device(int *id, struct pernet_operations *);
+extern void unregister_pernet_gen_device(int id, struct pernet_operations *);
 
 struct ctl_path;
 struct ctl_table;

net/core/net_namespace.c +4 -0

index 7b66083..7ef3bac 100644
@@ -5,6 +5,7 @@
 #include <linux/list.h>
 #include <linux/delay.h>
 #include <linux/sched.h>
+#include <linux/idr.h>
 #include <net/net_namespace.h>
 
 /*

@@ -253,6 +254,8 @@ static void unregister_pernet_operations(struct pernet_operations *ops)
 }
 #endif
 
+static DEFINE_IDA(net_generic_ids);
+
 /**
  *      register_pernet_subsys - register a network namespace subsystem
  *	@ops:  pernet operations structure for the subsystem

@@ -330,6 +333,28 @@ int register_pernet_device(struct pernet_operations *ops)
 }
 EXPORT_SYMBOL_GPL(register_pernet_device);
 
+int register_pernet_gen_device(int *id, struct pernet_operations *ops)

net/core/net_namespace.c +16 -0

+	mutex_unlock(&net_mutex);
+	return error;
+}
+EXPORT_SYMBOL_GPL(register_pernet_gen_device);
+
 /**
  *      unregister_pernet_device - unregister a network namespace netdevice
  *	@ops: pernet operations structure to manipulate

@@ -348,3 +373,14 @@ void unregister_pernet_device(struct pernet_operations *ops)
 	mutex_unlock(&net_mutex);
 }
 EXPORT_SYMBOL_GPL(unregister_pernet_device);
+
+void unregister_pernet_gen_device(int id, struct pernet_operations *ops)
+{
+	mutex_lock(&net_mutex);
+	if (&ops->list == first_device)
+		first_device = first_device->next;
+	unregister_pernet_operations(ops);
+	ida_remove(&net_generic_ids, id);
+	mutex_unlock(&net_mutex);
+}
+EXPORT_SYMBOL_GPL(unregister_pernet_gen_device);

net/core/net_namespace.c int register_pernet_device(struct pernet_operations *ops) · line 346

Quoted 11 lines

net/core/net_namespace.c +11 -0

+int register_pernet_gen_device(int *id, struct pernet_operations *ops)
+{
+	int error;
+	mutex_lock(&net_mutex);
+again:
+	error = ida_get_new_above(&net_generic_ids, 1, id);
+	if (error) {
+		if (error == -EAGAIN) {
+			ida_pre_get(&net_generic_ids, GFP_KERNEL);
+			goto again;
+		}

		goto out;

net/core/net_namespace.c int register_pernet_device(struct pernet_operations *ops) · line 352

Quoted 6 lines

net/core/net_namespace.c +6 -0

+	}
+	error = register_pernet_operations(first_device, ops);
+	if (error)
+		ida_remove(&net_generic_ids, *id);
+	else if (first_device == &pernet_list)
+		first_device = &ops->list;

out:

This is the first step in making tuntap devices work in net namespaces. The structure mentioned is pointed by generic net pointer with tun_net_id id, and tun driver fills one on its load. It will contain only the tun devices list.

So declare this structure and introduce net init and exit hooks.

Signed-off-by Pavel Emelyanov <[email protected]>

---
 drivers/net/tun.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 1 deletions(-)

drivers/net/tun.c +52 -1

index 7b816a0..9bfba02 100644
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <net/net_namespace.h>
+#include <net/netns/generic.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>

@@ -73,6 +74,11 @@ static int debug;
 
 /* Network device part of the driver */
 
+static unsigned int tun_net_id;
+struct tun_net {
+	struct list_head dev_list;
+};
+
 static LIST_HEAD(tun_dev_list);
 static const struct ethtool_ops tun_ethtool_ops;
 

@@ -873,6 +879,37 @@ static const struct ethtool_ops tun_ethtool_ops = {
 	.set_rx_csum	= tun_set_rx_csum
 };
 
+static int tun_init_net(struct net *net)
+{
+	struct tun_net *tn;
+
+	tn = kmalloc(sizeof(*tn), GFP_KERNEL);
+	if (tn == NULL)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&tn->dev_list);
+
+	if (net_assign_generic(net, tun_net_id, tn)) {
+		kfree(tn);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void tun_exit_net(struct net *net)
+{
+	struct tun_net *tn;
+
+	tn = net_generic(net, tun_net_id);
+	kfree(tn);
+}
+
+static struct pernet_operations tun_net_ops = {
+	.init = tun_init_net,
+	.exit = tun_exit_net,
+};
+
 static int __init tun_init(void)
 {
 	int ret = 0;

@@ -880,9 +917,22 @@ static int __init tun_init(void)
 	printk(KERN_INFO "tun: %s, %s\n", DRV_DESCRIPTION, DRV_VERSION);
 	printk(KERN_INFO "tun: %s\n", DRV_COPYRIGHT);
 
+	ret = register_pernet_gen_device(&tun_net_id, &tun_net_ops);
+	if (ret) {
+		printk(KERN_ERR "tun: Can't register pernet ops\n");
+		goto err_pernet;
+	}
+
 	ret = misc_register(&tun_miscdev);
-	if (ret)
+	if (ret) {
 		printk(KERN_ERR "tun: Can't register misc device %d\n", TUN_MINOR);
+		goto err_misc;
+	}
+	return 0;
+
+err_misc:
+	unregister_pernet_gen_device(tun_net_id, &tun_net_ops);
+err_pernet:
 	return ret;
 }
 

@@ -899,6 +949,7 @@ static void tun_cleanup(void)
 	}
 	rtnl_unlock();
 
+	unregister_pernet_gen_device(tun_net_id, &tun_net_ops);
 }
 
 module_init(tun_init);

Signature · 6 lines

1.5.3.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 5 lines · Pavel Emelyanov

This is the first step in making tuntap devices work in net namespaces. The structure mentioned is pointed by generic net pointer with tun_net_id id, and tun driver fills one on its load. It will contain only the tun devices list.

Quoted 1 line

So declare this structure and introduce net init and exit hooks.

OK, I have to ask... What prevents someone else from invoking net_generic() concurrently with a call to tun_exit_net(), potentially obtaining a pointer to the structure that tun_exit_net() is about to kfree()?

Thanx, Paul

Quoted 107 lines

Signed-off-by Pavel Emelyanov <[email protected]>

---
 drivers/net/tun.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 1 deletions(-)

drivers/net/tun.c +52 -1

index 7b816a0..9bfba02 100644
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <net/net_namespace.h>
+#include <net/netns/generic.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>

@@ -73,6 +74,11 @@ static int debug;
 
 /* Network device part of the driver */
 
+static unsigned int tun_net_id;
+struct tun_net {
+	struct list_head dev_list;
+};
+
 static LIST_HEAD(tun_dev_list);
 static const struct ethtool_ops tun_ethtool_ops;
 

@@ -873,6 +879,37 @@ static const struct ethtool_ops tun_ethtool_ops = {
 	.set_rx_csum	= tun_set_rx_csum
 };
 
+static int tun_init_net(struct net *net)
+{
+	struct tun_net *tn;
+
+	tn = kmalloc(sizeof(*tn), GFP_KERNEL);
+	if (tn == NULL)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&tn->dev_list);
+
+	if (net_assign_generic(net, tun_net_id, tn)) {
+		kfree(tn);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void tun_exit_net(struct net *net)
+{
+	struct tun_net *tn;
+
+	tn = net_generic(net, tun_net_id);
+	kfree(tn);
+}
+
+static struct pernet_operations tun_net_ops = {
+	.init = tun_init_net,
+	.exit = tun_exit_net,
+};
+
 static int __init tun_init(void)
 {
 	int ret = 0;

@@ -880,9 +917,22 @@ static int __init tun_init(void)
 	printk(KERN_INFO "tun: %s, %s\n", DRV_DESCRIPTION, DRV_VERSION);
 	printk(KERN_INFO "tun: %s\n", DRV_COPYRIGHT);
 
+	ret = register_pernet_gen_device(&tun_net_id, &tun_net_ops);
+	if (ret) {
+		printk(KERN_ERR "tun: Can't register pernet ops\n");
+		goto err_pernet;
+	}
+
 	ret = misc_register(&tun_miscdev);
-	if (ret)
+	if (ret) {
 		printk(KERN_ERR "tun: Can't register misc device %d\n", TUN_MINOR);
+		goto err_misc;
+	}
+	return 0;
+
+err_misc:
+	unregister_pernet_gen_device(tun_net_id, &tun_net_ops);
+err_pernet:
 	return ret;
 }
 

@@ -899,6 +949,7 @@ static void tun_cleanup(void)
 	}
 	rtnl_unlock();
 
+	unregister_pernet_gen_device(tun_net_id, &tun_net_ops);
 }
 
 module_init(tun_init);

Signature · 6 lines

1.5.3.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines · Paul E. McKenney

Quoted 5 lines · Pavel Emelyanov

This is the first step in making tuntap devices work in net namespaces. The structure mentioned is pointed by generic net pointer with tun_net_id id, and tun driver fills one on its load. It will contain only the tun devices list.

Quoted 6 lines

Quoted 1 line

So declare this structure and introduce net init and exit hooks.

OK, I have to ask... What prevents someone else from invoking net_generic() concurrently with a call to tun_exit_net(), potentially obtaining a pointer to the structure that tun_exit_net() is about to kfree()?

It's the same as if the tun_net was directly pointed by the struct net. Nobody can grant, that the pointer got by you from the struct net is not going to become free, unless you provide this security by yourself.

But if you call net_generic to get some pointer other than tun_net, then you're fine (due to RCU), providing you play the same rules with the pointer you're getting.

Maybe I'm missing something in your question, can you provide some testcase, that you suspect may cause an OOPS?

Quoted 112 lines

Quoted 107 lines

Signed-off-by Pavel Emelyanov <[email protected]>

---
 drivers/net/tun.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 1 deletions(-)

drivers/net/tun.c +52 -1

index 7b816a0..9bfba02 100644
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <net/net_namespace.h>
+#include <net/netns/generic.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>

@@ -73,6 +74,11 @@ static int debug;
 
 /* Network device part of the driver */
 
+static unsigned int tun_net_id;
+struct tun_net {
+	struct list_head dev_list;
+};
+
 static LIST_HEAD(tun_dev_list);
 static const struct ethtool_ops tun_ethtool_ops;
 

@@ -873,6 +879,37 @@ static const struct ethtool_ops tun_ethtool_ops = {
 	.set_rx_csum	= tun_set_rx_csum
 };
 
+static int tun_init_net(struct net *net)
+{
+	struct tun_net *tn;
+
+	tn = kmalloc(sizeof(*tn), GFP_KERNEL);
+	if (tn == NULL)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&tn->dev_list);
+
+	if (net_assign_generic(net, tun_net_id, tn)) {
+		kfree(tn);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void tun_exit_net(struct net *net)
+{
+	struct tun_net *tn;
+
+	tn = net_generic(net, tun_net_id);
+	kfree(tn);
+}
+
+static struct pernet_operations tun_net_ops = {
+	.init = tun_init_net,
+	.exit = tun_exit_net,
+};
+
 static int __init tun_init(void)
 {
 	int ret = 0;

@@ -880,9 +917,22 @@ static int __init tun_init(void)
 	printk(KERN_INFO "tun: %s, %s\n", DRV_DESCRIPTION, DRV_VERSION);
 	printk(KERN_INFO "tun: %s\n", DRV_COPYRIGHT);
 
+	ret = register_pernet_gen_device(&tun_net_id, &tun_net_ops);
+	if (ret) {
+		printk(KERN_ERR "tun: Can't register pernet ops\n");
+		goto err_pernet;
+	}
+
 	ret = misc_register(&tun_miscdev);
-	if (ret)
+	if (ret) {
 		printk(KERN_ERR "tun: Can't register misc device %d\n", TUN_MINOR);
+		goto err_misc;
+	}
+	return 0;
+
+err_misc:
+	unregister_pernet_gen_device(tun_net_id, &tun_net_ops);
+err_pernet:
 	return ret;
 }
 

@@ -899,6 +949,7 @@ static void tun_cleanup(void)
 	}
 	rtnl_unlock();
 
+	unregister_pernet_gen_device(tun_net_id, &tun_net_ops);
 }
 
 module_init(tun_init);

Signature · 6 lines

1.5.3.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 12 lines · Pavel Emelyanov

Quoted 11 lines · Paul E. McKenney

Quoted 6 lines · Pavel Emelyanov

This is the first step in making tuntap devices work in net namespaces. The structure mentioned is pointed by generic net pointer with tun_net_id id, and tun driver fills one on its load. It will contain only the tun devices list.

So declare this structure and introduce net init and exit hooks.

OK, I have to ask... What prevents someone else from invoking net_generic() concurrently with a call to tun_exit_net(), potentially obtaining a pointer to the structure that tun_exit_net() is about to kfree()?

Quoted 4 lines

It's the same as if the tun_net was directly pointed by the struct net. Nobody can grant, that the pointer got by you from the struct net is not going to become free, unless you provide this security by yourself.

So tun_net acquires some lock before calling net_generic(), and that same lock is held when calling tun_exit_net()? Or is there but a

single tun_net task, so that it will never call tun_net_exit() at the same time that it calls net_generic() for the tun_net pointer?

Quoted 3 lines

But if you call net_generic to get some pointer other than tun_net, then you're fine (due to RCU), providing you play the same rules with the pointer you're getting.

Agreed, RCU protects the net_generic structure, but not the structures pointed to by that structure.

Quoted 2 lines

Maybe I'm missing something in your question, can you provide some testcase, that you suspect may cause an OOPS?

Just trying to understand what prevents one task from calling net_generic() to pick up the tun_net pointer at the same time some other task calls tun_net_exit().

Thanx, Pau

Quoted 13 lines · Paul E. McKenney

Quoted 12 lines · Pavel Emelyanov

Quoted 11 lines · Paul E. McKenney

Quoted 6 lines · Pavel Emelyanov

This is the first step in making tuntap devices work in net namespaces. The structure mentioned is pointed by generic net pointer with tun_net_id id, and tun driver fills one on its load. It will contain only the tun devices list.

So declare this structure and introduce net init and exit hooks.

OK, I have to ask... What prevents someone else from invoking net_generic() concurrently with a call to tun_exit_net(), potentially obtaining a pointer to the structure that tun_exit_net() is about to kfree()?

Quoted 7 lines

Quoted 4 lines

It's the same as if the tun_net was directly pointed by the struct net. Nobody can grant, that the pointer got by you from the struct net is not going to become free, unless you provide this security by yourself.

So tun_net acquires some lock before calling net_generic(), and that same lock is held when calling tun_exit_net()? Or is there but a

No.

Quoted 2 lines

single tun_net task, so that it will never call tun_net_exit() at the same time that it calls net_generic() for the tun_net pointer?

tun_net_exit is called only when a struct net is no longer referenced and is going to be kfree-ed itself, so it's impossible (or BUGy by its own) that someone still has a pointer on this net.

Providing the struct net is alive (!), the net->gen array is alive (or is scheduled for kfree after RCU grace period). Thus, if your code holds the net and uses the net_generic() call, then it will get alive net->gen array and alive tun_net pointer.

Next, what happens after net_generic() completes and leaves the RCU-read section? Simple - the struct net is (should be) still referenced, so the tun_net_exit cannot yet be called and thus the tun_net pointer obtained earlier is alive. Unlike the (possibly) former instance of the net_generic array, but nobody references this one in my code (and should not do so, hm... I think I'll add this rule to the comments).

Quoted 6 lines

Quoted 3 lines

But if you call net_generic to get some pointer other than tun_net, then you're fine (due to RCU), providing you play the same rules with the pointer you're getting.

Agreed, RCU protects the net_generic structure, but not the structures pointed to by that structure.

They are protected by struct net reference counting.

Quoted 6 lines

Quoted 2 lines

Maybe I'm missing something in your question, can you provide some testcase, that you suspect may cause an OOPS?

Just trying to understand what prevents one task from calling net_generic() to pick up the tun_net pointer at the same time some other task calls tun_net_exit().

If this task dereferences a "held" struct net, then should be OK. If this task does not, this will OOPs in any case.

Consider the struct net to look like

struct net {
	...
	void *ptrs[N];
}

and the net_generic to be just

static inline void net_generic(struct net *net, int id)

{
	BUG_ON(id >= N);
	return net->ptrs[id - 1];
}

That's the same to what I propose, except for the ptrs array is on the RCU protected memory.

So RCU is protecting -only- the net_generic structure that net_generic() is traversing, and the structure returned by net_generic() is protected by a reference counter in the upper-level struct net.

If this is the approach, I am happy. ;-)

Thanx, Paul

Pavel, please respin these patches since I applied the updated version of patches 1 and 2 which added the generic netns bits.

Thanks!

Are you willing to look at vlans and ip tunnels (ipip, gre, sit) as well? This will be 6 sets ~7 patches each.

... which will kill all the devices in the given net with the given rtnl_link_ops. Will be used in VLAN patches later.

Signed-off-by Pavel Emelyanov <[email protected]>

---
 include/net/rtnetlink.h |    1 +
 net/core/rtnetlink.c    |   29 +++++++++++++++++++++--------
 2 files changed, 22 insertions(+), 8 deletions(-)

include/net/rtnetlink.h +1 -0

index 793863e..3c1895e 100644
@@ -74,6 +74,7 @@ struct rtnl_link_ops {
 
 extern int	__rtnl_link_register(struct rtnl_link_ops *ops);
 extern void	__rtnl_link_unregister(struct rtnl_link_ops *ops);
+extern void	rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops);
 
 extern int	rtnl_link_register(struct rtnl_link_ops *ops);
 extern void	rtnl_link_unregister(struct rtnl_link_ops *ops);

net/core/rtnetlink.c +20 -1

index da99ac0..bc39e41 100644
@@ -269,6 +269,26 @@ int rtnl_link_register(struct rtnl_link_ops *ops)
 
 EXPORT_SYMBOL_GPL(rtnl_link_register);
 
+static void __rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
+{
+	struct net_device *dev;
+restart:
+	for_each_netdev(net, dev) {
+		if (dev->rtnl_link_ops == ops) {
+			ops->dellink(dev);
+			goto restart;
+		}
+	}
+}
+
+void rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
+{
+	rtnl_lock();
+	__rtnl_kill_links(net, ops);
+	rtnl_unlock();
+}
+EXPORT_SYMBOL_GPL(rtnl_kill_links);
+
 /**
  * __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink.
  * @ops: struct rtnl_link_ops * to unregister

@@ -277,17 +297,10 @@ EXPORT_SYMBOL_GPL(rtnl_link_register);
  */
 void __rtnl_link_unregister(struct rtnl_link_ops *ops)
 {
-	struct net_device *dev, *n;
 	struct net *net;

net/core/rtnetlink.c +0 -1

 	}
 	list_del(&ops->list);
 }
--

1.5.3.4

Quoted 38 lines

net/core/rtnetlink.c +20 -1

index da99ac0..bc39e41 100644
@@ -269,6 +269,26 @@ int rtnl_link_register(struct rtnl_link_ops *ops)
 
 EXPORT_SYMBOL_GPL(rtnl_link_register);
 
+static void __rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
+{
+	struct net_device *dev;
+restart:
+	for_each_netdev(net, dev) {
+		if (dev->rtnl_link_ops == ops) {
+			ops->dellink(dev);
+			goto restart;
+		}
+	}
+}
+
+void rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
+{
+	rtnl_lock();
+	__rtnl_kill_links(net, ops);
+	rtnl_unlock();
+}
+EXPORT_SYMBOL_GPL(rtnl_kill_links);
+
 /**
  * __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink.
  * @ops: struct rtnl_link_ops * to unregister

@@ -277,17 +297,10 @@ EXPORT_SYMBOL_GPL(rtnl_link_register);
  */
 void __rtnl_link_unregister(struct rtnl_link_ops *ops)
 {
-	struct net_device *dev, *n;
 	struct net *net;
 

net/core/rtnetlink.c EXPORT_SYMBOL_GPL(rtnl_link_register); · line 303

Quoted 9 lines

net/core/rtnetlink.c +1 -7

 	for_each_net(net) {
-restart:
-		for_each_netdev_safe(net, dev, n) {
-			if (dev->rtnl_link_ops == ops) {
-				ops->dellink(dev);
-				goto restart;
-			}
-		}
+		__rtnl_kill_links(net, ops);

This was _safe, and now it's not. Is that intentional?

Quoted 5 lines

net/core/rtnetlink.c +0 -1

 	}
 	list_del(&ops->list);
 }
--

1.5.3.4

Quoted 10 lines

net/core/rtnetlink.c EXPORT_SYMBOL_GPL(rtnl_link_register); · line 303

Quoted 9 lines

net/core/rtnetlink.c +1 -7

 	for_each_net(net) {
-restart:
-		for_each_netdev_safe(net, dev, n) {
-			if (dev->rtnl_link_ops == ops) {
-				ops->dellink(dev);
-				goto restart;
-			}
-		}
+		__rtnl_kill_links(net, ops);

This was _safe, and now it's not. Is that intentional?

Yup - we goto restart in case we del some link, so there's no need in _safe iteration.

This goto was added by Partick (commit 68365458 [NET]: rtnl_link: fix use-after-free) and I suspect he simply forgot to remove the _safe iterator (I put him in Cc to correct me if I'm wrong).

Add the elastic array of void * pointer to the struct net. The access rules are simple:

1. register the ops with register_pernet_gen_device to get the id of your private pointer
2. call net_assign_generic() to put the private data on the struct net (most preferably this should be done in the ->init callback of the ops registered)
3. do not change this pointer while the net is alive;
4. use the net_generic() to get the pointer.

When adding a new pointer, I copy the old array, replace it with a new one and schedule the old for kfree after an RCU grace period.

Since the net_generic explores the net->gen array inside rcu
read section and once set the net->gen->ptr[x] pointer never changes, this grants us a safe access to generic pointers.

Signed-off-by Pavel Emelyanov <[email protected]>

---
 include/net/net_namespace.h |    2 +
 include/net/netns/generic.h |   49 ++++++++++++++++++++++++++++++++++
 net/core/net_namespace.c    |   62 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 0 deletions(-)
 create mode 100644 include/net/netns/generic.h

include/net/net_namespace.h +2 -0

index 6971fdb..e3d4eb4 100644
@@ -19,6 +19,7 @@ struct proc_dir_entry;
 struct net_device;
 struct sock;
 struct ctl_table_header;
+struct net_generic;
 
 struct net {
 	atomic_t		count;		/* To decided when the network

@@ -57,6 +58,7 @@ struct net {
 #ifdef CONFIG_NETFILTER
 	struct netns_xt		xt;
 #endif
+	struct net_generic	*gen;
 };
 
 

include/net/netns/generic.h new file +49 -0

new file mode 100644
index 0000000..e8a6d27
@@ -0,0 +1,49 @@
+/*
+ * generic net pointers
+ */
+
+#ifndef __NET_GENERIC_H__
+#define __NET_GENERIC_H__
+
+#include <linux/rcupdate.h>
+
+/*
+ * Generic net pointers are to be used by modules
+ * to put some private stuff on the struct net without
+ * explicit struct net modification
+ *
+ * The rules are simple:
+ * 1. register the ops with register_pernet_gen_device to get
+ *    the id of your private pointer
+ * 2. call net_assign_generic() to put the private data on the
+ *    struct net (most preferably this should be done in the
+ *    ->init callback of the ops registered)
+ * 3. do not change this pointer while the net is alive.
+ *
+ * After accomplishing all of the above, the private pointer
+ * can be accessed with the net_generic() call.
+ */
+
+struct net_generic {
+	unsigned int len;
+	struct rcu_head rcu;
+
+	void *ptr[0];
+};
+
+static inline void *net_generic(struct net *net, int id)
+{
+	struct net_generic *ng;
+	void *ptr;
+
+	rcu_read_lock();
+	ng = rcu_dereference(net->gen);
+	BUG_ON(id == 0 || id > ng->len);
+	ptr = ng->ptr[id - 1];
+	rcu_read_unlock();
+
+	return ptr;
+}
+
+extern int net_assign_generic(struct net *net, int id, void *data);
+#endif

net/core/net_namespace.c +3 -0

index 7ef3bac..b384840 100644
@@ -7,6 +7,7 @@
 #include <linux/sched.h>
 #include <linux/idr.h>
 #include <net/net_namespace.h>
+#include <net/netns/generic.h>
 
 /*
  *	Our network namespace constructor/destructor lists

@@ -21,6 +22,8 @@ LIST_HEAD(net_namespace_list);
 struct net init_net;
 EXPORT_SYMBOL(init_net);
 
+#define INITIAL_NET_GEN_PTRS	13 /* +1 for len +2 for rcu_head */
+
 /*
  * setup_net runs the initializers for the network namespace object.
  */

@@ -29,10 +32,21 @@ static __net_init int setup_net(struct net *net)
 	/* Must be called with net_mutex held */

net/core/net_namespace.c +15 -0

+	if (ng == NULL)
+		goto out;
+
+	ng->len = INITIAL_NET_GEN_PTRS;
+	INIT_RCU_HEAD(&ng->rcu);
+	rcu_assign_pointer(net->gen, ng);
+
 	error = 0;
 	list_for_each_entry(ops, &pernet_list, list) {
 		if (ops->init) {

@@ -54,6 +68,7 @@ out_undo:
 	}
 
 	rcu_barrier();
+	kfree(ng);
 	goto out;
 }
 

@@ -384,3 +399,50 @@ void unregister_pernet_gen_device(int id, struct pernet_operations *ops)
 	mutex_unlock(&net_mutex);
 }
 EXPORT_SYMBOL_GPL(unregister_pernet_gen_device);
+
+static void net_generic_release(struct rcu_head *rcu)
+{
+	struct net_generic *ng;
+
+	ng = container_of(rcu, struct net_generic, rcu);
+	kfree(ng);

net/core/net_namespace.c +30 -0

+	if (old_ng->len >= id)
+		goto assign;
+
+	ng = kzalloc(sizeof(struct net_generic) +
+			id * sizeof(void *), GFP_KERNEL);
+	if (ng == NULL)
+		return -ENOMEM;
+
+	/*
+	 * Some synchronisation notes:
+	 *
+	 * The net_generic explores the net->gen array inside rcu
+	 * read section. Besides once set the net->gen->ptr[x]
+	 * pointer never changes (see rules in netns/generic.h).
+	 *
+	 * That said, we simply duplicate this array and schedule
+	 * the old copy for kfree after a grace period.
+	 */
+
+	ng->len = id;
+	INIT_RCU_HEAD(&ng->rcu);
+	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len);
+
+	rcu_assign_pointer(net->gen, ng);
+	call_rcu(&old_ng->rcu, net_generic_release);
+assign:
+	ng->ptr[id - 1] = data;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(net_assign_generic);

Quoted 6 lines

net/core/net_namespace.c +1 -0

@@ -29,10 +32,21 @@ static __net_init int setup_net(struct net *net)
 	/* Must be called with net_mutex held */
 	struct pernet_operations *ops;
 	int error;
+	struct net_generic *ng;
 

net/core/net_namespace.c static __net_init int setup_net(struct net *net) · line 42

Quoted 6 lines

net/core/net_namespace.c +3 -0

 	atomic_set(&net->count, 1);
 	atomic_set(&net->use_count, 0);
 
+	error = -ENOMEM;
+	ng = kzalloc(sizeof(struct net_generic) +
+			INITIAL_NET_GEN_PTRS * sizeof(void *), GFP_KERNEL);

Why do you need to allocate more than sizeof(struct net_generic) ?

net/core/net_namespace.c void unregister_pernet_gen_device(int id, struct pernet_operations *ops) · line 418

Quoted 8 lines

net/core/net_namespace.c +8 -0

+int net_assign_generic(struct net *net, int id, void *data)
+{
+	struct net_generic *ng, *old_ng;
+
+	BUG_ON(!mutex_is_locked(&net_mutex));
+	BUG_ON(id == 0);
+
+	ng = old_ng = net->gen;

shouldn't it be rcu_dereferenced ?

Hi, guys.

I've recently sent a TUN devices virtualization, but it was rejected by Dave, since the struct net is becoming a dumping ground.

I agree with him - we really need some way to register on-net data dynamically. That's my view of such a thing and two examples of how to use it (TUN and VLAN devices virtualization).

If this will be found good, I'll send these sets to David, hoping he will accept them :)

Quoted 9 lines · Pavel Emelyanov

Hi, guys.

I've recently sent a TUN devices virtualization, but it was rejected by Dave, since the struct net is becoming a dumping ground.

I agree with him - we really need some way to register on-net data dynamically. That's my view of such a thing and two examples of how to use it (TUN and VLAN devices virtualization).

Quoted 2 lines

If this will be found good, I'll send these sets to David, hoping he will accept them :)

Pavel,

seems to be a smart solution :)

I am just afraid with the performances when the network resources are to be accessed in the fast path like a routing table (that seems not to be the case for tun and vlan). Shall we assume the fast path should always go to struct net and non critical path can go to net_generic ?

-- Daniel

I think, that core kernel code and protocols should/may use the struct net, while modules are better to work via generic pointers. However, if the generic pointers cause noticeable performance degradation, then we may ask Dave to bear with on-net members :)