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.
v2:
Added missing includes Added missing forward declarations Rearranged debugfs code Fixed kernel doc Rebased on https://git.open-mesh.org/batman-adv.git master
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 | 119 ++++++++++++++++++++++++++++++++++++- net/batman-adv/hard-interface.c | 36 ++++++++--- net/batman-adv/hard-interface.h | 3 +- net/batman-adv/soft-interface.c | 9 ++- net/batman-adv/soft-interface.h | 3 +- net/batman-adv/sysfs.c | 3 +- net/batman-adv/translation-table.c | 4 +- 7 files changed, 157 insertions(+), 20 deletions(-)
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 --- 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 7679f3a..f4aa60a 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -955,7 +955,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
On Tuesday 01 March 2016 22:19:05 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
net/batman-adv/soft-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Sven Eckelmann sven@narfation.org
Andrew, would it be ok for you to apply this patch first while the last two/three patches are still discussed? At least this could (when Marek agrees) reduce the amount of patches you have to resend each time.
Kind regards, Sven
On Sun, Mar 13, 2016 at 10:29:21AM +0100, Sven Eckelmann wrote:
On Tuesday 01 March 2016 22:19:05 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
net/batman-adv/soft-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Sven Eckelmann sven@narfation.org
Andrew, would it be ok for you to apply this patch first while the last two/three patches are still discussed?
Sure, no problem. Please take it.
Andrew
On Sunday, March 13, 2016 16:24:23 Andrew Lunn wrote:
On Sun, Mar 13, 2016 at 10:29:21AM +0100, Sven Eckelmann wrote:
On Tuesday 01 March 2016 22:19:05 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
net/batman-adv/soft-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Sven Eckelmann sven@narfation.org
Andrew, would it be ok for you to apply this patch first while the last two/three patches are still discussed?
Sure, no problem. Please take it.
Applied in revision 34199a6.
Thanks, Marek
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(-)
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);
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
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
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 namespace, so also retrieve the parents name space when finding the parent and use it when doing the comparison.
Signed-off-by: Andrew Lunn andrew@lunn.ch Acked-by: Antonio Quartulli a@untable.cc --- net/batman-adv/hard-interface.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index b28eacc..51e9e68 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -85,24 +85,37 @@ out:
/** * batadv_mutual_parents - check if two devices are each others parent - * @dev1: 1st net_device - * @dev2: 2nd net_device + * @dev1: 1st net dev + * @net1: 1st devices netns + * @dev2: 2nd net dev + * @net2: 2nd devices netns * * 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_device *dev2) + const struct net *net1, + const struct net_device *dev2, + const struct net *net2) { int dev1_parent_iflink = dev_get_iflink(dev1); int dev2_parent_iflink = dev_get_iflink(dev2); + const struct net *dev1_parent_net = net1; + const struct net *dev2_parent_net = net2; + + 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); + (dev2_parent_iflink == dev1->ifindex) && + net_eq(dev1_parent_net, net2) && + net_eq(dev2_parent_net, net1); }
/** @@ -120,8 +133,9 @@ 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); + struct net_device *parent_dev; + struct net *parent_net = net; bool ret;
/* check if this is a batman-adv mesh interface */ @@ -133,13 +147,16 @@ 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, parent_dev)) + if (batadv_mutual_parents(net_dev, net, parent_dev, parent_net)) return false;
ret = batadv_is_on_batman_iface(parent_dev);
On Tuesday 01 March 2016 22:19:07 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 namespace, so also retrieve the parents name space when finding the parent and use it when doing the comparison.
Signed-off-by: Andrew Lunn andrew@lunn.ch Acked-by: Antonio Quartulli a@untable.cc
net/batman-adv/hard-interface.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)
Include missing in net/batman-adv/hard-interface.c
#include <net/rtnetlink.h>
Does anyone (not only Andrew) have a proposal regarding the compat code? Patch 1+2 should be unproblematic (maybe these can already be applied?).
This patch here is the first problematic one because it uses get_link_net which was first introduced in v4.0 with d37512a277df ("rtnl: add link netns id to interface messages"). I personally see some function substractions coming at us. But then it would be better to have this code in a smaller function to make "patching" easier (but I don't have a complete solution right now):
dev_parent_net = default_net; if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) dev_parent_net = dev->rtnl_link_ops->get_link_net(dev);
The next problematic part in regards of compat stuff is the patch 4. linux/ns_common.h is missing (can be added without a problem in compat-includes). More problematic is the net::ns substruct (to be more precise the net::ns.inum) which was first introduced in 3.19 with 435d5f4bb2cc ("common object embedded into various struct ....ns") and was previously called net::proc_inum (3.8-3.18) and did not exist before v3.8 98f842e675f9 ("proc: Usable inode numbers for the namespace file descriptors.").
Kind regards, Sven
On Fri, Mar 04, 2016 at 01:35:01PM +0100, Sven Eckelmann wrote:
On Tuesday 01 March 2016 22:19:07 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 namespace, so also retrieve the parents name space when finding the parent and use it when doing the comparison.
Signed-off-by: Andrew Lunn andrew@lunn.ch Acked-by: Antonio Quartulli a@untable.cc
net/batman-adv/hard-interface.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)
Include missing in net/batman-adv/hard-interface.c
#include <net/rtnetlink.h>
Hi Sven
How are you determining this? It compiled fine for me, which is my usual test.
Does anyone (not only Andrew) have a proposal regarding the compat code? Patch 1+2 should be unproblematic (maybe these can already be applied?).
I've not much history with netns. One thing which might be interesting it know, is when did it become mature enough to the usable? Maybe for kernels older than v4.0, deny that netns exists, and find a way for functions like get_link_net() to return the default net. compat.h could contain
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #if IS_ENABLED(CONFIG_NET_NS) #error Network Namespaces not support with this vintage of kernel. Please disable #endif #endif
You then know everything is going to be in the default namespace for these old kernels.
This patch here is the first problematic one because it uses get_link_net which was first introduced in v4.0 with d37512a277df ("rtnl: add link netns id to interface messages"). I personally see some function substractions coming at us. But then it would be better to have this code in a smaller function to make "patching" easier (but I don't have a complete solution right now):
dev_parent_net = default_net; if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) dev_parent_net = dev->rtnl_link_ops->get_link_net(dev);
Horrible, and i have no idea if it actually works, but how about
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #define dev->rtnl_link_ops->get_link_net true #define dev->rtnl_link_ops->get_link_net(dev) init_net #endif
so we get the default namespace.
The next problematic part in regards of compat stuff is the patch 4. linux/ns_common.h is missing (can be added without a problem in compat-includes). More problematic is the net::ns substruct (to be more precise the net::ns.inum) which was first introduced in 3.19 with 435d5f4bb2cc ("common object embedded into various struct ....ns") and was previously called net::proc_inum (3.8-3.18) and did not exist before v3.8 98f842e675f9 ("proc: Usable inode numbers for the namespace file descriptors.").
How about something along the lines of
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #define init_net.ns.inum 42 #define net->ns.inum 42 #endif
Andrew
On Monday 07 March 2016 15:31:30 Andrew Lunn wrote:
On Fri, Mar 04, 2016 at 01:35:01PM +0100, Sven Eckelmann wrote:
On Tuesday 01 March 2016 22:19:07 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 namespace, so also retrieve the parents name space when finding the parent and use it when doing the comparison.
Signed-off-by: Andrew Lunn andrew@lunn.ch Acked-by: Antonio Quartulli a@untable.cc
net/batman-adv/hard-interface.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)
Include missing in net/batman-adv/hard-interface.c
#include <net/rtnetlink.h>Hi Sven
How are you determining this? It compiled fine for me, which is my usual test.
https://git.open-mesh.org/build_test.git/blob/a246d227882b7bc03ae602b41cf650...
Does anyone (not only Andrew) have a proposal regarding the compat code? Patch 1+2 should be unproblematic (maybe these can already be applied?).
I've not much history with netns. One thing which might be interesting it know, is when did it become mature enough to the usable? Maybe for kernels older than v4.0, deny that netns exists, and find a way for functions like get_link_net() to return the default net. compat.h could contain
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #if IS_ENABLED(CONFIG_NET_NS) #error Network Namespaces not support with this vintage of kernel. Please disable #endif #endif
You then know everything is going to be in the default namespace for these old kernels.
I think it is a good idea to deactivate it for too old kernels. But it will explode in everyones face when build against the distro kernel. But maybe letting some of the functions "fail" for old kernels might be possible.
[...]
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #define dev->rtnl_link_ops->get_link_net true #define dev->rtnl_link_ops->get_link_net(dev) init_net #endif
[...]
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #define init_net.ns.inum 42 #define net->ns.inum 42 #endif
These defines should not work (I haven't actually tested them but they should not work with all the . and ->). The preprocessor should fail because it ends the preprocessor macro name with init_net and then doesn't expect '.' to follow (only '(' or tab/space).
Kind regards, Sven
On Monday 07 March 2016 15:31:30 Andrew Lunn wrote: [...]
right now): dev_parent_net = default_net;
if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net)
dev_parent_net = dev->rtnl_link_ops->get_link_net(dev);Horrible, and i have no idea if it actually works, but how about
#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)) #define dev->rtnl_link_ops->get_link_net true #define dev->rtnl_link_ops->get_link_net(dev) init_net #endif
I have prepared a quick draft of a different idea [1].
You have to excuse me now but I have a longer meeting with my shower to get rid of the feeling that I have done something horrible/dirty. :)
Kind regards, Sven
[1] https://git.open-mesh.org/batman-adv.git/commit/fb7b5eb8c81969dc8bea356d0a3f...
On Sat, Mar 12, 2016 at 12:37:17AM +0100, Sven Eckelmann wrote:
[1] https://git.open-mesh.org/batman-adv.git/commit/fb7b5eb8c81969dc8bea356d0a3f...
Sven, what is the README there for ?
Cheers,
On Saturday 12 March 2016 12:42:36 Antonio Quartulli wrote:
On Sat, Mar 12, 2016 at 12:37:17AM +0100, Sven Eckelmann wrote:
[1] https://git.open-mesh.org/batman-adv.git/commit/fb7b5eb8c81969dc8bea356d0 a3f0b5e72d2da49
Sven, what is the README there for ?
It serves different purposes. It makes sure that the directory is created by git on checkout even when there is no patch in the directory. It also makes sure that the Makefile dependency glob for compat-patches/* is at least generating one dependency.
But it should also be used to define the rules under which a patch in this directory is acceptable (of course, the rules are not yet defined - so I've just used a placeholder for this draft). The requirements for such a patch should be really high because patches tend to fail when the code or the surrounding code is changed. This is also the reason why I've combined the critical code to really small functions which avoids more patch chunks for more places in the code.
Kind regards, Sven
On Saturday 12 March 2016 08:34:35 Sven Eckelmann wrote:
On Saturday 12 March 2016 12:42:36 Antonio Quartulli wrote:
On Sat, Mar 12, 2016 at 12:37:17AM +0100, Sven Eckelmann wrote:
[1] https://git.open-mesh.org/batman-adv.git/commit/fb7b5eb8c81969dc8bea356d 0 a3f0b5e72d2da49
Sven, what is the README there for ?
It serves different purposes. It makes sure that the directory is created by git on checkout even when there is no patch in the directory. It also makes sure that the Makefile dependency glob for compat-patches/* is at least generating one dependency.
But it should also be used to define the rules under which a patch in this directory is acceptable (of course, the rules are not yet defined - so I've just used a placeholder for this draft). The requirements for such a patch should be really high because patches tend to fail when the code or the surrounding code is changed. This is also the reason why I've combined the critical code to really small functions which avoids more patch chunks for more places in the code.
And maybe it should also describe how to generate patches for this directory
git format-patch --abbrev=7 -U3 --diff-algorithm=histogram --no-signature --format=format:'From: %an <%ae>%nDate: %aD%nSubject: [PATCH] %B' -1
Kind regards, Sven
Hi Andrew,
Thanks for looking into this!
It seems some compat code might be missing, compiling against a 3.19 kernel fails for me (4.0 to 4.4 compiles fine):
----- rm -f compat-autoconf.h* make -C /lib/modules/3.2.0-4-amd64/build M=/home/tux/dev/batman-adv-t_x/net/batman-adv CONFIG_BATMAN_ADV=m CONFIG_BATMAN_ADV_DEBUG=n CONFIG_BATMAN_ADV_BLA=y CONFIG_BATMAN_ADV_DAT=y CONFIG_BATMAN_ADV_NC=n CONFIG_BATMAN_ADV_MCAST=y CONFIG_BATMAN_ADV_BATMAN_V=n INSTALL_MOD_DIR=updates/net/batman-adv/ clean make[1]: Entering directory `/usr/src/linux-headers-3.2.0-4-amd64' CLEAN /home/tux/dev/batman-adv-t_x/net/batman-adv/.tmp_versions make[1]: Leaving directory `/usr/src/linux-headers-3.2.0-4-amd64' make clean 0,26s user 0,18s system 66% cpu 0,666 total /home/tux/dev/batman-adv-t_x/gen-compat-autoconf.sh /home/tux/dev/batman-adv-t_x/compat-autoconf.h /usr/bin/make -C /home/tux/dev/linux/headers/usr/src/linux-headers-3.19.0+ M=/home/tux/dev/batman-adv-t_x/net/batman-adv CONFIG_BATMAN_ADV=m CONFIG_BATMAN_ADV_DEBUG=n CONFIG_BATMAN_ADV_BLA=y CONFIG_BATMAN_ADV_DAT=y CONFIG_BATMAN_ADV_NC=n CONFIG_BATMAN_ADV_MCAST=y CONFIG_BATMAN_ADV_BATMAN_V=n INSTALL_MOD_DIR=updates/net/batman-adv/ modules make[1]: Entering directory `/home/tux/dev/linux/headers/usr/src/linux-headers-3.19.0+' CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/bat_iv_ogm.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/bitarray.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/bridge_loop_avoidance.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/debugfs.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/distributed-arp-table.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/fragmentation.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/gateway_client.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/gateway_common.o CC [M] /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.o /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c: In function ‘batadv_mutual_parents’: /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c:107:48: error: ‘const struct rtnl_link_ops’ has no member named ‘get_link_net’ /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c:108:40: error: ‘const struct rtnl_link_ops’ has no member named ‘get_link_net’ /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c:109:48: error: ‘const struct rtnl_link_ops’ has no member named ‘get_link_net’ /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c:110:40: error: ‘const struct rtnl_link_ops’ has no member named ‘get_link_net’ /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c: In function ‘batadv_is_on_batman_iface’: /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c:150:54: error: ‘const struct rtnl_link_ops’ has no member named ‘get_link_net’ /home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.c:151:38: error: ‘const struct rtnl_link_ops’ has no member named ‘get_link_net’ make[2]: *** [/home/tux/dev/batman-adv-t_x/net/batman-adv/hard-interface.o] Error 1 make[1]: *** [_module_/home/tux/dev/batman-adv-t_x/net/batman-adv] Error 2 make[1]: Leaving directory `/home/tux/dev/linux/headers/usr/src/linux-headers-3.19.0+' make: *** [all] Error 2 -----
Regards, Linus
On Mon, Mar 07, 2016 at 05:21:48AM +0100, Linus Lüssing wrote:
Hi Andrew,
Thanks for looking into this!
It seems some compat code might be missing, compiling against a 3.19 kernel fails for me (4.0 to 4.4 compiles fine):
Hi Linus
I didn't look at compat code at all. I mostly work on net-next/master, and the last -rc series.
Sven started a thread about this, and i can take a closer look to see if i can help.
Andrew
On Tuesday 01 March 2016 22:19:07 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 namespace, so also retrieve the parents name space when finding the parent and use it when doing the comparison.
Signed-off-by: Andrew Lunn andrew@lunn.ch Acked-by: Antonio Quartulli a@untable.cc
[...]
return (dev1_parent_iflink == dev2->ifindex) &&
(dev2_parent_iflink == dev1->ifindex);
(dev2_parent_iflink == dev1->ifindex) &&net_eq(dev1_parent_net, net2) &&net_eq(dev2_parent_net, net1);}
Just spotted this:
"(dev2_parent_iflink == dev1->ifindex) &&" and all following lines are not correctly aligned anymore (7 spaces vs 1 tab).
[...]
- const struct net *dev1_parent_net = net1;
- const struct net *dev2_parent_net = net2;
- 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);
[...]
struct net *parent_net = net;
[...]
- 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);
My proposal is there to move this in an extra function [1] so adding compat code is easier [2].
Already said this in an earlier mail but just for completeness:
Include missing in net/batman-adv/hard-interface.c
#include <net/rtnetlink.h>
Kind regards, Sven
[1] https://git.open-mesh.org/batman-adv.git/blob/aef6842ec0facda9ec52736f8a6ce8... [2] https://git.open-mesh.org/batman-adv.git/blob/fb7b5eb8c81969dc8bea356d0a3f0b...
- const struct net *dev1_parent_net = net1;
- const struct net *dev2_parent_net = net2;
- 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);[...]
struct net *parent_net = net;[...]
- 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);My proposal is there to move this in an extra function [1] so adding compat code is easier [2].
Hi Sven
Do you want to squash your compact changes in and add your own Signed-off-by?
Andrew
On Sunday 13 March 2016 16:38:08 Andrew Lunn wrote: [...]
My proposal is there to move this in an extra function [1] so adding compat code is easier [2].
[...]
Do you want to squash your compact changes in and add your own Signed-off-by?
You mean that I should finish the proposal [1] from Friday and resent it together with (slightly) polished versions of your patches?
Yes, I can do that. Not sure if I will do it tonight or tomorrow. But you should see the new patches (v3) soon on the mailing list.
Kind regards, Sven
[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-March/014661.html
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 ---
v2: Add missing includes --- net/batman-adv/debugfs.c | 119 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 116 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c index 3dc5208..1c6b71c 100644 --- a/net/batman-adv/debugfs.c +++ b/net/batman-adv/debugfs.c @@ -27,8 +27,12 @@ #include <linux/fs.h> #include <linux/jiffies.h> #include <linux/kernel.h> +#include <linux/kref.h> +#include <linux/list.h> #include <linux/module.h> +#include <linux/mutex.h> #include <linux/netdevice.h> +#include <linux/ns_common.h> #include <linux/poll.h> #include <linux/printk.h> #include <linux/sched.h> /* for linux/wait.h */ @@ -42,6 +46,7 @@ #include <linux/types.h> #include <linux/uaccess.h> #include <linux/wait.h> +#include <net/net_namespace.h> #include <stdarg.h>
#include "bridge_loop_avoidance.h" @@ -53,6 +58,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) @@ -451,6 +523,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)) @@ -471,6 +544,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); @@ -492,14 +574,24 @@ void batadv_debugfs_destroy(void) */ int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface) { + struct net *net = dev_net(hard_iface->net_dev); + char *name = hard_iface->net_dev->name; 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); + 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); if (!hard_iface->debug_dir) goto out;
@@ -517,6 +609,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; } @@ -528,22 +622,36 @@ 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); + debugfs_ns_dir = 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); if (!bat_priv->debug_dir) goto out;
@@ -572,6 +680,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; } @@ -579,6 +689,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);
@@ -586,4 +697,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); }
On 03/01/2016 10:19 PM, 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
By the way, the netns support is another good reason to switch from the debugfs interfaces to a netlink-based interface (as the netlink interface wouldn't need userspace applications like batctl to be aware of the namespaces). I guess I should finally finish the patches I started writing for that...
This becomes even more important when namespaces are used for isolation (e.g. by LXC/docker/...), as debugfs is really broken and would allow root in any namespace to trigger use-after-frees and make the kernel hold the RTNL lock indefinitely, besides tons of other debug interfaces a container root could abuse. Running batman-adv in LXC or docker would be really nice though...
Regards, Matthias
v2: Add missing includes
net/batman-adv/debugfs.c | 119 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 116 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c index 3dc5208..1c6b71c 100644 --- a/net/batman-adv/debugfs.c +++ b/net/batman-adv/debugfs.c @@ -27,8 +27,12 @@ #include <linux/fs.h> #include <linux/jiffies.h> #include <linux/kernel.h> +#include <linux/kref.h> +#include <linux/list.h> #include <linux/module.h> +#include <linux/mutex.h> #include <linux/netdevice.h> +#include <linux/ns_common.h> #include <linux/poll.h> #include <linux/printk.h> #include <linux/sched.h> /* for linux/wait.h */ @@ -42,6 +46,7 @@ #include <linux/types.h> #include <linux/uaccess.h> #include <linux/wait.h> +#include <net/net_namespace.h> #include <stdarg.h>
#include "bridge_loop_avoidance.h" @@ -53,6 +58,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) @@ -451,6 +523,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))
@@ -471,6 +544,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); @@ -492,14 +574,24 @@ void batadv_debugfs_destroy(void) */ int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface) {
struct net *net = dev_net(hard_iface->net_dev);
char *name = hard_iface->net_dev->name; 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);
- 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); if (!hard_iface->debug_dir) goto out;
@@ -517,6 +609,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; } @@ -528,22 +622,36 @@ 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);
- debugfs_ns_dir = 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); if (!bat_priv->debug_dir) goto out;
@@ -572,6 +680,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; } @@ -579,6 +689,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);
@@ -586,4 +697,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);}
On Monday 07 March 2016 15:21:07 Matthias Schiffer wrote: [...]
By the way, the netns support is another good reason to switch from the debugfs interfaces to a netlink-based interface (as the netlink interface wouldn't need userspace applications like batctl to be aware of the namespaces). I guess I should finally finish the patches I started writing for that...
So what is your suggestion here? Should the namespace support for namespaces be rejected and you send in your netlink implementation patches? Or should this patch be merged and be removed (together with the rest of the debugfs stuff) when your netlink support is integrated?
Kind regards, Sven
On Sun, Mar 13, 2016 at 10:12:07AM +0100, Sven Eckelmann wrote:
On Monday 07 March 2016 15:21:07 Matthias Schiffer wrote: [...]
By the way, the netns support is another good reason to switch from the debugfs interfaces to a netlink-based interface (as the netlink interface wouldn't need userspace applications like batctl to be aware of the namespaces). I guess I should finally finish the patches I started writing for that...
So what is your suggestion here? Should the namespace support for namespaces be rejected and you send in your netlink implementation patches? Or should this patch be merged and be removed (together with the rest of the debugfs stuff) when your netlink support is integrated?
Hi Sven
I would expect the debugfs code to stay around for a while, so people have a chance to upgrade their batctl and alfred to the new API. We probably need one release with both?
Andrew
Hi Andrew & list,
On Sunday 13 March 2016 16:42:22 Andrew Lunn wrote:
On Sun, Mar 13, 2016 at 10:12:07AM +0100, Sven Eckelmann wrote:
On Monday 07 March 2016 15:21:07 Matthias Schiffer wrote: [...]
By the way, the netns support is another good reason to switch from the debugfs interfaces to a netlink-based interface (as the netlink interface wouldn't need userspace applications like batctl to be aware of the namespaces). I guess I should finally finish the patches I started writing for that...
So what is your suggestion here? Should the namespace support for namespaces be rejected and you send in your netlink implementation patches? Or should this patch be merged and be removed (together with the rest of the debugfs stuff) when your netlink support is integrated?
Hi Sven
I would expect the debugfs code to stay around for a while, so people have a chance to upgrade their batctl and alfred to the new API. We probably need one release with both?
we had a phone discussion with Antonio, Marek, Sven and myself how to move forward with netlink and namespace support.
We concluded that having proper netlink support would be the more future proof option. We would then keep debugfs but slowly phase it out in the next coming years. New features would also be adopted in the netlink implementation.
It was also our impression that having the namespace support within netlink would be the cleaner approach, although it takes more work because it requires the netlink and appropriate userspace changes.
Andrew, what do you think? Would you like to check and rebase on Matthias' patchset?
Thanks, Simon
we had a phone discussion with Antonio, Marek, Sven and myself how to move forward with netlink and namespace support.
We concluded that having proper netlink support would be the more future proof option. We would then keep debugfs but slowly phase it out in the next coming years. New features would also be adopted in the netlink implementation.
It was also our impression that having the namespace support within netlink would be the cleaner approach, although it takes more work because it requires the netlink and appropriate userspace changes.
Andrew, what do you think? Would you like to check and rebase on Matthias' patchset?
I agree about debugfs. As i already said, i've played with netlink and netns. I changed my debugfs patch, so that it only creates files for the default netns. For all other netns, debugfs operations become a NOP, and you need to use netns.
Andrew
On Wed, Apr 20, 2016 at 04:36:40AM +0200, Andrew Lunn wrote:
I agree about debugfs. As i already said, i've played with netlink and netns. I changed my debugfs patch, so that it only creates files for the default netns. For all other netns, debugfs operations become a NOP, and you need to use netns.
I guess you meant "you need to use netlink" here ?
Thanks for your feedback.
Cheers,
On Wednesday 20 April 2016 04:36:40 Andrew Lunn wrote: [...]
I agree about debugfs. As i already said, i've played with netlink and netns. I changed my debugfs patch, so that it only creates files for the default netns. For all other netns, debugfs operations become a NOP, and you need to use netns.
Ok, so you already have some new patches. I will therefore mark the current patches as "Changes requested". The alfred/batctl patches will be marked as rejected because they would have to be replaced by something else. It is not about that I think they are bad but just because a different approach is preferred.
Kind regards, Sven
On 03/13/2016 10:12 AM, Sven Eckelmann wrote:
On Monday 07 March 2016 15:21:07 Matthias Schiffer wrote: [...]
By the way, the netns support is another good reason to switch from the debugfs interfaces to a netlink-based interface (as the netlink interface wouldn't need userspace applications like batctl to be aware of the namespaces). I guess I should finally finish the patches I started writing for that...
So what is your suggestion here? Should the namespace support for namespaces be rejected and you send in your netlink implementation patches? Or should this patch be merged and be removed (together with the rest of the debugfs stuff) when your netlink support is integrated?
Kind regards, Sven
As my netlink patches need more work, I guess it would make sense for me to rebase them onto the netns patchset.
At least the non-netns debugfs interface would need to continue being supported for a while I guess. As you know, both Linus Torvalds and David are very strict about kernel ABI regressions, and I know that at least Linus considers debugfs kernel ABI, so the same ABI stability guarantees as for the rest of the kernel apply. This makes me think that the netns support should not be merged into mainline until the netlink interface is done, so we don't add even more legacy interfaces.
I'll continue my work on the netlink patchset, I plan to send a v2 some time this week.
Matthias
b.a.t.m.a.n@lists.open-mesh.org
-
Andrew Lunn -
Antonio Quartulli -
Linus Lüssing -
Marek Lindner -
Matthias Schiffer -
Simon Wunderlich -
Sven Eckelmann