From patchwork Fri Sep 26 14:00:36 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesper Dangaard Brouer X-Patchwork-Id: 1684 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 7D78ADDDFD for ; Sat, 27 Sep 2008 00:33:11 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752533AbYIZOc6 (ORCPT ); Fri, 26 Sep 2008 10:32:58 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752511AbYIZOc6 (ORCPT ); Fri, 26 Sep 2008 10:32:58 -0400 Received: from lanfw001a.cxnet.dk ([87.72.215.196]:54017 "EHLO lanfw001a.cxnet.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752497AbYIZOcy (ORCPT ); Fri, 26 Sep 2008 10:32:54 -0400 X-Greylist: delayed 1934 seconds by postgrey-1.27 at vger.kernel.org; Fri, 26 Sep 2008 10:32:53 EDT Received: from comxexch02.comx.local (unknown [172.31.1.117]) by lanfw001a.cxnet.dk (Postfix) with ESMTP id C9663163566; Fri, 26 Sep 2008 16:01:17 +0200 (CEST) Received: from 172.31.4.93 ([172.31.4.93]) by comxexch02.comx.local ([172.31.1.117]) with Microsoft Exchange Server HTTP-DAV ; Fri, 26 Sep 2008 14:00:36 +0000 Received: from hawk by comxexch02.comx.local; 26 Sep 2008 16:00:36 +0200 Subject: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47) From: Jesper Dangaard Brouer Reply-To: jdb@comx.dk To: Patrick McHardy Cc: "netdev@vger.kernel.org" Organization: ComX Networks A/S Date: Fri, 26 Sep 2008 16:00:36 +0200 Message-Id: <1222437636.7598.14.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.6.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Patrick, Bisected me down to one of you changes commit 6c78dcbd47a68a [VLAN]: Fix promiscous/allmulti synchronization races Description: ------------ All other VLAN interfaces stop working, if a vlan is taken down (ifconfig eth1.1025 down) _while_ there is a tcpdump running on that interface. The problem is a result of promisc mode being removed on the real-device (eth1), when the vlan interface is taken down. This should not happen as other vlan devices exists that still need promisc mode on the real-device (eth1). Setup: ------ eth1 - the real-device eth1.1013 - VLAN device eth1.1025 - VLAN device Both VLAN devices has assigned ether address 00:11:22:33:44:55 (which is different from the real-device). Reproduce / test: ----------------- Lookfor "promiscuous mode" changes in kern.log # tail -n 40 -f /var/log/kern.log | grep "promiscuous mode" Start a tcpdump on VLAN device # tcpdump -ni eth1.1025 # LOG: # kernel: device eth1.1025 entered promiscuous mode Take VLAN device down # ifconfig eth1.1025 down Tcpdump dies, the problem is that promisc mode is removed from both the real-device and the VLAN device. That should NOT happen, as there are other VLAN devices up (eth1.1013). # LOG: # kernel: device eth1.1025 left promiscuous mode # kernel: device eth1 left promiscuous mode Bisect log: Attached Commit 6c78dcbd47: Attached See you in Paris! commit 6c78dcbd47a68a7d25d2bee7a6c74b9136cb5fde Author: Patrick McHardy Date: Sat Jul 14 18:52:56 2007 -0700 [VLAN]: Fix promiscous/allmulti synchronization races The set_multicast_list function may be called without holding the rtnl mutex, resulting in races when changing the underlying device's promiscous and allmulti state. Use the change_rx_mode hook, which is always invoked under the rtnl. Signed-off-by: Patrick McHardy Signed-off-by: David S. Miller diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index 61a57dc..7f71df4 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -132,8 +132,6 @@ struct vlan_dev_info { * made, in order to feed the right changes down * to the real hardware... */ - int old_allmulti; /* similar to above. */ - int old_promiscuity; /* similar to above. */ struct net_device *real_dev; /* the underlying device/interface */ unsigned char real_dev_addr[ETH_ALEN]; struct proc_dir_entry *dent; /* Holds the proc data */ diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c index abb9900..39bdcc2 100644 --- a/net/8021q/vlan.c +++ b/net/8021q/vlan.c @@ -373,6 +373,7 @@ void vlan_setup(struct net_device *new_dev) new_dev->open = vlan_dev_open; new_dev->stop = vlan_dev_stop; new_dev->set_multicast_list = vlan_dev_set_multicast_list; + new_dev->change_rx_flags = vlan_change_rx_flags; new_dev->destructor = free_netdev; new_dev->do_ioctl = vlan_dev_ioctl; diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h index 62ce1c5..7df5b29 100644 --- a/net/8021q/vlan.h +++ b/net/8021q/vlan.h @@ -69,6 +69,7 @@ int vlan_dev_set_vlan_flag(const struct net_device *dev, u32 flag, short flag_val); void vlan_dev_get_realdev_name(const struct net_device *dev, char *result); void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result); +void vlan_change_rx_flags(struct net_device *dev, int change); void vlan_dev_set_multicast_list(struct net_device *vlan_dev); int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id); diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index d4a62d1..dec7e62 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -712,6 +712,11 @@ int vlan_dev_open(struct net_device *dev) } memcpy(vlan->real_dev_addr, real_dev->dev_addr, ETH_ALEN); + if (dev->flags & IFF_ALLMULTI) + dev_set_allmulti(real_dev, 1); + if (dev->flags & IFF_PROMISC) + dev_set_promiscuity(real_dev, 1); + return 0; } @@ -721,6 +726,11 @@ int vlan_dev_stop(struct net_device *dev) vlan_flush_mc_list(dev); + if (dev->flags & IFF_ALLMULTI) + dev_set_allmulti(real_dev, -1); + if (dev->flags & IFF_PROMISC) + dev_set_promiscuity(real_dev, -1); + if (compare_ether_addr(dev->dev_addr, real_dev->dev_addr)) dev_unicast_delete(real_dev, dev->dev_addr, dev->addr_len); @@ -754,34 +764,26 @@ int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) return err; } +void vlan_change_rx_flags(struct net_device *dev, int change) +{ + struct net_device *real_dev = VLAN_DEV_INFO(dev)->real_dev; + + if (change & IFF_ALLMULTI) + dev_set_allmulti(real_dev, dev->flags & IFF_ALLMULTI ? 1 : -1); + if (change & IFF_PROMISC) + dev_set_promiscuity(real_dev, dev->flags & IFF_PROMISC ? 1 : -1); +} + /** Taken from Gleb + Lennert's VLAN code, and modified... */ void vlan_dev_set_multicast_list(struct net_device *vlan_dev) { struct dev_mc_list *dmi; struct net_device *real_dev; - int inc; if (vlan_dev && (vlan_dev->priv_flags & IFF_802_1Q_VLAN)) { /* Then it's a real vlan device, as far as we can tell.. */ real_dev = VLAN_DEV_INFO(vlan_dev)->real_dev; - /* compare the current promiscuity to the last promisc we had.. */ - inc = vlan_dev->promiscuity - VLAN_DEV_INFO(vlan_dev)->old_promiscuity; - if (inc) { - printk(KERN_INFO "%s: dev_set_promiscuity(master, %d)\n", - vlan_dev->name, inc); - dev_set_promiscuity(real_dev, inc); /* found in dev.c */ - VLAN_DEV_INFO(vlan_dev)->old_promiscuity = vlan_dev->promiscuity; - } - - inc = vlan_dev->allmulti - VLAN_DEV_INFO(vlan_dev)->old_allmulti; - if (inc) { - printk(KERN_INFO "%s: dev_set_allmulti(master, %d)\n", - vlan_dev->name, inc); - dev_set_allmulti(real_dev, inc); /* dev.c */ - VLAN_DEV_INFO(vlan_dev)->old_allmulti = vlan_dev->allmulti; - } - /* looking for addresses to add to master's list */ for (dmi = vlan_dev->mc_list; dmi != NULL; dmi = dmi->next) { if (vlan_should_add_mc(dmi, VLAN_DEV_INFO(vlan_dev)->old_mc_list)) {