diff mbox

[v2,2/4] batman-adv: Create batman soft interfaces within correct netns.

Message ID 1456867148-31883-3-git-send-email-andrew@lunn.ch (mailing list archive)
State Superseded, archived
Delegated to: Marek Lindner
Headers

Commit Message

Andrew Lunn March 1, 2016, 9:19 p.m. UTC 
  When creating a soft interface, create it in the same netns as the
hard interface. Replace all references to init_net with the correct
name space for the interface being manipulated.

Suggested-by: Daniel Ehlers <danielehlers@mindeye.net>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Antonio Quartulli <a@unstable.cc>
---

v2:
  struct net forward declarations
  Removed unneeded net/net_namespace.h
  Rebased on https://git.open-mesh.org/batman-adv.git master
---
 net/batman-adv/hard-interface.c    | 9 +++++----
 net/batman-adv/hard-interface.h    | 3 ++-
 net/batman-adv/soft-interface.c    | 7 +++++--
 net/batman-adv/soft-interface.h    | 3 ++-
 net/batman-adv/sysfs.c             | 3 ++-
 net/batman-adv/translation-table.c | 4 ++--
 6 files changed, 18 insertions(+), 11 deletions(-)

Comments

Sven Eckelmann March 13, 2016, 10:48 a.m. UTC | #1 
On Tuesday 01 March 2016 22:19:06 Andrew Lunn wrote:
> When creating a soft interface, create it in the same netns as the
> hard interface. Replace all references to init_net with the correct
> name space for the interface being manipulated.
> 
> Suggested-by: Daniel Ehlers <danielehlers@mindeye.net>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Acked-by: Antonio Quartulli <a@unstable.cc>
> ---
> 
> v2:
>   struct net forward declarations
>   Removed unneeded net/net_namespace.h
>   Rebased on https://git.open-mesh.org/batman-adv.git master
> ---
>  net/batman-adv/hard-interface.c    | 9 +++++----
>  net/batman-adv/hard-interface.h    | 3 ++-
>  net/batman-adv/soft-interface.c    | 7 +++++--
>  net/batman-adv/soft-interface.h    | 3 ++-
>  net/batman-adv/sysfs.c             | 3 ++-
>  net/batman-adv/translation-table.c | 4 ++--
>  6 files changed, 18 insertions(+), 11 deletions(-)

Just so that I have mentioned it: In theory '#include <net/net_namespace.h>' 
should be removed by this patch (hard-interface.c) and added again in the next 
patch. But this would be rather useless when applying the two patches 
together. It may be different when this patch is applied first without the 
next patch.

Just to be sure (and so that I don't have to test it here :) ): When this 
patch is applied, batadv_softif_create would already fail when the two 
namespaces have a batadv interface with the same name because the debugfs 
function would fail. But it would work when the batman-adv interfaces + the 
slave interfaces (for per-slave interface information) have different names. 
Right?

Just some of my notes (just in case someone else asks himself the same 
questions):

It looks to me that the only reason a device from a different namespace isn't 
added is because batadv_softif_slave_add + batadv_store_mesh_iface is getting 
the namespace of the new slave device and batadv_hardif_enable_interface then 
only searches the soft-interface (batX) in this namespace.

It is currently not prevented that a slave device changes the namespace after 
it was added. But this should not be a problem because the device will be 
first removed from the original namespace (so it will be removed from batX) 
and later added to the target namespace (see dev_change_net_namespace).


Reviewed-by: Sven Eckelmann <sven@narfation.org>

Kind regards,
	Sven
Andrew Lunn March 13, 2016, 3:35 p.m. UTC | #2 
> Just to be sure (and so that I don't have to test it here :) ): When this 
> patch is applied, batadv_softif_create would already fail when the two 
> namespaces have a batadv interface with the same name because the debugfs 
> function would fail. But it would work when the batman-adv interfaces + the 
> slave interfaces (for per-slave interface information) have different names. 
> Right?

That is correct. As far as i remember, it failed in a good way,
i.e. not an opps. Which is why i put this patch first. We could swap
the order, have the debugfs patch first, but i though the 'why' of the
debugfs patch is probably clearer to somebody who has just reviewed
this patch first.

> It is currently not prevented that a slave device changes the namespace after 
> it was added. But this should not be a problem because the device will be 
> first removed from the original namespace (so it will be removed from batX) 
> and later added to the target namespace (see dev_change_net_namespace).

Yes, and this is the same mechanism used for other master devices,
like bridges, teams/bonding. The slave is hot-unplugged from one name
space and hot-plugged into another name space.

> Reviewed-by: Sven Eckelmann <sven@narfation.org>

Thanks
	Andrew
diff mbox

Patch

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 3240a67..b28eacc 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -121,6 +121,7 @@  static bool batadv_mutual_parents(const struct net_device *dev1,
 static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 {
 	struct net_device *parent_dev;
+	struct net *net = dev_net(net_dev);
 	bool ret;
 
 	/* check if this is a batman-adv mesh interface */
@@ -133,7 +134,7 @@  static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 		return false;
 
 	/* recurse over the parent device */
-	parent_dev = __dev_get_by_index(&init_net, dev_get_iflink(net_dev));
+	parent_dev = __dev_get_by_index(net, dev_get_iflink(net_dev));
 	/* if we got a NULL parent_dev there is something broken.. */
 	if (WARN(!parent_dev, "Cannot find parent device"))
 		return false;
@@ -453,7 +454,7 @@  static int batadv_master_del_slave(struct batadv_hard_iface *slave,
 }
 
 int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
-				   const char *iface_name)
+				   struct net *net, const char *iface_name)
 {
 	struct batadv_priv *bat_priv;
 	struct net_device *soft_iface, *master;
@@ -467,10 +468,10 @@  int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
 	if (!kref_get_unless_zero(&hard_iface->refcount))
 		goto out;
 
-	soft_iface = dev_get_by_name(&init_net, iface_name);
+	soft_iface = dev_get_by_name(net, iface_name);
 
 	if (!soft_iface) {
-		soft_iface = batadv_softif_create(iface_name);
+		soft_iface = batadv_softif_create(net, iface_name);
 
 		if (!soft_iface) {
 			ret = -ENOMEM;
diff --git a/net/batman-adv/hard-interface.h b/net/batman-adv/hard-interface.h
index d74f198..a76724d 100644
--- a/net/batman-adv/hard-interface.h
+++ b/net/batman-adv/hard-interface.h
@@ -28,6 +28,7 @@ 
 #include <linux/types.h>
 
 struct net_device;
+struct net;
 
 enum batadv_hard_if_state {
 	BATADV_IF_NOT_IN_USE,
@@ -55,7 +56,7 @@  bool batadv_is_wifi_iface(int ifindex);
 struct batadv_hard_iface*
 batadv_hardif_get_by_netdev(const struct net_device *net_dev);
 int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
-				   const char *iface_name);
+				   struct net *net, const char *iface_name);
 void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface,
 				     enum batadv_hard_if_cleanup autodel);
 void batadv_hardif_remove_interfaces(void);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index f4aa60a..14cf689 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -868,13 +868,14 @@  static int batadv_softif_slave_add(struct net_device *dev,
 				   struct net_device *slave_dev)
 {
 	struct batadv_hard_iface *hard_iface;
+	struct net *net = dev_net(dev);
 	int ret = -EINVAL;
 
 	hard_iface = batadv_hardif_get_by_netdev(slave_dev);
 	if (!hard_iface || hard_iface->soft_iface)
 		goto out;
 
-	ret = batadv_hardif_enable_interface(hard_iface, dev->name);
+	ret = batadv_hardif_enable_interface(hard_iface, net, dev->name);
 
 out:
 	if (hard_iface)
@@ -971,7 +972,7 @@  static void batadv_softif_init_early(struct net_device *dev)
 	memset(priv, 0, sizeof(*priv));
 }
 
-struct net_device *batadv_softif_create(const char *name)
+struct net_device *batadv_softif_create(struct net *net, const char *name)
 {
 	struct net_device *soft_iface;
 	int ret;
@@ -981,6 +982,8 @@  struct net_device *batadv_softif_create(const char *name)
 	if (!soft_iface)
 		return NULL;
 
+	dev_net_set(soft_iface, net);
+
 	soft_iface->rtnl_link_ops = &batadv_link_ops;
 
 	ret = register_netdevice(soft_iface);
diff --git a/net/batman-adv/soft-interface.h b/net/batman-adv/soft-interface.h
index 417d30a..f7b3dbf 100644
--- a/net/batman-adv/soft-interface.h
+++ b/net/batman-adv/soft-interface.h
@@ -23,13 +23,14 @@ 
 #include <net/rtnetlink.h>
 
 struct net_device;
+struct net;
 struct sk_buff;
 
 int batadv_skb_head_push(struct sk_buff *skb, unsigned int len);
 void batadv_interface_rx(struct net_device *soft_iface,
 			 struct sk_buff *skb, struct batadv_hard_iface *recv_if,
 			 int hdr_size, struct batadv_orig_node *orig_node);
-struct net_device *batadv_softif_create(const char *name);
+struct net_device *batadv_softif_create(struct net *net, const char *name);
 void batadv_softif_destroy_sysfs(struct net_device *soft_iface);
 bool batadv_softif_is_valid(const struct net_device *net_dev);
 extern struct rtnl_link_ops batadv_link_ops;
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index e7cf513..6b1e54f 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -830,6 +830,7 @@  static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
 				       size_t count)
 {
 	struct net_device *net_dev = batadv_kobj_to_netdev(kobj);
+	struct net *net = dev_net(net_dev);
 	struct batadv_hard_iface *hard_iface;
 	int status_tmp = -1;
 	int ret = count;
@@ -873,7 +874,7 @@  static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
 		batadv_hardif_disable_interface(hard_iface,
 						BATADV_IF_CLEANUP_AUTO);
 
-	ret = batadv_hardif_enable_interface(hard_iface, buff);
+	ret = batadv_hardif_enable_interface(hard_iface, net, buff);
 
 unlock:
 	rtnl_unlock();
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 2ed55f4..ce63a51 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -43,7 +43,6 @@ 
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/workqueue.h>
-#include <net/net_namespace.h>
 
 #include "bridge_loop_avoidance.h"
 #include "hard-interface.h"
@@ -583,6 +582,7 @@  bool batadv_tt_local_add(struct net_device *soft_iface, const u8 *addr,
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
 	struct batadv_tt_local_entry *tt_local;
 	struct batadv_tt_global_entry *tt_global = NULL;
+	struct net *net = dev_net(soft_iface);
 	struct batadv_softif_vlan *vlan;
 	struct net_device *in_dev = NULL;
 	struct hlist_head *head;
@@ -594,7 +594,7 @@  bool batadv_tt_local_add(struct net_device *soft_iface, const u8 *addr,
 	u32 match_mark;
 
 	if (ifindex != BATADV_NULL_IFINDEX)
-		in_dev = dev_get_by_index(&init_net, ifindex);
+		in_dev = dev_get_by_index(net, ifindex);
 
 	tt_local = batadv_tt_local_hash_find(bat_priv, addr, vid);