[PATCH] macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

[PATCH] macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

[Peter Christensen]

Clearing the IFF_ALLMULTI flag on a down interface could cause an allmulti
overflow on the underlying interface.

Attempting the set IFF_ALLMULTI on the underlying interface would cause an
error and the log message:

"allmulti touches root, set allmulti failed."

Signed-off-by: Peter Christensen <pch@ordbogen.com>
---
 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 b0e2865..c5fb9cf 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -458,8 +458,10 @@ static void macvlan_change_rx_flags(struct net_device *dev, int change)
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	struct net_device *lowerdev = vlan->lowerdev;
 
-	if (change & IFF_ALLMULTI)
-		dev_set_allmulti(lowerdev, dev->flags & IFF_ALLMULTI ? 1 : -1);
+	if (dev->flags & IFF_UP) {
+		if (change & IFF_ALLMULTI)
+			dev_set_allmulti(lowerdev, dev->flags & IFF_ALLMULTI ? 1 : -1);
+	}
 }
 
 static void macvlan_set_mac_lists(struct net_device *dev)
-- 
1.7.10.4

Re: [PATCH] macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

[David Miller]

From: Peter Christensen <pch@ordbogen.com>
Date: Thu,  8 May 2014 11:15:37 +0200

> Clearing the IFF_ALLMULTI flag on a down interface could cause an allmulti
> overflow on the underlying interface.
> 
> Attempting the set IFF_ALLMULTI on the underlying interface would cause an
> error and the log message:
> 
> "allmulti touches root, set allmulti failed."
> 
> Signed-off-by: Peter Christensen <pch@ordbogen.com>

Hmmm, but won't we lose this event once the interface is brought
back up?

Re: [PATCH] macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

[Peter Christensen]

On Fri, 09 May 2014 16:02:25 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Peter Christensen <pch@ordbogen.com>
> Date: Thu,  8 May 2014 11:15:37 +0200
> 
> > Clearing the IFF_ALLMULTI flag on a down interface could cause an
> > allmulti overflow on the underlying interface.
> > 
> > Attempting the set IFF_ALLMULTI on the underlying interface would
> > cause an error and the log message:
> > 
> > "allmulti touches root, set allmulti failed."
> > 
> > Signed-off-by: Peter Christensen <pch@ordbogen.com>
> 
> Hmmm, but won't we lose this event once the interface is brought
> back up?

No, macvlan_open and macvlan_stop already takes care of that (which is
part of the reason the allmulti counter overflowed). Either
macvlan_open and macvlan_close shouldn't touch the flag on the
underlying interface, or
macvlan_change_rx_flags shouldn't touch the underlying interface if the
MACVLAN interface is down.

I though that it was probably better to make MACVLAN as non-intrusive
to the underlying interface when the MACVLAN interface is down, so I
opted for this solution.

Apparently the 802.1Q VLAN driver behaves
exactly like that, and back in 2011 it got a patch similar to this one
(commit deede2fabe24e00bd7e246eb81cd5767dc6fcfc7).

Re: [PATCH] macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

[David Miller]

From: Peter Christensen <pch@ordbogen.com>
Date: Mon, 12 May 2014 09:59:53 +0200

> On Fri, 09 May 2014 16:02:25 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Peter Christensen <pch@ordbogen.com>
>> Date: Thu,  8 May 2014 11:15:37 +0200
>> 
>> > Clearing the IFF_ALLMULTI flag on a down interface could cause an
>> > allmulti overflow on the underlying interface.
>> > 
>> > Attempting the set IFF_ALLMULTI on the underlying interface would
>> > cause an error and the log message:
>> > 
>> > "allmulti touches root, set allmulti failed."
>> > 
>> > Signed-off-by: Peter Christensen <pch@ordbogen.com>
>> 
>> Hmmm, but won't we lose this event once the interface is brought
>> back up?
> 
> No, macvlan_open and macvlan_stop already takes care of that (which is
> part of the reason the allmulti counter overflowed). Either
> macvlan_open and macvlan_close shouldn't touch the flag on the
> underlying interface, or
> macvlan_change_rx_flags shouldn't touch the underlying interface if the
> MACVLAN interface is down.
> 
> I though that it was probably better to make MACVLAN as non-intrusive
> to the underlying interface when the MACVLAN interface is down, so I
> opted for this solution.
> 
> Apparently the 802.1Q VLAN driver behaves
> exactly like that, and back in 2011 it got a patch similar to this one
> (commit deede2fabe24e00bd7e246eb81cd5767dc6fcfc7).

Thanks for the detailed explanation, applied and queued up for -stable, thanks!