diff mbox

Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)

Message ID 48E9F07E.9060609@trash.net
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Patrick McHardy Oct. 6, 2008, 11:03 a.m. UTC 
Jesper Dangaard Brouer wrote:
> 
> Here is a quick fix for the bug...
> Patrick promised he would do a more clean fix later ;-)

And here it is. Could you verify that it also fixes the problem?
Thanks!
commit 79a95e393b63c26f270c676075b95c9421d3faba
Author: Patrick McHardy <kaber@trash.net>
Date:   Mon Oct 6 13:01:26 2008 +0200

    net: only invoke dev->change_rx_flags when device is UP
    
    Jesper Dangaard Brouer <hawk@comx.dk> reported a bug when setting a VLAN
    device down that is in promiscous mode:
    
    When the VLAN device is set down, the promiscous count on the real
    device is decremented by one by vlan_dev_stop(). When removing the
    promiscous flag from the VLAN device afterwards, the promiscous
    count on the real device is decremented a second time by the
    vlan_change_rx_flags() callback.
    
    The root cause for this is that the ->change_rx_flags() callback is
    invoked while the device is down. The synchronization is meant to mirror
    the behaviour of the ->set_rx_mode callbacks, meaning the ->open function
    is responsible for doing a full sync on open, the ->close() function is
    responsible for doing full cleanup on ->stop() and ->change_rx_flags()
    is meant to do incremental changes while the device is UP.
    
    Only invoke ->change_rx_flags() while the device is UP to provide the
    intended behaviour.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

Comments

Jesper Dangaard Brouer Oct. 7, 2008, 11:03 a.m. UTC | #1 
On Mon, 2008-10-06 at 13:03 +0200, Patrick McHardy wrote:
> Jesper Dangaard Brouer wrote:
> > 
> > Patrick promised he would do a more clean fix later ;-)
> 
> And here it is. Could you verify that it also fixes the problem?

It works and fixes the problem -- thanks!

Tested-by: Jesper Dangaard Brouer <jdb@comx.dk>
Patrick McHardy Oct. 7, 2008, 11:10 a.m. UTC | #2 
Jesper Dangaard Brouer wrote:
> On Mon, 2008-10-06 at 13:03 +0200, Patrick McHardy wrote:
>> Jesper Dangaard Brouer wrote:
>>> Patrick promised he would do a more clean fix later ;-)
>> And here it is. Could you verify that it also fixes the problem?
> 
> It works and fixes the problem -- thanks!
> 
> Tested-by: Jesper Dangaard Brouer <jdb@comx.dk>

Thanks Jesper.

--
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/net/core/dev.c b/net/core/dev.c
index e8eb2b4..fd992c0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2918,6 +2918,12 @@  int netdev_set_master(struct net_device *slave, struct net_device *master)
 	return 0;
 }
 
+static void dev_change_rx_flags(struct net_device *dev, int flags)
+{
+	if (dev->flags & IFF_UP && dev->change_rx_flags)
+		dev->change_rx_flags(dev, flags);
+}
+
 static int __dev_set_promiscuity(struct net_device *dev, int inc)
 {
 	unsigned short old_flags = dev->flags;
@@ -2955,8 +2961,7 @@  static int __dev_set_promiscuity(struct net_device *dev, int inc)
 				current->uid, current->gid,
 				audit_get_sessionid(current));
 
-		if (dev->change_rx_flags)
-			dev->change_rx_flags(dev, IFF_PROMISC);
+		dev_change_rx_flags(dev, IFF_PROMISC);
 	}
 	return 0;
 }
@@ -3022,8 +3027,7 @@  int dev_set_allmulti(struct net_device *dev, int inc)
 		}
 	}
 	if (dev->flags ^ old_flags) {
-		if (dev->change_rx_flags)
-			dev->change_rx_flags(dev, IFF_ALLMULTI);
+		dev_change_rx_flags(dev, IFF_ALLMULTI);
 		dev_set_rx_mode(dev);
 	}
 	return 0;
@@ -3347,8 +3351,8 @@  int dev_change_flags(struct net_device *dev, unsigned flags)
 	 *	Load in the correct multicast list now the flags have changed.
 	 */
 
-	if (dev->change_rx_flags && (old_flags ^ flags) & IFF_MULTICAST)
-		dev->change_rx_flags(dev, IFF_MULTICAST);
+	if ((old_flags ^ flags) & IFF_MULTICAST)
+		dev_change_rx_flags(dev, IFF_MULTICAST);
 
 	dev_set_rx_mode(dev);