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(-)
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
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,
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
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,
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,
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
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
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);
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;
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
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,
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
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
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;
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
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
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
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!
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,
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
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
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); }
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;
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
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,
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
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 ?
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
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
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
b.a.t.m.a.n@lists.open-mesh.org
-
Andrew Lunn -
Antonio Quartulli -
Sven Eckelmann