* Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
@ 2008-09-26 14:00 Jesper Dangaard Brouer
2008-09-26 16:14 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2008-09-26 14:00 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1622 bytes --]
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!
--
Med venlig hilsen / Best regards
Jesper Brouer
ComX Networks A/S
Linux Network developer
Cand. Scient Datalog / MSc.
Author of http://adsl-optimizer.dk
LinkedIn: http://www.linkedin.com/in/brouer
[-- Attachment #2: bisect03-found-bug.log --]
[-- Type: text/x-log, Size: 1706 bytes --]
git-bisect start
# good: [de46c33745f5e2ad594c72f2cf5f490861b16ce1] Linux 2.6.21
git-bisect good de46c33745f5e2ad594c72f2cf5f490861b16ce1
# bad: [bbf25010f1a6b761914430f5fca081ec8c7accd1] Linux 2.6.23
git-bisect bad bbf25010f1a6b761914430f5fca081ec8c7accd1
# good: [71ba22fa739029bb158144813b9e82c00326497c] Merge branch 'upstream-linus' of master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6
git-bisect good 71ba22fa739029bb158144813b9e82c00326497c
# bad: [2d9ce177e68645945e3366cfe2d66ee3c28cd4f2] i386: Allow KVM on i386 nonpae
git-bisect bad 2d9ce177e68645945e3366cfe2d66ee3c28cd4f2
# bad: [da58a1617343e345d435953a0f32024997a95164] /proc/*/environ: wrong placing of ptrace_may_attach() check
git-bisect bad da58a1617343e345d435953a0f32024997a95164
# good: [79e15ae424afa0a40b1a0c4478046d6ba0b71e20] dm: bio_list prefetch removal
git-bisect good 79e15ae424afa0a40b1a0c4478046d6ba0b71e20
# good: [e46662be7fddde3464bf208317542c2f8df13d0b] net/9p: change net/9p module name to 9pnet
git-bisect good e46662be7fddde3464bf208317542c2f8df13d0b
# bad: [b3b0b681b12478a7afa7d1f3d58be96830e16c7d] [TCP]: tcp probe add back ssthresh field
git-bisect bad b3b0b681b12478a7afa7d1f3d58be96830e16c7d
# bad: [6c78dcbd47a68a7d25d2bee7a6c74b9136cb5fde] [VLAN]: Fix promiscous/allmulti synchronization races
git-bisect bad 6c78dcbd47a68a7d25d2bee7a6c74b9136cb5fde
# good: [24023451c8df726692e2f52288a20870d13b501f] [NET]: Add net_device change_rx_mode callback
git-bisect good 24023451c8df726692e2f52288a20870d13b501f
# good: [a0a400d79e3dd7843e7e81baa3ef2957bdc292d0] [NET]: dev_mcast: add multicast list synchronization helpers
git-bisect good a0a400d79e3dd7843e7e81baa3ef2957bdc292d0
[-- Attachment #3: bisect03-found-bug.patch --]
[-- Type: text/x-patch, Size: 4902 bytes --]
commit 6c78dcbd47a68a7d25d2bee7a6c74b9136cb5fde
Author: Patrick McHardy <kaber@trash.net>
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 <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
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)) {
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 14:00 Bisect'ed BUG in VLAN promisc mode (6c78dcbd47) Jesper Dangaard Brouer
@ 2008-09-26 16:14 ` Patrick McHardy
2008-09-26 19:22 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2008-09-26 16:14 UTC (permalink / raw)
To: jdb; +Cc: netdev@vger.kernel.org
Jesper Dangaard Brouer wrote:
> 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).
I'm pretty sure we fixed this already in commit 0ed21b32.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 16:14 ` Patrick McHardy
@ 2008-09-26 19:22 ` Jesper Dangaard Brouer
2008-09-26 19:24 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2008-09-26 19:22 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev@vger.kernel.org
On Fri, 2008-09-26 at 18:14 +0200, Patrick McHardy wrote:
> Jesper Dangaard Brouer wrote:
> > 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).
>
> I'm pretty sure we fixed this already in commit 0ed21b32.
I think that I have tested including this commit. The first thing I did
was to test with latest DaveM net-2.6 git tree.
I'll test it later... perhaps during the hacking days...
Need to pack.. See you soon!
--
Med venlig hilsen / Best regards
Jesper Brouer
ComX Networks A/S
Linux Network developer
Cand. Scient Datalog / MSc.
Author of http://adsl-optimizer.dk
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 19:22 ` Jesper Dangaard Brouer
@ 2008-09-26 19:24 ` Patrick McHardy
2008-09-26 19:28 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2008-09-26 19:24 UTC (permalink / raw)
To: jdb; +Cc: netdev@vger.kernel.org
Jesper Dangaard Brouer wrote:
> On Fri, 2008-09-26 at 18:14 +0200, Patrick McHardy wrote:
>> Jesper Dangaard Brouer wrote:
>>> 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).
>> I'm pretty sure we fixed this already in commit 0ed21b32.
>
> I think that I have tested including this commit. The first thing I did
> was to test with latest DaveM net-2.6 git tree.
Let me see if I can reproduce it ...
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 19:24 ` Patrick McHardy
@ 2008-09-26 19:28 ` Patrick McHardy
2008-09-26 19:34 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2008-09-26 19:28 UTC (permalink / raw)
To: jdb; +Cc: netdev@vger.kernel.org
Patrick McHardy wrote:
> Jesper Dangaard Brouer wrote:
>> On Fri, 2008-09-26 at 18:14 +0200, Patrick McHardy wrote:
>>> Jesper Dangaard Brouer wrote:
>>>> 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).
>>> I'm pretty sure we fixed this already in commit 0ed21b32.
>>
>> I think that I have tested including this commit. The first thing I did
>> was to test with latest DaveM net-2.6 git tree.
>
> Let me see if I can reproduce it ...
Actually - one question: you're saying you're using different MAC
addresses on the VLAN devices, so I guess thats why you're expecting
the underlying device to still be in promiscous mode after you set
eth1.1025 down. For devices that support multiple unicast addresses
in hardware, we don't put the device in promiscous mode anymore.
So the question is: is something actually not working, or did you
just notice that the real device is no longer in promiscous mode?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 19:28 ` Patrick McHardy
@ 2008-09-26 19:34 ` Jesper Dangaard Brouer
2008-09-26 19:41 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2008-09-26 19:34 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev@vger.kernel.org
On Fri, 2008-09-26 at 21:28 +0200, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Jesper Dangaard Brouer wrote:
> >> On Fri, 2008-09-26 at 18:14 +0200, Patrick McHardy wrote:
> >>> Jesper Dangaard Brouer wrote:
> >>>> 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).
> >>> I'm pretty sure we fixed this already in commit 0ed21b32.
> >>
> >> I think that I have tested including this commit. The first thing I did
> >> was to test with latest DaveM net-2.6 git tree.
> >
> > Let me see if I can reproduce it ...
>
> Actually - one question: you're saying you're using different MAC
> addresses on the VLAN devices, so I guess thats why you're expecting
> the underlying device to still be in promiscous mode after you set
> eth1.1025 down. For devices that support multiple unicast addresses
> in hardware, we don't put the device in promiscous mode anymore.
> So the question is: is something actually not working, or did you
> just notice that the real device is no longer in promiscous mode?
It stopped working! In the test setup I do have a machine connected to
eth1.1013, where I have a ping running, that stop working...
I use the tg3 driver for both netcards. Don't know if it supports it?
--
Med venlig hilsen / Best regards
Jesper Brouer
ComX Networks A/S
Linux Network developer
Cand. Scient Datalog / MSc.
Author of http://adsl-optimizer.dk
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 19:34 ` Jesper Dangaard Brouer
@ 2008-09-26 19:41 ` Patrick McHardy
2008-09-26 20:10 ` Patrick McHardy
2008-10-02 16:38 ` Jesper Dangaard Brouer
0 siblings, 2 replies; 12+ messages in thread
From: Patrick McHardy @ 2008-09-26 19:41 UTC (permalink / raw)
To: jdb; +Cc: netdev@vger.kernel.org
Jesper Dangaard Brouer wrote:
> On Fri, 2008-09-26 at 21:28 +0200, Patrick McHardy wrote:
>> Actually - one question: you're saying you're using different MAC
>> addresses on the VLAN devices, so I guess thats why you're expecting
>> the underlying device to still be in promiscous mode after you set
>> eth1.1025 down. For devices that support multiple unicast addresses
>> in hardware, we don't put the device in promiscous mode anymore.
>> So the question is: is something actually not working, or did you
>> just notice that the real device is no longer in promiscous mode?
>
> It stopped working! In the test setup I do have a machine connected to
> eth1.1013, where I have a ping running, that stop working...
Found it in the bugreport :) OK, I'll try to reproduce it now.
> I use the tg3 driver for both netcards. Don't know if it supports it?
The driver doesn't support it.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 19:41 ` Patrick McHardy
@ 2008-09-26 20:10 ` Patrick McHardy
2008-10-02 16:38 ` Jesper Dangaard Brouer
1 sibling, 0 replies; 12+ messages in thread
From: Patrick McHardy @ 2008-09-26 20:10 UTC (permalink / raw)
To: jdb; +Cc: netdev@vger.kernel.org
Patrick McHardy wrote:
> Jesper Dangaard Brouer wrote:
>> On Fri, 2008-09-26 at 21:28 +0200, Patrick McHardy wrote:
>>> Actually - one question: you're saying you're using different MAC
>>> addresses on the VLAN devices, so I guess thats why you're expecting
>>> the underlying device to still be in promiscous mode after you set
>>> eth1.1025 down. For devices that support multiple unicast addresses
>>> in hardware, we don't put the device in promiscous mode anymore.
>>> So the question is: is something actually not working, or did you
>>> just notice that the real device is no longer in promiscous mode?
>>
>> It stopped working! In the test setup I do have a machine connected to
>> eth1.1013, where I have a ping running, that stop working...
>
> Found it in the bugreport :) OK, I'll try to reproduce it now.
Found it without reproduing, but unfortunately I also have to leave
now, will look at it later again.
Anyways, the problem appears to be that the promiscous count is
decremented twice for the VLAN device, once in vlan_stop() because
the device is still in promiscous mode, once when af_packet takes
the VLAN device out of promisc in the NETDEV_UNREGISTER notifier
chain, which triggers the VLAN ->change_rx_mode callback and
removes another promiscous count from the real device.
I think the correct fix would be to not invoke ->change_rx_flags
while the device is down, similar to ->set_multicast_list and
->set_rx_mode, but I need to check the other drivers first.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-09-26 19:41 ` Patrick McHardy
2008-09-26 20:10 ` Patrick McHardy
@ 2008-10-02 16:38 ` Jesper Dangaard Brouer
2008-10-06 11:03 ` Patrick McHardy
1 sibling, 1 reply; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2008-10-02 16:38 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jesper Dangaard Brouer, netdev@vger.kernel.org
Here is a quick fix for the bug...
Patrick promised he would do a more clean fix later ;-)
commit 356d09baea53abb7f6a6aabf2264ba104cdb9b5b
Author: Jesper Dangaard Brouer <hawk@comx.dk>
Date: Thu Oct 2 12:24:19 2008 +0200
Quick fix from Patrick to solve the VLAN promisc mode bug.
Bisect'ed BUG in VLAN promisc mode
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).
Need promisc mode because VLAN devices uses another ether address.
Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 4bf014e..a79daee 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -551,6 +551,8 @@ static void vlan_dev_change_rx_flags(struct net_device *dev, int change)
{
struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
+ if (!netif_running(dev))
+ return;
if (change & IFF_ALLMULTI)
dev_set_allmulti(real_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
if (change & IFF_PROMISC)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-10-02 16:38 ` Jesper Dangaard Brouer
@ 2008-10-06 11:03 ` Patrick McHardy
2008-10-07 11:03 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2008-10-06 11:03 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Jesper Dangaard Brouer, netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 204 bytes --]
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!
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2701 bytes --]
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>
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);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-10-06 11:03 ` Patrick McHardy
@ 2008-10-07 11:03 ` Jesper Dangaard Brouer
2008-10-07 11:10 ` Patrick McHardy
0 siblings, 1 reply; 12+ messages in thread
From: Jesper Dangaard Brouer @ 2008-10-07 11:03 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jesper Dangaard Brouer, netdev@vger.kernel.org
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>
--
Med venlig hilsen / Best regards
Jesper Brouer
ComX Networks A/S
Linux Network developer
Cand. Scient Datalog / MSc.
Author of http://adsl-optimizer.dk
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Bisect'ed BUG in VLAN promisc mode (6c78dcbd47)
2008-10-07 11:03 ` Jesper Dangaard Brouer
@ 2008-10-07 11:10 ` Patrick McHardy
0 siblings, 0 replies; 12+ messages in thread
From: Patrick McHardy @ 2008-10-07 11:10 UTC (permalink / raw)
To: jdb; +Cc: Jesper Dangaard Brouer, netdev@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-10-07 11:10 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-26 14:00 Bisect'ed BUG in VLAN promisc mode (6c78dcbd47) Jesper Dangaard Brouer
2008-09-26 16:14 ` Patrick McHardy
2008-09-26 19:22 ` Jesper Dangaard Brouer
2008-09-26 19:24 ` Patrick McHardy
2008-09-26 19:28 ` Patrick McHardy
2008-09-26 19:34 ` Jesper Dangaard Brouer
2008-09-26 19:41 ` Patrick McHardy
2008-09-26 20:10 ` Patrick McHardy
2008-10-02 16:38 ` Jesper Dangaard Brouer
2008-10-06 11:03 ` Patrick McHardy
2008-10-07 11:03 ` Jesper Dangaard Brouer
2008-10-07 11:10 ` Patrick McHardy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).