Flow cache flush oops

Flow cache flush oops

[Tom Lendacky]

I consistently receive the following oops on 2.5.72 on an SMP machine
after flushing some IPSec policy's and sa's that I had established and
sent data through.  I have not been able to reproduce this on a UP
machine.  I haven't done a lot of debugging in an SMP environment so
if anyone else can reproduce this and help debug, it would be greatly
appreciated.  The oops data is below.

Thanks,
Tom

Unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
c02bd20d
*pde = 00000000
Oops: 0000 [#1]
CPU:    1
EIP:    0060:[<c02bd20d>]    Not tainted
EFLAGS: 00010286
EIP is at flow_cache_flush_tasklet+0x3d/0xb0
eax: 00000000   ebx: 00000000   ecx: 0000000a   edx: 01408f80
esi: f7e7ff0c   edi: c04d2e80   ebp: 00000001   esp: f7e7fee8
ds: 007b   es: 007b   ss: 0068
Process events/1 (pid: 7, threadinfo=f7e7e000 task=f7f44ca0)
Stack: c0405920 f7e7ff0c f7e7ff14 00000293 c02bd391 f7e7ff0c f7e7ff0c 00000001
       00000000 00000000 00000001 00000001 00000001 f7e7ff1c f7e7ff1c f7532c60
       f7e7e000 f7035c00 f7e7ff4c f7f8c060 c0329a91 f7035c14 f7e7ff4c c0329b5b
Call Trace:
 [<c02bd391>] flow_cache_flush+0xa1/0xbf
 [<c0329a91>] xfrm_policy_gc_kill+0x71/0xa0
 [<c0329b5b>] xfrm_policy_gc_task+0x9b/0xb0
 [<c0134cf7>] worker_thread+0x237/0x330
 [<c0329ac0>] xfrm_policy_gc_task+0x0/0xb0
 [<c011fe30>] default_wake_function+0x0/0x30
 [<c010aff2>] ret_from_fork+0x6/0x14
 [<c011fe30>] default_wake_function+0x0/0x30
 [<c0134ac0>] worker_thread+0x0/0x330
 [<c0108ded>] kernel_thread_helper+0x5/0x18

Code: 8b 14 98 85 d2 74 36 8d b6 00 00 00 00 8d bf 00 00 00 00 8b

Re: Flow cache flush oops

[David S. Miller]

   From: Tom Lendacky <toml@us.ibm.com>
   Date: 18 Jun 2003 14:22:33 -0500

   I consistently receive the following oops on 2.5.72 on an SMP machine
   after flushing some IPSec policy's and sa's that I had established and
   sent data through.

I've forwarded your report to Herbert, who worked on this stuff.
He should be able to figure it out.

Re: Flow cache flush oops

[Tom Lendacky]

   If your compiler is producing anything like mine, then edi should be
   smp_processor_id() which looks a bit weird.  Please send me your
   net/core/flow.o file so that I can compare it.

   What compiler are you using? And how many CPUs?

The compiler version output is:
  gcc version 3.2 20020903 (Red Hat Linux 8.0 3.2-7)

The system has 4 CPUs.

Herbert, I'll send the flow.o file to you separately.

Thanks,
Tom

Re: Flow cache flush oops

[Herbert Xu]

On Wed, Jun 18, 2003 at 06:10:48PM -0500, Tom Lendacky wrote:
> 
> The compiler version output is:
>   gcc version 3.2 20020903 (Red Hat Linux 8.0 3.2-7)

Thanks, you've found two bugs in my code :) Firstly it's only initialising
one CPU so only that one has a flow cache.  Secondly flow_flush_cache is
not checking whether the calling CPU has been initialised.

I'll post a patch tonight.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Flow cache flush oops

[Herbert Xu]

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

On Thu, Jun 19, 2003 at 10:07:04AM +1000, herbert wrote:
> 
> Thanks, you've found two bugs in my code :) Firstly it's only initialising
> one CPU so only that one has a flow cache.  Secondly flow_flush_cache is
> not checking whether the calling CPU has been initialised.
> 
> I'll post a patch tonight.

Here is the patch.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2041 bytes --]

Index: kernel-source-2.5/net/core/flow.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/core/flow.c,v
retrieving revision 1.5
diff -u -r1.5 flow.c
--- kernel-source-2.5/net/core/flow.c	13 Jun 2003 11:22:17 -0000	1.5
+++ kernel-source-2.5/net/core/flow.c	19 Jun 2003 09:23:38 -0000
@@ -300,7 +300,8 @@
 
 	local_bh_disable();
 	smp_call_function(flow_cache_flush_per_cpu, &info, 1, 0);
-	flow_cache_flush_tasklet((unsigned long)&info);
+	if (test_bit(smp_processor_id(), &info.cpumap))
+		flow_cache_flush_tasklet((unsigned long)&info);
 	local_bh_enable();
 
 	wait_for_completion(&info.completion);
@@ -308,12 +309,13 @@
 	up(&flow_flush_sem);
 }
 
-static void __devinit flow_cache_cpu_online(int cpu)
+static void __devinit flow_cache_cpu_prepare(int cpu)
 {
 	struct tasklet_struct *tasklet;
 	unsigned long order;
 
 	flow_hash_rnd_recalc(cpu) = 1;
+	flow_count(cpu) = 0;
 
 	for (order = 0;
 	     (PAGE_SIZE << order) <
@@ -328,7 +330,10 @@
 
 	tasklet = flow_flush_tasklet(cpu);
 	tasklet_init(tasklet, flow_cache_flush_tasklet, 0);
+}
 
+static void __devinit flow_cache_cpu_online(int cpu)
+{
 	down(&flow_cache_cpu_sem);
 	set_bit(cpu, &flow_cache_cpu_map);
 	flow_cache_cpu_count++;
@@ -341,6 +346,9 @@
 	unsigned long cpu = (unsigned long)cpu;
 	switch (action) {
 	case CPU_UP_PREPARE:
+		flow_cache_cpu_prepare(cpu);
+		break;
+	case CPU_ONLINE:
 		flow_cache_cpu_online(cpu);
 		break;
 	}
@@ -353,6 +361,8 @@
 
 static int __init flow_cache_init(void)
 {
+	int i;
+
 	flow_cachep = kmem_cache_create("flow_cache",
 					sizeof(struct flow_cache_entry),
 					0, SLAB_HWCACHE_ALIGN,
@@ -370,8 +380,12 @@
 	flow_hash_rnd_timer.expires = jiffies + FLOW_HASH_RND_PERIOD;
 	add_timer(&flow_hash_rnd_timer);
 
-	flow_cache_cpu_online(smp_processor_id());
 	register_cpu_notifier(&flow_cache_cpu_nb);
+	for (i = 0; i < NR_CPUS; i++)
+		if (cpu_online(i)) {
+			flow_cache_cpu_prepare(i);
+			flow_cache_cpu_online(i);
+		}
 
 	return 0;
 }

Re: Flow cache flush oops

[Tom Lendacky]

   Here is the patch.

Installed and working.  Thank you for the quick response.


Tom

Re: Flow cache flush oops

[David S. Miller]

   From: Herbert Xu <herbert@gondor.apana.org.au>
   Date: Thu, 19 Jun 2003 19:35:18 +1000

   Here is the patch.

Applied, thanks Herbert.

Reviewing this made me notice that we don't check the
__get_free_pages() return value.  I'll fix that.

Thanks again.

Re: Flow cache flush oops

[Herbert Xu]

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

On Thu, Jun 19, 2003 at 12:11:08PM -0700, David S. Miller wrote:
> 
> Reviewing this made me notice that we don't check the
> __get_free_pages() return value.  I'll fix that.

Thanks.  Perhaps we should check that in the init function as well.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 611 bytes --]

--- x/net/core/flow.c.orig	2003-06-20 08:35:54.000000000 +1000
+++ x/net/core/flow.c	2003-06-20 08:41:37.000000000 +1000
@@ -388,11 +388,14 @@
 	add_timer(&flow_hash_rnd_timer);
 
 	register_cpu_notifier(&flow_cache_cpu_nb);
-	for (i = 0; i < NR_CPUS; i++)
-		if (cpu_online(i)) {
-			flow_cache_cpu_prepare(i);
-			flow_cache_cpu_online(i);
-		}
+	for (i = 0; i < NR_CPUS; i++) {
+		if (!cpu_online(i))
+			continue;
+		if (flow_cache_cpu_prepare(i) == NOTIFY_OK &&
+		    flow_cache_cpu_online(i) == NOTIFY_OK)
+			continue;
+		panic("NET: failed to initialise flow cache hash table\n");
+	}
 
 	return 0;
 }

Re: Flow cache flush oops

[David S. Miller]

   From: Herbert Xu <herbert@gondor.apana.org.au>
   Date: Fri, 20 Jun 2003 08:43:08 +1000
   
   Perhaps we should check that in the init function as well.

Yep, I've applied your patch.

Thanks.