Netdev List
 help / color / mirror / Atom feed
* Re: BUG ? ipip unregister_netdevice_many()
From: Hans Schillstrom @ 2010-10-08 11:53 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: netdev@vger.kernel.org, Eric W. Biederman
In-Reply-To: <4CAEFE2C.3010007@free.fr>

On Friday 08 October 2010 13:19:08 Daniel Lezcano wrote:
Hello
> On 10/07/2010 10:48 AM, Hans Schillstrom wrote:
> > Hello
> > I'm trying to exit a network name space and it doesn't work (or am I doing something wrong?)
> > The only netdevices left are lo and the tunnels ip6tnl0, sit0 and tunl0 when exiting netns.
> >
> > A netns is created by lxc-execute with two interfaces eth0 eth1 (macvlan)
> > (see conf file at the end)
> >
> > Kernel: net-next-2.6 top from 4 october 2010
> >
> 
> Hi Hans,
> 
> I tried to reproduce your problem but I just get a big kernel crash when
> exiting the container :/
> 
> The stack is different but it may be related to the same problem.
> 
> BUG: unable to handle kernel paging request at ffff88003ba453a0
> IP: [<ffffffff813020b6>] macvlan_stop+0x57/0x7d
> PGD 180b063 PUD 180f063 PMD 1ffdb067 PTE 3ba45160
> Oops: 0002 [#1] DEBUG_PAGEALLOC
> last sysfs file: /sys/devices/virtual/net/mc0PyXBA/type
> CPU 0
> Pid: 5, comm: kworker/u:0 Not tainted 2.6.36-rc7-next-20101007+ #11 /Bochs
> RIP: 0010:[<ffffffff813020b6>]  [<ffffffff813020b6>] macvlan_stop+0x57/0x7d
> RSP: 0018:ffff88003f111c30  EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff88003bdd1e60 RCX: 000000000000c100
> RDX: ffff88003ba453a0 RSI: ffff88003f111d70 RDI: ffffffff810300e5
> RBP: ffff88003f111c50 R08: ffff88003f111d70 R09: 00000000000000cc
> R10: 0000000000000001 R11: ffff88003f111ba0 R12: ffff88003bdd1800
> R13: ffff880039fec800 R14: ffff88003f111d70 R15: ffff88003ba06830
> FS:  0000000000000000(0000) GS:ffffffff8181b000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: ffff88003ba453a0 CR3: 000000003c284000 CR4: 00000000000006f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process kworker/u:0 (pid: 5, threadinfo ffff88003f110000, task
> ffff88003f0f20a0)
> Stack:
>   ffffffff81657a10 ffff88003bdd1800 ffffffff81657a10 ffff88003bb23800
> <0> ffff88003f111c70 ffffffff81362d09 ffff88003bdd1800 ffff88003f111cf0
> <0> ffff88003f111c90 ffffffff81362d31 ffff88003ba067c0 ffff88003bdd1800
> Call Trace:
>   [<ffffffff81362d09>] __dev_close+0x75/0x83
>   [<ffffffff81362d31>] dev_close+0x1a/0x3f
>   [<ffffffff81362e38>] rollback_registered_many+0xe2/0x21c
>   [<ffffffff81362f88>] unregister_netdevice_many+0x16/0x6d
>   [<ffffffff8136314d>] default_device_exit_batch+0xa7/0xbb
>   [<ffffffff8135db06>] ops_exit_list+0x4e/0x56
>   [<ffffffff8135e285>] cleanup_net+0xf5/0x195
>   [<ffffffff8103e084>] process_one_work+0x25d/0x3e7
>   [<ffffffff8103e027>] ? process_one_work+0x200/0x3e7
>   [<ffffffff8135e190>] ? cleanup_net+0x0/0x195
>   [<ffffffff8103e54a>] worker_thread+0x1b5/0x342
>   [<ffffffff8103e395>] ? worker_thread+0x0/0x342
>   [<ffffffff81041495>] kthread+0x7c/0x84
>   [<ffffffff810034f4>] kernel_thread_helper+0x4/0x10
>   [<ffffffff814389ba>] ? restore_args+0x0/0x30
>   [<ffffffff81041419>] ? kthread+0x0/0x84
>   [<ffffffff810034f0>] ? kernel_thread_helper+0x0/0x10
> Code: 00 00 02 74 0b 83 ce ff 4c 89 ef e8 ab eb 05 00 49 8b b4 24 a0 02
> 00 00 4c 89 ef e8 b9 52 06 00 48 8b 43 18 48 8b 53 20 48 85 c0 <48> 89
> 02 74 04 48 89 50 08 48 be 00 02 20 00 00 00 ad de 48 89
> RIP  [<ffffffff813020b6>] macvlan_stop+0x57/0x7d
>   RSP <ffff88003f111c30>
> CR2: ffff88003ba453a0
> ---[ end trace 05c41c2103816005 ]---
> BUG: unable to handle kernel paging request at fffffffffffffff8
> IP: [<ffffffff810410bf>] kthread_data+0xb/0x11
> PGD 180c067 PUD 180d067 PMD 0
> Oops: 0000 [#2] DEBUG_PAGEALLOC
> last sysfs file: /sys/devices/virtual/net/mc0PyXBA/type
> CPU 0
> Pid: 5, comm: kworker/u:0 Tainted: G      D
> 2.6.36-rc7-next-20101007+ #11 /Bochs
> RIP: 0010:[<ffffffff810410bf>]  [<ffffffff810410bf>] kthread_data+0xb/0x11
> RSP: 0018:ffff88003f111868  EFLAGS: 00010096
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88003f111fd8
> RDX: ffff88003f0f20a0 RSI: 0000000000000000 RDI: ffff88003f0f20a0
> RBP: ffff88003f111868 R08: 0000000000000002 R09: 0000000000000001
> R10: 0000000000000246 R11: 09f911029d74e35b R12: 0000000000000000
> R13: ffff88003f111948 R14: ffff88003f0c60a0 R15: ffff88003f0f2218
> FS:  0000000000000000(0000) GS:ffffffff8181b000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: fffffffffffffff8 CR3: 000000003cb19000 CR4: 00000000000006f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process kworker/u:0 (pid: 5, threadinfo ffff88003f110000, task
> ffff88003f0f20a0)
> Stack:
>   ffff88003f111888 ffffffff8103d4e4 ffff88003f111888 ffff88003f0f2310
> <0> ffff88003f110010 ffff88003f0f20a0 ffff88003f111fd8 ffff88003f111fd8
> Call Trace:
>   [<ffffffff8103d4e4>] wq_worker_sleeping+0x10/0x76
>   [<ffffffff81435ffe>] schedule+0xf4/0x405
>   [<ffffffff8102ebc4>] do_exit+0x647/0x660
>   [<ffffffff81005ba0>] oops_end+0xb3/0xbb
>   [<ffffffff8101c6b8>] no_context+0x1f5/0x204
>   [<ffffffff8101c854>] __bad_area_nosemaphore+0x18d/0x1b0
>   [<ffffffff8101c885>] bad_area_nosemaphore+0xe/0x10
>   [<ffffffff8101cb52>] do_page_fault+0x16b/0x34d
>   [<ffffffff81300a85>] ? ei_set_multicast_list+0x1f/0x3d
>   [<ffffffff81437f46>] ? trace_hardirqs_off_thunk+0x3a/0x3c
>   [<ffffffff81438b9f>] page_fault+0x1f/0x30
>   [<ffffffff810300e5>] ? local_bh_enable_ip+0xb7/0xbd
>   [<ffffffff813020b6>] ? macvlan_stop+0x57/0x7d
>   [<ffffffff81362d09>] __dev_close+0x75/0x83
>   [<ffffffff81362d31>] dev_close+0x1a/0x3f
>   [<ffffffff81362e38>] rollback_registered_many+0xe2/0x21c
>   [<ffffffff81362f88>] unregister_netdevice_many+0x16/0x6d
>   [<ffffffff8136314d>] default_device_exit_batch+0xa7/0xbb
>   [<ffffffff8135db06>] ops_exit_list+0x4e/0x56
>   [<ffffffff8135e285>] cleanup_net+0xf5/0x195
>   [<ffffffff8103e084>] process_one_work+0x25d/0x3e7
>   [<ffffffff8103e027>] ? process_one_work+0x200/0x3e7
>   [<ffffffff8135e190>] ? cleanup_net+0x0/0x195
>   [<ffffffff8103e54a>] worker_thread+0x1b5/0x342
>   [<ffffffff8103e395>] ? worker_thread+0x0/0x342
>   [<ffffffff81041495>] kthread+0x7c/0x84
>   [<ffffffff810034f4>] kernel_thread_helper+0x4/0x10
>   [<ffffffff814389ba>] ? restore_args+0x0/0x30
>   [<ffffffff81041419>] ? kthread+0x0/0x84
>   [<ffffffff810034f0>] ? kernel_thread_helper+0x0/0x10
> Code: 5c 41 5d 41 5e c9 c3 90 55 48 8b 04 25 40 a0 81 81 48 8b 80 18 02
> 00 00 48 89 e5 8b 40 f0 c9 c3 48 8b 87 18 02 00 00 55 48 89 e5 <48> 8b
> 40 f8 c9 c3 48 89 f0 c1 ee 06 55 89 f6 83 e0 3f 48 c1 e6
> RIP  [<ffffffff810410bf>] kthread_data+0xb/0x11
>   RSP <ffff88003f111868>
> CR2: fffffffffffffff8
> ---[ end trace 05c41c2103816006 ]---
> 
> 
> 
> Thanks
>    -- Daniel

I did the same setup without any tunnel modules loaded and then it almost worked 
except free-ing of the loopback interface :-(
"unregister_netdevice: waiting for lo to become free. Usage count = 4"

When adding a tunnel module (here ip6_tunnel) you'll have the crash

ex: 
/var/lib/lxc # ifconfig -a
ip6tnl0   Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00  
          NOARP  MTU:1460  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

lo        Link encap:Local Loopback  
          LOOPBACK  MTU:16436  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

 # ifconfig
 # ps
  PID USER       VSZ STAT COMMAND
    1 root     12412 S    /usr/lib64/lxc/lxc-init -- /var/bin/init
    2 root      4540 S    /bin/ash /var/bin/init
    6 root      4544 S    /bin/ash
   16 root      4544 R    ps
 # kill 2
 # ^D (exit of ns)
 # ------------[ cut here ]------------
WARNING: at /home/hans/evip/kvm/net-next-2.6/kernel/sysctl.c:1953 unregister_sysctl_table+0xc7/0xf9()
Hardware name: Bochs
Modules linked in: macvlan ip6_tunnel tunnel6 pcnet32 tg3 libphy
Pid: 5, comm: kworker/u:0 Not tainted 2.6.36-rc3 #2
Call Trace:
 [<ffffffff8103e281>] warn_slowpath_common+0x85/0x9d
 [<ffffffff8103e2b3>] warn_slowpath_null+0x1a/0x1c
 [<ffffffff81045e64>] unregister_sysctl_table+0xc7/0xf9
 [<ffffffff812c86a5>] neigh_sysctl_unregister+0x27/0x3f
 [<ffffffff81340c75>] addrconf_ifdown+0x415/0x45e
 [<ffffffff81341705>] addrconf_notify+0x756/0x7fe
 [<ffffffff812cacfb>] ? neigh_ifdown+0xc3/0xd4
 [<ffffffff81360eb3>] ? ip6mr_device_event+0x8d/0x9e
 [<ffffffff8105eddb>] notifier_call_chain+0x37/0x63
 [<ffffffff8105ee8b>] raw_notifier_call_chain+0x14/0x16
 [<ffffffff812c15c7>] call_netdevice_notifiers+0x4a/0x4f
 [<ffffffff812c1c1b>] rollback_registered_many+0x121/0x208
 [<ffffffff812c1d1d>] unregister_netdevice_many+0x1b/0x71
 [<ffffffffa0047244>] ip6_tnl_exit_net+0xa4/0xb8 [ip6_tunnel]
 [<ffffffff812bc941>] ? cleanup_net+0x0/0x198
 [<ffffffff812bc2cf>] ops_exit_list+0x2a/0x5b
 [<ffffffff812bca39>] cleanup_net+0xf8/0x198
 [<ffffffff810568c7>] process_one_work+0x2a2/0x44d
 [<ffffffff81056e35>] worker_thread+0x1db/0x34e
 [<ffffffff81056c5a>] ? worker_thread+0x0/0x34e
 [<ffffffff8105a030>] kthread+0x82/0x8a
 [<ffffffff81003954>] kernel_thread_helper+0x4/0x10
 [<ffffffff81059fae>] ? kthread+0x0/0x8a
 [<ffffffff81003950>] ? kernel_thread_helper+0x0/0x10
---[ end trace eb3bc950cf9a8748 ]---
unregister_netdevice: waiting for lo to become free. Usage count = 4
unregister_netdevice: waiting for lo to become free. Usage count = 4
unregister_netdevice: waiting for lo to become free. Usage count = 4

Regards 
Hans

> 
> > I added some printk's inn ipip.c  ipip_exit_net()
> > ...
> >          rtnl_lock();
> >          printk(KERN_ERR "ipip_exit_net(enter)\n");
> >          ipip_destroy_tunnels(ipn,&list);
> >          printk(KERN_ERR "ipip_exit_net(1)\n");
> >          unregister_netdevice_queue(ipn->fb_tunnel_dev,&list);
> >          printk(KERN_ERR "ipip_exit_net(2)\n");
> >          unregister_netdevice_many(&list);
> >          printk(KERN_ERR "ipip_exit_net(3)\n");
> >          rtnl_unlock();
> >          printk(KERN_ERR "ipip_exit_net(exit)\n");
> >
> >
> > Exit steps:
> > ===== Screen dump =====
> >
> >   # ifconfig eth0  0.0.0.0  down
> >   # ifconfig eth1  0.0.0.0  down
> >   # ifconfig lo  0.0.0.0  down
> >   # ip li de eth0
> >   # ip li de eth1
> >   # ifconfig -a
> > ip6tnl0   Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
> >            NOARP  MTU:1460  Metric:1
> >            RX packets:0 errors:0 dropped:0 overruns:0 frame:0
> >            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
> >            collisions:0 txqueuelen:0
> >            RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
> >
> > lo        Link encap:Local Loopback
> >            inet addr:127.0.0.1  Mask:255.0.0.0
> >            LOOPBACK  MTU:16436  Metric:1
> >            RX packets:0 errors:0 dropped:0 overruns:0 frame:0
> >            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
> >            collisions:0 txqueuelen:0
> >            RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
> >
> > sit0      Link encap:IPv6-in-IPv4
> >            NOARP  MTU:1480  Metric:1
> >            RX packets:0 errors:0 dropped:0 overruns:0 frame:0
> >            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
> >            collisions:0 txqueuelen:0
> >            RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
> >
> > tunl0     Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
> >            NOARP  MTU:1480  Metric:1
> >            RX packets:0 errors:0 dropped:0 overruns:0 frame:0
> >            TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
> >            collisions:0 txqueuelen:0
> >            RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
> >
> >   # ps
> >    PID USER       VSZ STAT COMMAND
> >      1 root     12412 S    /usr/lib64/lxc/lxc-init -- /var/bin/init
> >      2 root      4540 S    /bin/ash /var/bin/init
> >      7 root      6640 S    inetd
> >      8 root      4544 S    /bin/ash
> >     26 root      4544 R    ps
> >   # lsmod
> > Module                  Size  Used by    Not tainted
> > macvlan                 8709  0
> > pcnet32                29549  0
> > tg3                   112093  0
> > libphy                 21043  1 tg3
> >   # kill 7 2
> >   # ps
> >    PID USER       VSZ STAT COMMAND
> >      1 root     12412 S    /usr/lib64/lxc/lxc-init -- /var/bin/init
> >      8 root      4544 S    /bin/ash
> >     28 root      4544 R    ps
> >   # exit  ( here is the exit from netns  )
> >   # ipip_exit_net(enter)
> > ipip_exit_net(1)
> > ipip_exit_net(2)
> > ------------[ cut here ]------------
> > WARNING: at /home/hans/evip/kvm/net-next-2.6/kernel/sysctl.c:1953 unregister_sysctl_table+0xc7/0xf9()
> > Hardware name: Bochs
> > Modules linked in: macvlan pcnet32 tg3 libphy
> > Pid: 5, comm: kworker/u:0 Not tainted 2.6.36-rc3+ #7
> > Call Trace:
> >   [<ffffffff8103e281>] warn_slowpath_common+0x85/0x9d
> >   [<ffffffff8103e2b3>] warn_slowpath_null+0x1a/0x1c
> >   [<ffffffff81045e64>] unregister_sysctl_table+0xc7/0xf9
> >   [<ffffffff812c86a5>] neigh_sysctl_unregister+0x27/0x3f
> >   [<ffffffff81342108>] addrconf_ifdown+0x415/0x45e
> >   [<ffffffff81342b98>] addrconf_notify+0x756/0x7fe
> >   [<ffffffff812cacfb>] ? neigh_ifdown+0xc3/0xd4
> >   [<ffffffff813622b3>] ? ip6mr_device_event+0x8d/0x9e
> >   [<ffffffff8105eddb>] notifier_call_chain+0x37/0x63
> >   [<ffffffff8105ee8b>] raw_notifier_call_chain+0x14/0x16
> >   [<ffffffff812c15c7>] call_netdevice_notifiers+0x4a/0x4f
> >   [<ffffffff812c1c1b>] rollback_registered_many+0x121/0x208
> >   [<ffffffff812c1d1d>] unregister_netdevice_many+0x1b/0x71
> >   [<ffffffff81324209>] ipip_exit_net+0xea/0x11a
> >   [<ffffffff812bc941>] ? cleanup_net+0x0/0x198
> >   [<ffffffff812bc2cf>] ops_exit_list+0x2a/0x5b
> >   [<ffffffff812bca39>] cleanup_net+0xf8/0x198
> >   [<ffffffff810568c7>] process_one_work+0x2a2/0x44d
> >   [<ffffffff81056e35>] worker_thread+0x1db/0x34e
> >   [<ffffffff81056c5a>] ? worker_thread+0x0/0x34e
> >   [<ffffffff8105a030>] kthread+0x82/0x8a
> >   [<ffffffff81003954>] kernel_thread_helper+0x4/0x10
> >   [<ffffffff81059fae>] ? kthread+0x0/0x8a
> >   [<ffffffff81003950>] ? kernel_thread_helper+0x0/0x10
> > ---[ end trace 939b5185219f32e7 ]---
> > ipip_exit_net(3)
> > ipip_exit_net(exit)
> > unregister_netdevice: waiting for lo to become free. Usage count = 4
> > unregister_netdevice: waiting for lo to become free. Usage count = 4
> > unregister_netdevice: waiting for lo to become free. Usage count = 4
> > ....
> > ...
> > ===== End of screen dump =====
> >
> > lxc conf file:
> > # Container with network virtualized using the vlan device driver
> > # Local eth0 uplink
> > lxc.utsname = fee_0
> > lxc.network.type = macvlan
> > lxc.network.flags = up
> > lxc.network.link = eth1
> > lxc.network.hwaddr = 00:00:04:01:01:01
> > lxc.network.ipv4 = 192.168.1.21/24
> > lxc.network.ipv6 = 2003::2:1:1/96
> > # local eth1 downlink - to the RS farm
> > lxc.network.type = macvlan
> > lxc.network.flags = up
> > lxc.network.link = eth0
> > lxc.network.hwaddr = 00:00:03:01:01:01
> > lxc.network.ipv4 = 192.168.0.21/24
> > lxc.network.ipv6 = 2003::1:1:1/96
> > lxc.mount.entry = /var/lib/lxc/fee_0/var /var none rw,bind 0 0
> >
> 

-- 
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>

^ permalink raw reply

* Re: BUG ? ipip unregister_netdevice_many()
From: Hans Schillstrom @ 2010-10-08 12:28 UTC (permalink / raw)
  To: Daniel Lezcano, Eric W. Biederman; +Cc: netdev@vger.kernel.org
In-Reply-To: <201010081353.28056.hans.schillstrom@ericsson.com>

Hi Eric,
Any advice how to trace this down ?
This rollback_registered_many() seems to have on the lists before...
All IPv4 and IPv6 tunnels causes this crash, all you have to do is load the tunnel module(s)
enter a new ns and exit from it.

Have not tested any more devices than tunnels, 
I did an "ip link delete" on my macvlans before exiting the ns.


snip
>  # ------------[ cut here ]------------
> WARNING: at /home/hans/evip/kvm/net-next-2.6/kernel/sysctl.c:1953 unregister_sysctl_table+0xc7/0xf9()
> Hardware name: Bochs
> Modules linked in: macvlan ip6_tunnel tunnel6 pcnet32 tg3 libphy
> Pid: 5, comm: kworker/u:0 Not tainted 2.6.36-rc3 #2
> Call Trace:
>  [<ffffffff8103e281>] warn_slowpath_common+0x85/0x9d
>  [<ffffffff8103e2b3>] warn_slowpath_null+0x1a/0x1c
>  [<ffffffff81045e64>] unregister_sysctl_table+0xc7/0xf9
>  [<ffffffff812c86a5>] neigh_sysctl_unregister+0x27/0x3f
>  [<ffffffff81340c75>] addrconf_ifdown+0x415/0x45e
>  [<ffffffff81341705>] addrconf_notify+0x756/0x7fe
>  [<ffffffff812cacfb>] ? neigh_ifdown+0xc3/0xd4
>  [<ffffffff81360eb3>] ? ip6mr_device_event+0x8d/0x9e
>  [<ffffffff8105eddb>] notifier_call_chain+0x37/0x63
>  [<ffffffff8105ee8b>] raw_notifier_call_chain+0x14/0x16
>  [<ffffffff812c15c7>] call_netdevice_notifiers+0x4a/0x4f
>  [<ffffffff812c1c1b>] rollback_registered_many+0x121/0x208
>  [<ffffffff812c1d1d>] unregister_netdevice_many+0x1b/0x71
>  [<ffffffffa0047244>] ip6_tnl_exit_net+0xa4/0xb8 [ip6_tunnel]
>  [<ffffffff812bc941>] ? cleanup_net+0x0/0x198
>  [<ffffffff812bc2cf>] ops_exit_list+0x2a/0x5b
>  [<ffffffff812bca39>] cleanup_net+0xf8/0x198
>  [<ffffffff810568c7>] process_one_work+0x2a2/0x44d
>  [<ffffffff81056e35>] worker_thread+0x1db/0x34e
>  [<ffffffff81056c5a>] ? worker_thread+0x0/0x34e
>  [<ffffffff8105a030>] kthread+0x82/0x8a
>  [<ffffffff81003954>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81059fae>] ? kthread+0x0/0x8a
>  [<ffffffff81003950>] ? kernel_thread_helper+0x0/0x10
> ---[ end trace eb3bc950cf9a8748 ]---
> unregister_netdevice: waiting for lo to become free. Usage count = 4
> unregister_netdevice: waiting for lo to become free. Usage count = 4
> unregister_netdevice: waiting for lo to become free. Usage count = 4


-- 
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>

^ permalink raw reply

* [PATCH 1/4] Phonet: add to MAINTAINERS and add myself
From: Rémi Denis-Courmont @ 2010-10-08 14:02 UTC (permalink / raw)
  To: netdev; +Cc: Rémi Denis-Courmont

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 MAINTAINERS |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ddb5ac..1fd58ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4541,6 +4541,14 @@ L:	linux-abi-devel@lists.sourceforge.net
 S:	Maintained
 F:	include/linux/personality.h
 
+PHONET PROTOCOL
+M:	Remi Denis-Courmont <remi.denis-courmont@nokia.com>
+S:	Supported
+F:	Documentation/networking/phonet.txt
+F:	include/linux/phonet.h
+F:	include/net/phonet/
+F:	net/phonet/
+
 PHRAM MTD DRIVER
 M:	Joern Engel <joern@lazybastard.org>
 L:	linux-mtd@lists.infradead.org
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/4] Phonet: advise against enabling the pipe controller
From: Rémi Denis-Courmont @ 2010-10-08 14:02 UTC (permalink / raw)
  To: netdev; +Cc: Rémi Denis-Courmont
In-Reply-To: <1286546523-3340-1-git-send-email-remi@remlab.net>

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

As it currently is, the new code path is not compatible with existing
Nokia modems. This would break existing userspace for Nokia modem, such
as the existing oFono ISI driver.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/Kconfig |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/phonet/Kconfig b/net/phonet/Kconfig
index 901956a..a4fceb8 100644
--- a/net/phonet/Kconfig
+++ b/net/phonet/Kconfig
@@ -24,4 +24,5 @@ config PHONET_PIPECTRLR
 	  data with Nokia Slim modems like WG2.5 used on ST-Ericsson U8500
 	  platform.
 
-	  If unsure, say N.
+	  This option is incompatible with older Nokia modems.
+	  Say N here unless you really know what you are doing.
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/4] Phonet: cleanup pipe enable socket option
From: Rémi Denis-Courmont @ 2010-10-08 14:02 UTC (permalink / raw)
  To: netdev; +Cc: Rémi Denis-Courmont, Kumar Sanghvi
In-Reply-To: <1286546523-3340-2-git-send-email-remi@remlab.net>

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

The current code works like this:

  int garbage, status;
  socklen_t len = sizeof(status);

  /* enable pipe */
  setsockopt(fd, SOL_PNPIPE, PNPIPE_ENABLE, &garbage, sizeof(garbage));
  /* disable pipe */
  setsockopt(fd, SOL_PNPIPE, PNPIPE_DISABLE, &garbage, sizeof(garbage));
  /* get status */
  getsockopt(fd, SOL_PNPIPE, PNPIPE_INQ, &status, &len);

...which does not follow the usual socket option pattern. This patch
merges all three "options" into a single gettable&settable option,
before Linux 2.6.37 gets out:

  int status;
  socklen_t len = sizeof(status);

  /* enable pipe */
  status = 1;
  setsockopt(fd, SOL_PNPIPE, PNPIPE_ENABLE, &status, sizeof(status));
  /* disable pipe */
  status = 0;
  setsockopt(fd, SOL_PNPIPE, PNPIPE_ENABLE, &status, sizeof(status));
  /* get status */
  getsockopt(fd, SOL_PNPIPE, PNPIPE_ENABLE, &status, &len);

This also fixes the error code from EFAULT to ENOTCONN.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Cc: Kumar Sanghvi <kumar.sanghvi@stericsson.com>
---
 Documentation/networking/phonet.txt |   15 +------
 include/linux/phonet.h              |    3 +-
 net/phonet/pep.c                    |   72 ++++++++++++++--------------------
 3 files changed, 34 insertions(+), 56 deletions(-)

diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index cccf5ff..2d9bc2b 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -213,12 +213,9 @@ The implementation adds socket options at SOL_PNPIPE level:
 	It then updates the pipe state associated with the sequenced socket to
 	be PIPE_DISABLED.
 
-  PNPIPE_ENABLE
-	It follows the same sequence as above for enabling a pipe by sending
-	PNS_PEP_ENABLE_REQ initially and then sending PNS_PEP_ENABLED_IND after
-	getting responses from sequenced socket and remote-pep.
-	It will also update the pipe state associated with the sequenced socket
-	to PIPE_ENABLED.
+  PNPIPE_ENABLE accepts one integer value (int). If set to zero, the pipe
+    is disabled. If the value is non-zero, the pipe is enabled. If the pipe
+    is not (yet) connected, ENOTCONN is error is returned.
 
    PNPIPE_DESTROY
 	This will send out PNS_PEP_DISCONNECT_REQ on the sequenced socket and
@@ -226,12 +223,6 @@ The implementation adds socket options at SOL_PNPIPE level:
 	It will also update the pipe state associated with the sequenced socket
 	to PIPE_IDLE
 
-   PNPIPE_INQ
-	This getsocktopt allows the user-space running on the sequenced socket
-	to examine the pipe state associated with that socket ie. whether the
-	pipe is created (PIPE_DISABLED) or enabled (PIPE_ENABLED) or disabled
-	(PIPE_DISABLED) or no pipe exists (PIPE_IDLE).
-
 After a pipe has been created and enabled successfully, the Pipe data can be
 exchanged between the host-pep and remote-pep (modem).
 
diff --git a/include/linux/phonet.h b/include/linux/phonet.h
index 96f5625..e27cbf9 100644
--- a/include/linux/phonet.h
+++ b/include/linux/phonet.h
@@ -38,9 +38,8 @@
 #define PNPIPE_IFINDEX		2
 #define PNPIPE_CREATE           3
 #define PNPIPE_ENABLE           4
-#define PNPIPE_DISABLE          5
+/* unused slot */
 #define PNPIPE_DESTROY          6
-#define PNPIPE_INQ              7
 
 #define PNADDR_ANY		0
 #define PNADDR_BROADCAST	0xFC
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index aa3d870..f818f76 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -327,29 +327,20 @@ static int pipe_handler_send_ind(struct sock *sk, u16 dobj, u8 utid,
 	return pn_skb_send(sk, skb, &spn);
 }
 
-static int pipe_handler_enable_pipe(struct sock *sk, int cmd)
+static int pipe_handler_enable_pipe(struct sock *sk, int enable)
 {
-	int ret;
 	struct pep_sock *pn = pep_sk(sk);
-
-	switch (cmd) {
-	case PNPIPE_ENABLE:
-		ret = pipe_handler_send_req(sk, pn->pn_sk.sobject,
-				PNS_PIPE_ENABLE_UTID, PNS_PEP_ENABLE_REQ,
-				pn->pipe_handle, GFP_ATOMIC);
-		break;
-
-	case PNPIPE_DISABLE:
-		ret = pipe_handler_send_req(sk, pn->pn_sk.sobject,
-				PNS_PIPE_DISABLE_UTID, PNS_PEP_DISABLE_REQ,
-				pn->pipe_handle, GFP_ATOMIC);
-		break;
-
-	default:
-		ret = -EINVAL;
+	int utid, req;
+
+	if (enable) {
+		utid = PNS_PIPE_ENABLE_UTID;
+		req = PNS_PEP_ENABLE_REQ;
+	} else {
+		utid = PNS_PIPE_DISABLE_UTID;
+		req = PNS_PEP_DISABLE_REQ;
 	}
-
-	return ret;
+	return pipe_handler_send_req(sk, pn->pn_sk.sobject, utid, req,
+			pn->pipe_handle, GFP_ATOMIC);
 }
 
 static int pipe_handler_create_pipe(struct sock *sk, int pipe_handle, int cmd)
@@ -1187,23 +1178,6 @@ static int pep_setsockopt(struct sock *sk, int level, int optname,
 			break;
 		}
 
-	case PNPIPE_ENABLE:
-		if (pn->pipe_state != PIPE_DISABLED) {
-			err = -EFAULT;
-			break;
-		}
-		err = pipe_handler_enable_pipe(sk, PNPIPE_ENABLE);
-		break;
-
-	case PNPIPE_DISABLE:
-		if (pn->pipe_state != PIPE_ENABLED) {
-			err = -EFAULT;
-			break;
-		}
-
-		err = pipe_handler_enable_pipe(sk, PNPIPE_DISABLE);
-		break;
-
 	case PNPIPE_DESTROY:
 		if (pn->pipe_state < PIPE_DISABLED) {
 			err = -EFAULT;
@@ -1239,6 +1213,17 @@ static int pep_setsockopt(struct sock *sk, int level, int optname,
 			err = 0;
 		}
 		goto out_norel;
+
+#ifdef CONFIG_PHONET_PIPECTRLR
+	case PNPIPE_ENABLE:
+		if (pn->pipe_state <= PIPE_IDLE) {
+			err = -ENOTCONN;
+			break;
+		}
+		err = pipe_handler_enable_pipe(sk, val);
+		break;
+#endif
+
 	default:
 		err = -ENOPROTOOPT;
 	}
@@ -1264,15 +1249,18 @@ static int pep_getsockopt(struct sock *sk, int level, int optname,
 		val = pn->ifindex ? PNPIPE_ENCAP_IP : PNPIPE_ENCAP_NONE;
 		break;
 
+	case PNPIPE_IFINDEX:
+		val = pn->ifindex;
+		break;
+
 #ifdef CONFIG_PHONET_PIPECTRLR
-	case PNPIPE_INQ:
-		val = pn->pipe_state;
+	case PNPIPE_ENABLE:
+		if (pn->pipe_state <= PIPE_IDLE)
+			return -ENOTCONN;
+		val = pn->pipe_state != PIPE_DISABLED;
 		break;
 #endif
 
-	case PNPIPE_IFINDEX:
-		val = pn->ifindex;
-		break;
 	default:
 		return -ENOPROTOOPT;
 	}
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 4/4] Phonet: mark the pipe controller as EXPERIMENTAL
From: Rémi Denis-Courmont @ 2010-10-08 14:02 UTC (permalink / raw)
  To: netdev; +Cc: Rémi Denis-Courmont
In-Reply-To: <1286546523-3340-3-git-send-email-remi@remlab.net>

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

There are a bunch of issues that need to be fixed, including:
 - GFP_KERNEL allocations from atomic context
   (and GFP_ATOMIC in process context),
 - abuse of the setsockopt() call convention,
 - unprotected/unlocked static variables...

IMHO, we will need to alter the userspace ABI when we fix it. So mark
the configuration option as EXPERIMENTAL for the time being (or should
it be BROKEN instead?).

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/phonet/Kconfig b/net/phonet/Kconfig
index a4fceb8..0d9b8a2 100644
--- a/net/phonet/Kconfig
+++ b/net/phonet/Kconfig
@@ -16,8 +16,8 @@ config PHONET
 	  will be called phonet. If unsure, say N.
 
 config PHONET_PIPECTRLR
-	bool "Phonet Pipe Controller"
-	depends on PHONET
+	bool "Phonet Pipe Controller (EXPERIMENTAL)"
+	depends on PHONET && EXPERIMENTAL
 	default N
 	help
 	  The Pipe Controller implementation in Phonet stack to support Pipe
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH] ehea: Fix a checksum issue on the receive path
From: Breno Leitao @ 2010-10-08 14:14 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, netdev, Jay Vosburgh
In-Reply-To: <1286513130.6536.467.camel@edumazet-laptop>

Hi Eric

On 10/08/2010 01:45 AM, Eric Dumazet wrote:
> Just to be clear : packets with wrong checksums are not given to upper
> stack, so a tcpdump can not display them ? I am not sure many drivers do
> that.
Well, what my code does is: 1) if the current packet is a UDP/TCP, then 
the checksum is not necessary, since we would check the checksum on 
ehea_proc_rwqes(), specific at this part of the code:

                if (!ehea_check_cqe(cqe, &rq)) {
			// Send the packet to the up layers

And ehea_check_cqe() checks for wrong checksumed packets on:
	
         if ((cqe->status & EHEA_CQE_STAT_ERR_MASK) == 0)
                 return 0;


Botton line, TCP/UDP packets with wrong checksums are dropped by 
ehea_proc_rwqes(), others go to the up layer.

So, back to your question, you are saying that we shouldn't do that, 
meaning that we should send to the upper layers all packets ? even those 
that have the wrong checksum ?

Thanks
Breno

^ permalink raw reply

* [PATCH 1/2] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Stanislaw Gruszka @ 2010-10-08 14:25 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Stanislaw Gruszka

We have fedora bug report where driver fail to initialize after
suspend/resume because of memory allocation errors:
https://bugzilla.redhat.com/show_bug.cgi?id=629158

To fix use GFP_KERNEL allocation where possible.

Tested-by: Neal Becker <ndbecker2@gmail.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/r8169.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index fe3b762..a7fb044 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4006,7 +4006,7 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
 static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
 					    struct net_device *dev,
 					    struct RxDesc *desc, int rx_buf_sz,
-					    unsigned int align)
+					    unsigned int align, gfp_t gfp)
 {
 	struct sk_buff *skb;
 	dma_addr_t mapping;
@@ -4014,7 +4014,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
 
 	pad = align ? align : NET_IP_ALIGN;
 
-	skb = netdev_alloc_skb(dev, rx_buf_sz + pad);
+	skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
 	if (!skb)
 		goto err_out;
 
@@ -4045,7 +4045,7 @@ static void rtl8169_rx_clear(struct rtl8169_private *tp)
 }
 
 static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
-			   u32 start, u32 end)
+			   u32 start, u32 end, gfp_t gfp)
 {
 	u32 cur;
 
@@ -4060,7 +4060,7 @@ static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
 
 		skb = rtl8169_alloc_rx_skb(tp->pci_dev, dev,
 					   tp->RxDescArray + i,
-					   tp->rx_buf_sz, tp->align);
+					   tp->rx_buf_sz, tp->align, gfp);
 		if (!skb)
 			break;
 
@@ -4088,7 +4088,7 @@ static int rtl8169_init_ring(struct net_device *dev)
 	memset(tp->tx_skb, 0x0, NUM_TX_DESC * sizeof(struct ring_info));
 	memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
 
-	if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC) != NUM_RX_DESC)
+	if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
 		goto err_out;
 
 	rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
@@ -4587,7 +4587,7 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 	count = cur_rx - tp->cur_rx;
 	tp->cur_rx = cur_rx;
 
-	delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx);
+	delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
 	if (!delta && count)
 		netif_info(tp, intr, dev, "no Rx buffer allocated\n");
 	tp->dirty_rx += delta;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/2] r8169: use device model DMA API
From: Stanislaw Gruszka @ 2010-10-08 14:25 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Stanislaw Gruszka
In-Reply-To: <1286547901-10782-1-git-send-email-sgruszka@redhat.com>

Use DMA API as PCI equivalents will be deprecated. This change also
allow to allocate with GFP_KERNEL where possible.

Tested-by: Neal Becker <ndbecker2@gmail.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/r8169.c |   53 +++++++++++++++++++++++++++-----------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index a7fb044..bc669a4 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1217,7 +1217,8 @@ static void rtl8169_update_counters(struct net_device *dev)
 	if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0)
 		return;
 
-	counters = pci_alloc_consistent(tp->pci_dev, sizeof(*counters), &paddr);
+	counters = dma_alloc_coherent(&tp->pci_dev->dev, sizeof(*counters),
+				      &paddr, GFP_KERNEL);
 	if (!counters)
 		return;
 
@@ -1238,7 +1239,8 @@ static void rtl8169_update_counters(struct net_device *dev)
 	RTL_W32(CounterAddrLow, 0);
 	RTL_W32(CounterAddrHigh, 0);
 
-	pci_free_consistent(tp->pci_dev, sizeof(*counters), counters, paddr);
+	dma_free_coherent(&tp->pci_dev->dev, sizeof(*counters), counters,
+			  paddr);
 }
 
 static void rtl8169_get_ethtool_stats(struct net_device *dev,
@@ -3298,15 +3300,15 @@ static int rtl8169_open(struct net_device *dev)
 
 	/*
 	 * Rx and Tx desscriptors needs 256 bytes alignment.
-	 * pci_alloc_consistent provides more.
+	 * dma_alloc_coherent provides more.
 	 */
-	tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
-					       &tp->TxPhyAddr);
+	tp->TxDescArray = dma_alloc_coherent(&pdev->dev, R8169_TX_RING_BYTES,
+					     &tp->TxPhyAddr, GFP_KERNEL);
 	if (!tp->TxDescArray)
 		goto err_pm_runtime_put;
 
-	tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
-					       &tp->RxPhyAddr);
+	tp->RxDescArray = dma_alloc_coherent(&pdev->dev, R8169_RX_RING_BYTES,
+					     &tp->RxPhyAddr, GFP_KERNEL);
 	if (!tp->RxDescArray)
 		goto err_free_tx_0;
 
@@ -3340,12 +3342,12 @@ out:
 err_release_ring_2:
 	rtl8169_rx_clear(tp);
 err_free_rx_1:
-	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
-			    tp->RxPhyAddr);
+	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
+			  tp->RxPhyAddr);
 	tp->RxDescArray = NULL;
 err_free_tx_0:
-	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
-			    tp->TxPhyAddr);
+	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
+			  tp->TxPhyAddr);
 	tp->TxDescArray = NULL;
 err_pm_runtime_put:
 	pm_runtime_put_noidle(&pdev->dev);
@@ -3981,7 +3983,7 @@ static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
 {
 	struct pci_dev *pdev = tp->pci_dev;
 
-	pci_unmap_single(pdev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
+	dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
 			 PCI_DMA_FROMDEVICE);
 	dev_kfree_skb(*sk_buff);
 	*sk_buff = NULL;
@@ -4020,7 +4022,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
 
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
-	mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
+	mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
 				 PCI_DMA_FROMDEVICE);
 
 	rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
@@ -4105,7 +4107,8 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
 {
 	unsigned int len = tx_skb->len;
 
-	pci_unmap_single(pdev, le64_to_cpu(desc->addr), len, PCI_DMA_TODEVICE);
+	dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
+			 PCI_DMA_TODEVICE);
 	desc->opts1 = 0x00;
 	desc->opts2 = 0x00;
 	desc->addr = 0x00;
@@ -4249,7 +4252,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 		txd = tp->TxDescArray + entry;
 		len = frag->size;
 		addr = ((void *) page_address(frag->page)) + frag->page_offset;
-		mapping = pci_map_single(tp->pci_dev, addr, len, PCI_DMA_TODEVICE);
+		mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
+					 PCI_DMA_TODEVICE);
 
 		/* anti gcc 2.95.3 bugware (sic) */
 		status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
@@ -4319,7 +4323,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 		tp->tx_skb[entry].skb = skb;
 	}
 
-	mapping = pci_map_single(tp->pci_dev, skb->data, len, PCI_DMA_TODEVICE);
+	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
+				 PCI_DMA_TODEVICE);
 
 	tp->tx_skb[entry].len = len;
 	txd->addr = cpu_to_le64(mapping);
@@ -4482,8 +4487,8 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
 	if (!skb)
 		goto out;
 
-	pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
-				    PCI_DMA_FROMDEVICE);
+	dma_sync_single_for_cpu(&tp->pci_dev->dev, addr, pkt_size,
+				PCI_DMA_FROMDEVICE);
 	skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
 	*sk_buff = skb;
 	done = true;
@@ -4552,11 +4557,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 			}
 
 			if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
-				pci_dma_sync_single_for_device(pdev, addr,
+				dma_sync_single_for_device(&pdev->dev, addr,
 					pkt_size, PCI_DMA_FROMDEVICE);
 				rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
 			} else {
-				pci_unmap_single(pdev, addr, tp->rx_buf_sz,
+				dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
 						 PCI_DMA_FROMDEVICE);
 				tp->Rx_skbuff[entry] = NULL;
 			}
@@ -4773,10 +4778,10 @@ static int rtl8169_close(struct net_device *dev)
 
 	free_irq(dev->irq, dev);
 
-	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
-			    tp->RxPhyAddr);
-	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
-			    tp->TxPhyAddr);
+	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
+			  tp->RxPhyAddr);
+	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
+			  tp->TxPhyAddr);
 	tp->TxDescArray = NULL;
 	tp->RxDescArray = NULL;
 
-- 
1.7.1


^ permalink raw reply related

* [RFC PATCH 1/2] r8169: check dma mapping failures
From: Stanislaw Gruszka @ 2010-10-08 14:30 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka

This is on top on my two r8169 patches just send.

Check possible dma mapping errors and do clean up if it happens.
Patch was not tested.

BTW: I see many drivers do not check these, so is really possible to
have this errors, and if yes, when ?
---
 drivers/net/r8169.c |   36 ++++++++++++++++++++++++++++--------
 1 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index bc669a4..b3b28b1 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4018,20 +4018,24 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
 
 	skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
 	if (!skb)
-		goto err_out;
+		goto err0;
 
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
 	mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
 				 PCI_DMA_FROMDEVICE);
+	if (dma_mapping_error(&pdev->dev, mapping))
+		goto err1;
 
 	rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
-out:
+
 	return skb;
 
-err_out:
+err1:
+	dev_kfree_skb(skb);
+err0:
 	rtl8169_make_unusable_by_asic(desc);
-	goto out;
+	return NULL;
 }
 
 static void rtl8169_rx_clear(struct rtl8169_private *tp)
@@ -4115,11 +4119,11 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
 	tx_skb->len = 0;
 }
 
-static void rtl8169_tx_clear(struct rtl8169_private *tp)
+static void rtl8169_tx_clear_range(struct rtl8169_private *tp, u32 start, u32 end)
 {
-	unsigned int i;
+	u32 i;
 
-	for (i = tp->dirty_tx; i < tp->dirty_tx + NUM_TX_DESC; i++) {
+	for (i = start; i < end; i++) {
 		unsigned int entry = i % NUM_TX_DESC;
 		struct ring_info *tx_skb = tp->tx_skb + entry;
 		unsigned int len = tx_skb->len;
@@ -4136,6 +4140,11 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 			tp->dev->stats.tx_dropped++;
 		}
 	}
+}
+
+static inline void rtl8169_tx_clear(struct rtl8169_private *tp)
+{
+	rtl8169_tx_clear_range(tp, tp->dirty_tx, tp->dirty_tx + NUM_TX_DESC);
 	tp->cur_tx = tp->dirty_tx = 0;
 }
 
@@ -4254,6 +4263,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
 		addr = ((void *) page_address(frag->page)) + frag->page_offset;
 		mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
 					 PCI_DMA_TODEVICE);
+		if (dma_mapping_error(&tp->pci_dev->dev, mapping))
+			return -cur_frag;
 
 		/* anti gcc 2.95.3 bugware (sic) */
 		status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
@@ -4314,7 +4325,10 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	opts1 = DescOwn | rtl8169_tso_csum(skb, dev);
 
 	frags = rtl8169_xmit_frags(tp, skb, opts1);
-	if (frags) {
+	if (frags < 0) {
+		frags = -frags;
+		goto err_dma;
+	} else if (frags) {
 		len = skb_headlen(skb);
 		opts1 |= FirstFrag;
 	} else {
@@ -4325,6 +4339,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
 				 PCI_DMA_TODEVICE);
+	if (dma_mapping_error(&tp->pci_dev->dev, mapping))
+		goto err_dma;
 
 	tp->tx_skb[entry].len = len;
 	txd->addr = cpu_to_le64(mapping);
@@ -4355,6 +4371,10 @@ err_stop:
 	netif_stop_queue(dev);
 	dev->stats.tx_dropped++;
 	return NETDEV_TX_BUSY;
+
+err_dma:
+	rtl8169_tx_clear_range(tp, entry, entry + frags + 1);
+	return NETDEV_TX_OK;
 }
 
 static void rtl8169_pcierr_interrupt(struct net_device *dev)
-- 
1.7.1


^ permalink raw reply related

* [RFC PATCH 2/2] r8169: reduce number of functions arguments
From: Stanislaw Gruszka @ 2010-10-08 14:30 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Denis Kirjanov, Stanislaw Gruszka
In-Reply-To: <1286548203-10831-1-git-send-email-sgruszka@redhat.com>

We don't need to pass arguments on stack since we have them in per
device private structure. Patch was not tested.
---
 drivers/net/r8169.c |   30 ++++++++++++------------------
 1 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index b3b28b1..65d4219 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4005,29 +4005,26 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
 	rtl8169_mark_to_asic(desc, rx_buf_sz);
 }
 
-static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
-					    struct net_device *dev,
-					    struct RxDesc *desc, int rx_buf_sz,
-					    unsigned int align, gfp_t gfp)
+static struct sk_buff *rtl8169_alloc_rx_skb(struct rtl8169_private *tp,
+					    struct RxDesc *desc, gfp_t gfp)
 {
 	struct sk_buff *skb;
 	dma_addr_t mapping;
-	unsigned int pad;
+	unsigned int align = tp->align;
+	unsigned int pad = align ? align : NET_IP_ALIGN;
 
-	pad = align ? align : NET_IP_ALIGN;
-
-	skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
+	skb = __netdev_alloc_skb(tp->dev, tp->rx_buf_sz + pad, gfp);
 	if (!skb)
 		goto err0;
 
 	skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
 
-	mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
+	mapping = dma_map_single(&tp->pci_dev->dev, skb->data, tp->rx_buf_sz,
 				 PCI_DMA_FROMDEVICE);
-	if (dma_mapping_error(&pdev->dev, mapping))
+	if (dma_mapping_error(&tp->pci_dev->dev, mapping))
 		goto err1;
 
-	rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
+	rtl8169_map_to_asic(desc, mapping, tp->rx_buf_sz);
 
 	return skb;
 
@@ -4050,8 +4047,7 @@ static void rtl8169_rx_clear(struct rtl8169_private *tp)
 	}
 }
 
-static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
-			   u32 start, u32 end, gfp_t gfp)
+static u32 rtl8169_rx_fill(struct rtl8169_private *tp, u32 start, u32 end, gfp_t gfp)
 {
 	u32 cur;
 
@@ -4064,9 +4060,7 @@ static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
 		if (tp->Rx_skbuff[i])
 			continue;
 
-		skb = rtl8169_alloc_rx_skb(tp->pci_dev, dev,
-					   tp->RxDescArray + i,
-					   tp->rx_buf_sz, tp->align, gfp);
+		skb = rtl8169_alloc_rx_skb(tp, tp->RxDescArray + i, gfp);
 		if (!skb)
 			break;
 
@@ -4094,7 +4088,7 @@ static int rtl8169_init_ring(struct net_device *dev)
 	memset(tp->tx_skb, 0x0, NUM_TX_DESC * sizeof(struct ring_info));
 	memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
 
-	if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
+	if (rtl8169_rx_fill(tp, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
 		goto err_out;
 
 	rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
@@ -4612,7 +4606,7 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
 	count = cur_rx - tp->cur_rx;
 	tp->cur_rx = cur_rx;
 
-	delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
+	delta = rtl8169_rx_fill(tp, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
 	if (!delta && count)
 		netif_info(tp, intr, dev, "no Rx buffer allocated\n");
 	tp->dirty_rx += delta;
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH] ehea: Fix a checksum issue on the receive path
From: Eric Dumazet @ 2010-10-08 14:36 UTC (permalink / raw)
  To: Breno Leitao; +Cc: davem, netdev, Jay Vosburgh
In-Reply-To: <4CAF2732.90703@linux.vnet.ibm.com>

Le vendredi 08 octobre 2010 à 11:14 -0300, Breno Leitao a écrit :
> Hi Eric
> 
> On 10/08/2010 01:45 AM, Eric Dumazet wrote:
> > Just to be clear : packets with wrong checksums are not given to upper
> > stack, so a tcpdump can not display them ? I am not sure many drivers do
> > that.
> Well, what my code does is: 1) if the current packet is a UDP/TCP, then 
> the checksum is not necessary, since we would check the checksum on 
> ehea_proc_rwqes(), specific at this part of the code:
> 
>                 if (!ehea_check_cqe(cqe, &rq)) {
> 			// Send the packet to the up layers
> 
> And ehea_check_cqe() checks for wrong checksumed packets on:
> 	
>          if ((cqe->status & EHEA_CQE_STAT_ERR_MASK) == 0)
>                  return 0;
> 
> 
> Botton line, TCP/UDP packets with wrong checksums are dropped by 
> ehea_proc_rwqes(), others go to the up layer.
> 
> So, back to your question, you are saying that we shouldn't do that, 
> meaning that we should send to the upper layers all packets ? even those 
> that have the wrong checksum ?
> 

I am pretty sure most (if not all) netdev drivers pass the packet with
invalid checksum to upper stack, so that we can increment appropriate
SNMP counters, in IP stack or UDP/TCP/whatever stack.

tg3, bnx2, e1000, skge, sky2, bnx2x, niu, r8169, igb, ... seems to do
that.




^ permalink raw reply

* Re: [PATCH 1/2] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Stanislaw Gruszka @ 2010-10-08 14:52 UTC (permalink / raw)
  To: Francois Romieu, netdev
In-Reply-To: <1286547901-10782-1-git-send-email-sgruszka@redhat.com>

On Fri, Oct 08, 2010 at 04:25:00PM +0200, Stanislaw Gruszka wrote:
> We have fedora bug report where driver fail to initialize after
> suspend/resume because of memory allocation errors:
> https://bugzilla.redhat.com/show_bug.cgi?id=629158

There is also one more thing to do regarding above. Calltraces from bug
reports, shows that order 3 allocation fail. On arch with 4kB pages,
order 3 mean 32kB allocation. We want to alloc 16kB, but there is also
internal sk_buff data what make that we exceed the boundary and take
32kB from allocator, getting almost 50% wastage.

To fix we can use similar method as in niu or iwlwifi drivers, alloc
pages directly form buddy allocator and attach them to skb (by
skb_add_rx_frag for example). I'm going to prepare such patch, but
I have one doubt, what happens if page size in system is bigger
than 16kB, should I care about such case? 

Stanislaw

^ permalink raw reply

* Re: [PATCH 1/2] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-10-08 15:04 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Francois Romieu, netdev
In-Reply-To: <20101008145256.GB10393@redhat.com>

Le vendredi 08 octobre 2010 à 16:52 +0200, Stanislaw Gruszka a écrit :
> On Fri, Oct 08, 2010 at 04:25:00PM +0200, Stanislaw Gruszka wrote:
> > We have fedora bug report where driver fail to initialize after
> > suspend/resume because of memory allocation errors:
> > https://bugzilla.redhat.com/show_bug.cgi?id=629158
> 
> There is also one more thing to do regarding above. Calltraces from bug
> reports, shows that order 3 allocation fail. On arch with 4kB pages,
> order 3 mean 32kB allocation. We want to alloc 16kB, but there is also
> internal sk_buff data what make that we exceed the boundary and take
> 32kB from allocator, getting almost 50% wastage.
> 

Or its only an 1460+overhead allocation, and SLUB uses order-3 pages to
satisfy 2048 bytes allocations.

# grep 2048 /proc/slabinfo 
kmalloc-2048        8664   8752   2048   16    8 : tunables    0    0
0 : slabdata    547    547      0


8 in the <pagesperslab> column just says that : order-3 pages, even for
small allocations.

Switch to SLAB -> no more problem ;)


> To fix we can use similar method as in niu or iwlwifi drivers, alloc
> pages directly form buddy allocator and attach them to skb (by
> skb_add_rx_frag for example). I'm going to prepare such patch, but
> I have one doubt, what happens if page size in system is bigger
> than 16kB, should I care about such case? 

Seems tricky. Should we patch all drivers to do something like that ?




^ permalink raw reply

* Re: Linux 2.6.36-rc7
From: James Bottomley @ 2010-10-08 15:05 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Linus Torvalds, Linux Kernel Mailing List, Russell King,
	David Miller, netdev, John W. Linville, Michal Marek,
	Dmitry Torokhov
In-Reply-To: <20101007114938.ad3d2c76.sfr@canb.auug.org.au>

On Thu, 2010-10-07 at 11:49 +1100, Stephen Rothwell wrote:
> Hi Linus,
> 
> On Wed, 6 Oct 2010 14:45:13 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> > This should be the last -rc, I'm not seeing any reason to keep
> > delaying a real release. There was still more changes to
> > drivers/gpu/drm than I really would have hoped for, but they all look
> > harmless and good. Famous last words.
> 
> I have no idea how critical any of this stuff is, but linux-next contain
> the following in it's "current" trees i.e. stuff that is supposed to go
> into 2.6.36.  These are from the arm-current, scsi-rc-fixes, net-current,
> wireless-current, kbuild-current, input-current and ide-curent trees
> (contacts cc'd).

The SCSI rc-fixes stuff is critical if you run into the bugs, but the
bugs are fairly rare cases for most people.  I'd still like to get them
in, though (and I have another 3 rc fixes candidates going through the
test pipeline).

James

^ permalink raw reply

* Re: BUG ? ipip unregister_netdevice_many()
From: Daniel Lezcano @ 2010-10-08 15:53 UTC (permalink / raw)
  To: Hans Schillstrom; +Cc: Eric W. Biederman, netdev@vger.kernel.org
In-Reply-To: <201010081428.37639.hans.schillstrom@ericsson.com>

On 10/08/2010 02:28 PM, Hans Schillstrom wrote:
> Hi Eric,
> Any advice how to trace this down ?
> This rollback_registered_many() seems to have on the lists before...
> All IPv4 and IPv6 tunnels causes this crash, all you have to do is load the tunnel module(s)
> enter a new ns and exit from it.
>
> Have not tested any more devices than tunnels,
> I did an "ip link delete" on my macvlans before exiting the ns.
>    

Ah ! I succeed to reproduce it.
It does not appear immediately in fact.

I am trying to simplify the configuration but I am falling in the bug I 
talked about in the previous email.

> snip
>    
>>   # ------------[ cut here ]------------
>> WARNING: at /home/hans/evip/kvm/net-next-2.6/kernel/sysctl.c:1953 unregister_sysctl_table+0xc7/0xf9()
>> Hardware name: Bochs
>> Modules linked in: macvlan ip6_tunnel tunnel6 pcnet32 tg3 libphy
>> Pid: 5, comm: kworker/u:0 Not tainted 2.6.36-rc3 #2
>> Call Trace:
>>   [<ffffffff8103e281>] warn_slowpath_common+0x85/0x9d
>>   [<ffffffff8103e2b3>] warn_slowpath_null+0x1a/0x1c
>>   [<ffffffff81045e64>] unregister_sysctl_table+0xc7/0xf9
>>   [<ffffffff812c86a5>] neigh_sysctl_unregister+0x27/0x3f
>>   [<ffffffff81340c75>] addrconf_ifdown+0x415/0x45e
>>   [<ffffffff81341705>] addrconf_notify+0x756/0x7fe
>>   [<ffffffff812cacfb>] ? neigh_ifdown+0xc3/0xd4
>>   [<ffffffff81360eb3>] ? ip6mr_device_event+0x8d/0x9e
>>   [<ffffffff8105eddb>] notifier_call_chain+0x37/0x63
>>   [<ffffffff8105ee8b>] raw_notifier_call_chain+0x14/0x16
>>   [<ffffffff812c15c7>] call_netdevice_notifiers+0x4a/0x4f
>>   [<ffffffff812c1c1b>] rollback_registered_many+0x121/0x208
>>   [<ffffffff812c1d1d>] unregister_netdevice_many+0x1b/0x71
>>   [<ffffffffa0047244>] ip6_tnl_exit_net+0xa4/0xb8 [ip6_tunnel]
>>   [<ffffffff812bc941>] ? cleanup_net+0x0/0x198
>>   [<ffffffff812bc2cf>] ops_exit_list+0x2a/0x5b
>>   [<ffffffff812bca39>] cleanup_net+0xf8/0x198
>>   [<ffffffff810568c7>] process_one_work+0x2a2/0x44d
>>   [<ffffffff81056e35>] worker_thread+0x1db/0x34e
>>   [<ffffffff81056c5a>] ? worker_thread+0x0/0x34e
>>   [<ffffffff8105a030>] kthread+0x82/0x8a
>>   [<ffffffff81003954>] kernel_thread_helper+0x4/0x10
>>   [<ffffffff81059fae>] ? kthread+0x0/0x8a
>>   [<ffffffff81003950>] ? kernel_thread_helper+0x0/0x10
>> ---[ end trace eb3bc950cf9a8748 ]---
>> unregister_netdevice: waiting for lo to become free. Usage count = 4
>> unregister_netdevice: waiting for lo to become free. Usage count = 4
>> unregister_netdevice: waiting for lo to become free. Usage count = 4
>>      
>
>    


^ permalink raw reply

* Re: [PATCH 1/2] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Stanislaw Gruszka @ 2010-10-08 16:03 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Francois Romieu, netdev
In-Reply-To: <1286550247.2959.444.camel@edumazet-laptop>

On Fri, Oct 08, 2010 at 05:04:07PM +0200, Eric Dumazet wrote:
> Le vendredi 08 octobre 2010 à 16:52 +0200, Stanislaw Gruszka a écrit :
> > On Fri, Oct 08, 2010 at 04:25:00PM +0200, Stanislaw Gruszka wrote:
> > > We have fedora bug report where driver fail to initialize after
> > > suspend/resume because of memory allocation errors:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=629158
> > 
> > There is also one more thing to do regarding above. Calltraces from bug
> > reports, shows that order 3 allocation fail. On arch with 4kB pages,
> > order 3 mean 32kB allocation. We want to alloc 16kB, but there is also
> > internal sk_buff data what make that we exceed the boundary and take
> > 32kB from allocator, getting almost 50% wastage.
> > 
> 
> Or its only an 1460+overhead allocation, and SLUB uses order-3 pages to
> satisfy 2048 bytes allocations.

Rather not, trace show failure in rtl8169_rx_fill, where we allocate rx
buffers and these are 16kB big by default.

> Switch to SLAB -> no more problem ;)

yeh, I wish to, but fedora use SLUB because of some debugging
capabilities. 

> > To fix we can use similar method as in niu or iwlwifi drivers, alloc
> > pages directly form buddy allocator and attach them to skb (by
> > skb_add_rx_frag for example). I'm going to prepare such patch, but
> > I have one doubt, what happens if page size in system is bigger
> > than 16kB, should I care about such case? 
> 
> Seems tricky. Should we patch all drivers to do something like that ?

I think, only on these drivers which do alloc_skb(n*PAGE_SIZE).
As alternative we can be smarter in alloc_skb.

Stanislaw
> 
> 
> 

^ permalink raw reply

* Re: BUG ? ipip unregister_netdevice_many()
From: Eric W. Biederman @ 2010-10-08 16:06 UTC (permalink / raw)
  To: Hans Schillstrom; +Cc: netdev@vger.kernel.org, Daniel Lezcano
In-Reply-To: <201010071048.12817.hans.schillstrom@ericsson.com>

Hans Schillstrom <hans.schillstrom@ericsson.com> writes:

> Hello
> I'm trying to exit a network name space and it doesn't work (or am I doing something wrong?)
> The only netdevices left are lo and the tunnels ip6tnl0, sit0 and tunl0 when exiting netns.
>
> A netns is created by lxc-execute with two interfaces eth0 eth1 (macvlan)
> (see conf file at the end)
>
> Kernel: net-next-2.6 top from 4 october 2010
>
> I added some printk's inn ipip.c  ipip_exit_net()
> ...
>         rtnl_lock();
>         printk(KERN_ERR "ipip_exit_net(enter)\n");
>         ipip_destroy_tunnels(ipn, &list);
>         printk(KERN_ERR "ipip_exit_net(1)\n");
>         unregister_netdevice_queue(ipn->fb_tunnel_dev, &list);
>         printk(KERN_ERR "ipip_exit_net(2)\n");
>         unregister_netdevice_many(&list);
>         printk(KERN_ERR "ipip_exit_net(3)\n");
>         rtnl_unlock();
>         printk(KERN_ERR "ipip_exit_net(exit)\n");
>
>
> Exit steps:
> ===== Screen dump =====
>
>  # ifconfig eth0  0.0.0.0  down
>  # ifconfig eth1  0.0.0.0  down
>  # ifconfig lo  0.0.0.0  down
>  # ip li de eth0
>  # ip li de eth1
>  # ifconfig -a
> ip6tnl0   Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00  
>           NOARP  MTU:1460  Metric:1
>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>           collisions:0 txqueuelen:0 
>           RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
>
> lo        Link encap:Local Loopback  
>           inet addr:127.0.0.1  Mask:255.0.0.0
>           LOOPBACK  MTU:16436  Metric:1
>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>           collisions:0 txqueuelen:0 
>           RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
>
> sit0      Link encap:IPv6-in-IPv4  
>           NOARP  MTU:1480  Metric:1
>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>           collisions:0 txqueuelen:0 
>           RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
>
> tunl0     Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00  
>           NOARP  MTU:1480  Metric:1
>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>           collisions:0 txqueuelen:0 
>           RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
>
>  # ps
>   PID USER       VSZ STAT COMMAND
>     1 root     12412 S    /usr/lib64/lxc/lxc-init -- /var/bin/init
>     2 root      4540 S    /bin/ash /var/bin/init
>     7 root      6640 S    inetd
>     8 root      4544 S    /bin/ash
>    26 root      4544 R    ps
>  # lsmod 
> Module                  Size  Used by    Not tainted
> macvlan                 8709  0 
> pcnet32                29549  0 
> tg3                   112093  0 
> libphy                 21043  1 tg3
>  # kill 7 2
>  # ps
>   PID USER       VSZ STAT COMMAND
>     1 root     12412 S    /usr/lib64/lxc/lxc-init -- /var/bin/init
>     8 root      4544 S    /bin/ash
>    28 root      4544 R    ps
>  # exit  ( here is the exit from netns  )
>  # ipip_exit_net(enter)
> ipip_exit_net(1)
> ipip_exit_net(2)
> ------------[ cut here ]------------
> WARNING: at /home/hans/evip/kvm/net-next-2.6/kernel/sysctl.c:1953
>   unregister_sysctl_table+0xc7/0xf9()

This warning is caused by removing the parent directory
before the child in the sysctl tables.  Not strictly fatal but
it is a problem.  It may be worth looking at which sysctl
tables ipip registers to see if we can rectify this.

> Hardware name: Bochs
> Modules linked in: macvlan pcnet32 tg3 libphy
> Pid: 5, comm: kworker/u:0 Not tainted 2.6.36-rc3+ #7
> Call Trace:
>  [<ffffffff8103e281>] warn_slowpath_common+0x85/0x9d
>  [<ffffffff8103e2b3>] warn_slowpath_null+0x1a/0x1c
>  [<ffffffff81045e64>] unregister_sysctl_table+0xc7/0xf9
>  [<ffffffff812c86a5>] neigh_sysctl_unregister+0x27/0x3f
>  [<ffffffff81342108>] addrconf_ifdown+0x415/0x45e
>  [<ffffffff81342b98>] addrconf_notify+0x756/0x7fe
>  [<ffffffff812cacfb>] ? neigh_ifdown+0xc3/0xd4
>  [<ffffffff813622b3>] ? ip6mr_device_event+0x8d/0x9e
>  [<ffffffff8105eddb>] notifier_call_chain+0x37/0x63
>  [<ffffffff8105ee8b>] raw_notifier_call_chain+0x14/0x16
>  [<ffffffff812c15c7>] call_netdevice_notifiers+0x4a/0x4f
>  [<ffffffff812c1c1b>] rollback_registered_many+0x121/0x208
>  [<ffffffff812c1d1d>] unregister_netdevice_many+0x1b/0x71
>  [<ffffffff81324209>] ipip_exit_net+0xea/0x11a
>  [<ffffffff812bc941>] ? cleanup_net+0x0/0x198
>  [<ffffffff812bc2cf>] ops_exit_list+0x2a/0x5b
>  [<ffffffff812bca39>] cleanup_net+0xf8/0x198
>  [<ffffffff810568c7>] process_one_work+0x2a2/0x44d
>  [<ffffffff81056e35>] worker_thread+0x1db/0x34e
>  [<ffffffff81056c5a>] ? worker_thread+0x0/0x34e
>  [<ffffffff8105a030>] kthread+0x82/0x8a
>  [<ffffffff81003954>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81059fae>] ? kthread+0x0/0x8a
>  [<ffffffff81003950>] ? kernel_thread_helper+0x0/0x10
> ---[ end trace 939b5185219f32e7 ]---
> ipip_exit_net(3)
> ipip_exit_net(exit)
> unregister_netdevice: waiting for lo to become free. Usage count = 4
> unregister_netdevice: waiting for lo to become free. Usage count = 4
> unregister_netdevice: waiting for lo to become free. Usage count = 4

Nasty. Someone has left a reference lying around to one of the network
devices.  It is a reference that we can transfer to the loopback device
at device exit time, but we never drop the reference and so the loopback
interface never frees up.

Ouch!

There is the painful method of instrumenting of dev_hold and dev_release
that may give you a clue.  It may also be worth seeing which kinds of
device reference we transfer from the loopback device when a device
exits.

Eric

^ permalink raw reply

* Re: [PATCH] sysctl: fix min/max handling in __do_proc_doulongvec_minmax()
From: Américo Wang @ 2010-10-08 16:13 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Américo Wang, Robin Holt, Andrew Morton, linux-kernel,
	Willy Tarreau, David S. Miller, netdev, James Morris,
	Pekka Savola (ipv6), Patrick McHardy, Alexey Kuznetsov, ebiederm
In-Reply-To: <1286445081.2912.15.camel@edumazet-laptop>

On Thu, Oct 07, 2010 at 11:51:21AM +0200, Eric Dumazet wrote:
>Le jeudi 07 octobre 2010 à 17:25 +0800, Américo Wang a écrit :
>> >>
>> >
>> >Here is the final one.
>> 
>> Oops, that one is not correct. Hopefully this one
>> is correct.
>> 
>> --------------->
>> 
>> Eric D. noticed that we may trigger an OOPS if we leave ->extra{1,2}
>> to NULL when we use proc_doulongvec_minmax().
>> 
>> Actually, we don't need to store min/max values in a vector,
>> because all the elements in the vector should share the same min/max
>> value, like what proc_dointvec_minmax() does.
>> 
>
>If we assert same min/max limits are to be applied to all elements,
>a much simpler fix than yours would be :
>
>diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>index f88552c..8e45451 100644
>--- a/kernel/sysctl.c
>+++ b/kernel/sysctl.c
>@@ -2485,7 +2485,7 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
> 		kbuf[left] = 0;
> 	}
> 
>-	for (; left && vleft--; i++, min++, max++, first=0) {
>+	for (; left && vleft--; i++, first=0) {
> 		unsigned long val;
> 
> 		if (write) {
>
>
>Please dont send huge patches like this to 'fix' a bug,
>especially on slow path.

Well, my patch makes that horrible code a little better. :)

>
>First we fix the bug, _then_ we can try to make code more 
>efficient or more pretty or shorter.
>
>So the _real_ question is :
>
>Should the min/max limits should be a single pair,
>shared by all elements, or a vector of limits.
>

Yes, actually I talked with Eric W. about this before
sending the patch.

I also checked the users of proc_doulongvec_minmax(),
none of them are using more than one limit, so it is
safe to remove that.


-- 
Live like a child, think like the god.
 

^ permalink raw reply

* Re: BUG ? ipip unregister_netdevice_many()
From: Daniel Lezcano @ 2010-10-08 16:17 UTC (permalink / raw)
  Cc: Hans Schillstrom, Eric W. Biederman, netdev@vger.kernel.org
In-Reply-To: <4CAF3E78.8030202@free.fr>

On 10/08/2010 05:53 PM, Daniel Lezcano wrote:
> On 10/08/2010 02:28 PM, Hans Schillstrom wrote:
>> Hi Eric,
>> Any advice how to trace this down ?
>> This rollback_registered_many() seems to have on the lists before...
>> All IPv4 and IPv6 tunnels causes this crash, all you have to do is
>> load the tunnel module(s)
>> enter a new ns and exit from it.
>>
>> Have not tested any more devices than tunnels,
>> I did an "ip link delete" on my macvlans before exiting the ns.
>
> Ah ! I succeed to reproduce it.
> It does not appear immediately in fact.
>
> I am trying to simplify the configuration but I am falling in the bug I
> talked about in the previous email.

Ok, so after investigating, we just need a macvlan and specify an ipv6 
address for it (inside a new netns of course), and the loopback is not 
released. I compiled out the tunnels, so they are not related to this 
problem I think.

That reduces the scope of investigation :)

Looking forward ...

   -- Daniel

^ permalink raw reply

* Re: [PATCH] sysctl: fix min/max handling in __do_proc_doulongvec_minmax()
From: Américo Wang @ 2010-10-08 16:22 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Andrew Morton, Eric Dumazet, Américo Wang, Robin Holt,
	linux-kernel, Willy Tarreau, David S. Miller, netdev,
	James Morris, Pekka Savola (ipv6), Patrick McHardy,
	Alexey Kuznetsov
In-Reply-To: <m1vd5d3ia9.fsf@fess.ebiederm.org>

On Thu, Oct 07, 2010 at 12:38:22PM -0700, Eric W. Biederman wrote:
>Andrew Morton <akpm@linux-foundation.org> writes:
>
>> On Thu, 07 Oct 2010 18:59:03 +0200
>> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>>> Thats fine by me, thanks Eric.
>>> 
>>> Andrew, please remove previous patch from your tree and replace it by
>>> following one :
>>> 
>>> [PATCH v2] sysctl: fix min/max handling in __do_proc_doulongvec_minmax()
>>> 
>>> When proc_doulongvec_minmax() is used with an array of longs,
>>> and no min/max check requested (.extra1 or .extra2 being NULL), we
>>> dereference a NULL pointer for the second element of the array.
>>> 
>>> Noticed while doing some changes in network stack for the "16TB problem"
>>> 
>>> Fix is to not change min & max pointers in
>>> __do_proc_doulongvec_minmax(), so that all elements of the vector share
>>> an unique min/max limit, like proc_dointvec_minmax().
>>> 
>>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>>> ---
>>>  kernel/sysctl.c |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>>> index f88552c..8e45451 100644
>>> --- a/kernel/sysctl.c
>>> +++ b/kernel/sysctl.c
>>> @@ -2485,7 +2485,7 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
>>>  		kbuf[left] = 0;
>>>  	}
>>>  
>>> -	for (; left && vleft--; i++, min++, max++, first=0) {
>>> +	for (; left && vleft--; i++, first=0) {
>>>  		unsigned long val;
>>>  
>>>  		if (write) {
>>
>> Did we check to see whether any present callers are passing in pointers
>> to arrays of min/max values?
>
>In 2.6.36 there are not any callers that pass in a vector of anything, I
>don't know about linux-next.  It looks to me like incrementing min and
>max was simply a bug.
>

Agreed, I checked them too.

>> I wonder if there's any documentation for this interface which just
>> became wrong.
>
>Or it just became right.  Clearly no one has been expecting min
>and max to be vectors.
>

I think we need to document this before we rewrite the code.

-- 
Live like a child, think like the god.
 

^ permalink raw reply

* Re: [PATCH 1/2] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-10-08 16:27 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Francois Romieu, netdev
In-Reply-To: <20101008160341.GC10393@redhat.com>

Le vendredi 08 octobre 2010 à 18:03 +0200, Stanislaw Gruszka a écrit :
> On Fri, Oct 08, 2010 at 05:04:07PM +0200, Eric Dumazet wrote:
> > Le vendredi 08 octobre 2010 à 16:52 +0200, Stanislaw Gruszka a écrit :
> > > On Fri, Oct 08, 2010 at 04:25:00PM +0200, Stanislaw Gruszka wrote:
> > > > We have fedora bug report where driver fail to initialize after
> > > > suspend/resume because of memory allocation errors:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=629158
> > > 
> > > There is also one more thing to do regarding above. Calltraces from bug
> > > reports, shows that order 3 allocation fail. On arch with 4kB pages,
> > > order 3 mean 32kB allocation. We want to alloc 16kB, but there is also
> > > internal sk_buff data what make that we exceed the boundary and take
> > > 32kB from allocator, getting almost 50% wastage.
> > > 
> > 
> > Or its only an 1460+overhead allocation, and SLUB uses order-3 pages to
> > satisfy 2048 bytes allocations.
> 
> Rather not, trace show failure in rtl8169_rx_fill, where we allocate rx
> buffers and these are 16kB big by default.
> 

Only when gfp_t is GFP_KERNEL to fill rx buffers. (after your patch
applied of course). This should succeed. If not, driver cannot load and
function, since this NIC really needs 16KB buffers in order to avoid a
hardware bug.

Once allocated for RX rings, we never free them (never give this skb to
upper stack) : When we receive a frame, we copybreak it, (using
GFP_ATOMIC) so it depends on MTU.

With MTU=1500, I am pretty sure we allocate 2048 bytes chunks, not more.


> I think, only on these drivers which do alloc_skb(n*PAGE_SIZE).
> As alternative we can be smarter in alloc_skb.

Only if MTU is non standard, then.

I repeat : With standard MTU=1500, we dont allocate huge skbs in rx
path, only small (<2048 bytes) ones.

For bigger frames, then you might allocate fragments, using pages, and
dont care if PAGE_SIZE is 64Kbytes.




^ permalink raw reply

* Re: IPv4: sysctl table check failed [was: mmotm 2010-10-07-14-08 uploaded]
From: Américo Wang @ 2010-10-08 16:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Eric Dumazet, Jiri Slaby, linux-kernel, mm-commits, ML netdev,
	David S. Miller, Eric W. Biederman
In-Reply-To: <20101007152806.119d1522.akpm@linux-foundation.org>

On Thu, Oct 07, 2010 at 03:28:06PM -0700, Andrew Morton wrote:
>On Fri, 08 Oct 2010 00:22:15 +0200
>Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Le vendredi 08 octobre 2010 __ 00:06 +0200, Jiri Slaby a __crit :
>> > On 10/07/2010 11:08 PM, akpm@linux-foundation.org wrote:
>> > > The mm-of-the-moment snapshot 2010-10-07-14-08 has been uploaded to
>> > 
>> > Hi, I got bunch of "sysctl table check failed" below. All seem to be
>> > related to ipv4:
>> 
>> I would say, sysctl check is buggy :(
>> 
>> min/max are optional
>> 
>> [PATCH] sysctl: min/max bounds are optional
>> 
>> sysctl check complains when proc_doulongvec_minmax or
>> proc_doulongvec_ms_jiffies_minmax are used by a vector of longs (with
>> more than one element), with no min or max value specified.
>> 
>> This is unexpected, given we had a bug on this min/max handling :)
>> 
>> Reported-by: Jiri Slaby <jirislaby@gmail.com>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>>  kernel/sysctl_check.c |    9 ---------
>>  1 file changed, 9 deletions(-)
>> 
>> diff --git a/kernel/sysctl_check.c b/kernel/sysctl_check.c
>> index 04cdcf7..10b90d8 100644
>> --- a/kernel/sysctl_check.c
>> +++ b/kernel/sysctl_check.c
>> @@ -143,15 +143,6 @@ int sysctl_check_table(struct nsproxy *namespaces, struct ctl_table *table)
>>  				if (!table->maxlen)
>>  					set_fail(&fail, table, "No maxlen");
>>  			}
>> -			if ((table->proc_handler == proc_doulongvec_minmax) ||
>> -			    (table->proc_handler == proc_doulongvec_ms_jiffies_minmax)) {
>> -				if (table->maxlen > sizeof (unsigned long)) {
>> -					if (!table->extra1)
>> -						set_fail(&fail, table, "No min");
>> -					if (!table->extra2)
>> -						set_fail(&fail, table, "No max");
>> -				}
>> -			}
>>  #ifdef CONFIG_PROC_SYSCTL
>>  			if (table->procname && !table->proc_handler)
>>  				set_fail(&fail, table, "No proc_handler");
>
>That will probably fix it ;)


Yeah, it looks good for me too,

Acked-by: WANG Cong <xiyou.wangcong@gmail.com>

>
>net-avoid-limits-overflow.patch is dependent on this patch.  Unless
>Eric B squeaks I'll plan on sending this patch in for 2.6.37.
>

Eirc B reminded me we should check the code in sysctl_check.c,
but I forgot. The patch from Eric D is exactly what we need here.

Thanks.


^ permalink raw reply

* [PATCH net-next] net dst: use a percpu_counter to track entries
From: Eric Dumazet @ 2010-10-08 16:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

struct dst_ops tracks number of allocated dst in an atomic_t field,
subject to high cache line contention in stress workload.

Switch to a percpu_counter, to reduce number of time we need to dirty a
central location. Place it on a separate cache line to avoid dirtying
read only fields.

Stress test :

(Sending 160.000.000 UDP frames,
IP route cache disabled, dual E5540 @2.53GHz,
32bit kernel, FIB_TRIE, SLUB/NUMA)

Before:

real    0m51.179s
user    0m15.329s
sys     10m15.942s

After:

real	0m45.570s
user	0m15.525s
sys	9m56.669s

With a small reordering of struct neighbour fields, subject of a
following patch, (to separate refcnt from other read mostly fields)

real	0m41.841s
user	0m15.261s
sys	8m45.949s

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/dst_ops.h     |   37 +++++++++++++++++++++++++++++++++++-
 net/bridge/br_netfilter.c |   11 ++++++++--
 net/core/dst.c            |    6 ++---
 net/decnet/dn_route.c     |    3 +-
 net/ipv4/route.c          |   36 +++++++++++++++++++++--------------
 net/ipv4/xfrm4_policy.c   |    4 +--
 net/ipv6/route.c          |   28 +++++++++++++++++++--------
 net/ipv6/xfrm6_policy.c   |   10 +++++----
 8 files changed, 100 insertions(+), 35 deletions(-)

diff --git a/include/net/dst_ops.h b/include/net/dst_ops.h
index d1ff9b7..1fa5306 100644
--- a/include/net/dst_ops.h
+++ b/include/net/dst_ops.h
@@ -1,6 +1,7 @@
 #ifndef _NET_DST_OPS_H
 #define _NET_DST_OPS_H
 #include <linux/types.h>
+#include <linux/percpu_counter.h>
 
 struct dst_entry;
 struct kmem_cachep;
@@ -22,7 +23,41 @@ struct dst_ops {
 	void			(*update_pmtu)(struct dst_entry *dst, u32 mtu);
 	int			(*local_out)(struct sk_buff *skb);
 
-	atomic_t		entries;
 	struct kmem_cache	*kmem_cachep;
+
+	struct percpu_counter	pcpuc_entries ____cacheline_aligned_in_smp;
 };
+
+static inline int dst_entries_get_fast(struct dst_ops *dst)
+{
+	return percpu_counter_read_positive(&dst->pcpuc_entries);
+}
+
+static inline int dst_entries_get_slow(struct dst_ops *dst)
+{
+	int res;
+
+	local_bh_disable();
+	res = percpu_counter_sum_positive(&dst->pcpuc_entries);
+	local_bh_enable();
+	return res;
+}
+
+static inline void dst_entries_add(struct dst_ops *dst, int val)
+{
+	local_bh_disable();
+	percpu_counter_add(&dst->pcpuc_entries, val);
+	local_bh_enable();
+}
+
+static inline int dst_entries_init(struct dst_ops *dst)
+{
+	return percpu_counter_init(&dst->pcpuc_entries, 0);
+}
+
+static inline void dst_entries_destroy(struct dst_ops *dst)
+{
+	percpu_counter_destroy(&dst->pcpuc_entries);
+}
+
 #endif
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 77f7b5f..7f9ce96 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -106,7 +106,6 @@ static struct dst_ops fake_dst_ops = {
 	.family =		AF_INET,
 	.protocol =		cpu_to_be16(ETH_P_IP),
 	.update_pmtu =		fake_update_pmtu,
-	.entries =		ATOMIC_INIT(0),
 };
 
 /*
@@ -1003,15 +1002,22 @@ int __init br_netfilter_init(void)
 {
 	int ret;
 
-	ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+	ret = dst_entries_init(&fake_dst_ops);
 	if (ret < 0)
 		return ret;
+
+	ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+	if (ret < 0) {
+		dst_entries_destroy(&fake_dst_ops);
+		return ret;
+	}
 #ifdef CONFIG_SYSCTL
 	brnf_sysctl_header = register_sysctl_paths(brnf_path, brnf_table);
 	if (brnf_sysctl_header == NULL) {
 		printk(KERN_WARNING
 		       "br_netfilter: can't register to sysctl.\n");
 		nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+		dst_entries_destroy(&fake_dst_ops);
 		return -ENOMEM;
 	}
 #endif
@@ -1025,4 +1031,5 @@ void br_netfilter_fini(void)
 #ifdef CONFIG_SYSCTL
 	unregister_sysctl_table(brnf_sysctl_header);
 #endif
+	dst_entries_destroy(&fake_dst_ops);
 }
diff --git a/net/core/dst.c b/net/core/dst.c
index 6c41b1f..eebcf4f 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -168,7 +168,7 @@ void *dst_alloc(struct dst_ops *ops)
 {
 	struct dst_entry *dst;
 
-	if (ops->gc && atomic_read(&ops->entries) > ops->gc_thresh) {
+	if (ops->gc && dst_entries_get_fast(ops) > ops->gc_thresh) {
 		if (ops->gc(ops))
 			return NULL;
 	}
@@ -183,7 +183,7 @@ void *dst_alloc(struct dst_ops *ops)
 #if RT_CACHE_DEBUG >= 2
 	atomic_inc(&dst_total);
 #endif
-	atomic_inc(&ops->entries);
+	dst_entries_add(ops, 1);
 	return dst;
 }
 EXPORT_SYMBOL(dst_alloc);
@@ -236,7 +236,7 @@ again:
 		neigh_release(neigh);
 	}
 
-	atomic_dec(&dst->ops->entries);
+	dst_entries_add(dst->ops, -1);
 
 	if (dst->ops->destroy)
 		dst->ops->destroy(dst);
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 6585ea6..df0f3e5 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -132,7 +132,6 @@ static struct dst_ops dn_dst_ops = {
 	.negative_advice =	dn_dst_negative_advice,
 	.link_failure =		dn_dst_link_failure,
 	.update_pmtu =		dn_dst_update_pmtu,
-	.entries =		ATOMIC_INIT(0),
 };
 
 static __inline__ unsigned dn_hash(__le16 src, __le16 dst)
@@ -1758,6 +1757,7 @@ void __init dn_route_init(void)
 	dn_dst_ops.kmem_cachep =
 		kmem_cache_create("dn_dst_cache", sizeof(struct dn_route), 0,
 				  SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+	dst_entries_init(&dn_dst_ops);
 	setup_timer(&dn_route_timer, dn_dst_check_expire, 0);
 	dn_route_timer.expires = jiffies + decnet_dst_gc_interval * HZ;
 	add_timer(&dn_route_timer);
@@ -1816,5 +1816,6 @@ void __exit dn_route_cleanup(void)
 	dn_run_flush(0);
 
 	proc_net_remove(&init_net, "decnet_cache");
+	dst_entries_destroy(&dn_dst_ops);
 }
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 7864d0c..831c6d4 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -159,7 +159,6 @@ static struct dst_ops ipv4_dst_ops = {
 	.link_failure =		ipv4_link_failure,
 	.update_pmtu =		ip_rt_update_pmtu,
 	.local_out =		__ip_local_out,
-	.entries =		ATOMIC_INIT(0),
 };
 
 #define ECN_OR_COST(class)	TC_PRIO_##class
@@ -466,7 +465,7 @@ static int rt_cpu_seq_show(struct seq_file *seq, void *v)
 
 	seq_printf(seq,"%08x  %08x %08x %08x %08x %08x %08x %08x "
 		   " %08x %08x %08x %08x %08x %08x %08x %08x %08x \n",
-		   atomic_read(&ipv4_dst_ops.entries),
+		   dst_entries_get_slow(&ipv4_dst_ops),
 		   st->in_hit,
 		   st->in_slow_tot,
 		   st->in_slow_mc,
@@ -945,6 +944,7 @@ static int rt_garbage_collect(struct dst_ops *ops)
 	struct rtable *rth, **rthp;
 	unsigned long now = jiffies;
 	int goal;
+	int entries = dst_entries_get_fast(&ipv4_dst_ops);
 
 	/*
 	 * Garbage collection is pretty expensive,
@@ -954,28 +954,28 @@ static int rt_garbage_collect(struct dst_ops *ops)
 	RT_CACHE_STAT_INC(gc_total);
 
 	if (now - last_gc < ip_rt_gc_min_interval &&
-	    atomic_read(&ipv4_dst_ops.entries) < ip_rt_max_size) {
+	    entries < ip_rt_max_size) {
 		RT_CACHE_STAT_INC(gc_ignored);
 		goto out;
 	}
 
+	entries = dst_entries_get_slow(&ipv4_dst_ops);
 	/* Calculate number of entries, which we want to expire now. */
-	goal = atomic_read(&ipv4_dst_ops.entries) -
-		(ip_rt_gc_elasticity << rt_hash_log);
+	goal = entries - (ip_rt_gc_elasticity << rt_hash_log);
 	if (goal <= 0) {
 		if (equilibrium < ipv4_dst_ops.gc_thresh)
 			equilibrium = ipv4_dst_ops.gc_thresh;
-		goal = atomic_read(&ipv4_dst_ops.entries) - equilibrium;
+		goal = entries - equilibrium;
 		if (goal > 0) {
 			equilibrium += min_t(unsigned int, goal >> 1, rt_hash_mask + 1);
-			goal = atomic_read(&ipv4_dst_ops.entries) - equilibrium;
+			goal = entries - equilibrium;
 		}
 	} else {
 		/* We are in dangerous area. Try to reduce cache really
 		 * aggressively.
 		 */
 		goal = max_t(unsigned int, goal >> 1, rt_hash_mask + 1);
-		equilibrium = atomic_read(&ipv4_dst_ops.entries) - goal;
+		equilibrium = entries - goal;
 	}
 
 	if (now - last_gc >= ip_rt_gc_min_interval)
@@ -1032,14 +1032,16 @@ static int rt_garbage_collect(struct dst_ops *ops)
 		expire >>= 1;
 #if RT_CACHE_DEBUG >= 2
 		printk(KERN_DEBUG "expire>> %u %d %d %d\n", expire,
-				atomic_read(&ipv4_dst_ops.entries), goal, i);
+				dst_entries_get_fast(&ipv4_dst_ops), goal, i);
 #endif
 
-		if (atomic_read(&ipv4_dst_ops.entries) < ip_rt_max_size)
+		if (dst_entries_get_fast(&ipv4_dst_ops) < ip_rt_max_size)
 			goto out;
 	} while (!in_softirq() && time_before_eq(jiffies, now));
 
-	if (atomic_read(&ipv4_dst_ops.entries) < ip_rt_max_size)
+	if (dst_entries_get_fast(&ipv4_dst_ops) < ip_rt_max_size)
+		goto out;
+	if (dst_entries_get_slow(&ipv4_dst_ops) < ip_rt_max_size)
 		goto out;
 	if (net_ratelimit())
 		printk(KERN_WARNING "dst cache overflow\n");
@@ -1049,11 +1051,12 @@ static int rt_garbage_collect(struct dst_ops *ops)
 work_done:
 	expire += ip_rt_gc_min_interval;
 	if (expire > ip_rt_gc_timeout ||
-	    atomic_read(&ipv4_dst_ops.entries) < ipv4_dst_ops.gc_thresh)
+	    dst_entries_get_fast(&ipv4_dst_ops) < ipv4_dst_ops.gc_thresh ||
+	    dst_entries_get_slow(&ipv4_dst_ops) < ipv4_dst_ops.gc_thresh)
 		expire = ip_rt_gc_timeout;
 #if RT_CACHE_DEBUG >= 2
 	printk(KERN_DEBUG "expire++ %u %d %d %d\n", expire,
-			atomic_read(&ipv4_dst_ops.entries), goal, rover);
+			dst_entries_get_fast(&ipv4_dst_ops), goal, rover);
 #endif
 out:	return 0;
 }
@@ -2719,7 +2722,6 @@ static struct dst_ops ipv4_dst_blackhole_ops = {
 	.destroy		=	ipv4_dst_destroy,
 	.check			=	ipv4_blackhole_dst_check,
 	.update_pmtu		=	ipv4_rt_blackhole_update_pmtu,
-	.entries		=	ATOMIC_INIT(0),
 };
 
 
@@ -3289,6 +3291,12 @@ int __init ip_rt_init(void)
 
 	ipv4_dst_blackhole_ops.kmem_cachep = ipv4_dst_ops.kmem_cachep;
 
+	if (dst_entries_init(&ipv4_dst_ops) < 0)
+		panic("IP: failed to allocate ipv4_dst_ops counter\n");
+
+	if (dst_entries_init(&ipv4_dst_blackhole_ops) < 0)
+		panic("IP: failed to allocate ipv4_dst_blackhole_ops counter\n");
+
 	rt_hash_table = (struct rt_hash_bucket *)
 		alloc_large_system_hash("IP route cache",
 					sizeof(struct rt_hash_bucket),
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index a580349..4464f3b 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -174,7 +174,7 @@ static inline int xfrm4_garbage_collect(struct dst_ops *ops)
 	struct net *net = container_of(ops, struct net, xfrm.xfrm4_dst_ops);
 
 	xfrm4_policy_afinfo.garbage_collect(net);
-	return (atomic_read(&ops->entries) > ops->gc_thresh * 2);
+	return (dst_entries_get_slow(ops) > ops->gc_thresh * 2);
 }
 
 static void xfrm4_update_pmtu(struct dst_entry *dst, u32 mtu)
@@ -232,7 +232,6 @@ static struct dst_ops xfrm4_dst_ops = {
 	.ifdown =		xfrm4_dst_ifdown,
 	.local_out =		__ip_local_out,
 	.gc_thresh =		1024,
-	.entries =		ATOMIC_INIT(0),
 };
 
 static struct xfrm_policy_afinfo xfrm4_policy_afinfo = {
@@ -288,6 +287,7 @@ void __init xfrm4_init(int rt_max_size)
 	 * and start cleaning when were 1/2 full
 	 */
 	xfrm4_dst_ops.gc_thresh = rt_max_size/2;
+	dst_entries_init(&xfrm4_dst_ops);
 
 	xfrm4_state_init();
 	xfrm4_policy_init();
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 17e2179..25661f9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -109,7 +109,6 @@ static struct dst_ops ip6_dst_ops_template = {
 	.link_failure		=	ip6_link_failure,
 	.update_pmtu		=	ip6_rt_update_pmtu,
 	.local_out		=	__ip6_local_out,
-	.entries		=	ATOMIC_INIT(0),
 };
 
 static void ip6_rt_blackhole_update_pmtu(struct dst_entry *dst, u32 mtu)
@@ -122,7 +121,6 @@ static struct dst_ops ip6_dst_blackhole_ops = {
 	.destroy		=	ip6_dst_destroy,
 	.check			=	ip6_dst_check,
 	.update_pmtu		=	ip6_rt_blackhole_update_pmtu,
-	.entries		=	ATOMIC_INIT(0),
 };
 
 static struct rt6_info ip6_null_entry_template = {
@@ -1058,19 +1056,22 @@ static int ip6_dst_gc(struct dst_ops *ops)
 	int rt_elasticity = net->ipv6.sysctl.ip6_rt_gc_elasticity;
 	int rt_gc_timeout = net->ipv6.sysctl.ip6_rt_gc_timeout;
 	unsigned long rt_last_gc = net->ipv6.ip6_rt_last_gc;
+	int entries;
 
+	entries = dst_entries_get_fast(ops);
 	if (time_after(rt_last_gc + rt_min_interval, now) &&
-	    atomic_read(&ops->entries) <= rt_max_size)
+	    entries <= rt_max_size)
 		goto out;
 
 	net->ipv6.ip6_rt_gc_expire++;
 	fib6_run_gc(net->ipv6.ip6_rt_gc_expire, net);
 	net->ipv6.ip6_rt_last_gc = now;
-	if (atomic_read(&ops->entries) < ops->gc_thresh)
+	entries = dst_entries_get_slow(ops);
+	if (entries < ops->gc_thresh)
 		net->ipv6.ip6_rt_gc_expire = rt_gc_timeout>>1;
 out:
 	net->ipv6.ip6_rt_gc_expire -= net->ipv6.ip6_rt_gc_expire>>rt_elasticity;
-	return atomic_read(&ops->entries) > rt_max_size;
+	return entries > rt_max_size;
 }
 
 /* Clean host part of a prefix. Not necessary in radix tree,
@@ -2524,7 +2525,7 @@ static int rt6_stats_seq_show(struct seq_file *seq, void *v)
 		   net->ipv6.rt6_stats->fib_rt_alloc,
 		   net->ipv6.rt6_stats->fib_rt_entries,
 		   net->ipv6.rt6_stats->fib_rt_cache,
-		   atomic_read(&net->ipv6.ip6_dst_ops.entries),
+		   dst_entries_get_slow(&net->ipv6.ip6_dst_ops),
 		   net->ipv6.rt6_stats->fib_discarded_routes);
 
 	return 0;
@@ -2666,11 +2667,14 @@ static int __net_init ip6_route_net_init(struct net *net)
 	memcpy(&net->ipv6.ip6_dst_ops, &ip6_dst_ops_template,
 	       sizeof(net->ipv6.ip6_dst_ops));
 
+	if (dst_entries_init(&net->ipv6.ip6_dst_ops) < 0)
+		goto out_ip6_dst_ops;
+
 	net->ipv6.ip6_null_entry = kmemdup(&ip6_null_entry_template,
 					   sizeof(*net->ipv6.ip6_null_entry),
 					   GFP_KERNEL);
 	if (!net->ipv6.ip6_null_entry)
-		goto out_ip6_dst_ops;
+		goto out_ip6_dst_entries;
 	net->ipv6.ip6_null_entry->dst.path =
 		(struct dst_entry *)net->ipv6.ip6_null_entry;
 	net->ipv6.ip6_null_entry->dst.ops = &net->ipv6.ip6_dst_ops;
@@ -2720,6 +2724,8 @@ out_ip6_prohibit_entry:
 out_ip6_null_entry:
 	kfree(net->ipv6.ip6_null_entry);
 #endif
+out_ip6_dst_entries:
+	dst_entries_destroy(&net->ipv6.ip6_dst_ops);
 out_ip6_dst_ops:
 	goto out;
 }
@@ -2758,10 +2764,14 @@ int __init ip6_route_init(void)
 	if (!ip6_dst_ops_template.kmem_cachep)
 		goto out;
 
-	ret = register_pernet_subsys(&ip6_route_net_ops);
+	ret = dst_entries_init(&ip6_dst_blackhole_ops);
 	if (ret)
 		goto out_kmem_cache;
 
+	ret = register_pernet_subsys(&ip6_route_net_ops);
+	if (ret)
+		goto out_dst_entries;
+
 	ip6_dst_blackhole_ops.kmem_cachep = ip6_dst_ops_template.kmem_cachep;
 
 	/* Registering of the loopback is done before this portion of code,
@@ -2808,6 +2818,8 @@ out_fib6_init:
 	fib6_gc_cleanup();
 out_register_subsys:
 	unregister_pernet_subsys(&ip6_route_net_ops);
+out_dst_entries:
+	dst_entries_destroy(&ip6_dst_blackhole_ops);
 out_kmem_cache:
 	kmem_cache_destroy(ip6_dst_ops_template.kmem_cachep);
 	goto out;
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 39676ea..7e74023 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -199,7 +199,7 @@ static inline int xfrm6_garbage_collect(struct dst_ops *ops)
 	struct net *net = container_of(ops, struct net, xfrm.xfrm6_dst_ops);
 
 	xfrm6_policy_afinfo.garbage_collect(net);
-	return atomic_read(&ops->entries) > ops->gc_thresh * 2;
+	return dst_entries_get_fast(ops) > ops->gc_thresh * 2;
 }
 
 static void xfrm6_update_pmtu(struct dst_entry *dst, u32 mtu)
@@ -255,7 +255,6 @@ static struct dst_ops xfrm6_dst_ops = {
 	.ifdown =		xfrm6_dst_ifdown,
 	.local_out =		__ip6_local_out,
 	.gc_thresh =		1024,
-	.entries =		ATOMIC_INIT(0),
 };
 
 static struct xfrm_policy_afinfo xfrm6_policy_afinfo = {
@@ -312,11 +311,13 @@ int __init xfrm6_init(void)
 	 */
 	gc_thresh = FIB6_TABLE_HASHSZ * 8;
 	xfrm6_dst_ops.gc_thresh = (gc_thresh < 1024) ? 1024 : gc_thresh;
+	dst_entries_init(&xfrm6_dst_ops);
 
 	ret = xfrm6_policy_init();
-	if (ret)
+	if (ret) {
+		dst_entries_destroy(&xfrm6_dst_ops);
 		goto out;
-
+	}
 	ret = xfrm6_state_init();
 	if (ret)
 		goto out_policy;
@@ -341,4 +342,5 @@ void xfrm6_fini(void)
 	//xfrm6_input_fini();
 	xfrm6_policy_fini();
 	xfrm6_state_fini();
+	dst_entries_destroy(&xfrm6_dst_ops);
 }



^ permalink raw reply related

* Re: BUG ? ipip unregister_netdevice_many()
From: Eric W. Biederman @ 2010-10-08 16:45 UTC (permalink / raw)
  To: Hans Schillstrom; +Cc: Daniel Lezcano, netdev@vger.kernel.org
In-Reply-To: <201010081428.37639.hans.schillstrom@ericsson.com>

Hans Schillstrom <hans.schillstrom@ericsson.com> writes:

> Hi Eric,
> Any advice how to trace this down ?
> This rollback_registered_many() seems to have on the lists before...

That is just the core piece that does device registration.

> All IPv4 and IPv6 tunnels causes this crash, all you have to do is load the tunnel module(s)
> enter a new ns and exit from it.

Ouch.  

> Have not tested any more devices than tunnels, 
> I did an "ip link delete" on my macvlans before exiting the ns.

My hunch is that we have dst entry problems, as I know those hop network
interfaces when we destroy network devices, but I have seen weird issues
with the route cache as well.

Grumble.

Eric

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox