[B.A.T.M.A.N.] [PATCH 0/4] batman-adv: Add network name space support

[B.A.T.M.A.N.] [PATCH 0/4] batman-adv: Add network name space support

[Andrew Lunn]

This patchset makes batman-adv network namespace aware. A soft
interface can be creates in a namespace and hard interfaces added to
it. soft interfaces cannot be moved between name spaces.

The biggest change is to debugfs, which is not natively netns aware,
unlike sysfs which is. A new netns directory has been added and within
that, a directory per network name space which batman is used within.
The changes are backwards compatible, in that interfaces in the global
namespace are not placed into a subdirectory.

/sys/kernel/debug/batman_adv
├── eth0
│   └── originators
├── eth1
│   └── originators
├── eth2
│   └── originators
├── netns
│   ├── 4026531957 -> ..
│   ├── 4026532176
│   │   ├── bat0
│   │   │   ├── bla_backbone_table
│   │   │   ├── bla_claim_table
│   │   │   ├── dat_cache
│   │   │   ├── gateways
│   │   │   ├── nc
│   │   │   │   ├── max_buffer_time
│   │   │   │   ├── max_fwd_delay
│   │   │   │   └── min_tq
│   │   │   ├── nc_nodes
│   │   │   ├── originators
│   │   │   ├── socket
│   │   │   ├── transtable_global
│   │   │   └── transtable_local
│   │   ├── emu1-veth1
│   │   │   └── originators
│   │   └── emu1-veth2
│   │       └── originators
│   ├── 4026532225
│   │   ├── bat0
│   │   │   ├── bla_backbone_table
│   │   │   ├── bla_claim_table
│   │   │   ├── dat_cache
│   │   │   ├── gateways
│   │   │   ├── nc
│   │   │   │   ├── max_buffer_time
│   │   │   │   ├── max_fwd_delay
│   │   │   │   └── min_tq
│   │   │   ├── nc_nodes
│   │   │   ├── originators
│   │   │   ├── socket
│   │   │   ├── transtable_global
│   │   │   └── transtable_local
│   │   ├── emu2-veth1
│   │   │   └── originators
│   │   └── emu2-veth2
│   │       └── originators

Patches are available for batctl and alfred, to make use of this new
structure.

The bat-over-bat detection has been enhanced, so that it is also netns
aware. It can also now cope with veth interfaces which are mutual
parents.

Andrew Lunn (4):
  batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves
  batman-adv: Create batman soft interfaces within correct netns.
  batman-adv: Handle parent interfaces in a different netns
  batman-adv: debugfs: Add netns support

 net/batman-adv/debugfs.c           | 118 ++++++++++++++++++++++++++++++++++++-
 net/batman-adv/hard-interface.c    |  49 +++++++++++++--
 net/batman-adv/hard-interface.h    |   2 +-
 net/batman-adv/soft-interface.c    |   9 ++-
 net/batman-adv/soft-interface.h    |   2 +-
 net/batman-adv/sysfs.c             |   3 +-
 net/batman-adv/translation-table.c |   3 +-
 7 files changed, 172 insertions(+), 14 deletions(-)

-- 
2.7.0.rc3

[B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Andrew Lunn]

The batX soft interface should not be moved between network name
spaces. This is similar to bridges, bonds, tunnels, which are not
allowed to move between network namespaces.

Suggested-by: Daniel Ehlers <danielehlers@mindeye.net>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/batman-adv/soft-interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index ac4d08d..f3f096c0 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -940,7 +940,7 @@ static void batadv_softif_init_early(struct net_device *dev)
 
 	dev->netdev_ops = &batadv_netdev_ops;
 	dev->destructor = batadv_softif_free;
-	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_NETNS_LOCAL;
 	dev->priv_flags |= IFF_NO_QUEUE;
 
 	/* can't call min_mtu, because the needed variables
-- 
2.7.0.rc3

[B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Create batman soft interfaces within correct netns.

[Andrew Lunn]

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>
---
 net/batman-adv/hard-interface.c    | 9 +++++----
 net/batman-adv/hard-interface.h    | 2 +-
 net/batman-adv/soft-interface.c    | 7 +++++--
 net/batman-adv/soft-interface.h    | 2 +-
 net/batman-adv/sysfs.c             | 3 ++-
 net/batman-adv/translation-table.c | 3 ++-
 6 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index f11345e..a5ce58a 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -90,6 +90,7 @@ out:
 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 */
@@ -102,7 +103,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;
@@ -418,7 +419,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;
@@ -432,10 +433,10 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
 	if (!atomic_inc_not_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 5a31420..4ed737f 100644
--- a/net/batman-adv/hard-interface.h
+++ b/net/batman-adv/hard-interface.h
@@ -55,7 +55,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 f3f096c0..2eeec3d 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -853,13 +853,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)
@@ -956,7 +957,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;
@@ -966,6 +967,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 8e82176..d616cd5 100644
--- a/net/batman-adv/soft-interface.h
+++ b/net/batman-adv/soft-interface.h
@@ -29,7 +29,7 @@ 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);
 int 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 9de3c88..d6867da 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -773,6 +773,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;
@@ -816,7 +817,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 76f19ba..0dbda83 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -534,6 +534,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;
@@ -545,7 +546,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);
 
-- 
2.7.0.rc3

[B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Andrew Lunn]

batman-adv tries to prevent the user from placing a batX soft
interface into another batman mesh as a hard interface. It does this
by walking up the devices list of parents and ensures they are all
none batX interfaces. iflink can point to an interface in a different
nameplace, so also retrieve the parents name space when finding the
parent.

Additionally, veth devices come in pairs and they are each others
parents. Don't infinitely recurse in this situation.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/batman-adv/hard-interface.c | 44 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index a5ce58a..8d51a4a 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -75,6 +75,39 @@ out:
 }
 
 /**
+ * batadv_mutual_parents - check if two devices are each others parent
+ * @net_dev - 1st parent
+ * @parent_dev - 2nd parent
+ *
+ * veth devices come in pairs and each is the parent of the other!
+ *
+ * Return true if the devices are each others parent, otherwise false
+ */
+static bool batadv_mutual_parents(const struct net_device *dev1,
+				  const struct net *dev1_net,
+				  const struct net_device *dev2,
+				  const struct net *dev2_net)
+{
+	int dev1_parent_iflink = dev_get_iflink(dev1);
+	int dev2_parent_iflink = dev_get_iflink(dev2);
+	const struct net *dev1_parent_net = dev1_net;
+	const struct net *dev2_parent_net = dev2_net;
+
+	if (dev1->rtnl_link_ops && dev1->rtnl_link_ops->get_link_net)
+		dev1_parent_net = dev1->rtnl_link_ops->get_link_net(dev1);
+	if (dev2->rtnl_link_ops && dev2->rtnl_link_ops->get_link_net)
+		dev2_parent_net = dev2->rtnl_link_ops->get_link_net(dev2);
+
+	if (!dev1_parent_iflink || !dev2_parent_iflink)
+		return false;
+
+	return (dev1_parent_iflink == dev2->ifindex) &&
+		(dev2_parent_iflink == dev1->ifindex) &&
+		net_eq(dev1_parent_net, dev2_net) &&
+		net_eq(dev2_parent_net, dev1_net);
+}
+
+/**
  * batadv_is_on_batman_iface - check if a device is a batman iface descendant
  * @net_dev: the device to check
  *
@@ -89,8 +122,9 @@ out:
  */
 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);
+	struct net_device *parent_dev;
+	struct net *parent_net = net;
 	bool ret;
 
 	/* check if this is a batman-adv mesh interface */
@@ -102,12 +136,18 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 	    dev_get_iflink(net_dev) == net_dev->ifindex)
 		return false;
 
+	if (net_dev->rtnl_link_ops && net_dev->rtnl_link_ops->get_link_net)
+		parent_net = net_dev->rtnl_link_ops->get_link_net(net_dev);
+
 	/* recurse over the parent device */
-	parent_dev = __dev_get_by_index(net, dev_get_iflink(net_dev));
+	parent_dev = __dev_get_by_index(parent_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;
 
+	if (batadv_mutual_parents(net_dev, net, parent_dev, parent_net))
+		return false;
+
 	ret = batadv_is_on_batman_iface(parent_dev);
 
 	return ret;
-- 
2.7.0.rc3

[B.A.T.M.A.N.] [PATCH 4/4] batman-adv: debugfs: Add netns support

[Andrew Lunn]

Unlike sysfs, debugfs is not netns aware. So batman has to take care
to avoid namespace clashes.

Each namespace is given a directory within debugfs/batman-adv/netns,
using the namespaces inum as the directory name.

Files for namespaces other than the global namespace are placed within
the namespace specific directory. Additionally, a symbolic link is
used to link the global namespaces inum back to debugfs/batman-adv/ so
tools do not need to differentiate between the global namespace and
other namespaces.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/batman-adv/debugfs.c | 118 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 115 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c
index c4c1e80..b87ffad 100644
--- a/net/batman-adv/debugfs.c
+++ b/net/batman-adv/debugfs.c
@@ -53,6 +53,73 @@
 #include "translation-table.h"
 
 static struct dentry *batadv_debugfs;
+static struct dentry *batadv_ns_debugfs;
+
+struct batadv_debugfs_ns_entry {
+	struct net *net;
+	struct dentry *dir;
+	struct kref refcount;
+	struct list_head link;
+};
+
+static LIST_HEAD(batadv_debugfs_ns);
+static DEFINE_MUTEX(batadv_debugfs_ns_mutex);
+
+static struct dentry *batadv_debugfs_ns_get(struct net *net)
+{
+	struct batadv_debugfs_ns_entry *ns_entry;
+	char name[32];
+
+	mutex_lock(&batadv_debugfs_ns_mutex);
+	list_for_each_entry(ns_entry, &batadv_debugfs_ns, link) {
+		if (ns_entry->net == net) {
+			kref_get(&ns_entry->refcount);
+			mutex_unlock(&batadv_debugfs_ns_mutex);
+			return ns_entry->dir;
+		}
+	}
+
+	ns_entry = kzalloc(sizeof(*ns_entry), GFP_ATOMIC);
+	if (ns_entry) {
+		INIT_LIST_HEAD(&ns_entry->link);
+		ns_entry->net = net;
+		kref_init(&ns_entry->refcount);
+		sprintf(name, "%u", net->ns.inum);
+		ns_entry->dir = debugfs_create_dir(name, batadv_ns_debugfs);
+		if (!ns_entry->dir) {
+			kfree(ns_entry);
+			mutex_unlock(&batadv_debugfs_ns_mutex);
+			return NULL;
+		}
+		list_add(&ns_entry->link, &batadv_debugfs_ns);
+	}
+	mutex_unlock(&batadv_debugfs_ns_mutex);
+	return ns_entry->dir;
+}
+
+static void batadv_ns_entry_release(struct kref *ref)
+{
+	struct batadv_debugfs_ns_entry *ns_entry;
+
+	ns_entry = container_of(ref, struct batadv_debugfs_ns_entry, refcount);
+	debugfs_remove_recursive(ns_entry->dir);
+	list_del(&ns_entry->link);
+	kfree(ns_entry);
+}
+
+static void batadv_debugfs_ns_put(struct net *net)
+{
+	struct batadv_debugfs_ns_entry *ns_entry;
+
+	mutex_lock(&batadv_debugfs_ns_mutex);
+	list_for_each_entry(ns_entry, &batadv_debugfs_ns, link) {
+		if (ns_entry->net == net) {
+			kref_put(&ns_entry->refcount, batadv_ns_entry_release);
+			break;
+		}
+	}
+	mutex_unlock(&batadv_debugfs_ns_mutex);
+}
 
 #ifdef CONFIG_BATMAN_ADV_DEBUG
 #define BATADV_LOG_BUFF_MASK (batadv_log_buff_len - 1)
@@ -438,6 +505,7 @@ void batadv_debugfs_init(void)
 {
 	struct batadv_debuginfo **bat_debug;
 	struct dentry *file;
+	char name[32];
 
 	batadv_debugfs = debugfs_create_dir(BATADV_DEBUGFS_SUBDIR, NULL);
 	if (batadv_debugfs == ERR_PTR(-ENODEV))
@@ -458,6 +526,15 @@ void batadv_debugfs_init(void)
 		}
 	}
 
+	batadv_ns_debugfs = debugfs_create_dir("netns", batadv_debugfs);
+	if (!batadv_ns_debugfs)
+		goto err;
+
+	/* Create a symlink for the default name space */
+	sprintf(name, "%u", init_net.ns.inum);
+	if (!debugfs_create_symlink(name, batadv_ns_debugfs, ".."))
+		goto err;
+
 	return;
 err:
 	debugfs_remove_recursive(batadv_debugfs);
@@ -477,14 +554,26 @@ void batadv_debugfs_destroy(void)
  */
 int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface)
 {
+	char *name = hard_iface->net_dev->name;
+	struct net *net = dev_net(hard_iface->net_dev);
 	struct batadv_debuginfo **bat_debug;
+	struct dentry *debugfs_ns_dir;
 	struct dentry *file;
 
 	if (!batadv_debugfs)
 		goto out;
 
-	hard_iface->debug_dir = debugfs_create_dir(hard_iface->net_dev->name,
-						   batadv_debugfs);
+	if (net != &init_net) {
+		debugfs_ns_dir = batadv_debugfs_ns_get(net);
+		if (!debugfs_ns_dir)
+			goto out;
+		hard_iface->debug_dir = debugfs_create_dir(name,
+							   debugfs_ns_dir);
+	} else {
+		hard_iface->debug_dir = debugfs_create_dir(name,
+							   batadv_debugfs);
+	}
+
 	if (!hard_iface->debug_dir)
 		goto out;
 
@@ -502,6 +591,8 @@ int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface)
 rem_attr:
 	debugfs_remove_recursive(hard_iface->debug_dir);
 	hard_iface->debug_dir = NULL;
+	if (net != &init_net)
+		batadv_debugfs_ns_put(net);
 out:
 	return -ENOMEM;
 }
@@ -513,22 +604,38 @@ out:
  */
 void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface)
 {
+	struct net *net = dev_net(hard_iface->net_dev);
+
 	if (batadv_debugfs) {
 		debugfs_remove_recursive(hard_iface->debug_dir);
 		hard_iface->debug_dir = NULL;
 	}
+	if (net != &init_net)
+		batadv_debugfs_ns_put(net);
 }
 
 int batadv_debugfs_add_meshif(struct net_device *dev)
 {
 	struct batadv_priv *bat_priv = netdev_priv(dev);
 	struct batadv_debuginfo **bat_debug;
+	struct net *net = dev_net(dev);
+	struct dentry *debugfs_ns_dir;
 	struct dentry *file;
 
 	if (!batadv_debugfs)
 		goto out;
 
-	bat_priv->debug_dir = debugfs_create_dir(dev->name, batadv_debugfs);
+	if (net != &init_net) {
+		debugfs_ns_dir = batadv_debugfs_ns_get(net);
+		if (!debugfs_ns_dir)
+			goto out;
+		bat_priv->debug_dir = debugfs_create_dir(dev->name,
+							 debugfs_ns_dir);
+	} else {
+		bat_priv->debug_dir = debugfs_create_dir(dev->name,
+							 batadv_debugfs);
+	}
+
 	if (!bat_priv->debug_dir)
 		goto out;
 
@@ -557,6 +664,8 @@ int batadv_debugfs_add_meshif(struct net_device *dev)
 rem_attr:
 	debugfs_remove_recursive(bat_priv->debug_dir);
 	bat_priv->debug_dir = NULL;
+	if (net != &init_net)
+		batadv_debugfs_ns_put(net);
 out:
 	return -ENOMEM;
 }
@@ -564,6 +673,7 @@ out:
 void batadv_debugfs_del_meshif(struct net_device *dev)
 {
 	struct batadv_priv *bat_priv = netdev_priv(dev);
+	struct net *net = dev_net(dev);
 
 	batadv_debug_log_cleanup(bat_priv);
 
@@ -571,4 +681,6 @@ void batadv_debugfs_del_meshif(struct net_device *dev)
 		debugfs_remove_recursive(bat_priv->debug_dir);
 		bat_priv->debug_dir = NULL;
 	}
+	if (net != &init_net)
+		batadv_debugfs_ns_put(net);
 }
-- 
2.7.0.rc3

Re: [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Antonio Quartulli]

On Wed, Jan 20, 2016 at 06:48:27PM +0100, Andrew Lunn wrote:
> The batX soft interface should not be moved between network name
> spaces. This is similar to bridges, bonds, tunnels, which are not
> allowed to move between network namespaces.
> 

Andrew,
I understand bridges and other similar interfaces do the same, but what
is the real reason for preventing a batman soft-interface from moving
between namespaces? I tried to figure this put myself, but I couldn't
find a real technical reason for that

(note - I am not a namespace expert as I am just learning now how they
work)

Cheers,

-- 
Antonio Quartulli

Re: [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Andrew Lunn]

On Sat, Jan 23, 2016 at 10:06:37AM +0800, Antonio Quartulli wrote:
> On Wed, Jan 20, 2016 at 06:48:27PM +0100, Andrew Lunn wrote:
> > The batX soft interface should not be moved between network name
> > spaces. This is similar to bridges, bonds, tunnels, which are not
> > allowed to move between network namespaces.
> > 
> 
> Andrew,
> I understand bridges and other similar interfaces do the same, but what
> is the real reason for preventing a batman soft-interface from moving
> between namespaces?

Humm, actually, a good question. I blindly copied this code without
thinking about it.

One clear answer is the notification mechanism, used by
batadv_hard_if_event(). As far as i can see, such events are only sent
within the interfaces namespace. So for example you hot unplug a hard
interface which the soft interface is using, you would not get the
NETDEV_DOWN event if the soft interface is in a different namespace.

Now that i think of it, there are a few other reasons. All the
interface management calls, and displaying of interface names would
have to contains the namespace. Otherwise you look at the output for
'batctl o' and have no idea which namespace that wlan0 interface is
from. Remember, interface names are only unique within a namespace,
not across namespaces.

namespaces are supposed to be as transparent as possible. Applications
are not supposed to know they are in a name space. That goes right out
the window as soon as you need to include a namespace into every
interface name.

	  Andrew

Re: [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Antonio Quartulli]

On Sat, Jan 23, 2016 at 06:20:06PM +0100, Andrew Lunn wrote:
> On Sat, Jan 23, 2016 at 10:06:37AM +0800, Antonio Quartulli wrote:
> > On Wed, Jan 20, 2016 at 06:48:27PM +0100, Andrew Lunn wrote:
> > > The batX soft interface should not be moved between network name
> > > spaces. This is similar to bridges, bonds, tunnels, which are not
> > > allowed to move between network namespaces.
> > > 
> > 
> > Andrew,
> > I understand bridges and other similar interfaces do the same, but what
> > is the real reason for preventing a batman soft-interface from moving
> > between namespaces?
> 
> Humm, actually, a good question. I blindly copied this code without
> thinking about it.
> 
> One clear answer is the notification mechanism, used by
> batadv_hard_if_event(). As far as i can see, such events are only sent
> within the interfaces namespace. So for example you hot unplug a hard
> interface which the soft interface is using, you would not get the
> NETDEV_DOWN event if the soft interface is in a different namespace.

This raises another question (maybe addressed by another of your patch?): what
happens if an hard-interface currently enslaved in bat0 changes namespace?
Will this result in an UNREGISTER event like when the interface gets destroyed?
If not, I am not sure how the whole situation can still be consistent.

> 
> Now that i think of it, there are a few other reasons. All the
> interface management calls, and displaying of interface names would
> have to contains the namespace. Otherwise you look at the output for
> 'batctl o' and have no idea which namespace that wlan0 interface is
> from. Remember, interface names are only unique within a namespace,
> not across namespaces.

Yeah, it make sense. If the softif is in the same ns as the hardifs, it is
definitely necessary to prevent bat0 from jumping from a namespace to another.

Cheers,

-- 
Antonio Quartulli

Re: [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Antonio Quartulli]

On Sun, Jan 24, 2016 at 02:19:59PM +0800, Antonio Quartulli wrote:
> On Sat, Jan 23, 2016 at 06:20:06PM +0100, Andrew Lunn wrote:
> > On Sat, Jan 23, 2016 at 10:06:37AM +0800, Antonio Quartulli wrote:
> > > On Wed, Jan 20, 2016 at 06:48:27PM +0100, Andrew Lunn wrote:
> > > > The batX soft interface should not be moved between network name
> > > > spaces. This is similar to bridges, bonds, tunnels, which are not
> > > > allowed to move between network namespaces.
> > > > 
> > > 
> > > Andrew,
> > > I understand bridges and other similar interfaces do the same, but what
> > > is the real reason for preventing a batman soft-interface from moving
> > > between namespaces?
> > 
> > Humm, actually, a good question. I blindly copied this code without
> > thinking about it.
> > 
> > One clear answer is the notification mechanism, used by
> > batadv_hard_if_event(). As far as i can see, such events are only sent
> > within the interfaces namespace. So for example you hot unplug a hard
> > interface which the soft interface is using, you would not get the
> > NETDEV_DOWN event if the soft interface is in a different namespace.
> 
> This raises another question (maybe addressed by another of your patch?): what
> happens if an hard-interface currently enslaved in bat0 changes namespace?
> Will this result in an UNREGISTER event like when the interface gets destroyed?
> If not, I am not sure how the whole situation can still be consistent.

It seems that an UNREGISTER event is broadcasted when an hardif goes to another
namespace. At net/dev/core.c:4915

Therefore batman-adv will do the right thing :)

Cheers,


-- 
Antonio Quartulli

Re: [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Antonio Quartulli]

On Wed, Jan 20, 2016 at 06:48:27PM +0100, Andrew Lunn wrote:
> The batX soft interface should not be moved between network name
> spaces. This is similar to bridges, bonds, tunnels, which are not
> allowed to move between network namespaces.
> 
> Suggested-by: Daniel Ehlers <danielehlers@mindeye.net>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Antonio Quartulli <a@unstable.cc>

-- 
Antonio Quartulli

Re: [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves

[Andrew Lunn]

> > This raises another question (maybe addressed by another of your patch?): what
> > happens if an hard-interface currently enslaved in bat0 changes namespace?
> > Will this result in an UNREGISTER event like when the interface gets destroyed?
> > If not, I am not sure how the whole situation can still be consistent.
> 
> It seems that an UNREGISTER event is broadcasted when an hardif goes to another
> namespace. At net/dev/core.c:4915
> 
> Therefore batman-adv will do the right thing :)

Correct. An interface is effectively hotplugged from one namespace to
another.

	Andrew

Re: [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Create batman soft interfaces within correct netns.

[Antonio Quartulli]

On Wed, Jan 20, 2016 at 06:48:28PM +0100, Andrew Lunn wrote:
>  int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
> -				   const char *iface_name)
> +				   struct net *net, const char *iface_name)

Andrew,
minor style note here: instead of passing the namespace as argument, could we
just invoke dev_net() on hard_iface->net_dev inside
batadv_hardif_enable_interface() ?

>  {
>  	struct batadv_priv *bat_priv;
>  	struct net_device *soft_iface, *master;
> @@ -432,10 +433,10 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
>  	if (!atomic_inc_not_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;


-- 
Antonio Quartulli

Re: [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Create batman soft interfaces within correct netns.

[Andrew Lunn]

On Mon, Jan 25, 2016 at 11:28:53AM +0800, Antonio Quartulli wrote:
> On Wed, Jan 20, 2016 at 06:48:28PM +0100, Andrew Lunn wrote:
> >  int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
> > -				   const char *iface_name)
> > +				   struct net *net, const char *iface_name)
> 
> Andrew,
> minor style note here: instead of passing the namespace as argument, could we
> just invoke dev_net() on hard_iface->net_dev inside
> batadv_hardif_enable_interface() ?

Hi Antonio

The problem with that is register_netdevice() is used to register the
soft interface in batadv_softif_create(). Calling it after
registrations would mean it needs to change netns. The default
namespace might already have a bat0, so it is given the name bat1, but
then gets moved to the target netns, and will keeps its name, unless
there already is a bat1 interface. But people expect the newly created
interface to be called bat0.

I think passing the namespace is correct, so the softif can be created
in the correct place to start with.

   Andrew

Re: [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Create batman soft interfaces within correct netns.

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

On Mon, Jan 25, 2016 at 02:12:39PM +0100, Andrew Lunn wrote:
> On Mon, Jan 25, 2016 at 11:28:53AM +0800, Antonio Quartulli wrote:
> > On Wed, Jan 20, 2016 at 06:48:28PM +0100, Andrew Lunn wrote:
> > >  int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
> > > -				   const char *iface_name)
> > > +				   struct net *net, const char *iface_name)
> > 
> > Andrew,
> > minor style note here: instead of passing the namespace as argument, could we
> > just invoke dev_net() on hard_iface->net_dev inside
> > batadv_hardif_enable_interface() ?
> 
> Hi Antonio
> 
> The problem with that is register_netdevice() is used to register the
> soft interface in batadv_softif_create(). Calling it after
> registrations would mean it needs to change netns. The default
> namespace might already have a bat0, so it is given the name bat1, but
> then gets moved to the target netns, and will keeps its name, unless
> there already is a bat1 interface. But people expect the newly created
> interface to be called bat0.
> 
> I think passing the namespace is correct, so the softif can be created
> in the correct place to start with.
> 


Yeah, you are right, I overlooked something in my review.
The patch looks good to me.

Cheers,

-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Create batman soft interfaces within correct netns.

[Antonio Quartulli]

On Wed, Jan 20, 2016 at 06:48:28PM +0100, 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>

-- 
Antonio Quartulli

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

On Wed, Jan 20, 2016 at 06:48:29PM +0100, Andrew Lunn wrote:
>  /**
> + * batadv_mutual_parents - check if two devices are each others parent
> + * @net_dev - 1st parent
> + * @parent_dev - 2nd parent
> + *
> + * veth devices come in pairs and each is the parent of the other!
> + *
> + * Return true if the devices are each others parent, otherwise false

Andrew, minor style issue: you should put the ':' after the word Return.
Please check the other comments in this file as example. Sven recently changed
all the comments to be coherent with this style.

Other than this, the patch looks good to me.
You can add my

Acked-by: Antonio Quartulli <a@untable.cc>

-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

On Tuesday 26 January 2016 20:53:14 Antonio Quartulli wrote:
> On Wed, Jan 20, 2016 at 06:48:29PM +0100, Andrew Lunn wrote:
> >  /**
> > + * batadv_mutual_parents - check if two devices are each others parent
> > + * @net_dev - 1st parent
> > + * @parent_dev - 2nd parent
> > + *
> > + * veth devices come in pairs and each is the parent of the other!
> > + *
> > + * Return true if the devices are each others parent, otherwise false
> 
> Andrew, minor style issue: you should put the ':' after the word Return.
> Please check the other comments in this file as example. Sven recently changed
> all the comments to be coherent with this style.

It is not about being coherent but about kernel-doc being able to parse it.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> nameplace, so also retrieve the parents name space when finding the

s/nameplace/namespace/ ?

> Additionally, veth devices come in pairs and they are each others
> parents. Don't infinitely recurse in this situation.

Can we have this part as extra patch so Antonio can ask David for a stable 
backport?

Thanks,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 332 bytes --]

On Wed, Jan 27, 2016 at 11:13:53AM +0100, Sven Eckelmann wrote:
> > Additionally, veth devices come in pairs and they are each others
> > parents. Don't infinitely recurse in this situation.
> 
> Can we have this part as extra patch so Antonio can ask David for a stable 
> backport?

good idea!


-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: debugfs: Add netns support

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

On Wed, Jan 20, 2016 at 06:48:30PM +0100, Andrew Lunn wrote:

[...]

> +static DEFINE_MUTEX(batadv_debugfs_ns_mutex);

[...]

> +static void batadv_debugfs_ns_put(struct net *net)
> +{
> +	struct batadv_debugfs_ns_entry *ns_entry;
> +
> +	mutex_lock(&batadv_debugfs_ns_mutex);
> +	list_for_each_entry(ns_entry, &batadv_debugfs_ns, link) {
> +		if (ns_entry->net == net) {
> +			kref_put(&ns_entry->refcount, batadv_ns_entry_release);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&batadv_debugfs_ns_mutex);
> +}
>  

Andrew, is there any particular reason why in this case we use a mutex instead
of a spinlock + RCU ? I imagine it is something related to debugfs..or just to
make the code simpler as we don't really require a full-blown RCU strategy ?



>  int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface)
>  {
> +	char *name = hard_iface->net_dev->name;
> +	struct net *net = dev_net(hard_iface->net_dev);

here we should respect David's inverted Christmas tree rule :)

>  	struct batadv_debuginfo **bat_debug;
> +	struct dentry *debugfs_ns_dir;
>  	struct dentry *file;
>  
>  	if (!batadv_debugfs)
>  		goto out;
>  
> -	hard_iface->debug_dir = debugfs_create_dir(hard_iface->net_dev->name,
> -						   batadv_debugfs);

how about:

	debugfs_ns_dir = batadv_debugfs;

> +	if (net != &init_net) {
> +		debugfs_ns_dir = batadv_debugfs_ns_get(net);
> +		if (!debugfs_ns_dir)
> +			goto out;
	}

	hard_iface->debug_dir = debugfs_create_dir(name, debugfs_ns_dir);

isn't it nicer? :)


>  	if (!hard_iface->debug_dir)
>  		goto out;
>  

[...]

> -	bat_priv->debug_dir = debugfs_create_dir(dev->name, batadv_debugfs);
> +	if (net != &init_net) {
> +		debugfs_ns_dir = batadv_debugfs_ns_get(net);
> +		if (!debugfs_ns_dir)
> +			goto out;
> +		bat_priv->debug_dir = debugfs_create_dir(dev->name,
> +							 debugfs_ns_dir);
> +	} else {
> +		bat_priv->debug_dir = debugfs_create_dir(dev->name,
> +							 batadv_debugfs);
> +	}

same as above


> +
>  	if (!bat_priv->debug_dir)
>  		goto out;
>  



-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: debugfs: Add netns support

[Andrew Lunn]

On Thu, Jan 28, 2016 at 09:28:07AM +0800, Antonio Quartulli wrote:
> On Wed, Jan 20, 2016 at 06:48:30PM +0100, Andrew Lunn wrote:
> 
> [...]
> 
> > +static DEFINE_MUTEX(batadv_debugfs_ns_mutex);
> 
> [...]
> 
> > +static void batadv_debugfs_ns_put(struct net *net)
> > +{
> > +	struct batadv_debugfs_ns_entry *ns_entry;
> > +
> > +	mutex_lock(&batadv_debugfs_ns_mutex);
> > +	list_for_each_entry(ns_entry, &batadv_debugfs_ns, link) {
> > +		if (ns_entry->net == net) {
> > +			kref_put(&ns_entry->refcount, batadv_ns_entry_release);
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&batadv_debugfs_ns_mutex);
> > +}
> >  
> 
> Andrew, is there any particular reason why in this case we use a mutex instead
> of a spinlock + RCU ? I imagine it is something related to debugfs..or just to
> make the code simpler as we don't really require a full-blown RCU strategy ?
 
I just wanted it to be KISS. We are on a very slow path, only called
when interfaces are added and removed. Keeping it KISS makes it easier
to review, make sure i have an unlock for every lock, etc. It is also
what is the kref documentation suggests.
 
> >  int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface)
> >  {
> > +	char *name = hard_iface->net_dev->name;
> > +	struct net *net = dev_net(hard_iface->net_dev);
> 
> here we should respect David's inverted Christmas tree rule :)

Yes, i will change this.

> >  	struct batadv_debuginfo **bat_debug;
> > +	struct dentry *debugfs_ns_dir;
> >  	struct dentry *file;
> >  
> >  	if (!batadv_debugfs)
> >  		goto out;
> >  
> > -	hard_iface->debug_dir = debugfs_create_dir(hard_iface->net_dev->name,
> > -						   batadv_debugfs);
> 
> how about:
> 
> 	debugfs_ns_dir = batadv_debugfs;
> 
> > +	if (net != &init_net) {
> > +		debugfs_ns_dir = batadv_debugfs_ns_get(net);
> > +		if (!debugfs_ns_dir)
> > +			goto out;
> 	}
> 
> 	hard_iface->debug_dir = debugfs_create_dir(name, debugfs_ns_dir);
> 
> isn't it nicer? :)

O.K, i will rearrange.

     Andrew

Re: [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: debugfs: Add netns support

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

On Thu, Jan 28, 2016 at 02:40:07AM +0100, Andrew Lunn wrote:
> On Thu, Jan 28, 2016 at 09:28:07AM +0800, Antonio Quartulli wrote:
> > On Wed, Jan 20, 2016 at 06:48:30PM +0100, Andrew Lunn wrote:
> > 
> > [...]
> > 
> > > +static DEFINE_MUTEX(batadv_debugfs_ns_mutex);
> > 
> > [...]
> > 
> > > +static void batadv_debugfs_ns_put(struct net *net)
> > > +{
> > > +	struct batadv_debugfs_ns_entry *ns_entry;
> > > +
> > > +	mutex_lock(&batadv_debugfs_ns_mutex);
> > > +	list_for_each_entry(ns_entry, &batadv_debugfs_ns, link) {
> > > +		if (ns_entry->net == net) {
> > > +			kref_put(&ns_entry->refcount, batadv_ns_entry_release);
> > > +			break;
> > > +		}
> > > +	}
> > > +	mutex_unlock(&batadv_debugfs_ns_mutex);
> > > +}
> > >  
> > 
> > Andrew, is there any particular reason why in this case we use a mutex instead
> > of a spinlock + RCU ? I imagine it is something related to debugfs..or just to
> > make the code simpler as we don't really require a full-blown RCU strategy ?
>  
> I just wanted it to be KISS. We are on a very slow path, only called
> when interfaces are added and removed. Keeping it KISS makes it easier
> to review, make sure i have an unlock for every lock, etc. It is also
> what is the kref documentation suggests.

ok, thanks for explaining, Andrew!
I agree with that, but I wanted to be sure I was understanding the ratio
behind :)


> > how about:
> > 
> > 	debugfs_ns_dir = batadv_debugfs;
> > 
> > > +	if (net != &init_net) {
> > > +		debugfs_ns_dir = batadv_debugfs_ns_get(net);
> > > +		if (!debugfs_ns_dir)
> > > +			goto out;
> > 	}
> > 
> > 	hard_iface->debug_dir = debugfs_create_dir(name, debugfs_ns_dir);
> > 
> > isn't it nicer? :)
> 
> O.K, i will rearrange.

The last chunk reported in my reply could be rearranged the same way.




Thanks a lot!
Other than these points the patch seems sound to me.

Cheers,


-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: Add network name space support

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On Wed, Jan 20, 2016 at 06:48:26PM +0100, Andrew Lunn wrote:
> This patchset makes batman-adv network namespace aware. A soft
> interface can be creates in a namespace and hard interfaces added to
> it. soft interfaces cannot be moved between name spaces.
> 
> The biggest change is to debugfs, which is not natively netns aware,
> unlike sysfs which is. A new netns directory has been added and within
> that, a directory per network name space which batman is used within.
> The changes are backwards compatible, in that interfaces in the global
> namespace are not placed into a subdirectory.

Andrew,

I guess that after this patchset gets applied we need to change batctl and
provide some "netns" argument similar to ip ?

-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: Add network name space support

[Andrew Lunn]

> Andrew,
> 
> I guess that after this patchset gets applied we need to change batctl and
> provide some "netns" argument similar to ip ?

I have patches already. No new arguments needed. batctl and alfred
simply manipulate BATMAN in the current netns.

       Andrew

Re: [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: Add network name space support

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 3737 bytes --]

On Wednesday 20 January 2016 18:48:26 Andrew Lunn wrote:
[...]
> Andrew Lunn (4):
>   batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves
>   batman-adv: Create batman soft interfaces within correct netns.
>   batman-adv: Handle parent interfaces in a different netns
>   batman-adv: debugfs: Add netns support
> 
>  net/batman-adv/debugfs.c           | 118
> ++++++++++++++++++++++++++++++++++++- net/batman-adv/hard-interface.c    | 
> 49 +++++++++++++--
>  net/batman-adv/hard-interface.h    |   2 +-
>  net/batman-adv/soft-interface.c    |   9 ++-
>  net/batman-adv/soft-interface.h    |   2 +-
>  net/batman-adv/sysfs.c             |   3 +-
>  net/batman-adv/translation-table.c |   3 +-
>  7 files changed, 172 insertions(+), 14 deletions(-)

Just to let everybody know: This patchset will break batman-adv compatibility
for kernels older than 4.0 (see d37512a277dfb2cef8a578e25a3246f61399a55a):

hard-interface.c:105:55: error: no member 'get_link_net' in struct rtnl_link_ops
    hard-interface.c:106:54: error: no member 'get_link_net' in struct rtnl_link_ops
    hard-interface.c:107:55: error: no member 'get_link_net' in struct rtnl_link_ops
    hard-interface.c:108:54: error: no member 'get_link_net' in struct rtnl_link_ops
    hard-interface.c:148:61: error: no member 'get_link_net' in struct rtnl_link_ops
    hard-interface.c:149:52: error: no member 'get_link_net' in struct rtnl_link_ops
    hard-interface.c: In function 'batadv_mutual_parents':
    hard-interface.c:105:48: error: 'const struct rtnl_link_ops' has no member named 'get_link_net'
      if (dev1->rtnl_link_ops && dev1->rtnl_link_ops->get_link_net)
                                                    ^
    hard-interface.c:106:40: error: 'const struct rtnl_link_ops' has no member named 'get_link_net'
       dev1_parent_net = dev1->rtnl_link_ops->get_link_net(dev1);
                                            ^
    hard-interface.c:107:48: error: 'const struct rtnl_link_ops' has no member named 'get_link_net'
      if (dev2->rtnl_link_ops && dev2->rtnl_link_ops->get_link_net)
                                                    ^
    hard-interface.c:108:40: error: 'const struct rtnl_link_ops' has no member named 'get_link_net'
       dev2_parent_net = dev2->rtnl_link_ops->get_link_net(dev2);
                                            ^
    hard-interface.c: In function 'batadv_is_on_batman_iface':
    hard-interface.c:148:54: error: 'const struct rtnl_link_ops' has no member named 'get_link_net'
      if (net_dev->rtnl_link_ops && net_dev->rtnl_link_ops->get_link_net)
                                                          ^
    hard-interface.c:149:38: error: 'const struct rtnl_link_ops' has no member named 'get_link_net'
       parent_net = net_dev->rtnl_link_ops->get_link_net(net_dev);
                                          ^
    make[2]: *** [hard-interface.o] Error 1
    make[1]: *** [_module_] Error 2
    make: *** [all] Error 2

(build stopped here and there may be a lot more problems)

And with 3.18 you will only get until (see
435d5f4bb2ccba3b791d9ef61d2590e30b8e806e):

    debugfs.c:87:40: error: no member 'ns' in struct net
    debugfs.c:547:37: error: no member 'ns' in struct net
    debugfs.c: In function 'batadv_debugfs_ns_get':
    debugfs.c:87:26: error: 'struct net' has no member named 'ns'
       sprintf(name, "%u", net->ns.inum);
                              ^
    debugfs.c: In function 'batadv_debugfs_init':
    debugfs.c:547:30: error: 'struct net' has no member named 'ns'
      sprintf(name, "%u", init_net.ns.inum);
                                  ^
    make[2]: *** [debugfs.o] Error 1
    make[1]: *** [_module_] Error 2
    make: *** [all] Error 2


Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Create batman soft interfaces within correct netns.

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]

On Wednesday 20 January 2016 18:48:28 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>
> ---
>  net/batman-adv/hard-interface.c    | 9 +++++----
>  net/batman-adv/hard-interface.h    | 2 +-
>  net/batman-adv/soft-interface.c    | 7 +++++--
>  net/batman-adv/soft-interface.h    | 2 +-
>  net/batman-adv/sysfs.c             | 3 ++-
>  net/batman-adv/translation-table.c | 3 ++-
>  6 files changed, 16 insertions(+), 10 deletions(-)


This patch doesn't apply (via git-am/git-apply; and only unclean with GNU 
patch). Can you please rebase it.

[...]
> diff --git a/net/batman-adv/hard-interface.h
> b/net/batman-adv/hard-interface.h index 5a31420..4ed737f 100644
> --- a/net/batman-adv/hard-interface.h
> +++ b/net/batman-adv/hard-interface.h

New forward declaration missing:

struct net;

> @@ -55,7 +55,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.h
> b/net/batman-adv/soft-interface.h index 8e82176..d616cd5 100644
> --- a/net/batman-adv/soft-interface.h
> +++ b/net/batman-adv/soft-interface.h

New forward declaration missing:

struct net;

> @@ -29,7 +29,7 @@ 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);
>  int batadv_softif_is_valid(const struct net_device *net_dev);
>  extern struct rtnl_link_ops batadv_link_ops;
[...]
> diff --git a/net/batman-adv/translation-table.c
> b/net/batman-adv/translation-table.c index 76f19ba..0dbda83 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c

init_net is not used anymore and thus following include should be removed:

#include <net/net_namespace.h>

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> batman-adv tries to prevent the user from placing a batX soft
> interface into another batman mesh as a hard interface. It does this
> by walking up the devices list of parents and ensures they are all
> none batX interfaces. iflink can point to an interface in a different
> nameplace, so also retrieve the parents name space when finding the
> parent.
> 
> Additionally, veth devices come in pairs and they are each others
> parents. Don't infinitely recurse in this situation.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/batman-adv/hard-interface.c | 44
> +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+),
> 2 deletions(-)
> 
> diff --git a/net/batman-adv/hard-interface.c
> b/net/batman-adv/hard-interface.c index a5ce58a..8d51a4a 100644
> --- a/net/batman-adv/hard-interface.c
> +++ b/net/batman-adv/hard-interface.c

Include missing:

#include <net/rtnetlink.h>

> @@ -75,6 +75,39 @@ out:
>  }
> 
>  /**
> + * batadv_mutual_parents - check if two devices are each others parent
> + * @net_dev - 1st parent
> + * @parent_dev - 2nd parent
> + *
> + * veth devices come in pairs and each is the parent of the other!
> + *
> + * Return true if the devices are each others parent, otherwise false
> + */
> +static bool batadv_mutual_parents(const struct net_device *dev1,
> +				  const struct net *dev1_net,
> +				  const struct net_device *dev2,
> +				  const struct net *dev2_net)


 * What is net_dev and parent_dev?
 * Missing parameter docs are:
   - dev1
   - dev1_net
   - dev2
   - dev2_net.
 * Return -> Return:

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: debugfs: Add netns support

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

On Wednesday 20 January 2016 18:48:30 Andrew Lunn wrote:
> Unlike sysfs, debugfs is not netns aware. So batman has to take care
> to avoid namespace clashes.
> 
> Each namespace is given a directory within debugfs/batman-adv/netns,
> using the namespaces inum as the directory name.
> 
> Files for namespaces other than the global namespace are placed within
> the namespace specific directory. Additionally, a symbolic link is
> used to link the global namespaces inum back to debugfs/batman-adv/ so
> tools do not need to differentiate between the global namespace and
> other namespaces.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/batman-adv/debugfs.c | 118
> +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 115
> insertions(+), 3 deletions(-)
> 
> diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c
> index c4c1e80..b87ffad 100644
> --- a/net/batman-adv/debugfs.c
> +++ b/net/batman-adv/debugfs.c

Missing includes:

#include <linux/kref.h>
#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/ns_common.h>
#include <net/net_namespace.h>

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On Wed, Jan 27, 2016 at 11:13:53AM +0100, Sven Eckelmann wrote:
> On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> > nameplace, so also retrieve the parents name space when finding the
> 
> s/nameplace/namespace/ ?
> 
> > Additionally, veth devices come in pairs and they are each others
> > parents. Don't infinitely recurse in this situation.
> 
> Can we have this part as extra patch so Antonio can ask David for a stable 
> backport?

Andrew,

do you have any plan about resending this fix ?
Sven also sent a pair of fixes and it would be nice I could send everything in
on pull request.

Cheers,


-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Handle parent interfaces in a different netns

[Andrew Lunn]

On Mon, Feb 01, 2016 at 10:57:55AM +0800, Antonio Quartulli wrote:
> On Wed, Jan 27, 2016 at 11:13:53AM +0100, Sven Eckelmann wrote:
> > On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> > > nameplace, so also retrieve the parents name space when finding the
> > 
> > s/nameplace/namespace/ ?
> > 
> > > Additionally, veth devices come in pairs and they are each others
> > > parents. Don't infinitely recurse in this situation.
> > 
> > Can we have this part as extra patch so Antonio can ask David for a stable 
> > backport?
> 
> Andrew,
> 
> do you have any plan about resending this fix ?
> Sven also sent a pair of fixes and it would be nice I could send everything in
> on pull request.

Sorry, been in the wilderness the last few days.

I will try to resend tomorrow. If not, it will be Thursday.  If you
want it sooner, feel free to fix it up yourself and add your own
Signed-off by, next to mine.

	   Andrew

Re: [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: Add network name space support

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 4133 bytes --]

On Sunday 31 January 2016 14:26:27 Sven Eckelmann wrote:
[...]
> Just to let everybody know: This patchset will break batman-adv
> compatibility for kernels older than 4.0 (see
> d37512a277dfb2cef8a578e25a3246f61399a55a):
> 
> hard-interface.c:105:55: error: no member 'get_link_net' in struct
> rtnl_link_ops hard-interface.c:106:54: error: no member 'get_link_net' in
> struct rtnl_link_ops hard-interface.c:107:55: error: no member
> 'get_link_net' in struct rtnl_link_ops hard-interface.c:108:54: error: no
> member 'get_link_net' in struct rtnl_link_ops hard-interface.c:148:61:
> error: no member 'get_link_net' in struct rtnl_link_ops
> hard-interface.c:149:52: error: no member 'get_link_net' in struct
> rtnl_link_ops hard-interface.c: In function 'batadv_mutual_parents':
>     hard-interface.c:105:48: error: 'const struct rtnl_link_ops' has no
> member named 'get_link_net' if (dev1->rtnl_link_ops &&
> dev1->rtnl_link_ops->get_link_net) ^
>     hard-interface.c:106:40: error: 'const struct rtnl_link_ops' has no
> member named 'get_link_net' dev1_parent_net =
> dev1->rtnl_link_ops->get_link_net(dev1);
>                                             ^
>     hard-interface.c:107:48: error: 'const struct rtnl_link_ops' has no
> member named 'get_link_net' if (dev2->rtnl_link_ops &&
> dev2->rtnl_link_ops->get_link_net) ^
>     hard-interface.c:108:40: error: 'const struct rtnl_link_ops' has no
> member named 'get_link_net' dev2_parent_net =
> dev2->rtnl_link_ops->get_link_net(dev2);
>                                             ^
>     hard-interface.c: In function 'batadv_is_on_batman_iface':
>     hard-interface.c:148:54: error: 'const struct rtnl_link_ops' has no
> member named 'get_link_net' if (net_dev->rtnl_link_ops &&
> net_dev->rtnl_link_ops->get_link_net) ^
>     hard-interface.c:149:38: error: 'const struct rtnl_link_ops' has no
> member named 'get_link_net' parent_net =
> net_dev->rtnl_link_ops->get_link_net(net_dev); ^
>     make[2]: *** [hard-interface.o] Error 1
>     make[1]: *** [_module_] Error 2
>     make: *** [all] Error 2
> 
> (build stopped here and there may be a lot more problems)
> 
> And with 3.18 you will only get until (see
> 435d5f4bb2ccba3b791d9ef61d2590e30b8e806e):
> 
>     debugfs.c:87:40: error: no member 'ns' in struct net
>     debugfs.c:547:37: error: no member 'ns' in struct net
>     debugfs.c: In function 'batadv_debugfs_ns_get':
>     debugfs.c:87:26: error: 'struct net' has no member named 'ns'
>        sprintf(name, "%u", net->ns.inum);
>                               ^
>     debugfs.c: In function 'batadv_debugfs_init':
>     debugfs.c:547:30: error: 'struct net' has no member named 'ns'
>       sprintf(name, "%u", init_net.ns.inum);
>                                   ^
>     make[2]: *** [debugfs.o] Error 1
>     make[1]: *** [_module_] Error 2
>     make: *** [all] Error 2



On Friday 12 February 2016 16:25:36 Andrew Lunn wrote:
> But what about the netns case? This is mainly a backward compatible
> issue. It sounds like some of the older kernels you have via compat.h
> are going to have issues with netns support.

There is now more than compat.h. So maybe we can find a more complex way to 
work around the problem?

> What do the maintainers
> what to do about this? NACK my patches, drop support for some of the
> older kernels? Something else?

I personally don't be one of the maintainers but I see the reduction of 
supported kernel as being a significant problem. I am also for reducing the 
supported kernel in the near future but only to something like 3.2 (which 
should be the oldest not EOL kernel in some months). But reducing the 
compatibility to only 4.0-4.5 is is quite a cut. Especially when people like 
Andreas Pape still want to use 2.6.32 (bad idea btw.).

But even the OpenWrt trunk (3.18, 4.1, 4.3, 4.4) would be affected by this 
change. Maybe the OpenWrt case is not that problematic because someone could 
just apply some patches to get support for older kernels.

On the other hand, there are multiple people interested in this feature. So 
just NACKing them also would not be a good idea.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]