[v6,01/11] batman-adv: Handle parent interfaces in a different netns
Commit Message
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>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Acked-by: Antonio Quartulli <a@untable.cc>
---
compat.h | 7 ++++++
net/batman-adv/hard-interface.c | 50 +++++++++++++++++++++++++++++++++++------
2 files changed, 50 insertions(+), 7 deletions(-)
Comments
On Monday 16 May 2016 19:27:59 Andrew Lunn wrote:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)
> +
> +/* WARNING for batadv_getlink_net */
> +#define get_link_net get_xstats_size || 0 || netdev->rtnl_link_ops-
>get_xstats_size
> +
> +#endif /* < KERNEL_VERSION(4, 0, 0) */
This should be "|| 1 ||" to enforce the immediate return in the function
batadv_get_link_net. This was also the version submitted to gather some
comments [1] about the compat-hack.
* v2 had "|| 1 ||": https://patchwork.open-mesh.org/patch/16003/
* v3 had "|| 0 ||": https://patchwork.open-mesh.org/patch/16024/
I think I removed this multiple times but this misalignment seems to be
immortal:
> 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);
Kind regards,
Sven
[1] https://patchwork.open-mesh.org/patch/15996/
> I think I removed this multiple times but this misalignment seems to be
> immortal:
>
> > 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);
According to checkpatch and emacs, it is also correct as it is :-)
If you really want to enforce some other arbitrary white space
alignment policy than checkpatch.pl, it would help if the scripts you
are using are placed into ./tools, just like checkpatch.pl is. Better
still, provide patches to checkpatch.pl. You should also put all your
other tools there. Checkpatch mostly works because it is easily
available, making it easy to push back against when people don't use
it. Developing code for BATMAN is easy, but making it pass all your
arbitrary style checks, header file requirements, structure forward
declaration, negative value kerneldoc function headers, etc is hard
and takes much much longer than developing the code. Putting these
scripts at the figures of the developers,
./scripts/batadv-checkpatch.pl, and you stand a better chance of
compliance.
I'm actually getting close to the point where i'm going to give up.
Andrew
On Tuesday 17 May 2016 14:18:35 Andrew Lunn wrote:
> I'm actually getting close to the point where i'm going to give up.
Ok, I will try to not review anything from you again. Sorry for trying to
inform you about things which would make it harder to get these patches
accepted and sent upstream.
Kind regards,
Sven
On Tue, May 17, 2016 at 02:22:29PM +0200, Sven Eckelmann wrote:
> On Tuesday 17 May 2016 14:18:35 Andrew Lunn wrote:
> > I'm actually getting close to the point where i'm going to give up.
>
> Ok, I will try to not review anything from you again. Sorry for trying to
> inform you about things which would make it harder to get these patches
> accepted and sent upstream.
I'm quite happy to get bug reports, and now i look at, 1 is definitely
correct.
However, most of the rest has nothing about getting DaveM to accept
the patches. Of the 120 patches i've got into mailine via DaveM, i
don't remember him every complaining about indentation, or kerneldoc
headers, or forward declarations.
Andrew
@@ -146,6 +146,13 @@ static int __batadv_interface_kill_vid(struct net_device *dev, __be16 proto,\
#endif /* < KERNEL_VERSION(3, 15, 0) */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 0, 0)
+
+/* WARNING for batadv_getlink_net */
+#define get_link_net get_xstats_size || 0 || netdev->rtnl_link_ops->get_xstats_size
+
+#endif /* < KERNEL_VERSION(4, 0, 0) */
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0)
#define IFF_NO_QUEUE 0; dev->tx_queue_len = 0
@@ -36,6 +36,8 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
+#include <net/net_namespace.h>
+#include <net/rtnetlink.h>
#include "bridge_loop_avoidance.h"
#include "debugfs.h"
@@ -83,25 +85,55 @@ out:
}
/**
+ * batadv_getlink_net - return link net namespace (of use fallback)
+ * @netdev: net_device to check
+ * @fallback_net: return in case get_link_net is not available for @netdev
+ *
+ * Return: result of rtnl_link_ops->get_link_net or @fallback_net
+ */
+static const struct net *batadv_getlink_net(const struct net_device *netdev,
+ const struct net *fallback_net)
+{
+ if (!netdev->rtnl_link_ops)
+ return fallback_net;
+
+ if (!netdev->rtnl_link_ops->get_link_net)
+ return fallback_net;
+
+ return netdev->rtnl_link_ops->get_link_net(netdev);
+}
+
+/**
* 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;
+ const struct net *dev2_parent_net;
+
+ dev1_parent_net = batadv_getlink_net(dev1, net1);
+ dev2_parent_net = batadv_getlink_net(dev2, net2);
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);
}
/**
@@ -119,8 +151,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;
+ const struct net *parent_net;
bool ret;
/* check if this is a batman-adv mesh interface */
@@ -132,13 +165,16 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
dev_get_iflink(net_dev) == net_dev->ifindex)
return false;
+ parent_net = batadv_getlink_net(net_dev, net);
+
/* recurse over the parent device */
- parent_dev = __dev_get_by_index(net, dev_get_iflink(net_dev));
+ parent_dev = __dev_get_by_index((struct net *)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);