Takes > 1 second to delete macvlan with global IPv6 address on it.

Takes > 1 second to delete macvlan with global IPv6 address on it.

[Ben Greear]

This is on an otherwise lightly loaded 2.6.36 + hacks system, 12 physical interfaces,
and two VETH interfaces.

It's much faster to delete an interface when it has no IPv6 address:

[root@ct503-60 lanforge]# time ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan

real	0m0.005s
user	0m0.001s
sys	0m0.004s
[root@ct503-60 lanforge]# time ip link delete eth5#0

real	0m0.033s
user	0m0.001s
sys	0m0.005s
[root@ct503-60 lanforge]# ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan

[root@ct503-60 lanforge]# ip -6 addr add 2002::1/64 dev eth5#0
[root@ct503-60 lanforge]# time ip link delete eth5#0

real	0m1.030s
user	0m0.000s
sys	0m0.013s


Funny enough, if you explicitly remove the IPv6 addr first it seems
to run at normal speed (adding both operation's times together)

[root@ct503-60 lanforge]# ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
[root@ct503-60 lanforge]# ip -6 addr add 2002::1/64 dev eth5#0
[root@ct503-60 lanforge]# time ip -6 addr delete 2002::1/64 dev eth5#0

real	0m0.001s
user	0m0.000s
sys	0m0.001s
[root@ct503-60 lanforge]# time ip link delete eth5#0

real	0m0.028s
user	0m0.001s
sys	0m0.005s


Take it easy,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Takes > 1 second to delete macvlan with global IPv6 address on it.

[Eric Dumazet]

Le lundi 08 novembre 2010 à 16:20 -0800, Ben Greear a écrit :
> This is on an otherwise lightly loaded 2.6.36 + hacks system, 12 physical interfaces,
> and two VETH interfaces.
> 
> It's much faster to delete an interface when it has no IPv6 address:
> 
> [root@ct503-60 lanforge]# time ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
> 
> real	0m0.005s
> user	0m0.001s
> sys	0m0.004s
> [root@ct503-60 lanforge]# time ip link delete eth5#0
> 
> real	0m0.033s
> user	0m0.001s
> sys	0m0.005s
> [root@ct503-60 lanforge]# ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
> 
> [root@ct503-60 lanforge]# ip -6 addr add 2002::1/64 dev eth5#0
> [root@ct503-60 lanforge]# time ip link delete eth5#0
> 
> real	0m1.030s
> user	0m0.000s
> sys	0m0.013s
> 
> 
> Funny enough, if you explicitly remove the IPv6 addr first it seems
> to run at normal speed (adding both operation's times together)
> 
> [root@ct503-60 lanforge]# ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
> [root@ct503-60 lanforge]# ip -6 addr add 2002::1/64 dev eth5#0
> [root@ct503-60 lanforge]# time ip -6 addr delete 2002::1/64 dev eth5#0
> 
> real	0m0.001s
> user	0m0.000s
> sys	0m0.001s
> [root@ct503-60 lanforge]# time ip link delete eth5#0
> 
> real	0m0.028s
> user	0m0.001s
> sys	0m0.005s
> 

The key here is you have to wait a bit (2 seconds) between 
"ip -6 addr add..." and the "ip link delete", or it is fast.

So ipv6 misses a cleanup somewhere and a device refcount is held.

here is a debugging patch on current kernels :

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 072652d..820d9ed 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1799,6 +1799,7 @@ extern void netdev_run_todo(void);
  */
 static inline void dev_put(struct net_device *dev)
 {
+	WARN_ON(dev->reg_state == NETREG_UNREGISTERED);
 	irqsafe_cpu_dec(*dev->pcpu_refcnt);
 }
 
gives :

[  418.614227] ------------[ cut here ]------------
[  418.614281] WARNING: at include/linux/netdevice.h:1802 in6_dev_finish_destroy+0xc9/0xf0()
[  418.614348] Hardware name: ProLiant BL460c G6
[  418.614392] Modules linked in: macvlan ipmi_devintf ipmi_si ipmi_msghandler dm_mod tg3 libphy sg [last unloaded: x_tables]
[  418.614804] Pid: 5403, comm: ip Tainted: G        W   2.6.37-rc1-00186-g5c6f178-dirty #271
[  418.614857] Call Trace:
[  418.614901]  [<ffffffff814ecac9>] ? in6_dev_finish_destroy+0xc9/0xf0
[  418.614952]  [<ffffffff81046440>] warn_slowpath_common+0x90/0xc0
[  418.615002]  [<ffffffff8104648a>] warn_slowpath_null+0x1a/0x20
[  418.615051]  [<ffffffff814ecac9>] in6_dev_finish_destroy+0xc9/0xf0
[  418.615101]  [<ffffffff814f469e>] ip6_dst_ifdown+0x5e/0x60
[  418.615150]  [<ffffffff81448318>] dst_ifdown+0x38/0x110
[  418.615198]  [<ffffffff81448457>] dst_dev_event+0x67/0x130
[  418.615247]  [<ffffffff815d2888>] notifier_call_chain+0x58/0x80
[  418.615298]  [<ffffffff8106b86e>] __raw_notifier_call_chain+0xe/0x10
[  418.615348]  [<ffffffff8106b886>] raw_notifier_call_chain+0x16/0x20
[  418.615432]  [<ffffffff814408d7>] call_netdevice_notifiers+0x37/0x70
[  418.615496]  [<ffffffff81440a47>] netdev_run_todo+0x137/0x260
[  418.615560]  [<ffffffff8144f11e>] rtnl_unlock+0xe/0x10
[  418.615621]  [<ffffffff8144f18a>] rtnetlink_rcv+0x2a/0x40
[  418.615684]  [<ffffffff8148b043>] netlink_unicast+0x2c3/0x2d0
[  418.615747]  [<ffffffff81438a8b>] ? memcpy_fromiovec+0x7b/0xa0
[  418.615810]  [<ffffffff8148bddd>] netlink_sendmsg+0x24d/0x380
[  418.615874]  [<ffffffff8142dad0>] sock_sendmsg+0xc0/0xf0
[  418.615938]  [<ffffffff81458370>] ? verify_compat_iovec+0x80/0x130
[  418.616002]  [<ffffffff8142e894>] sys_sendmsg+0x1a4/0x340
[  418.616065]  [<ffffffff810dad46>] ? handle_mm_fault+0x676/0x8b0
[  418.616129]  [<ffffffff815d2610>] ? do_page_fault+0x2a0/0x4c0
[  418.616192]  [<ffffffff8142df09>] ? sys_recvmsg+0x49/0x70
[  418.616254]  [<ffffffff81457f14>] compat_sys_sendmsg+0x14/0x20
[  418.616317]  [<ffffffff81458cbf>] compat_sys_socketcall+0x1cf/0x220
[  418.616380]  [<ffffffff815cf1e5>] ? page_fault+0x25/0x30
[  418.616443]  [<ffffffff8102ec60>] sysenter_dispatch+0x7/0x2e
[  418.616520] ---[ end trace c2d75997b525ef59 ]---

[PATCH] net/dst: dst_dev_event() called after other notifiers

[Eric Dumazet]

Le lundi 08 novembre 2010 à 16:20 -0800, Ben Greear a écrit :
> This is on an otherwise lightly loaded 2.6.36 + hacks system, 12 physical interfaces,
> and two VETH interfaces.
> 
> It's much faster to delete an interface when it has no IPv6 address:
> 
> [root@ct503-60 lanforge]# time ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
> 
> real	0m0.005s
> user	0m0.001s
> sys	0m0.004s
> [root@ct503-60 lanforge]# time ip link delete eth5#0
> 
> real	0m0.033s
> user	0m0.001s
> sys	0m0.005s
> [root@ct503-60 lanforge]# ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
> 
> [root@ct503-60 lanforge]# ip -6 addr add 2002::1/64 dev eth5#0
> [root@ct503-60 lanforge]# time ip link delete eth5#0
> 
> real	0m1.030s
> user	0m0.000s
> sys	0m0.013s
> 
> 
> Funny enough, if you explicitly remove the IPv6 addr first it seems
> to run at normal speed (adding both operation's times together)
> 
> [root@ct503-60 lanforge]# ip link add link eth5 up name eth5#0 address 00:00:00:00:00:01 type macvlan
> [root@ct503-60 lanforge]# ip -6 addr add 2002::1/64 dev eth5#0
> [root@ct503-60 lanforge]# time ip -6 addr delete 2002::1/64 dev eth5#0
> 
> real	0m0.001s
> user	0m0.000s
> sys	0m0.001s
> [root@ct503-60 lanforge]# time ip link delete eth5#0
> 
> real	0m0.028s
> user	0m0.001s
> sys	0m0.005s
> 

OK, I confirm I already knew how to correct the problem.

http://www.spinics.net/lists/netdev/msg140729.html

quote :

I also believe the order of netdevice notifiers is wrong (we dont set
priority), and that we should call fib_netdev_event() _before_
dst_dev_event(). This needs another patch.


Thanks

[PATCH] net/dst: dst_dev_event() called after other notifiers

Followup of commit ef885afbf8a37689 (net: use rcu_barrier() in
rollback_registered_many)

dst_dev_event() scans a garbage dst list that might be feeded by various
network notifiers at device dismantle time.

Its important to call dst_dev_event() after other notifiers, or we might
enter the infamous msleep(250) in netdev_wait_allrefs(), and wait one
second before calling again call_netdevice_notifiers(NETDEV_UNREGISTER,
dev) to properly remove last device references.

Use priority -10 to let dst_dev_notifier be called after other network
notifiers (they have the default 0 priority)

Reported-by: Ben Greear <greearb@candelatech.com>
Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Reported-by: Octavian Purdila <opurdila@ixiacom.com>
Reported-by: Benjamin LaHaise <bcrl@kvack.org>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/dst.c |    1 +
 1 files changed, 1 insertion(+)

diff --git a/net/core/dst.c b/net/core/dst.c
index 8abe628..e234bf1 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -370,6 +370,7 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
 
 static struct notifier_block dst_dev_notifier = {
 	.notifier_call  = dst_dev_event,
+	.priority = -10, /* must be called after other network notifiers */
 };
 
 void __init dst_init(void)

Re: [PATCH] net/dst: dst_dev_event() called after other notifiers

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 09 Nov 2010 20:37:55 +0100

> [PATCH] net/dst: dst_dev_event() called after other notifiers

Nice, applied.

However, I had to apply this by hand:

>  static struct notifier_block dst_dev_notifier = {
>  	.notifier_call  = dst_dev_event,
> +	.priority = -10, /* must be called after other network notifiers */
>  };

The character after ".notifier_call" in my tree is a TAB character but
in your patch it is a sequence of spaces.  This isn't looking like the
usual email corruption, because the leading TAB characters on these
lines are properly there.

Please figure out why this happened so that it doesn't repeat in
future patches :-)

Thanks!

Re: [PATCH] net/dst: dst_dev_event() called after other notifiers

[Ben Greear]

On 11/09/2010 11:48 AM, David Miller wrote:
> From: Eric Dumazet<eric.dumazet@gmail.com>
> Date: Tue, 09 Nov 2010 20:37:55 +0100
>
>> [PATCH] net/dst: dst_dev_event() called after other notifiers
>
> Nice, applied.
>
> However, I had to apply this by hand:
>
>>   static struct notifier_block dst_dev_notifier = {
>>   	.notifier_call  = dst_dev_event,
>> +	.priority = -10, /* must be called after other network notifiers */
>>   };
>
> The character after ".notifier_call" in my tree is a TAB character but
> in your patch it is a sequence of spaces.  This isn't looking like the
> usual email corruption, because the leading TAB characters on these
> lines are properly there.
>
> Please figure out why this happened so that it doesn't repeat in
> future patches :-)

I manually applied this as well and can confirm that interface deletion
with a global IPv6 address on it is now comparable to any other device
delete (about 30ms).

Tested-by:  Ben Greear <greearb@candelatech.com>

I'd love to test patches that made all interface deletes faster,
btw :)

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] net/dst: dst_dev_event() called after other notifiers

[Eric Dumazet]

Le mardi 09 novembre 2010 à 11:48 -0800, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 09 Nov 2010 20:37:55 +0100
> 
> > [PATCH] net/dst: dst_dev_event() called after other notifiers
> 
> Nice, applied.
> 
> However, I had to apply this by hand:
> 
> >  static struct notifier_block dst_dev_notifier = {
> >  	.notifier_call  = dst_dev_event,
> > +	.priority = -10, /* must be called after other network notifiers */
> >  };
> 
> The character after ".notifier_call" in my tree is a TAB character but
> in your patch it is a sequence of spaces.  This isn't looking like the
> usual email corruption, because the leading TAB characters on these
> lines are properly there.
> 
> Please figure out why this happened so that it doesn't repeat in
> future patches :-)
> 

I am very sorry David, I had to run yesterday night and did a stupid
hand editing right before doing so. It was a human error, not a tool
error. Next time, I'll delay the patch to next day :)

Thanks !