diff mbox

[net] macvlan: add NETIF_F_NETNS_LOCAL flag

Message ID 1392082593-7742-1-git-send-email-xiyou.wangcong@gmail.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Cong Wang Feb. 11, 2014, 1:36 a.m. UTC
From: Cong Wang <cwang@twopensource.com>

BZ: https://bugzilla.kernel.org/show_bug.cgi?id=66691

There is no point to allow moving a macvlan device to
another namespace while the lower device is still in
this namespace. tunnels already set this flag.

Cc: Patrick McHardy <kaber@trash.net>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <cwang@twopensource.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

---
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Hannes Frederic Sowa Feb. 11, 2014, 1:45 a.m. UTC | #1
On Mon, Feb 10, 2014 at 05:36:33PM -0800, Cong Wang wrote:
> From: Cong Wang <cwang@twopensource.com>
> 
> BZ: https://bugzilla.kernel.org/show_bug.cgi?id=66691
> 
> There is no point to allow moving a macvlan device to
> another namespace while the lower device is still in
> this namespace. tunnels already set this flag.

Can't we solve this somehow differently, like not showing anything at all
etc.? I guess this is a feature some people use and haven't noticed yet.

Greetings,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cong Wang Feb. 11, 2014, 2:25 a.m. UTC | #2
On Mon, Feb 10, 2014 at 5:45 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Mon, Feb 10, 2014 at 05:36:33PM -0800, Cong Wang wrote:
>> From: Cong Wang <cwang@twopensource.com>
>>
>> BZ: https://bugzilla.kernel.org/show_bug.cgi?id=66691
>>
>> There is no point to allow moving a macvlan device to
>> another namespace while the lower device is still in
>> this namespace. tunnels already set this flag.
>
> Can't we solve this somehow differently, like not showing anything at all
> etc.? I guess this is a feature some people use and haven't noticed yet.
>

I don't understand what you mean by "not showing anything at all".
I assume you mean mac1@xxx, not matter whether we show xxx
here, mac1 relies on xxx to function.

Please give a real use case rather than just guessing, I don't think
there is any valid case until we support moving multiple devices into
a netns atomically.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Frederic Sowa Feb. 11, 2014, 2:40 a.m. UTC | #3
On Mon, Feb 10, 2014 at 06:25:51PM -0800, Cong Wang wrote:
> On Mon, Feb 10, 2014 at 5:45 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > On Mon, Feb 10, 2014 at 05:36:33PM -0800, Cong Wang wrote:
> >> From: Cong Wang <cwang@twopensource.com>
> >>
> >> BZ: https://bugzilla.kernel.org/show_bug.cgi?id=66691
> >>
> >> There is no point to allow moving a macvlan device to
> >> another namespace while the lower device is still in
> >> this namespace. tunnels already set this flag.
> >
> > Can't we solve this somehow differently, like not showing anything at all
> > etc.? I guess this is a feature some people use and haven't noticed yet.
> >
> 
> I don't understand what you mean by "not showing anything at all".
> I assume you mean mac1@xxx, not matter whether we show xxx
> here, mac1 relies on xxx to function.

Sorry, I have no idea how to resolve this easily, maybe set the ifindex to
something generic. I'll try to think about it.

Maybe revserve an id and install a generic name for it, so old software
doesn't get confused.

> Please give a real use case rather than just guessing, I don't think
> there is any valid case until we support moving multiple devices into
> a netns atomically.

Setting up a macvlan and moving it into another namespace without moving
the parent device is a nice feature. I am not an administrator, so I don't
use that stuff often, but given you can easily spawn namespaces and put
applications into them, one of the easiest things to connect those to
local network without routing over veth and such is the macvlan interface.

Greetings,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Feb. 11, 2014, 4:14 a.m. UTC | #4
On Mon, 2014-02-10 at 17:36 -0800, Cong Wang wrote:
> From: Cong Wang <cwang@twopensource.com>
> 
> BZ: https://bugzilla.kernel.org/show_bug.cgi?id=66691
> 
> There is no point to allow moving a macvlan device to
> another namespace while the lower device is still in
> this namespace. tunnels already set this flag.

What do you mean ?

This is probably one of the needed/useful feature of macvlan !


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Feb. 11, 2014, 4:15 a.m. UTC | #5
On Tue, 2014-02-11 at 03:40 +0100, Hannes Frederic Sowa wrote:

> Setting up a macvlan and moving it into another namespace without moving
> the parent device is a nice feature. I am not an administrator, so I don't
> use that stuff often, but given you can easily spawn namespaces and put
> applications into them, one of the easiest things to connect those to
> local network without routing over veth and such is the macvlan interface.

Exactly.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cong Wang Feb. 11, 2014, 4:41 a.m. UTC | #6
On Mon, Feb 10, 2014 at 8:15 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-02-11 at 03:40 +0100, Hannes Frederic Sowa wrote:
>
>> Setting up a macvlan and moving it into another namespace without moving
>> the parent device is a nice feature. I am not an administrator, so I don't
>> use that stuff often, but given you can easily spawn namespaces and put
>> applications into them, one of the easiest things to connect those to
>> local network without routing over veth and such is the macvlan interface.
>
> Exactly.
>

Exactly broken by design.

IFA_LINK is an ifindex, ifindex is per-netns. macvlan should not use IFA_LINK.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Feb. 11, 2014, 5:21 a.m. UTC | #7
On Mon, 2014-02-10 at 20:41 -0800, Cong Wang wrote:

> Exactly broken by design.

Design of what ? Do you have a pointer to a document about this
'design' ?

> 
> IFA_LINK is an ifindex, ifindex is per-netns. macvlan should not use IFA_LINK.

When this was 'designed', ifindex were not per netns.

Apparently nobody spotted this when ifindexes become per netns.

I am sure we can find a solution, keeping this very useful
functionality.

BTW, are Cong Wang <cwang@twopensource.com> and Cong Wang
<xiyou.wangcong@gmail.com> different persons ?



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel Feb. 12, 2014, 9:47 a.m. UTC | #8
Le 11/02/2014 05:15, Eric Dumazet a écrit :
> On Tue, 2014-02-11 at 03:40 +0100, Hannes Frederic Sowa wrote:
>
>> Setting up a macvlan and moving it into another namespace without moving
>> the parent device is a nice feature. I am not an administrator, so I don't
>> use that stuff often, but given you can easily spawn namespaces and put
>> applications into them, one of the easiest things to connect those to
>> local network without routing over veth and such is the macvlan interface.
>
> Exactly.

I also agree.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 8433de4..9bc3b13 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -539,7 +539,7 @@  static int macvlan_init(struct net_device *dev)
 	dev->state		= (dev->state & ~MACVLAN_STATE_MASK) |
 				  (lowerdev->state & MACVLAN_STATE_MASK);
 	dev->features 		= lowerdev->features & MACVLAN_FEATURES;
-	dev->features		|= NETIF_F_LLTX;
+	dev->features		|= NETIF_F_LLTX | NETIF_F_NETNS_LOCAL;
 	dev->gso_max_size	= lowerdev->gso_max_size;
 	dev->iflink		= lowerdev->ifindex;
 	dev->hard_header_len	= lowerdev->hard_header_len;
@@ -699,7 +699,7 @@  static netdev_features_t macvlan_fix_features(struct net_device *dev,
 	features = netdev_increment_features(vlan->lowerdev->features,
 					     features,
 					     mask);
-	features |= NETIF_F_LLTX;
+	features |= NETIF_F_LLTX | NETIF_F_NETNS_LOCAL;
 
 	return features;
 }