Netdev List
 help / color / mirror / Atom feed
* Re: [Bugme-new] [Bug 15239] New: Problem with network performance after a while.
From: Brandeburg, Jesse @ 2010-02-08 23:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net,
	bugzilla-daemon@bugzilla.kernel.org,
	bugme-daemon@bugzilla.kernel.org, Kirsher, Jeffrey T,
	Allan, Bruce W, Duyck, Alexander H, Waskiewicz Jr, Peter P,
	Ronciak, John, pstaszewski@artcom.pl, stable@kernel.org
In-Reply-To: <20100208141513.b5d0be56.akpm@linux-foundation.org>


Andrew, thanks.

On Mon, 8 Feb 2010, Andrew Morton wrote:
> > Hello the problem is that after some time of working my router have some
> > performance problems.
> > 
> > The problem exist on kernel 2.6.32.7 and 2.6.33-rc6-git5
> > The problem dont exist on 2.6.32.2 kernel.
> 
> That would be bad.  That's a regression in 2.6.33-rc, and we've
> backported that regression into 2.6.32.x -stable.

there were no changes to e1000e from v2.6.32..v2.6.32.7
# git whatchanged v2.6.32..v2.6.32.7 drivers/net/e1000e

 
> > on 2.6.33-rc6-git5 i have this info:

> > BUG: soft lockup - CPU#1 stuck for 61s!
> > [events/1:28]

so why is this events/1 thing here?  I've see this kernel thread related 
to ipv6 stack hanging when under heavy incoming packet load as the 
interface comes up.  Was the interface just brought up or something 
changed in rules or something?

> > Modules linked in:
> > CPU 1
> > Pid: 28, comm: events/1 Not tainted 2.6.33-rc6-git5 #1 X7DCT/X7DCT
> > RIP: 0010:[<ffffffff810a3d89>]  [<ffffffff810a3d89>]
> > kmem_cache_free+0x11b/0x11c
> > RSP: 0018:ffff880028243e50  EFLAGS: 00000292
> > RAX: 0000000000000032 RBX: 000000000000007d RCX: ffff8803190683c0
> > RDX: 0000000000000031 RSI: ffff8803190683c0 RDI: ffff88031f83e680
> > RBP: ffffffff81002893 R08: 0000000000000000 R09: 000000000000007c
> > R10: ffff88030d776800 R11: ffff88030d7768a0 R12: ffff880028243dd0
> > R13: ffffc900008b2f80 R14: ffff88031fa7c800 R15: ffffffff81012da7
> > FS:  0000000000000000(0000) GS:ffff880028240000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > CR2: 00007fd61d5bd000 CR3: 000000031e55c000 CR4: 00000000000006a0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > Process events/1 (pid: 28, threadinfo ffff88031f9c8000, task ffff88031f9a4f80)
> > Stack:
> >  ffffffff8126826f ffff88031faa4600 ffffffff8126834a 000096ba00000023
> > <0> 01ffc90000000024 ffff88031fbb4000 ffff88031faa4600 0000000000000040
> > <0> 0000000000000040 ffff88031faa4788 ffff88031faa4600 0000000000000740
> > Call Trace:
> >  <IRQ>
> >  [<ffffffff8126826f>] ? e1000_put_txbuf+0x62/0x74
> >  [<ffffffff8126834a>] ? e1000_clean_tx_irq+0xc9/0x235

clean_tx_irq in e1000e has a break-out condition to keep the transmit 
cleanup for running forever (if another cpu is transmitting) so it seems 
that something else must have happened.  Can kmem_cache_free sleep or 
stall on a lock?


> >  [<ffffffff8126b71b>] ? e1000_clean+0x5c/0x21c
> >  [<ffffffff812f29a3>] ? net_rx_action+0x71/0x15d
> >  [<ffffffff81035311>] ? __do_softirq+0xd7/0x196
> >  [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> >  [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> >  [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> >  <EOI>
> >  [<ffffffff81004599>] ? do_softirq+0x31/0x63
> >  [<ffffffff81034ec1>] ? local_bh_enable_ip+0x75/0x86
> >  [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> >  [<ffffffff812f775d>] ? dst_gc_task+0xce/0x1a7
> >  [<ffffffff8136b08c>] ? schedule+0x82c/0x906
> >  [<ffffffff8103c44f>] ? lock_timer_base+0x26/0x4b
> >  [<ffffffff810a41d6>] ? cache_reap+0x0/0x11d
> >  [<ffffffff81044c38>] ? worker_thread+0x14c/0x1dc
> >  [<ffffffff81047dcd>] ? autoremove_wake_function+0x0/0x2e
> >  [<ffffffff81044aec>] ? worker_thread+0x0/0x1dc
> >  [<ffffffff810479bd>] ? kthread+0x79/0x81
> >  [<ffffffff81002cb4>] ? kernel_thread_helper+0x4/0x10
> >  [<ffffffff81047944>] ? kthread+0x0/0x81
> >  [<ffffffff81002cb0>] ? kernel_thread_helper+0x0/0x10
> > Code: fe 79 4c 00 48 85 db 74 14 48 8b 74 24 10 48 89 ef ff 13 48 83 c3 08 48
> > 83 3b 00 eb ea 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f <c3> 55 48 89 f5 53 48
> > 89 fb 48 83 ec 08 48 8b 76 18 48 2b 75 10
> > Call Trace:
> >  <IRQ>  [<ffffffff8126826f>] ? e1000_put_txbuf+0x62/0x74
> >  [<ffffffff8126834a>] ? e1000_clean_tx_irq+0xc9/0x235
> >  [<ffffffff8126b71b>] ? e1000_clean+0x5c/0x21c
> >  [<ffffffff812f29a3>] ? net_rx_action+0x71/0x15d
> >  [<ffffffff81035311>] ? __do_softirq+0xd7/0x196
> >  [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> >  [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> >  [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> >  <EOI>  [<ffffffff81004599>] ? do_softirq+0x31/0x63
> >  [<ffffffff81034ec1>] ? local_bh_enable_ip+0x75/0x86
> >  [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> >  [<ffffffff812f775d>] ? dst_gc_task+0xce/0x1a7
> >  [<ffffffff8136b08c>] ? schedule+0x82c/0x906
> >  [<ffffffff8103c44f>] ? lock_timer_base+0x26/0x4b
> >  [<ffffffff810a41d6>] ? cache_reap+0x0/0x11d
> >  [<ffffffff81044c38>] ? worker_thread+0x14c/0x1dc
> >  [<ffffffff81047dcd>] ? autoremove_wake_function+0x0/0x2e
> >  [<ffffffff81044aec>] ? worker_thread+0x0/0x1dc
> >  [<ffffffff810479bd>] ? kthread+0x79/0x81
> >  [<ffffffff81002cb4>] ? kernel_thread_helper+0x4/0x10
> >  [<ffffffff81047944>] ? kthread+0x0/0x81
> >  [<ffffffff81002cb0>] ? kernel_thread_helper+0x0/0x10
> > 
> > And on 2.6.32.7
> > Feb  4 13:12:40 TM_01_C1 ------------[ cut here ]------------
> > Feb  4 13:12:40 TM_01_C1 WARNING: at net/sched/sch_generic.c:261
> > dev_watchdog+0x130/0x1d6()
> > Feb  4 13:12:40 TM_01_C1 Hardware name: X7DCT
> > Feb  4 13:12:40 TM_01_C1 NETDEV WATCHDOG: eth0 (e1000e): transmit queue 0 timed
> > out
> > Feb  4 13:12:40 TM_01_C1 Modules linked in: oprofile
> > Feb  4 13:12:40 TM_01_C1 Pid: 0, comm: swapper Not tainted 2.6.32 #1

the above doesn't match your statement that it is 2.6.32.7

> > Feb  4 13:12:40 TM_01_C1 Call Trace:
> > Feb  4 13:12:40 TM_01_C1 <IRQ>  [<ffffffff812fcaf7>] ? dev_watchdog+0x130/0x1d6
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff812fcaf7>] ? dev_watchdog+0x130/0x1d6
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff81038811>] ? warn_slowpath_common+0x77/0xa3
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff81038899>] ? warn_slowpath_fmt+0x51/0x59
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8102897e>] ? activate_task+0x3f/0x4e
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff81034fe5>] ? try_to_wake_up+0x1eb/0x1f8
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff812eb768>] ? netdev_drivername+0x3b/0x40
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff812fcaf7>] ? dev_watchdog+0x130/0x1d6
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8102d1e3>] ? __wake_up+0x30/0x44
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff812fc9c7>] ? dev_watchdog+0x0/0x1d6
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff810448c4>] ? run_timer_softirq+0x1ff/0x29d
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff810556ab>] ? ktime_get+0x5f/0xb7
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8103e0fd>] ? __do_softirq+0xd7/0x196
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8100be7c>] ? call_softirq+0x1c/0x28
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8100d645>] ? do_softirq+0x31/0x66
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8101b148>] ?
> > smp_apic_timer_interrupt+0x87/0x95
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8100b873>] ? apic_timer_interrupt+0x13/0x20
> > Feb  4 13:12:40 TM_01_C1 <EOI>  [<ffffffff810111f5>] ? mwait_idle+0x9b/0xa0
> > Feb  4 13:12:40 TM_01_C1 [<ffffffff8100a236>] ? cpu_idle+0x49/0x7c
> > Feb  4 13:12:40 TM_01_C1 ---[ end trace c670a6a17be040e5 ]---
> > 
> > 
> > Machine is a BGP router with some iproute traffic management.

please send a full dmesg from boot.  also full lspci -vvv.

We sporadically see some issue with some people's use of traffic 
management with some e1000/e devices, we have no repro or common 
reproduction case, so haven't solved the issue yet.  I suspect there is a 
strange interaction with HTB or one of the other queueing disciplines that 
causes some odd skb or set of skb's to arrive, but that is just a theory.

I see two different bugs above, one is the tx hang, could it be that you 
didn't have SOFTLOCKUP detection enabled on 2.6.32?  A softlockup could 
cause a tx hang.

The other bug is the softlockup and I don't know how the driver could be 
at fault in that listed stack path.  We simply can't loop more than 
tx_ring->count times, which could never take > 60 seconds unless one of 
the calls we were making was getting stalled for some reason.

So your traffic is "heavy" forwarding at the time of the failure, do you 
have any input on average packet size?  I'm trying to figure out how we 
might reproduce this.

Jesse

^ permalink raw reply

* Re: [Bugme-new] [Bug 15239] New: Problem with network performance after a while.
From: David Miller @ 2010-02-08 23:14 UTC (permalink / raw)
  To: jesse.brandeburg
  Cc: akpm, netdev, e1000-devel, bugzilla-daemon, bugme-daemon,
	jeffrey.t.kirsher, bruce.w.allan, alexander.h.duyck,
	peter.p.waskiewicz.jr, john.ronciak, pstaszewski, stable
In-Reply-To: <alpine.WNT.2.00.1002081432390.6892@jbrandeb-desk1.amr.corp.intel.com>

From: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
Date: Mon, 8 Feb 2010 15:06:24 -0800 (Pacific Standard Time)

>> > on 2.6.33-rc6-git5 i have this info:
> 
>> > BUG: soft lockup - CPU#1 stuck for 61s!
>> > [events/1:28]
> 
> so why is this events/1 thing here?

Jesse, this is taken care of already, see:

http://marc.info/?l=linux-netdev&m=126565759512718&w=2

^ permalink raw reply

* Re: [Bugme-new] [Bug 15239] New: Problem with network performance after a while.
From: Rafael J. Wysocki @ 2010-02-08 23:04 UTC (permalink / raw)
  To: David Miller; +Cc: akpm, netdev, maciej.rutecki, Kernel Bugzilla
In-Reply-To: <20100208.145857.27251976.davem@davemloft.net>

On Monday 08 February 2010, David Miller wrote:
> From: "Rafael J. Wysocki" <rjw@sisk.pl>
> Date: Mon, 8 Feb 2010 23:51:30 +0100
> 
> > On Monday 08 February 2010, Andrew Morton wrote:
> >> On Sat, 6 Feb 2010 14:39:46 GMT
> >> bugzilla-daemon@bugzilla.kernel.org wrote:
> >> 
> >> > http://bugzilla.kernel.org/show_bug.cgi?id=15239
> >> 
> >> post-2.6.32 regression, backported into -stable :(
> > 
> > Added, CCing netdev.
> 
> No need, we already worked on this and have a fix,
> see:
> 
> http://marc.info/?l=linux-netdev&m=126565759512718&w=2

Great, thanks!

^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: Andrew Morton @ 2010-02-08 23:26 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Paweł Staszewski, David Miller,
	Linux Network Development list
In-Reply-To: <1265657560.4236.80.camel@edumazet-laptop>

On Mon, 08 Feb 2010 20:32:40 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> [PATCH] dst: call cond_resched() in dst_gc_task()
> 
> On some workloads, it is quite possible to get a huge dst list to
> process in dst_gc_task(), and trigger soft lockup detection.
> 
> Fix is to call cond_resched(), as we run in process context.
> 
> Reported-by: Pawel Staszewski <pstaszewski@itcare.pl>
> Tested-by: Pawel Staszewski <pstaszewski@itcare.pl>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 57bc4d5..cb1b348 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -17,6 +17,7 @@
>  #include <linux/string.h>
>  #include <linux/types.h>
>  #include <net/net_namespace.h>
> +#include <linux/sched.h>
>  
>  #include <net/dst.h>
>  
> @@ -79,6 +80,7 @@ loop:
>  	while ((dst = next) != NULL) {
>  		next = dst->next;
>  		prefetch(&next->next);
> +		cond_resched();
>  		if (likely(atomic_read(&dst->__refcnt))) {
>  			last->next = dst;
>  			last = dst;

Gad.  Am I understanding this right?  The softlockup threshold is sixty
seconds!

I assume that this function spends most of its time walking over busy
entries?  Is a more powerful data structure needed?


^ permalink raw reply

* Re: [PATCH 0/3 v4] macvtap driver
From: Ed Swierk @ 2010-02-08 23:30 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: arnd, netdev
In-Reply-To: <1265655334.31760.9.camel@w-sridhar.beaverton.ibm.com>

On Mon, 2010-02-08 at 10:55 -0800, Sridhar Samudrala wrote:
> I am also seeing this issue with net-next-2.6.
> Basically macvtap_put_user() and macvtap_get_user() call copy_to/from_user
> from within a RCU read-side critical section.
> 
> The following patch fixes this issue by releasing the RCU read lock before
> calling these routines, but instead hold a reference to q->sk.

Thanks, I tried your patch and it fixes the problem.

However, it seems to cause another minor problem.  macvlan_count_rx() is
now getting called from macvtap_put_user() with preemption enabled,
which causes smp_processor_id() to BUG:

Feb  8 20:31:38 ti102 kernel: BUG: using smp_processor_id() in
preemptible [00000000] code: qemu-kvm/4546 
Feb  8 20:31:38 ti102 kernel: caller is macvtap_aio_read+0x18c/0x221
[macvtap] 
Feb  8 20:31:38 ti102 kernel: Pid: 4546, comm: qemu-kvm Not tainted
2.6.29.6.Ar-224686.2009eswierk8.2 #1 
Feb  8 20:31:38 ti102 kernel: Call Trace: 
Feb  8 20:31:38 ti102 kernel: [<c0349546>] ? printk+0xf/0x11 
Feb  8 20:31:38 ti102 kernel: [<c02142c0>] debug_smp_processor_id
+0xa4/0xb8 
Feb  8 20:31:38 ti102 kernel: [<f8af581f>] macvtap_aio_read+0x18c/0x221
[macvtap] 
Feb  8 20:31:38 ti102 kernel: [<c011eaf7>] ? default_wake_function
+0x0/0xd 
Feb  8 20:31:38 ti102 kernel: [<c016c75f>] do_sync_read+0xab/0xe9 
Feb  8 20:31:38 ti102 kernel: [<c011933d>] ? update_curr+0x6c/0x147 
Feb  8 20:31:38 ti102 kernel: [<c0133933>] ? autoremove_wake_function
+0x0/0x33 
Feb  8 20:31:38 ti102 kernel: [<c0349fd0>] ? schedule+0x7af/0x7e3 
Feb  8 20:31:38 ti102 kernel: [<c016d101>] vfs_read+0xb5/0x129 
Feb  8 20:31:38 ti102 kernel: [<c016d20e>] sys_read+0x3b/0x60 
Feb  8 20:31:38 ti102 kernel: [<c0102e71>] sysenter_do_call+0x12/0x25 

I fixed this problem with the change below.  I'm not sure if replacing
smp_processor_id() with get_cpu() is the right thing to do but it works
for macvtap at least.

Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>

---
Index: linux-2.6.29.6/include/linux/if_macvlan.h
===================================================================
--- linux-2.6.29.6.orig/include/linux/if_macvlan.h
+++ linux-2.6.29.6/include/linux/if_macvlan.h
@@ -42,8 +42,9 @@ static inline void macvlan_count_rx(cons
 				    bool multicast)
 {
 	struct macvlan_rx_stats *rx_stats;
+	int cpu = get_cpu();
 
-	rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
+	rx_stats = per_cpu_ptr(vlan->rx_stats, cpu);
 	if (likely(success)) {
 		rx_stats->rx_packets++;;
 		rx_stats->rx_bytes += len;
@@ -52,6 +53,7 @@ static inline void macvlan_count_rx(cons
 	} else {
 		rx_stats->rx_errors++;
 	}
+	put_cpu();
 }
 
 extern int macvlan_common_newlink(struct net_device *dev,



^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: David Miller @ 2010-02-08 23:34 UTC (permalink / raw)
  To: akpm; +Cc: eric.dumazet, pstaszewski, netdev
In-Reply-To: <20100208152606.91c55722.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 8 Feb 2010 15:26:06 -0800

> I assume that this function spends most of its time walking over busy
> entries?  Is a more powerful data structure needed?

When you're getting pounded with millions of packets per second,
all mostly to different destinations (and thus resolving to
different routing cache entries), this is what happens.

For a busy router, really, this is normal behavior.


^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: Andrew Morton @ 2010-02-08 23:37 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, pstaszewski, netdev
In-Reply-To: <20100208.153406.123254133.davem@davemloft.net>

On Mon, 08 Feb 2010 15:34:06 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Mon, 8 Feb 2010 15:26:06 -0800
> 
> > I assume that this function spends most of its time walking over busy
> > entries?  Is a more powerful data structure needed?
> 
> When you're getting pounded with millions of packets per second,
> all mostly to different destinations (and thus resolving to
> different routing cache entries), this is what happens.
> 
> For a busy router, really, this is normal behavior.

Is the cache a net win in that scenario?

^ permalink raw reply

* Re: PROBLEM: oops w/ bridge in 2.6.32.7
From: Stephen Hemminger @ 2010-02-08 23:47 UTC (permalink / raw)
  To: Andrew Morton; +Cc: lkml, Linux Kernel-Liste, netdev, bridge
In-Reply-To: <20100208142044.6be97024.akpm@linux-foundation.org>

On Mon, 8 Feb 2010 14:20:44 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> > virtualbox-ose              3.1.2-dfsg-1+b1
> > virtualbox-ose-source       3.1.2-dfsg-1
> > 


Sorry, since virtualbox is not part of standard mainline kernel,
I suspect the problem is there.
Please reproduce with standard 2.6.32.X distribution.

^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: David Miller @ 2010-02-08 23:50 UTC (permalink / raw)
  To: akpm; +Cc: eric.dumazet, pstaszewski, netdev
In-Reply-To: <20100208153744.236158aa.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 8 Feb 2010 15:37:44 -0800

> On Mon, 08 Feb 2010 15:34:06 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
>> For a busy router, really, this is normal behavior.
> 
> Is the cache a net win in that scenario?

Absolutely.

^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: Stephen Hemminger @ 2010-02-08 23:50 UTC (permalink / raw)
  To: Andrew Morton; +Cc: David Miller, eric.dumazet, pstaszewski, netdev
In-Reply-To: <20100208153744.236158aa.akpm@linux-foundation.org>

On Mon, 8 Feb 2010 15:37:44 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 08 Feb 2010 15:34:06 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Mon, 8 Feb 2010 15:26:06 -0800
> > 
> > > I assume that this function spends most of its time walking over busy
> > > entries?  Is a more powerful data structure needed?
> > 
> > When you're getting pounded with millions of packets per second,
> > all mostly to different destinations (and thus resolving to
> > different routing cache entries), this is what happens.
> > 
> > For a busy router, really, this is normal behavior.
> 
> Is the cache a net win in that scenario?

No, cache doesn't help.

Robert who is the expert in this area, runs with FIB TRIE and
no routing cache.

^ permalink raw reply

* [PATCH 1/1 net-next] virtio_net: remove send queue
From: Shirley Ma @ 2010-02-09  0:14 UTC (permalink / raw)
  To: davem; +Cc: netdev, kvm, rusty, mst, linux-kernel

Now we have a virtio detach API (in commit
f9bfbebf34eab707b065116cdc9699d25ba4252a), we don't need to track xmit
skbs in the virio_net driver, which improves transmission performance.

Signed-off-by: Shirley Ma <xma@us.ibm.com>
Acked-by: Rusty Russell <rusty@rustcorp.com.au>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |   26 ++++++++------------------
 1 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9d8984a..8069c08 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -56,9 +56,6 @@ struct virtnet_info
 	/* Host will merge rx buffers for big packets (shake it! shake it!) */
 	bool mergeable_rx_bufs;
 
-	/* Send queue. */
-	struct sk_buff_head send;
-
 	/* Work struct for refilling if we run low on memory. */
 	struct delayed_work refill;
 
@@ -505,7 +502,6 @@ static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
 
 	while ((skb = vi->svq->vq_ops->get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
-		__skb_unlink(skb, &vi->send);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
 		tot_sgs += skb_vnet_hdr(skb)->num_sg;
@@ -588,15 +584,6 @@ again:
 	}
 	vi->svq->vq_ops->kick(vi->svq);
 
-	/*
-	 * Put new one in send queue.  You'd expect we'd need this before
-	 * xmit_skb calls add_buf(), since the callback can be triggered
-	 * immediately after that.  But since the callback just triggers
-	 * another call back here, normal network xmit locking prevents the
-	 * race.
-	 */
-	__skb_queue_head(&vi->send, skb);
-
 	/* Don't wait up for transmitted skbs to be freed. */
 	skb_orphan(skb);
 	nf_reset(skb);
@@ -977,9 +964,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 			dev->features |= NETIF_F_HW_VLAN_FILTER;
 	}
 
-	/* Initialize our empty send queue. */
-	skb_queue_head_init(&vi->send);
-
 	err = register_netdev(dev);
 	if (err) {
 		pr_debug("virtio_net: registering device failed\n");
@@ -1016,6 +1000,12 @@ static void free_unused_bufs(struct virtnet_info *vi)
 {
 	void *buf;
 	while (1) {
+		buf = vi->svq->vq_ops->detach_unused_buf(vi->svq);
+		if (!buf)
+			break;
+		dev_kfree_skb(buf);
+	}
+	while (1) {
 		buf = vi->rvq->vq_ops->detach_unused_buf(vi->rvq);
 		if (!buf)
 			break;
@@ -1035,11 +1025,11 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
 	/* Stop all the virtqueues. */
 	vdev->config->reset(vdev);
 
-	/* Free our skbs in send queue, if any. */
-	__skb_queue_purge(&vi->send);
 
 	unregister_netdev(vi->dev);
 	cancel_delayed_work_sync(&vi->refill);
+
+	/* Free unused buffers in both send and recv, if any. */
 	free_unused_bufs(vi);
 
 	vdev->config->del_vqs(vi->vdev);

^ permalink raw reply related

* [PATCH net-2.6] cxgb3: fix GRO checksum check
From: Divy Le Ray @ 2010-02-09  0:55 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, swise

From: Divy Le Ray <divy@chelsio.com>

Verify the HW checksum state for frames handed to GRO processing.

Signed-off-by: Divy Le Ray <divy@chelsio.com>
---

 drivers/net/cxgb3/sge.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)


diff --git a/drivers/net/cxgb3/sge.c b/drivers/net/cxgb3/sge.c
index bdbd147..318a018 100644
--- a/drivers/net/cxgb3/sge.c
+++ b/drivers/net/cxgb3/sge.c
@@ -2079,6 +2079,7 @@ static void lro_add_page(struct adapter *adap, struct sge_qset *qs,
 			 struct sge_fl *fl, int len, int complete)
 {
 	struct rx_sw_desc *sd = &fl->sdesc[fl->cidx];
+	struct port_info *pi = netdev_priv(qs->netdev);
 	struct sk_buff *skb = NULL;
 	struct cpl_rx_pkt *cpl;
 	struct skb_frag_struct *rx_frag;
@@ -2116,11 +2117,18 @@ static void lro_add_page(struct adapter *adap, struct sge_qset *qs,
 
 	if (!nr_frags) {
 		offset = 2 + sizeof(struct cpl_rx_pkt);
-		qs->lro_va = sd->pg_chunk.va + 2;
-	}
-	len -= offset;
+		cpl = qs->lro_va = sd->pg_chunk.va + 2;
 
-	prefetch(qs->lro_va);
+		if ((pi->rx_offload & T3_RX_CSUM) &&
+		     cpl->csum_valid && cpl->csum == htons(0xffff)) {
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+			qs->port_stats[SGE_PSTAT_RX_CSUM_GOOD]++;
+		} else
+			skb->ip_summed = CHECKSUM_NONE;
+	} else
+		cpl = qs->lro_va;
+
+	len -= offset;
 
 	rx_frag += nr_frags;
 	rx_frag->page = sd->pg_chunk.page;
@@ -2136,12 +2144,8 @@ static void lro_add_page(struct adapter *adap, struct sge_qset *qs,
 		return;
 
 	skb_record_rx_queue(skb, qs - &adap->sge.qs[0]);
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
-	cpl = qs->lro_va;
 
 	if (unlikely(cpl->vlan_valid)) {
-		struct net_device *dev = qs->netdev;
-		struct port_info *pi = netdev_priv(dev);
 		struct vlan_group *grp = pi->vlan_grp;
 
 		if (likely(grp != NULL)) {

^ permalink raw reply related

* linux-next: manual merge of the net tree with the wireless-current tree
From: Stephen Rothwell @ 2010-02-09  2:24 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Johannes Berg, John W. Linville

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

Hi all,

Today's linux-next merge of the net tree got a conflict in
net/mac80211/scan.c between commit
c0ce77b8323c1a0d4eeef97caf16c0ea971222a9 ("mac80211: fix deferred
hardware scan requests") from the wireless-current tree and commit
af6b63741cc4e4dfd575d06beb333b11a8a6e0c0 ("mac80211: generalise work
handling") from the net tree.

I resolved this the same way it is resolved in the merge of the
wireless-current tree into the wireless tree.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: linux-next: manual merge of the net tree with the wireless-current tree
From: John W. Linville @ 2010-02-09  3:20 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, netdev, linux-next, linux-kernel, Johannes Berg
In-Reply-To: <20100209132444.d90b48a6.sfr@canb.auug.org.au>

On Tue, Feb 09, 2010 at 01:24:44PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the net tree got a conflict in
> net/mac80211/scan.c between commit
> c0ce77b8323c1a0d4eeef97caf16c0ea971222a9 ("mac80211: fix deferred
> hardware scan requests") from the wireless-current tree and commit
> af6b63741cc4e4dfd575d06beb333b11a8a6e0c0 ("mac80211: generalise work
> handling") from the net tree.
> 
> I resolved this the same way it is resolved in the merge of the
> wireless-current tree into the wireless tree.

Maybe it would make sense to pull the wireless trees _before_ the
greater net trees?  That way you can get the benefit of my merge
conflict resolutions automatically.

John
-- 
John W. Linville                Someday the world will need a hero, and you
linville@tuxdriver.com                  might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH 0/3 v4] macvtap driver
From: Ed Swierk @ 2010-02-09  3:25 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: arnd, netdev
In-Reply-To: <1265655334.31760.9.camel@w-sridhar.beaverton.ibm.com>

On Mon, 2010-02-08 at 10:55 -0800, Sridhar Samudrala wrote:
> I am also seeing this issue with net-next-2.6.
> Basically macvtap_put_user() and macvtap_get_user() call copy_to/from_user
> from within a RCU read-side critical section.
> 
> The following patch fixes this issue by releasing the RCU read lock before
> calling these routines, but instead hold a reference to q->sk.

I've encountered some more problems, with various users of
macvtap_file_get_queue() either calling or neglecting to call
macvtap_file_put_queue() in error cases.

I modified your patch so that when macvtap_file_get_queue() returns 0,
it also calls rcu_read_unlock_bh(), and modified the users
appropriately.

This patch also incorporates my preemption fix for macvlan_count_rx().

Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>

---
On Mon, 2010-02-08 at 09:14 -0800, Ed Swierk wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Sat, 30 Jan 2010 23:22:15 +0100
> >
> >> This is the fourth version of the macvtap driver,
> >> based on the comments I got for the last version
> >> I got a few days ago. Very few changes:
> >>
> >> * release netdev in chardev open function so
> >>   we can destroy it properly.
> >> * Implement TUNSETSNDBUF
> >> * fix sleeping call in rcu_read_lock
> >> * Fix comment in namespace isolation patch
> >> * Fix small context difference to make it apply
> >>   to net-next
> >>
> >> I can't really test here while travelling, so please
> >> give it a go if you're interested in this driver.
> 
> I'm seeing complaints from might_sleep():
> 
> Feb  8 16:21:06 ti102 kernel: BUG: sleeping function called from
> invalid context at include/linux/kernel.h:155
> Feb  8 16:21:06 ti102 kernel: in_atomic(): 1, irqs_disabled(): 0, pid:
> 2881, name: qemu-kvm
> Feb  8 16:21:06 ti102 kernel: Pid: 2881, comm: qemu-kvm Not tainted
> 2.6.29.6.Ar-224527.2009eswierk8 #1
> Feb  8 16:21:06 ti102 kernel: Call Trace:
> Feb  8 16:21:06 ti102 kernel: [<c0119250>] __might_sleep+0xdc/0xe3
> Feb  8 16:21:06 ti102 kernel: [<c0210f7c>] copy_to_user+0x36/0x106
> Feb  8 16:21:06 ti102 kernel: [<c02af568>] memcpy_toiovec+0x2c/0x50
> Feb  8 16:21:06 ti102 kernel: [<c02afbb3>] skb_copy_datagram_iovec+0x47/0x184
> Feb  8 16:21:06 ti102 kernel: [<c034bd07>] ? _spin_unlock_irqrestore+0x17/0x2c
> Feb  8 16:21:06 ti102 kernel: [<f829a776>]
> macvtap_aio_read+0x102/0x158 [macvtap]
> Feb  8 16:21:06 ti102 kernel: [<c011eaf7>] ? default_wake_function+0x0/0xd
> Feb  8 16:21:06 ti102 kernel: [<c016c75f>] do_sync_read+0xab/0xe9
> Feb  8 16:21:06 ti102 kernel: [<c0133933>] ? autoremove_wake_function+0x0/0x33
> Feb  8 16:21:06 ti102 kernel: [<c019211f>] ? eventfd_read+0x121/0x156
> Feb  8 16:21:06 ti102 kernel: [<c011eaf7>] ? default_wake_function+0x0/0xd
> Feb  8 16:21:06 ti102 kernel: [<c016d101>] vfs_read+0xb5/0x129
> Feb  8 16:21:06 ti102 kernel: [<c016d20e>] sys_read+0x3b/0x60
> Feb  8 16:21:06 ti102 kernel: [<c0102e71>] sysenter_do_call+0x12/0x25

I am also seeing this issue with net-next-2.6.
Basically macvtap_put_user() and macvtap_get_user() call copy_to/from_user
from within a RCU read-side critical section.

The following patch fixes this issue by releasing the RCU read lock before
calling these routines, but instead hold a reference to q->sk.

Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

Index: linux-2.6.29.6/drivers/net/macvtap.c
===================================================================
--- linux-2.6.29.6.orig/drivers/net/macvtap.c
+++ linux-2.6.29.6/drivers/net/macvtap.c
@@ -160,8 +160,12 @@ static void macvtap_del_queues(struct ne
 
 static inline struct macvtap_queue *macvtap_file_get_queue(struct file *file)
 {
+	struct macvtap_queue *q;
 	rcu_read_lock_bh();
-	return rcu_dereference(file->private_data);
+	q = rcu_dereference(file->private_data);
+	if (!q)
+		rcu_read_unlock_bh();
+	return q;
 }
 
 static inline void macvtap_file_put_queue(void)
@@ -313,13 +317,14 @@ static unsigned int macvtap_poll(struct 
 	     sock_writeable(&q->sk)))
 		mask |= POLLOUT | POLLWRNORM;
 
-out:
 	macvtap_file_put_queue();
+
+out:
 	return mask;
 }
 
 /* Get packet from user space buffer */
-static ssize_t macvtap_get_user(struct macvtap_queue *q,
+static ssize_t macvtap_get_user(struct macvlan_dev *vlan, struct sock *sk,
 				struct iovec *iv, size_t count,
 				int noblock)
 {
@@ -330,10 +335,10 @@ static ssize_t macvtap_get_user(struct m
 	if (unlikely(len < ETH_HLEN))
 		return -EINVAL;
 
-	skb = sock_alloc_send_skb(&q->sk, NET_IP_ALIGN + len, noblock, &err);
+	skb = sock_alloc_send_skb(sk, NET_IP_ALIGN + len, noblock, &err);
 
 	if (!skb) {
-		macvlan_count_rx(q->vlan, 0, false, false);
+		macvlan_count_rx(vlan, 0, false, false);
 		return err;
 	}
 
@@ -341,14 +346,14 @@ static ssize_t macvtap_get_user(struct m
 	skb_put(skb, count);
 
 	if (skb_copy_datagram_from_iovec(skb, 0, iv, len)) {
-		macvlan_count_rx(q->vlan, 0, false, false);
+		macvlan_count_rx(vlan, 0, false, false);
 		kfree_skb(skb);
 		return -EFAULT;
 	}
 
 	skb_set_network_header(skb, ETH_HLEN);
 
-	macvlan_start_xmit(skb, q->vlan->dev);
+	macvlan_start_xmit(skb, vlan->dev);
 
 	return count;
 }
@@ -359,23 +364,29 @@ static ssize_t macvtap_aio_write(struct 
 	struct file *file = iocb->ki_filp;
 	ssize_t result = -ENOLINK;
 	struct macvtap_queue *q = macvtap_file_get_queue(file);
+	struct macvlan_dev *vlan;
+	struct sock *sk;
 
 	if (!q)
 		goto out;
 
-	result = macvtap_get_user(q, (struct iovec *) iv, iov_length(iv, count),
+	vlan = q->vlan;
+	sk = &q->sk;
+	sock_hold(sk);
+	macvtap_file_put_queue();
+
+	result = macvtap_get_user(vlan, sk, (struct iovec *) iv, iov_length(iv, count),
 			      file->f_flags & O_NONBLOCK);
+	sock_put(sk);
 out:
-	macvtap_file_put_queue();
 	return result;
 }
 
 /* Put packet to the user space buffer */
-static ssize_t macvtap_put_user(struct macvtap_queue *q,
+static ssize_t macvtap_put_user(struct macvlan_dev *vlan,
 				struct sk_buff *skb,
 				struct iovec *iv, int len)
 {
-	struct macvlan_dev *vlan = q->vlan;
 	int ret;
 
 	len = min_t(int, skb->len, len);
@@ -392,15 +403,20 @@ static ssize_t macvtap_aio_read(struct k
 {
 	struct file *file = iocb->ki_filp;
 	struct macvtap_queue *q = macvtap_file_get_queue(file);
+	struct macvlan_dev *vlan;
+	struct sock *sk;
 
 	DECLARE_WAITQUEUE(wait, current);
 	struct sk_buff *skb;
 	ssize_t len, ret = 0;
 
-	if (!q) {
-		ret = -ENOLINK;
-		goto out;
-	}
+	if (!q)
+		return -ENOLINK;
+
+	vlan = q->vlan;
+	sk = &q->sk;
+	sock_hold(sk);
+	macvtap_file_put_queue();
 
 	len = iov_length(iv, count);
 	if (len < 0) {
@@ -408,12 +424,12 @@ static ssize_t macvtap_aio_read(struct k
 		goto out;
 	}
 
-	add_wait_queue(q->sk.sk_sleep, &wait);
+	add_wait_queue(sk->sk_sleep, &wait);
 	while (len) {
 		current->state = TASK_INTERRUPTIBLE;
 
 		/* Read frames from the queue */
-		skb = skb_dequeue(&q->sk.sk_receive_queue);
+		skb = skb_dequeue(&sk->sk_receive_queue);
 		if (!skb) {
 			if (file->f_flags & O_NONBLOCK) {
 				ret = -EAGAIN;
@@ -427,16 +443,16 @@ static ssize_t macvtap_aio_read(struct k
 			schedule();
 			continue;
 		}
-		ret = macvtap_put_user(q, skb, (struct iovec *) iv, len);
+		ret = macvtap_put_user(vlan, skb, (struct iovec *) iv, len);
 		kfree_skb(skb);
 		break;
 	}
 
 	current->state = TASK_RUNNING;
-	remove_wait_queue(q->sk.sk_sleep, &wait);
+	remove_wait_queue(sk->sk_sleep, &wait);
 
 out:
-	macvtap_file_put_queue();
+	sock_put(sk);
 	return ret;
 }
 
Index: linux-2.6.29.6/include/linux/if_macvlan.h
===================================================================
--- linux-2.6.29.6.orig/include/linux/if_macvlan.h
+++ linux-2.6.29.6/include/linux/if_macvlan.h
@@ -42,8 +42,9 @@ static inline void macvlan_count_rx(cons
 				    bool multicast)
 {
 	struct macvlan_rx_stats *rx_stats;
+	int cpu = get_cpu();
 
-	rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
+	rx_stats = per_cpu_ptr(vlan->rx_stats, cpu);
 	if (likely(success)) {
 		rx_stats->rx_packets++;;
 		rx_stats->rx_bytes += len;
@@ -52,6 +53,7 @@ static inline void macvlan_count_rx(cons
 	} else {
 		rx_stats->rx_errors++;
 	}
+	put_cpu();
 }
 
 extern int macvlan_common_newlink(struct net_device *dev,



^ permalink raw reply

* [PATCH net-next-2.6] be2net: bug fix in be_read_eeprom
From: Ajit Khaparde @ 2010-02-09  3:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

The offset to read the eeprom data was missing and
wrong eeprom data was being dumped. This patch fixes this.

>From Suresh R <sureshr@serverengines.com>
Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
---
 drivers/net/benet/be_ethtool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/benet/be_ethtool.c b/drivers/net/benet/be_ethtool.c
index 09d8899..dcc7f37 100644
--- a/drivers/net/benet/be_ethtool.c
+++ b/drivers/net/benet/be_ethtool.c
@@ -614,7 +614,7 @@ be_read_eeprom(struct net_device *netdev, struct ethtool_eeprom *eeprom,
 
 	if (!status) {
 		resp = (struct be_cmd_resp_seeprom_read *) eeprom_cmd.va;
-		memcpy(data, resp->seeprom_data, eeprom->len);
+		memcpy(data, resp->seeprom_data + eeprom->offset, eeprom->len);
 	}
 	pci_free_consistent(adapter->pdev, eeprom_cmd.size, eeprom_cmd.va,
 			eeprom_cmd.dma);
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 2/2] IPv6: remove trivial nested _bh suffix
From: Stephen Hemminger @ 2010-02-09  5:48 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Miller,
	YOSHIFUJI Hideaki / 吉藤英明, netdev
In-Reply-To: <20100208214805.1f8f2c6d@nehalam>

Don't need to disable bottom half it is already down in the
previous lock. Move some blank lines to group locking in same
context.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/ipv6/addrconf.c	2010-02-08 21:37:31.589584963 -0800
+++ b/net/ipv6/addrconf.c	2010-02-08 21:37:41.114027790 -0800
@@ -2802,14 +2802,14 @@ static void addrconf_dad_start(struct in
 	read_lock_bh(&idev->lock);
 	if (ifp->dead)
 		goto out;
-	spin_lock_bh(&ifp->lock);
 
+	spin_lock(&ifp->lock);
 	if (dev->flags&(IFF_NOARP|IFF_LOOPBACK) ||
 	    idev->cnf.accept_dad < 1 ||
 	    !(ifp->flags&IFA_F_TENTATIVE) ||
 	    ifp->flags & IFA_F_NODAD) {
 		ifp->flags &= ~(IFA_F_TENTATIVE|IFA_F_OPTIMISTIC|IFA_F_DADFAILED);
-		spin_unlock_bh(&ifp->lock);
+		spin_unlock(&ifp->lock);
 		read_unlock_bh(&idev->lock);
 
 		addrconf_dad_completed(ifp);
@@ -2817,7 +2817,7 @@ static void addrconf_dad_start(struct in
 	}
 
 	if (!(idev->if_flags & IF_READY)) {
-		spin_unlock_bh(&ifp->lock);
+		spin_unlock(&ifp->lock);
 		read_unlock_bh(&idev->lock);
 		/*
 		 * If the device is not ready:
@@ -2837,7 +2837,7 @@ static void addrconf_dad_start(struct in
 		ip6_ins_rt(ifp->rt);
 
 	addrconf_dad_kick(ifp);
-	spin_unlock_bh(&ifp->lock);
+	spin_unlock(&ifp->lock);
 out:
 	read_unlock_bh(&idev->lock);
 }
@@ -2853,14 +2853,15 @@ static void addrconf_dad_timer(unsigned 
 		read_unlock_bh(&idev->lock);
 		goto out;
 	}
-	spin_lock_bh(&ifp->lock);
+
+	spin_lock(&ifp->lock);
 	if (ifp->probes == 0) {
 		/*
 		 * DAD was successful
 		 */
 
 		ifp->flags &= ~(IFA_F_TENTATIVE|IFA_F_OPTIMISTIC|IFA_F_DADFAILED);
-		spin_unlock_bh(&ifp->lock);
+		spin_unlock(&ifp->lock);
 		read_unlock_bh(&idev->lock);
 
 		addrconf_dad_completed(ifp);
@@ -2870,7 +2871,7 @@ static void addrconf_dad_timer(unsigned 
 
 	ifp->probes--;
 	addrconf_mod_timer(ifp, AC_DAD, ifp->idev->nd_parms->retrans_time);
-	spin_unlock_bh(&ifp->lock);
+	spin_unlock(&ifp->lock);
 	read_unlock_bh(&idev->lock);
 
 	/* send a neighbour solicitation for our addr */
@@ -2918,12 +2919,12 @@ static void addrconf_dad_run(struct inet
 
 	read_lock_bh(&idev->lock);
 	for (ifp = idev->addr_list; ifp; ifp = ifp->if_next) {
-		spin_lock_bh(&ifp->lock);
+		spin_lock(&ifp->lock);
 		if (!(ifp->flags & IFA_F_TENTATIVE)) {
-			spin_unlock_bh(&ifp->lock);
+			spin_unlock(&ifp->lock);
 			continue;
 		}
-		spin_unlock_bh(&ifp->lock);
+		spin_unlock(&ifp->lock);
 		addrconf_dad_kick(ifp);
 	}
 	read_unlock_bh(&idev->lock);


-- 

^ permalink raw reply

* [PATCH 1/2] IPv6: keep permanent addresses on admin down
From: Stephen Hemminger @ 2010-02-09  5:48 UTC (permalink / raw)
  To: David Miller,
	YOSHIFUJI Hideaki / 吉藤英明; +Cc: netdev

Permanent IPV6 addresses should not be removed when the link is
set to admin down, only when device is removed. 

When link is lost permanent addresses should be marked as tentative
so that when link comes back they are subject to duplicate address
detection (if DAD was enabled for that address).

Other routing systems keep manually configured IPv6 addresses
when link is set down.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
The logic here is getting long and twisted, doing some work
to use RCU here and refactor.

--- a/net/ipv6/addrconf.c	2010-02-08 21:05:04.798785751 -0800
+++ b/net/ipv6/addrconf.c	2010-02-08 21:38:58.734340743 -0800
@@ -2646,7 +2646,8 @@ static int addrconf_ifdown(struct net_de
 
 		write_lock_bh(&addrconf_hash_lock);
 		while ((ifa = *bifa) != NULL) {
-			if (ifa->idev == idev) {
+			if (ifa->idev == idev &&
+			    (how || !(ifa->flags&IFA_F_PERMANENT))) {
 				*bifa = ifa->lst_next;
 				ifa->lst_next = NULL;
 				addrconf_del_timer(ifa);
@@ -2686,18 +2687,30 @@ static int addrconf_ifdown(struct net_de
 		write_lock_bh(&idev->lock);
 	}
 #endif
-	while ((ifa = idev->addr_list) != NULL) {
-		idev->addr_list = ifa->if_next;
-		ifa->if_next = NULL;
-		ifa->dead = 1;
-		addrconf_del_timer(ifa);
-		write_unlock_bh(&idev->lock);
-
-		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
-		in6_ifa_put(ifa);
+	bifa = &idev->addr_list;
+	while ((ifa = *bifa) != NULL) {
+		if (how == 0 && (ifa->flags&IFA_F_PERMANENT)) {
+			/* Retain permanent address on admin down */
+			bifa = &ifa->if_next;
+
+			/* Restart DAD if needed when link comes back up */
+			if ( !((dev->flags&(IFF_NOARP|IFF_LOOPBACK)) ||
+			       idev->cnf.accept_dad <= 0 ||
+			       (ifa->flags & IFA_F_NODAD)))
+				ifa->flags |= IFA_F_TENTATIVE;
+		} else {
+			*bifa = ifa->if_next;
+			ifa->if_next = NULL;
+
+			ifa->dead = 1;
+			write_unlock_bh(&idev->lock);
+
+			__ipv6_ifa_notify(RTM_DELADDR, ifa);
+			atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+			in6_ifa_put(ifa);
 
-		write_lock_bh(&idev->lock);
+			write_lock_bh(&idev->lock);
+		}
 	}
 	write_unlock_bh(&idev->lock);
 

^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: Eric Dumazet @ 2010-02-09  6:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Andrew Morton, David Miller, pstaszewski, netdev
In-Reply-To: <20100208155053.4ad68484@nehalam>

Le lundi 08 février 2010 à 15:50 -0800, Stephen Hemminger a écrit :

> No, cache doesn't help.
> 
> Robert who is the expert in this area, runs with FIB TRIE and
> no routing cache.

Who knows, it probably depends on many factors. I always run with cache
enabled, because it saves cycles on moderate load.

FIB_TRIE is unrelated here, if routing table is very small, it fits HASH
or TRIE.

Pawel hit the bug with tunables that basically enabled the cache but in
a non helpful way (filling the list of busy dst). User error combined
with a lazy kernel function :)

Please note that conversion from softirq to workqueue, without
scheduling point, might/probably use same cpu for handling network irqs
and running dst_gc_task() :

On big routers, admins usually use irq affinities, so we can have very
litle cpu time available to run other tasks on those cpus.

After this patch, I believe that scheduler is allowed to migrate
dst_gc_task() to an idle cpu.

Another point (for 2.6.34) to address is the dst_gc_mutex that can delay
NETDEV_UNREGISTER/NETDEV_DOWN events for a long period.




^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: Eric Dumazet @ 2010-02-09  6:07 UTC (permalink / raw)
  To: David Miller; +Cc: pstaszewski, netdev
In-Reply-To: <20100208.150126.25065601.davem@davemloft.net>

Le lundi 08 février 2010 à 15:01 -0800, David Miller a écrit :

> 
> When fixing bugs with kernel bugzilla entries, please
> mention them in the commit message.  I fixed this up for
> you but please take care of it next time.
> 
> Thanks!

Sorry Dave, I was not aware of the bugzilla entry.



^ permalink raw reply

* Re: [net-next PATCH v2 2/3] net: TCP thin linear timeouts
From: Eric Dumazet @ 2010-02-09  6:31 UTC (permalink / raw)
  To: Andreas Petlund
  Cc: netdev@vger.kernel.org, Ilpo Järvinen, Arnd Hannemann, LKML,
	shemminger, David Miller, william.allen.simpson
In-Reply-To: <4B701EE2.1000006@simula.no>

Le lundi 08 février 2010 à 15:25 +0100, Andreas Petlund a écrit :
>  
> +	case TCP_THIN_LT:
> +		if (val)
> +			tp->thin_lt = 1;
> +		break;
> +

Why not allowing user to clear thin_lt ?

^ permalink raw reply

* Re: [PATCH] dst: call cond_resched() in dst_gc_task()
From: Andrew Morton @ 2010-02-09  6:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, David Miller, pstaszewski, netdev
In-Reply-To: <1265695598.4236.95.camel@edumazet-laptop>

On Tue, 09 Feb 2010 07:06:38 +0100 Eric Dumazet <eric.dumazet@gmail.com> wrote:

> After this patch, I believe that scheduler is allowed to migrate
> dst_gc_task() to an idle cpu.

No, keventd threads are each pinned to a single CPU (kthread_bind() in
start_workqueue_thread()), so dst_gc_task() gets run on the CPU which
ran schedule_delayed_work() and no other.


^ permalink raw reply

* Re: [PATCH net-2.6] cxgb3: fix GRO checksum check
From: David Miller @ 2010-02-09  6:37 UTC (permalink / raw)
  To: divy; +Cc: netdev, linux-kernel, swise
In-Reply-To: <20100209005544.12679.3987.stgit@speedy5.asicdesigners.com>

From: Divy Le Ray <divy@chelsio.com>
Date: Mon, 08 Feb 2010 16:55:44 -0800

> From: Divy Le Ray <divy@chelsio.com>
> 
> Verify the HW checksum state for frames handed to GRO processing.
> 
> Signed-off-by: Divy Le Ray <divy@chelsio.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net/sched: Fix module name in Kconfig
From: David Miller @ 2010-02-09  6:41 UTC (permalink / raw)
  To: hadi; +Cc: jluebbe, netdev
In-Reply-To: <1265574348.3688.37.camel@bigi>

From: jamal <hadi@cyberus.ca>
Date: Sun, 07 Feb 2010 15:25:48 -0500

> On Sat, 2010-02-06 at 18:40 +0100, Jan Luebbe wrote:
>> The action modules have been prefixed with 'act_', but the Kconfig
>> description was not changed.
>> 
>> Signed-off-by: Jan Luebbe <jluebbe@debian.org>
> 
> Acked-by: Jamal Hadi Salim <hadi@cyberus.ca>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: networking drivers - Add git net-next tree
From: David Miller @ 2010-02-09  6:42 UTC (permalink / raw)
  To: joe; +Cc: netdev, linux-kernel, akpm, maximlevitsky
In-Reply-To: <1265489035.2063.35.camel@Joe-Laptop.home>

From: Joe Perches <joe@perches.com>
Date: Sat, 06 Feb 2010 12:43:55 -0800

> During the rc period, patches that are not bugfixes
> should be done using the net-next tree.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied, thanks.

^ 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