* [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves
2016-03-01 21:19 [B.A.T.M.A.N.] [PATCHv2 0/4] batman-adv: Add network name space support Andrew Lunn
@ 2016-03-01 21:19 ` Andrew Lunn
2016-03-13 9:29 ` Sven Eckelmann
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: Create batman soft interfaces within correct netns Andrew Lunn
` (2 subsequent siblings)
3 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2016-03-01 21:19 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: B.A.T.M.A.N
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
--
2.7.0.rc3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves Andrew Lunn
@ 2016-03-13 9:29 ` Sven Eckelmann
2016-03-13 15:24 ` Andrew Lunn
0 siblings, 1 reply; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-13 9:29 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 766 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves
2016-03-13 9:29 ` Sven Eckelmann
@ 2016-03-13 15:24 ` Andrew Lunn
2016-04-21 10:48 ` Marek Lindner
0 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2016-03-13 15:24 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n, Antonio Quartulli
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
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves
2016-03-13 15:24 ` Andrew Lunn
@ 2016-04-21 10:48 ` Marek Lindner
0 siblings, 0 replies; 30+ messages in thread
From: Marek Lindner @ 2016-04-21 10:48 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 949 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: Create batman soft interfaces within correct netns.
2016-03-01 21:19 [B.A.T.M.A.N.] [PATCHv2 0/4] batman-adv: Add network name space support Andrew Lunn
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves Andrew Lunn
@ 2016-03-01 21:19 ` Andrew Lunn
2016-03-13 10:48 ` Sven Eckelmann
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns Andrew Lunn
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support Andrew Lunn
3 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2016-03-01 21:19 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: B.A.T.M.A.N
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);
--
2.7.0.rc3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: Create batman soft interfaces within correct netns.
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: Create batman soft interfaces within correct netns Andrew Lunn
@ 2016-03-13 10:48 ` Sven Eckelmann
2016-03-13 15:35 ` Andrew Lunn
0 siblings, 1 reply; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-13 10:48 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 2356 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: Create batman soft interfaces within correct netns.
2016-03-13 10:48 ` Sven Eckelmann
@ 2016-03-13 15:35 ` Andrew Lunn
0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2016-03-13 15:35 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n, Antonio Quartulli
> 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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-01 21:19 [B.A.T.M.A.N.] [PATCHv2 0/4] batman-adv: Add network name space support Andrew Lunn
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: NETIF_F_NETNS_LOCAL feature to prevent netns moves Andrew Lunn
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 2/4] batman-adv: Create batman soft interfaces within correct netns Andrew Lunn
@ 2016-03-01 21:19 ` Andrew Lunn
2016-03-04 12:35 ` Sven Eckelmann
` (2 more replies)
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support Andrew Lunn
3 siblings, 3 replies; 30+ messages in thread
From: Andrew Lunn @ 2016-03-01 21:19 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: B.A.T.M.A.N
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);
--
2.7.0.rc3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns Andrew Lunn
@ 2016-03-04 12:35 ` Sven Eckelmann
2016-03-07 14:31 ` Andrew Lunn
2016-03-07 4:21 ` Linus Lüssing
2016-03-13 11:02 ` Sven Eckelmann
2 siblings, 1 reply; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-04 12:35 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli, B.A.T.M.A.N
[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-04 12:35 ` Sven Eckelmann
@ 2016-03-07 14:31 ` Andrew Lunn
2016-03-07 14:45 ` Sven Eckelmann
2016-03-11 23:37 ` Sven Eckelmann
0 siblings, 2 replies; 30+ messages in thread
From: Andrew Lunn @ 2016-03-07 14:31 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: B.A.T.M.A.N, b.a.t.m.a.n, Antonio Quartulli
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
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-07 14:31 ` Andrew Lunn
@ 2016-03-07 14:45 ` Sven Eckelmann
2016-03-11 23:37 ` Sven Eckelmann
1 sibling, 0 replies; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-07 14:45 UTC (permalink / raw)
To: Andrew Lunn; +Cc: B.A.T.M.A.N, b.a.t.m.a.n, Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]
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/a246d227882b7bc03ae602b41cf65053397f9cc8:/checkstuff.sh#l311
> >
> > 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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-07 14:31 ` Andrew Lunn
2016-03-07 14:45 ` Sven Eckelmann
@ 2016-03-11 23:37 ` Sven Eckelmann
2016-03-12 4:42 ` Antonio Quartulli
1 sibling, 1 reply; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-11 23:37 UTC (permalink / raw)
To: Andrew Lunn; +Cc: b.a.t.m.a.n, Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 802 bytes --]
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/fb7b5eb8c81969dc8bea356d0a3f0b5e72d2da49
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-11 23:37 ` Sven Eckelmann
@ 2016-03-12 4:42 ` Antonio Quartulli
2016-03-12 7:34 ` Sven Eckelmann
0 siblings, 1 reply; 30+ messages in thread
From: Antonio Quartulli @ 2016-03-12 4:42 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 242 bytes --]
On Sat, Mar 12, 2016 at 12:37:17AM +0100, Sven Eckelmann wrote:
> [1] https://git.open-mesh.org/batman-adv.git/commit/fb7b5eb8c81969dc8bea356d0a3f0b5e72d2da49
Sven, what is the README there for ?
Cheers,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-12 4:42 ` Antonio Quartulli
@ 2016-03-12 7:34 ` Sven Eckelmann
2016-03-12 7:40 ` Sven Eckelmann
0 siblings, 1 reply; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-12 7:34 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-12 7:34 ` Sven Eckelmann
@ 2016-03-12 7:40 ` Sven Eckelmann
0 siblings, 0 replies; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-12 7:40 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns Andrew Lunn
2016-03-04 12:35 ` Sven Eckelmann
@ 2016-03-07 4:21 ` Linus Lüssing
2016-03-07 13:47 ` Andrew Lunn
2016-03-13 11:02 ` Sven Eckelmann
2 siblings, 1 reply; 30+ messages in thread
From: Linus Lüssing @ 2016-03-07 4:21 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-07 4:21 ` Linus Lüssing
@ 2016-03-07 13:47 ` Andrew Lunn
0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2016-03-07 13:47 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns Andrew Lunn
2016-03-04 12:35 ` Sven Eckelmann
2016-03-07 4:21 ` Linus Lüssing
@ 2016-03-13 11:02 ` Sven Eckelmann
2016-03-13 15:38 ` Andrew Lunn
2 siblings, 1 reply; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-13 11:02 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]
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/aef6842ec0facda9ec52736f8a6ce8ca4110e91a:/net/batman-adv/hard-interface.c#l87
[2] https://git.open-mesh.org/batman-adv.git/blob/fb7b5eb8c81969dc8bea356d0a3f0b5e72d2da49:/compat-patches/0001-get_link_net.patch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns
2016-03-13 11:02 ` Sven Eckelmann
@ 2016-03-13 15:38 ` Andrew Lunn
2016-03-13 15:59 ` Sven Eckelmann
0 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2016-03-13 15:38 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n, Antonio Quartulli
> > + 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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-03-01 21:19 [B.A.T.M.A.N.] [PATCHv2 0/4] batman-adv: Add network name space support Andrew Lunn
` (2 preceding siblings ...)
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 3/4] batman-adv: Handle parent interfaces in a different netns Andrew Lunn
@ 2016-03-01 21:19 ` Andrew Lunn
2016-03-07 14:21 ` Matthias Schiffer
3 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2016-03-01 21:19 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: B.A.T.M.A.N
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);
}
--
2.7.0.rc3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-03-01 21:19 ` [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support Andrew Lunn
@ 2016-03-07 14:21 ` Matthias Schiffer
2016-03-13 9:12 ` Sven Eckelmann
0 siblings, 1 reply; 30+ messages in thread
From: Matthias Schiffer @ 2016-03-07 14:21 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1.1: Type: text/plain, Size: 8145 bytes --]
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);
> }
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-03-07 14:21 ` Matthias Schiffer
@ 2016-03-13 9:12 ` Sven Eckelmann
2016-03-13 15:42 ` Andrew Lunn
2016-03-14 10:41 ` Matthias Schiffer
0 siblings, 2 replies; 30+ messages in thread
From: Sven Eckelmann @ 2016-03-13 9:12 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 674 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-03-13 9:12 ` Sven Eckelmann
@ 2016-03-13 15:42 ` Andrew Lunn
2016-04-18 14:41 ` Simon Wunderlich
2016-03-14 10:41 ` Matthias Schiffer
1 sibling, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2016-03-13 15:42 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-03-13 15:42 ` Andrew Lunn
@ 2016-04-18 14:41 ` Simon Wunderlich
2016-04-20 2:36 ` Andrew Lunn
0 siblings, 1 reply; 30+ messages in thread
From: Simon Wunderlich @ 2016-04-18 14:41 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-04-18 14:41 ` Simon Wunderlich
@ 2016-04-20 2:36 ` Andrew Lunn
2016-04-20 3:23 ` Antonio Quartulli
2016-04-20 7:02 ` Sven Eckelmann
0 siblings, 2 replies; 30+ messages in thread
From: Andrew Lunn @ 2016-04-20 2:36 UTC (permalink / raw)
To: Simon Wunderlich; +Cc: b.a.t.m.a.n
> 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
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-04-20 2:36 ` Andrew Lunn
@ 2016-04-20 3:23 ` Antonio Quartulli
2016-04-20 7:02 ` Sven Eckelmann
1 sibling, 0 replies; 30+ messages in thread
From: Antonio Quartulli @ 2016-04-20 3:23 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking,
Andrew Lunn
[-- Attachment #1: Type: text/plain, Size: 436 bytes --]
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,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-04-20 2:36 ` Andrew Lunn
2016-04-20 3:23 ` Antonio Quartulli
@ 2016-04-20 7:02 ` Sven Eckelmann
1 sibling, 0 replies; 30+ messages in thread
From: Sven Eckelmann @ 2016-04-20 7:02 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 647 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2 4/4] batman-adv: debugfs: Add netns support
2016-03-13 9:12 ` Sven Eckelmann
2016-03-13 15:42 ` Andrew Lunn
@ 2016-03-14 10:41 ` Matthias Schiffer
1 sibling, 0 replies; 30+ messages in thread
From: Matthias Schiffer @ 2016-03-14 10:41 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: b.a.t.m.a.n
[-- Attachment #1.1: Type: text/plain, Size: 1485 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread