diff mbox

[RFC] batman-adv: Remove recursive bat-on-bat netdevice check

Message ID 20160119144003.GH6554@lunn.ch (mailing list archive)
State RFC, archived
Headers

Commit Message

Andrew Lunn Jan. 19, 2016, 2:40 p.m. UTC 
  > > One of the issues i had is in this piece of code and with veth. The
> > other end of the veth can be in a different name space. Hence you
> > cannot look it up using the ifindex in the current name space. So i
> > made the code just exit with an O.K. if the parent device cannot be
> > found.
> 
> Is this really ok ? Isn't it possible to create "loops" by jumping from
> one namespace to the other ?

Quite possible. I took the easy option:


I didn't spend the time to think all the possible loop situations
though. It is not too bad in that the software interface cannot be
moved into a different netns. Also, all the hard interfaces have to be
in the same netns as the soft interface. If you move a hard interface
into a different netns, it will automatically be removed from the soft
interface using the notification mechanism.

There are some other restrictions which help avoid loops. You cannot
in general move 'soft interfaces' between netns. You cannot move
bridge interfaces, tunnel interfaces, team/bonding interfaces, etc.

> exactly my point above! :)
> So this means that an interface A might have A.iflink pointing to a
> device in another namespace ?

Partly correct. The current code for getting the parent is:

parent_dev = __dev_get_by_index(net, dev_get_iflink(net_dev));

This is actually totally broken, when used with veth. It possible can
return a completely different interface than the parent, since the
ifindex returned for a veth could be in a different namespace. ifindex
is only unique within a namespace, not across name spaces.

I need to look deeper into the code and see if there is a different
parenting mechanism that can be used.

	  Andrew
diff mbox

Patch

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index a5ce58a..a7a92ae 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -104,8 +104,12 @@  static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 
        /* recurse over the parent device */
        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"))
+       /* if we got a NULL parent_dev, it might mean the parent is in
+        * a different network namespace. Things then get really
+        * complex, so lets just assume the user knows what she is
+        * doing.
+        */
+       if (!parent_dev)
                return false;
 
        ret = batadv_is_on_batman_iface(parent_dev);