* [PATCH net-next-2.6] veth: Fix veth_dellink method
@ 2009-10-30 7:00 Eric Dumazet
2009-10-30 8:00 ` David Miller
2009-10-30 23:07 ` Eric W. Biederman
0 siblings, 2 replies; 7+ messages in thread
From: Eric Dumazet @ 2009-10-30 7:00 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, Pavel Emelyanov
In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
(net: add a list_head parameter to dellink() method),
I forgot to actually use this parameter in veth_dellink.
I remember feeling a bit uncomfortable about veth_close(),
because it does :
netif_carrier_off(dev);
netif_carrier_off(priv->peer);
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index ffb502d..9bed694 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
priv = netdev_priv(dev);
peer = priv->peer;
- unregister_netdevice(dev);
- unregister_netdevice(peer);
+ unregister_netdevice_queue(dev, head);
+ unregister_netdevice_queue(peer, head);
}
static const struct nla_policy veth_policy[VETH_INFO_MAX + 1];
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
2009-10-30 7:00 [PATCH net-next-2.6] veth: Fix veth_dellink method Eric Dumazet
@ 2009-10-30 8:00 ` David Miller
2009-10-30 23:07 ` Eric W. Biederman
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2009-10-30 8:00 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, xemul
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Oct 2009 08:00:31 +0100
> In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
> (net: add a list_head parameter to dellink() method),
> I forgot to actually use this parameter in veth_dellink.
>
> I remember feeling a bit uncomfortable about veth_close(),
> because it does :
>
> netif_carrier_off(dev);
> netif_carrier_off(priv->peer);
>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
2009-10-30 7:00 [PATCH net-next-2.6] veth: Fix veth_dellink method Eric Dumazet
2009-10-30 8:00 ` David Miller
@ 2009-10-30 23:07 ` Eric W. Biederman
2009-10-31 0:51 ` [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth Eric W. Biederman
2009-10-31 9:43 ` [PATCH net-next-2.6] veth: Fix veth_dellink method Eric Dumazet
1 sibling, 2 replies; 7+ messages in thread
From: Eric W. Biederman @ 2009-10-30 23:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
Eric Dumazet <eric.dumazet@gmail.com> writes:
> In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
> (net: add a list_head parameter to dellink() method),
> I forgot to actually use this parameter in veth_dellink.
>
> I remember feeling a bit uncomfortable about veth_close(),
> because it does :
>
> netif_carrier_off(dev);
> netif_carrier_off(priv->peer);
>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index ffb502d..9bed694 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
> priv = netdev_priv(dev);
> peer = priv->peer;
>
> - unregister_netdevice(dev);
> - unregister_netdevice(peer);
> + unregister_netdevice_queue(dev, head);
> + unregister_netdevice_queue(peer, head);
Unless I am mistaken you need to change the list_add_tail to
list_move_tail in unregister_netdevice_queue because we will be
adding each veth device twice.
Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
2009-10-30 23:07 ` Eric W. Biederman
@ 2009-10-31 0:51 ` Eric W. Biederman
2009-10-31 9:41 ` Eric Dumazet
2009-10-31 9:43 ` [PATCH net-next-2.6] veth: Fix veth_dellink method Eric Dumazet
1 sibling, 1 reply; 7+ messages in thread
From: Eric W. Biederman @ 2009-10-31 0:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
I tested the recent unregister many changes and got a weird,
nasty and seemingly unrelasted kernel oops. Changing
unregister_netdevice_queue to use list_move_tail fixes
the problem for me.
ip link add type veth
rmmod veth
ls /sys/class/net/
showed one of the veth devices still present.
A subsequent ip link oopsed the box.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index 94f42a1..9c0b202 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5227,6 +5227,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
netdev_init_queues(dev);
INIT_LIST_HEAD(&dev->napi_list);
+ INIT_LIST_HEAD(&dev->unreg_list);
dev->priv_flags = IFF_XMIT_DST_RELEASE;
setup(dev);
strcpy(dev->name, name);
@@ -5308,7 +5309,7 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head)
ASSERT_RTNL();
if (head) {
- list_add_tail(&dev->unreg_list, head);
+ list_move_tail(&dev->unreg_list, head);
} else {
rollback_registered(dev);
/* Finish processing unregister after unlock */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
2009-10-31 0:51 ` [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth Eric W. Biederman
@ 2009-10-31 9:41 ` Eric Dumazet
2009-11-02 7:56 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2009-10-31 9:41 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
Eric W. Biederman a écrit :
> I tested the recent unregister many changes and got a weird,
> nasty and seemingly unrelasted kernel oops. Changing
> unregister_netdevice_queue to use list_move_tail fixes
> the problem for me.
>
> ip link add type veth
> rmmod veth
>
> ls /sys/class/net/
> showed one of the veth devices still present.
>
> A subsequent ip link oopsed the box.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
Yes, problem is __rtnl_kill_links() doesnt anymore restart
its loop and does :
for_each_netdev(net, dev) {
if (dev->rtnl_link_ops == ops)
ops->dellink(dev, &list_kill);
}
unregister_netdevice_many(&list_kill);
As veth wants in its dellink() method to unregister two netdevices,
we really want your fix or corrupt list_kill
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
2009-10-30 23:07 ` Eric W. Biederman
2009-10-31 0:51 ` [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth Eric W. Biederman
@ 2009-10-31 9:43 ` Eric Dumazet
1 sibling, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2009-10-31 9:43 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
Eric W. Biederman a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>
>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>> index ffb502d..9bed694 100644
>> --- a/drivers/net/veth.c
>> +++ b/drivers/net/veth.c
>> @@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
>> priv = netdev_priv(dev);
>> peer = priv->peer;
>>
>> - unregister_netdevice(dev);
>> - unregister_netdevice(peer);
>> + unregister_netdevice_queue(dev, head);
>> + unregister_netdevice_queue(peer, head);
>
> Unless I am mistaken you need to change the list_add_tail to
> list_move_tail in unregister_netdevice_queue because we will be
> adding each veth device twice.
>
You are right, and your patch is the solution to this problem
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
2009-10-31 9:41 ` Eric Dumazet
@ 2009-11-02 7:56 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2009-11-02 7:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: ebiederm, netdev, xemul
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 31 Oct 2009 10:41:39 +0100
> Eric W. Biederman a écrit :
>> I tested the recent unregister many changes and got a weird,
>> nasty and seemingly unrelasted kernel oops. Changing
>> unregister_netdevice_queue to use list_move_tail fixes
>> the problem for me.
>>
>> ip link add type veth
>> rmmod veth
>>
>> ls /sys/class/net/
>> showed one of the veth devices still present.
>>
>> A subsequent ip link oopsed the box.
>>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-11-02 7:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-30 7:00 [PATCH net-next-2.6] veth: Fix veth_dellink method Eric Dumazet
2009-10-30 8:00 ` David Miller
2009-10-30 23:07 ` Eric W. Biederman
2009-10-31 0:51 ` [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth Eric W. Biederman
2009-10-31 9:41 ` Eric Dumazet
2009-11-02 7:56 ` David Miller
2009-10-31 9:43 ` [PATCH net-next-2.6] veth: Fix veth_dellink method Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.