Now that the new dev_upper API is used, the private check for enslaving loops is not needed anymore as this is performed by the new API itself.
Reported-by: Marek Lindner lindner_marek@yahoo.de Signed-off-by: Antonio Quartulli ordex@autistici.org ---
v2, v3, v4: - no change to this patch
v4: - added compat code
compat.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ compat.h | 5 ++++- hard-interface.c | 43 ------------------------------------------- 3 files changed, 57 insertions(+), 44 deletions(-)
diff --git a/compat.c b/compat.c index 764878f..da63ea6 100644 --- a/compat.c +++ b/compat.c @@ -24,6 +24,7 @@ #include <linux/in.h> #include <linux/version.h> #include "main.h" +#include "soft-interface.h"
#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 0, 0)
@@ -85,3 +86,55 @@ void batadv_free_rcu_nc_path(struct rcu_head *rcu) #endif
#endif /* < KERNEL_VERSION(3, 0, 0) */ + +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 9, 0) + +/** + * batadv_is_on_batman_iface - check if a device is a batman iface descendant + * @net_dev: the device to check + * + * If the user creates any virtual device on top of a batman-adv interface, it + * is important to prevent this new interface to be used to create a new mesh + * network (this behaviour would lead to a batman-over-batman configuration). + * This function recursively checks all the fathers of the device passed as + * argument looking for a batman-adv soft interface. + * + * Returns true if the device is descendant of a batman-adv mesh interface (or + * if it is a batman-adv interface itself), false otherwise + */ +static bool batadv_is_on_batman_iface(const struct net_device *net_dev) +{ + struct net_device *parent_dev; + bool ret; + + /* check if this is a batman-adv mesh interface */ + if (batadv_softif_is_valid(net_dev)) + return true; + + /* no more parents..stop recursion */ + if (net_dev->iflink == net_dev->ifindex) + return false; + + /* recurse over the parent device */ + parent_dev = dev_get_by_index(&init_net, net_dev->iflink); + /* if we got a NULL parent_dev there is something broken.. */ + if (WARN(!parent_dev, "Cannot find parent device")) + return false; + + ret = batadv_is_on_batman_iface(parent_dev); + + if (parent_dev) + dev_put(parent_dev); + return ret; +} + +int batadv_netdev_set_master(struct net_device *slave, + struct net_device *master) +{ + if (batadv_is_on_batman_iface(slave)) + return -1; + + return netdev_set_master(slave, master); +} + +#endif /* < KERNEL_VERSION(3, 9, 0) */ diff --git a/compat.h b/compat.h index 69c3dc2..79d4217 100644 --- a/compat.h +++ b/compat.h @@ -229,7 +229,10 @@ static int batadv_interface_set_mac_addr(struct net_device *dev, void *p) \ }\ static int __batadv_interface_set_mac_addr(x, y)
-#define netdev_master_upper_dev_link netdev_set_master +int batadv_netdev_set_master(struct net_device *slave, + struct net_device *master); + +#define netdev_master_upper_dev_link batadv_netdev_set_master #define netdev_upper_dev_unlink(slave, master) netdev_set_master(slave, NULL) #define netdev_master_upper_dev_get(dev) \ ({\ diff --git a/hard-interface.c b/hard-interface.c index fd99e42..b96f819 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -60,45 +60,6 @@ out: return hard_iface; }
-/** - * batadv_is_on_batman_iface - check if a device is a batman iface descendant - * @net_dev: the device to check - * - * If the user creates any virtual device on top of a batman-adv interface, it - * is important to prevent this new interface to be used to create a new mesh - * network (this behaviour would lead to a batman-over-batman configuration). - * This function recursively checks all the fathers of the device passed as - * argument looking for a batman-adv soft interface. - * - * Returns true if the device is descendant of a batman-adv mesh interface (or - * if it is a batman-adv interface itself), false otherwise - */ -static bool batadv_is_on_batman_iface(const struct net_device *net_dev) -{ - struct net_device *parent_dev; - bool ret; - - /* check if this is a batman-adv mesh interface */ - if (batadv_softif_is_valid(net_dev)) - return true; - - /* no more parents..stop recursion */ - if (net_dev->iflink == net_dev->ifindex) - return false; - - /* recurse over the parent device */ - parent_dev = dev_get_by_index(&init_net, net_dev->iflink); - /* if we got a NULL parent_dev there is something broken.. */ - if (WARN(!parent_dev, "Cannot find parent device")) - return false; - - ret = batadv_is_on_batman_iface(parent_dev); - - if (parent_dev) - dev_put(parent_dev); - return ret; -} - static int batadv_is_valid_iface(const struct net_device *net_dev) { if (net_dev->flags & IFF_LOOPBACK) @@ -110,10 +71,6 @@ static int batadv_is_valid_iface(const struct net_device *net_dev) if (net_dev->addr_len != ETH_ALEN) return 0;
- /* no batman over batman */ - if (batadv_is_on_batman_iface(net_dev)) - return 0; - return 1; }
Hi,
I've tested this for compatibility on 2.6.32 (Debian stable) in my virtual machine. Marek suggested to use:
batctl if add eth0 vconfig add bat0 5 batctl if add bat0.5
and the last command fails with: Error - can't write to file '/sys/class/net/bat0.5/batman_adv/mesh_iface': Operation not permitted
which looks good and expected to me.
Without the patch, the directory /sys/class/net/bat0.5/batman_adv does not even exist - probably because the check was removed in this patch.
Another thing I have tried, which behaves bad with and without this patch: batctl if add eth0 brctl addbr br0 brctl addif br0 bat0 batctl if add br0 (wait a few seconds) rmmod batman-adv
First, enslaving br0 into bat0 does not fail, but it makes no sense to enslave bat0 through br0 into bat0 again - we should probably prevent this too. Removing the module crashes the kernel, not always, but pretty reliably (I have the feeling it happens more often after applying this patch, but not sure). Unfortunately I couldn't get any useful backtraces so far, if it fails it usually instantly reboots or hangs without output (this is in kvm with -curses). I've succeeded to get some trace part, it's probably not useful but i've attached it to the bottom anyway [1].
Any thoughts on that? :)
Cheers, Simon
[1] [ 130.688334] [<c1270aaf>] ? do_page_fault+0x0/0x307 [ 130.688334] [<c101bb73>] ? bad_area_nosemaphore+0xa/0xc [ 130.688334] [<c126f303>] ? error_code+0x73/0x78 [ 130.688334] [<c8c60f89>] ? br_nf_pre_routing+0x28/0x4a8 [bridge] [ 130.688334] [<c11d007b>] ? sock_rmalloc+0x1e/0x68 [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c60f89>] ? br_nf_pre_routing+0x28/0x4a8 [bridge] [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c5d5b2>] ? br_handle_frame_finish+0xd2/0xff [bridge] [ 130.688334] [<c8c5d760>] ? br_handle_frame+0x181/0x195 [bridge] [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c5d5b2>] ? br_handle_frame_finish+0xd2/0xff [bridge] [ 130.688334] [<c8c5d760>] ? br_handle_frame+0x181/0x195 [bridge] [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c5d5b2>] ? br_handle_frame_finish+0xd2/0xff [bridge] [ 130.688334] [<c8c5d760>] ? br_handle_frame+0x181/0x195 [bridge] [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c5d5b2>] ? br_handle_frame_finish+0xd2/0xff [bridge] [ 130.688334] [<c8c5d760>] ? br_handle_frame+0x181/0x195 [bridge] [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c5d5b2>] ? br_handle_frame_finish+0xd2/0xff [bridge] [ 130.688334] [<c8c5d760>] ? br_handle_frame+0x181/0x195 [bridge] [ 130.688334] [<c11d9bee>] ? netif_receive_skb+0x2dd/0x3d6 [ 130.688334] [<c8c5d5b2>] ? br_handle_frame_finish+0xd2/0xff [bridge]
On Fri, Mar 01, 2013 at 10:44:47PM +0100, Antonio Quartulli wrote:
Now that the new dev_upper API is used, the private check for enslaving loops is not needed anymore as this is performed by the new API itself.
Reported-by: Marek Lindner lindner_marek@yahoo.de Signed-off-by: Antonio Quartulli ordex@autistici.org
v2, v3, v4:
- no change to this patch
v4:
- added compat code
compat.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ compat.h | 5 ++++- hard-interface.c | 43 ------------------------------------------- 3 files changed, 57 insertions(+), 44 deletions(-)
diff --git a/compat.c b/compat.c index 764878f..da63ea6 100644 --- a/compat.c +++ b/compat.c @@ -24,6 +24,7 @@ #include <linux/in.h> #include <linux/version.h> #include "main.h" +#include "soft-interface.h"
#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 0, 0)
@@ -85,3 +86,55 @@ void batadv_free_rcu_nc_path(struct rcu_head *rcu) #endif
#endif /* < KERNEL_VERSION(3, 0, 0) */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 9, 0)
+/**
- batadv_is_on_batman_iface - check if a device is a batman iface descendant
- @net_dev: the device to check
- If the user creates any virtual device on top of a batman-adv interface, it
- is important to prevent this new interface to be used to create a new mesh
- network (this behaviour would lead to a batman-over-batman configuration).
- This function recursively checks all the fathers of the device passed as
- argument looking for a batman-adv soft interface.
- Returns true if the device is descendant of a batman-adv mesh interface (or
- if it is a batman-adv interface itself), false otherwise
- */
+static bool batadv_is_on_batman_iface(const struct net_device *net_dev) +{
- struct net_device *parent_dev;
- bool ret;
- /* check if this is a batman-adv mesh interface */
- if (batadv_softif_is_valid(net_dev))
return true;- /* no more parents..stop recursion */
- if (net_dev->iflink == net_dev->ifindex)
return false;- /* recurse over the parent device */
- parent_dev = dev_get_by_index(&init_net, net_dev->iflink);
- /* if we got a NULL parent_dev there is something broken.. */
- if (WARN(!parent_dev, "Cannot find parent device"))
return false;- ret = batadv_is_on_batman_iface(parent_dev);
- if (parent_dev)
dev_put(parent_dev);- return ret;
+}
+int batadv_netdev_set_master(struct net_device *slave,
struct net_device *master)+{
- if (batadv_is_on_batman_iface(slave))
return -1;- return netdev_set_master(slave, master);
+}
+#endif /* < KERNEL_VERSION(3, 9, 0) */ diff --git a/compat.h b/compat.h index 69c3dc2..79d4217 100644 --- a/compat.h +++ b/compat.h @@ -229,7 +229,10 @@ static int batadv_interface_set_mac_addr(struct net_device *dev, void *p) \ }\ static int __batadv_interface_set_mac_addr(x, y)
-#define netdev_master_upper_dev_link netdev_set_master +int batadv_netdev_set_master(struct net_device *slave,
struct net_device *master);+#define netdev_master_upper_dev_link batadv_netdev_set_master #define netdev_upper_dev_unlink(slave, master) netdev_set_master(slave, NULL) #define netdev_master_upper_dev_get(dev) \ ({\ diff --git a/hard-interface.c b/hard-interface.c index fd99e42..b96f819 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -60,45 +60,6 @@ out: return hard_iface; }
-/**
- batadv_is_on_batman_iface - check if a device is a batman iface descendant
- @net_dev: the device to check
- If the user creates any virtual device on top of a batman-adv interface, it
- is important to prevent this new interface to be used to create a new mesh
- network (this behaviour would lead to a batman-over-batman configuration).
- This function recursively checks all the fathers of the device passed as
- argument looking for a batman-adv soft interface.
- Returns true if the device is descendant of a batman-adv mesh interface (or
- if it is a batman-adv interface itself), false otherwise
- */
-static bool batadv_is_on_batman_iface(const struct net_device *net_dev) -{
- struct net_device *parent_dev;
- bool ret;
- /* check if this is a batman-adv mesh interface */
- if (batadv_softif_is_valid(net_dev))
return true;- /* no more parents..stop recursion */
- if (net_dev->iflink == net_dev->ifindex)
return false;- /* recurse over the parent device */
- parent_dev = dev_get_by_index(&init_net, net_dev->iflink);
- /* if we got a NULL parent_dev there is something broken.. */
- if (WARN(!parent_dev, "Cannot find parent device"))
return false;- ret = batadv_is_on_batman_iface(parent_dev);
- if (parent_dev)
dev_put(parent_dev);- return ret;
-}
static int batadv_is_valid_iface(const struct net_device *net_dev) { if (net_dev->flags & IFF_LOOPBACK) @@ -110,10 +71,6 @@ static int batadv_is_valid_iface(const struct net_device *net_dev) if (net_dev->addr_len != ETH_ALEN) return 0;
- /* no batman over batman */
- if (batadv_is_on_batman_iface(net_dev))
return 0;- return 1;
}
-- 1.8.1.4
On Sat, Mar 02, 2013 at 03:01:14PM +0100, Simon Wunderlich wrote:
Hi,
I've tested this for compatibility on 2.6.32 (Debian stable) in my virtual machine.
Hello Simon and thank yuou,
Marek suggested to use:
batctl if add eth0 vconfig add bat0 5 batctl if add bat0.5
and the last command fails with: Error - can't write to file '/sys/class/net/bat0.5/batman_adv/mesh_iface': Operation not permitted
which looks good and expected to me.
yeah.
Without the patch, the directory /sys/class/net/bat0.5/batman_adv does not even exist - probably because the check was removed in this patch.
Well, with this patch the check is done later and therefore the folder is created. We may want to try to move the check earlier, like it was before the patch?
Another thing I have tried, which behaves bad with and without this patch: batctl if add eth0 brctl addbr br0 brctl addif br0 bat0 batctl if add br0 (wait a few seconds) rmmod batman-adv
First, enslaving br0 into bat0 does not fail, but it makes no sense to enslave bat0 through br0 into bat0 again - we should probably prevent this too. Removing the module crashes the kernel, not always, but pretty reliably (I have the feeling it happens more often after applying this patch, but not sure). Unfortunately I couldn't get any useful backtraces so far, if it fails it usually instantly reboots or hangs without output (this is in kvm with -curses). I've succeeded to get some trace part, it's probably not useful but i've attached it to the bottom anyway [1].
Any thoughts on that? :)
How sure are we about being batman-adv the culprit for this?
Cheers,
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Simon Wunderlich