Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] 8021q/vlan: process NETDEV_GOING_DOWN

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


eldad at fogrefinery

May 29, 2012, 3:30 PM

Post #1 of 7 (158 views)
Permalink
[PATCH] 8021q/vlan: process NETDEV_GOING_DOWN

In the current flow, when you take down a physical device that has
VLANs configured on it, the NETDEV_GOING_DOWN notification will be
sent too late, i.e., no data can be sent to the wire anymore.

static int __dev_close_many(struct list_head *head)
{
...
list_for_each_entry(dev, head, unreg_list) {
call_netdevice_notifiers(NETDEV_GOING_DOWN, dev);
...
}
...
list_for_each_entry(dev, head, unreg_list) {
if (ops->ndo_stop)
ops->ndo_stop(dev);
}
...
}

static int dev_close_many(struct list_head *head)
{
...
__dev_close_many(head);

list_for_each_entry(dev, head, unreg_list) {
rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING);
call_netdevice_notifiers(NETDEV_DOWN, dev);
}
}

In a setup like this: eth0 with VLANs 2, 3 the flow would be:

eth0 - NETDEV_GOING_DOWN
ndo_stop is called on the device
eth0.2 - NETDEV_GOING_DOWN
eth0.2 - NETDEV_DOWN
eth0.3 - NETDEV_GOING_DOWN
eth0.3 - NETDEV_DOWN
eth0 - NETDEV_DOWN

If instead NETDEV_GOING_DOWN is processed, the flow would be:
eth0.2 - NETDEV_GOING_DOWN
eth0.2 - NETDEV_DOWN
eth0.3 - NETDEV_GOING_DOWN
eth0.3 - NETDEV_DOWN
eth0 - NETDEV_GOING_DOWN
eth0 - NETDEV_DOWN
ndo_stop is called on the device

Signed-off-by: Eldad Zack <eldad [at] fogrefinery>
---
net/8021q/vlan.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 6089f0c..fd87ecc 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -402,6 +402,12 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,

break;

+ case NETDEV_GOING_DOWN: /* NETDEV_DOWN */
+ /* If the parent device is going down it will call ndo_stop after
+ * it sends out NETDEV_GOING_DOWN but before sending out NETDEV_DOWN,
+ * The effect of which is, that no data can be sent anymore
+ * by the time the VLAN device sends out its NETDEV_GOING_DOWN.
+ */
case NETDEV_DOWN:
/* Put all VLANs for this dev in the down state too. */
for (i = 0; i < VLAN_N_VID; i++) {
--
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


davem at davemloft

May 29, 2012, 5:40 PM

Post #2 of 7 (153 views)
Permalink
Re: [PATCH] 8021q/vlan: process NETDEV_GOING_DOWN [In reply to]

From: Eldad Zack <eldad [at] fogrefinery>
Date: Wed, 30 May 2012 00:30:35 +0200

> In the current flow, when you take down a physical device that has
> VLANs configured on it, the NETDEV_GOING_DOWN notification will be
> sent too late, i.e., no data can be sent to the wire anymore.

Why do you need to send data? Any queued up data should be purged not
sent.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


eldad at fogrefinery

May 30, 2012, 12:11 PM

Post #3 of 7 (152 views)
Permalink
Re: [PATCH] 8021q/vlan: process NETDEV_GOING_DOWN [In reply to]

On Tue, 29 May 2012, David Miller wrote:
> From: Eldad Zack <eldad [at] fogrefinery>
> Date: Wed, 30 May 2012 00:30:35 +0200
>
> > In the current flow, when you take down a physical device that has
> > VLANs configured on it, the NETDEV_GOING_DOWN notification will be
> > sent too late, i.e., no data can be sent to the wire anymore.
>
> Why do you need to send data? Any queued up data should be purged not
> sent.

In case a certain protocol needs to send a "dying gasp" packet, when you
administrativly shutdown the port (which is also what happens when you
restart the machine).

I'm working on an implementation of such protocol (LLDP) on my free
time. The specification says that it should send a (compact) shutdown
message, with the TTL field set to zero, so that other stations are informed
of the shutdown - and it works fine with the main interface, but not with
VLANs, since the notifier is called too late.
With that small change it works as well.

Another use for it would be the Ethernet CFM, which has a similar
requirement.

On the other hand, I might've missed something. Is there a better way to
be informed of a shutdown than listening for the GOING_DOWN
event, so the frame can be sent to the wire when it's appropriate?

Eldad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


davem at davemloft

May 30, 2012, 1:27 PM

Post #4 of 7 (151 views)
Permalink
Re: [PATCH] 8021q/vlan: process NETDEV_GOING_DOWN [In reply to]

From: Eldad Zack <eldad [at] fogrefinery>
Date: Wed, 30 May 2012 21:11:02 +0200 (CEST)

> In case a certain protocol needs to send a "dying gasp" packet, when you
> administrativly shutdown the port (which is also what happens when you
> restart the machine).

No in tree users have this requirement, therefore your patch is
inappropriate.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


eldad at fogrefinery

May 30, 2012, 2:47 PM

Post #5 of 7 (150 views)
Permalink
Re: [PATCH] 8021q/vlan: process NETDEV_GOING_DOWN [In reply to]

On Wed, 30 May 2012, David Miller wrote:
> From: Eldad Zack <eldad [at] fogrefinery>
> Date: Wed, 30 May 2012 21:11:02 +0200 (CEST)
>
> > In case a certain protocol needs to send a "dying gasp" packet, when you
> > administrativly shutdown the port (which is also what happens when you
> > restart the machine).
>
> No in tree users have this requirement, therefore your patch is
> inappropriate.

You are right in that, that no in tree users have this requirement
(yet), but in the same time it doesn't harm any existing code.

Don't you agree that it's the right order to do the notifications?
And if so, isn't that enough, considering that there are no side
effects and it's a tiny change?

Eldad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


davem at davemloft

May 30, 2012, 2:50 PM

Post #6 of 7 (152 views)
Permalink
Re: [PATCH] 8021q/vlan: process NETDEV_GOING_DOWN [In reply to]

From: Eldad Zack <eldad [at] fogrefinery>
Date: Wed, 30 May 2012 23:47:32 +0200 (CEST)

>
> On Wed, 30 May 2012, David Miller wrote:
>> From: Eldad Zack <eldad [at] fogrefinery>
>> Date: Wed, 30 May 2012 21:11:02 +0200 (CEST)
>>
>> > In case a certain protocol needs to send a "dying gasp" packet, when you
>> > administrativly shutdown the port (which is also what happens when you
>> > restart the machine).
>>
>> No in tree users have this requirement, therefore your patch is
>> inappropriate.
>
> You are right in that, that no in tree users have this requirement
> (yet), but in the same time it doesn't harm any existing code.
>
> Don't you agree that it's the right order to do the notifications?

It's not an issue that matters upstream, so I simply do not care.

When you, or someone else, submits code that needs this facility
then we can talk about it.

Otherwise it's just a waste of our time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


eldad at fogrefinery

May 30, 2012, 3:02 PM

Post #7 of 7 (152 views)
Permalink
Re: [PATCH] 8021q/vlan: process NETDEV_GOING_DOWN [In reply to]

On Wed, 30 May 2012, David Miller wrote:
> From: Eldad Zack <eldad [at] fogrefinery>
> Date: Wed, 30 May 2012 23:47:32 +0200 (CEST)
> It's not an issue that matters upstream, so I simply do not care.
>
> When you, or someone else, submits code that needs this facility
> then we can talk about it.
>
> Otherwise it's just a waste of our time.

I understand that. I thought it was better to send unrelated
patches earlier.
Thanks for the review!

Eldad

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.