[PATCH] macvlan: Support creating macvtaps from macvlans

[PATCH] macvlan: Support creating macvtaps from macvlans

[Kevin Wallace]

When running in a network namespace whose only link to the outside
world is a macvlan device, not being able to create a macvtap off of
it is a real pain.

So modify macvtap creation to automatically forward a creation of a
macvtap on a macvlan to become a creation of a macvtap on the
underlying network device, just like is currently done with
macvlan-on-macvlan devices.

Signed-off-by: Kevin Wallace <kevin@pentabarf.net>
---
 drivers/net/macvlan.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 16b43bf..3fa86cb 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -757,6 +757,8 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
 	return 0;
 }
 
+static struct rtnl_link_ops macvlan_link_ops;
+
 int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 			   struct nlattr *tb[], struct nlattr *data[],
 			   int (*receive)(struct sk_buff *skb),
@@ -775,10 +777,10 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	if (lowerdev == NULL)
 		return -ENODEV;
 
-	/* When creating macvlans on top of other macvlans - use
+	/* When creating macvlans or macvtaps on top of other macvlans - use
 	 * the real device as the lowerdev.
 	 */
-	if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
+	if (lowerdev->rtnl_link_ops == &macvlan_link_ops) {
 		struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
 		lowerdev = lowervlan->lowerdev;
 	}
-- 
1.8.3.2

Re: [PATCH] macvlan: Support creating macvtaps from macvlans

[Michal Kubecek]

On Mon, Dec 02, 2013 at 08:13:11PM -0800, Kevin Wallace wrote:
> -	/* When creating macvlans on top of other macvlans - use
> +	/* When creating macvlans or macvtaps on top of other macvlans - use
>  	 * the real device as the lowerdev.
>  	 */
> -	if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
> +	if (lowerdev->rtnl_link_ops == &macvlan_link_ops) {
>  		struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
>  		lowerdev = lowervlan->lowerdev;
>  	}

Perhaps we could use the helpers:

	if (netif_is_macvlan(lowerdev))
		lowerdev = macvlan_dev_real_dev(lowerdev);

to show more clearly what we are doing.

                                                        Michal Kubecek

Re: [PATCH] macvlan: Support creating macvtaps from macvlans

[Kevin Wallace]

Good call; I didn't see those.  I'll grab a more recent snapshot that
includes those helpers, and send out a v2 once I've tested.

Thanks!

Kevin

On Tue, Dec 3, 2013 at 12:24 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
> On Mon, Dec 02, 2013 at 08:13:11PM -0800, Kevin Wallace wrote:
>> -     /* When creating macvlans on top of other macvlans - use
>> +     /* When creating macvlans or macvtaps on top of other macvlans - use
>>        * the real device as the lowerdev.
>>        */
>> -     if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
>> +     if (lowerdev->rtnl_link_ops == &macvlan_link_ops) {
>>               struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
>>               lowerdev = lowervlan->lowerdev;
>>       }
>
> Perhaps we could use the helpers:
>
>         if (netif_is_macvlan(lowerdev))
>                 lowerdev = macvlan_dev_real_dev(lowerdev);
>
> to show more clearly what we are doing.
>
>                                                         Michal Kubecek
>

[PATCH v2] macvlan: Support creating macvtaps from macvlans

[Kevin Wallace]

When running in a network namespace whose only link to the outside
world is a macvlan device, not being able to create a macvtap off of
it is a real pain.

So modify macvtap creation to automatically forward a creation of a
macvtap on a macvlan to become a creation of a macvtap on the
underlying network device, just like is currently done with
macvlan-on-macvlan devices.

v2: Use netif_is_macvlan and macvlan_dev_real_dev helpers to make it
    more clear what we're doing.

Signed-off-by: Kevin Wallace <kevin@pentabarf.net>
---
 drivers/net/macvlan.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index acf9379..cfb9157 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -820,13 +820,11 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	if (lowerdev == NULL)
 		return -ENODEV;
 
-	/* When creating macvlans on top of other macvlans - use
+	/* When creating macvlans or macvtaps on top of other macvlans - use
 	 * the real device as the lowerdev.
 	 */
-	if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
-		struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
-		lowerdev = lowervlan->lowerdev;
-	}
+	if (netif_is_macvlan(lowerdev))
+		lowerdev = macvlan_dev_real_dev(lowerdev);
 
 	if (!tb[IFLA_MTU])
 		dev->mtu = lowerdev->mtu;
-- 
1.8.3.2

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[Vlad Yasevich]

On 12/03/2013 05:55 AM, Kevin Wallace wrote:
> When running in a network namespace whose only link to the outside
> world is a macvlan device, not being able to create a macvtap off of
> it is a real pain.
> 
> So modify macvtap creation to automatically forward a creation of a
> macvtap on a macvlan to become a creation of a macvtap on the
> underlying network device, just like is currently done with
> macvlan-on-macvlan devices.
> 
> v2: Use netif_is_macvlan and macvlan_dev_real_dev helpers to make it
>     more clear what we're doing.
> 
> Signed-off-by: Kevin Wallace <kevin@pentabarf.net>
> ---
>  drivers/net/macvlan.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index acf9379..cfb9157 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -820,13 +820,11 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>  	if (lowerdev == NULL)
>  		return -ENODEV;
>  
> -	/* When creating macvlans on top of other macvlans - use
> +	/* When creating macvlans or macvtaps on top of other macvlans - use
>  	 * the real device as the lowerdev.
>  	 */
> -	if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
> -		struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
> -		lowerdev = lowervlan->lowerdev;
> -	}
> +	if (netif_is_macvlan(lowerdev))
> +		lowerdev = macvlan_dev_real_dev(lowerdev);
>  
>  	if (!tb[IFLA_MTU])
>  		dev->mtu = lowerdev->mtu;
> 

the other question is should this be done in a loop?  What happens if
you have nested namespaces?

-vlad

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[Kevin Wallace]

On Tue, Dec 3, 2013 at 8:17 AM, Vlad Yasevich <vyasevich@gmail.com> wrote:
> the other question is should this be done in a loop?  What happens if
> you have nested namespaces?

I don't believe that will change anything -- this code makes the
situation necessary for multiple iterations (macvlan with a macvlan
lower) impossible.

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[Michal Kubecek]

On Tue, Dec 03, 2013 at 11:17:37AM -0500, Vlad Yasevich wrote:
> > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > index acf9379..cfb9157 100644
> > --- a/drivers/net/macvlan.c
> > +++ b/drivers/net/macvlan.c
> > @@ -820,13 +820,11 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> >  	if (lowerdev == NULL)
> >  		return -ENODEV;
> >  
> > -	/* When creating macvlans on top of other macvlans - use
> > +	/* When creating macvlans or macvtaps on top of other macvlans - use
> >  	 * the real device as the lowerdev.
> >  	 */
> > -	if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
> > -		struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
> > -		lowerdev = lowervlan->lowerdev;
> > -	}
> > +	if (netif_is_macvlan(lowerdev))
> > +		lowerdev = macvlan_dev_real_dev(lowerdev);
> >  
> >  	if (!tb[IFLA_MTU])
> >  		dev->mtu = lowerdev->mtu;
> > 
> 
> the other question is should this be done in a loop?  What happens if
> you have nested namespaces?

Nested namespaces are not a problem, what would be a problem, would be
having a macvlan (macvtap) device on top of another macvlan. But the
purpose of this particular code is to prevent it and use the underlying
"real" device instead. That's why unlike vlan_dev_real_dev(),
macvlan_dev_real_dev() doesn't need to recurse.

                                                         Michal Kubecek

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[Vlad Yasevich]

On 12/03/2013 02:47 PM, Michal Kubecek wrote:
> On Tue, Dec 03, 2013 at 11:17:37AM -0500, Vlad Yasevich wrote:
>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>> index acf9379..cfb9157 100644
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -820,13 +820,11 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>>>  	if (lowerdev == NULL)
>>>  		return -ENODEV;
>>>  
>>> -	/* When creating macvlans on top of other macvlans - use
>>> +	/* When creating macvlans or macvtaps on top of other macvlans - use
>>>  	 * the real device as the lowerdev.
>>>  	 */
>>> -	if (lowerdev->rtnl_link_ops == dev->rtnl_link_ops) {
>>> -		struct macvlan_dev *lowervlan = netdev_priv(lowerdev);
>>> -		lowerdev = lowervlan->lowerdev;
>>> -	}
>>> +	if (netif_is_macvlan(lowerdev))
>>> +		lowerdev = macvlan_dev_real_dev(lowerdev);
>>>  
>>>  	if (!tb[IFLA_MTU])
>>>  		dev->mtu = lowerdev->mtu;
>>>
>>
>> the other question is should this be done in a loop?  What happens if
>> you have nested namespaces?
> 
> Nested namespaces are not a problem, what would be a problem, would be
> having a macvlan (macvtap) device on top of another macvlan. But the
> purpose of this particular code is to prevent it and use the underlying
> "real" device instead. That's why unlike vlan_dev_real_dev(),
> macvlan_dev_real_dev() doesn't need to recurse.
> 
>                                                          Michal Kubecek
> 

Wait,  so you have a namespace that uses macvlan to access the net.
That macvlan is configured on top of another macvlan, so you need to
get to the lower level device.  I understand that.  What I am asking
is that what happens if you have a namespace within a namespace with
the same network access restrictions.  The code as is, will think that
the first level macvlan is the real device.  Is this setup practical...

The reason I ask is that there is nothing preventing it, and it would
break just the same as your setup did.

-vlad

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[Michal Kubecek]

On Wed, Dec 04, 2013 at 08:59:21AM -0500, Vlad Yasevich wrote:
> On 12/03/2013 02:47 PM, Michal Kubecek wrote:
> > On Tue, Dec 03, 2013 at 11:17:37AM -0500, Vlad Yasevich wrote:
> >>
> >> the other question is should this be done in a loop?  What happens if
> >> you have nested namespaces?
> > 
> > Nested namespaces are not a problem, what would be a problem, would be
> > having a macvlan (macvtap) device on top of another macvlan. But the
> > purpose of this particular code is to prevent it and use the underlying
> > "real" device instead. That's why unlike vlan_dev_real_dev(),
> > macvlan_dev_real_dev() doesn't need to recurse.
> 
> Wait,  so you have a namespace that uses macvlan to access the net.
> That macvlan is configured on top of another macvlan, so you need to
> get to the lower level device.  I understand that.  What I am asking
> is that what happens if you have a namespace within a namespace with
> the same network access restrictions.  The code as is, will think that
> the first level macvlan is the real device.  Is this setup practical...

My understanding is this:

We have eth0, a real ethernet device in init_net. We create a macvlan
device mv1 on top of it and put it into a namespace, say ns1. Inside
ns1, we can't see eth0 so that we cannot do

  ip link add mv2 link eth0 type macvlan mode bridge

The purpose of this code is to allow

  ip link add mv2 link mv1 type macvlan mode bridge

but to use eth0 as lowerdev of mv2 instead. If we then put mv2 into
another namespace ns2, there is still no problem because its lowerdev is
also eth0 (even if we cannot see it inside ns2) so that

  ip link add mv3 link mv2 type macvlan mode bridge

in fact creates mv3 with lowerdev eth0 again.

However, what I'm not sure about is whether there is something to
prevent building e.g. a macvlan on top of a (802.1q) VLAN on top of
a macvlan.

                                                     Michal Kubecek

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[Vlad Yasevich]

On 12/04/2013 09:23 AM, Michal Kubecek wrote:
> On Wed, Dec 04, 2013 at 08:59:21AM -0500, Vlad Yasevich wrote:
>> On 12/03/2013 02:47 PM, Michal Kubecek wrote:
>>> On Tue, Dec 03, 2013 at 11:17:37AM -0500, Vlad Yasevich wrote:
>>>>
>>>> the other question is should this be done in a loop?  What happens if
>>>> you have nested namespaces?
>>>
>>> Nested namespaces are not a problem, what would be a problem, would be
>>> having a macvlan (macvtap) device on top of another macvlan. But the
>>> purpose of this particular code is to prevent it and use the underlying
>>> "real" device instead. That's why unlike vlan_dev_real_dev(),
>>> macvlan_dev_real_dev() doesn't need to recurse.
>>
>> Wait,  so you have a namespace that uses macvlan to access the net.
>> That macvlan is configured on top of another macvlan, so you need to
>> get to the lower level device.  I understand that.  What I am asking
>> is that what happens if you have a namespace within a namespace with
>> the same network access restrictions.  The code as is, will think that
>> the first level macvlan is the real device.  Is this setup practical...
> 
> My understanding is this:
> 
> We have eth0, a real ethernet device in init_net. We create a macvlan
> device mv1 on top of it and put it into a namespace, say ns1. Inside
> ns1, we can't see eth0 so that we cannot do
> 
>   ip link add mv2 link eth0 type macvlan mode bridge
> 
> The purpose of this code is to allow
> 
>   ip link add mv2 link mv1 type macvlan mode bridge
> 
> but to use eth0 as lowerdev of mv2 instead. If we then put mv2 into
> another namespace ns2, there is still no problem because its lowerdev is
> also eth0 (even if we cannot see it inside ns2) so that
> 
>   ip link add mv3 link mv2 type macvlan mode bridge
> 
> in fact creates mv3 with lowerdev eth0 again.

I see now, this makes total sense...

> 
> However, what I'm not sure about is whether there is something to
> prevent building e.g. a macvlan on top of a (802.1q) VLAN on top of
> a macvlan.

That looks like it will create a new macvlan port on top of the VLAN
interface and use the VLAN as the lower_dev.  I think it'll still work.

-vlad
> 
>                                                      Michal Kubecek
> 

Re: [PATCH v2] macvlan: Support creating macvtaps from macvlans

[David Miller]

From: Kevin Wallace <kevin@pentabarf.net>
Date: Tue,  3 Dec 2013 02:55:22 -0800

> When running in a network namespace whose only link to the outside
> world is a macvlan device, not being able to create a macvtap off of
> it is a real pain.
> 
> So modify macvtap creation to automatically forward a creation of a
> macvtap on a macvlan to become a creation of a macvtap on the
> underlying network device, just like is currently done with
> macvlan-on-macvlan devices.
> 
> v2: Use netif_is_macvlan and macvlan_dev_real_dev helpers to make it
>     more clear what we're doing.
> 
> Signed-off-by: Kevin Wallace <kevin@pentabarf.net>

Applied to net-next, thanks.