netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
       [not found] ` <20070128114148.b8067721.randy.dunlap@oracle.com>
@ 2007-01-28 22:21   ` Adrian Bunk
  2007-01-28 23:53     ` David Miller
  0 siblings, 1 reply; 14+ messages in thread
From: Adrian Bunk @ 2007-01-28 22:21 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Andrew Morton, linux-kernel, netfilter, netdev

On Sun, Jan 28, 2007 at 11:41:48AM -0800, Randy Dunlap wrote:
>...
> net/built-in.o: In function `q931_help':
> nf_conntrack_h323_main.c:(.text.q931_help+0x6ad): undefined reference to `ip6_route_output'
> nf_conntrack_h323_main.c:(.text.q931_help+0x6c3): undefined reference to `ip6_route_output'
>...

You didn't send your .config, but it seems you had CONFIG_IPV6=m and
CONFIG_NF_CONNTRACK_H323=y.

In this case, the untested patch below should fix it.

> ~Randy

cu
Adrian


<--  snip  -->


CONFIG_IPV6=m, CONFIG_NF_CONNTRACK_H323=y results in a compile error.

Fix this by letting NF_CONNTRACK_H323 depend on (IPV6 || IPV6=n).

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.20-rc6-mm1/net/netfilter/Kconfig.old	2007-01-28 23:06:37.000000000 +0100
+++ linux-2.6.20-rc6-mm1/net/netfilter/Kconfig	2007-01-28 23:06:49.000000000 +0100
@@ -165,7 +165,7 @@
 
 config NF_CONNTRACK_H323
 	tristate "H.323 protocol support (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && NF_CONNTRACK
+	depends on EXPERIMENTAL && NF_CONNTRACK && (IPV6 || IPV6=n)
 	help
 	  H.323 is a VoIP signalling protocol from ITU-T. As one of the most
 	  important VoIP protocols, it is widely used by voice hardware and


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
       [not found] <20070127234928.64d8e437.akpm@osdl.org>
       [not found] ` <20070128114148.b8067721.randy.dunlap@oracle.com>
@ 2007-01-28 22:31 ` Michal Piotrowski
  2007-01-28 23:10   ` 2.6.20-rc6-mm1 Andrew Morton
  2007-01-29  5:17   ` 2.6.20-rc6-mm1 Herbert Xu
  1 sibling, 2 replies; 14+ messages in thread
From: Michal Piotrowski @ 2007-01-28 22:31 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, netdev

Andrew Morton napisał(a):
> Temporarily at
> 
> 	http://userweb.kernel.org/~akpm/2.6.20-rc6-mm1/
> 
> will appear one day at
> 
> 	ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm1/
> 
> 

Jan 28 22:58:29 euridica kernel: BUG: using smp_processor_id() in preemptible [00000001] code: yum-updatesd/2846
Jan 28 22:58:29 euridica kernel: caller is nf_conntrack_in+0x363/0x47f [nf_conntrack]
Jan 28 22:58:29 euridica kernel:  [<c01053c6>] show_trace_log_lvl+0x1a/0x2f
Jan 28 22:58:29 euridica kernel:  [<c0105ad6>] show_trace+0x12/0x14
Jan 28 22:58:29 euridica kernel:  [<c0105b98>] dump_stack+0x16/0x18
Jan 28 22:58:29 euridica kernel:  [<c0207803>] debug_smp_processor_id+0xb3/0xc8
Jan 28 22:58:29 euridica kernel:  [<fdbf8ad0>] nf_conntrack_in+0x363/0x47f [nf_conntrack]
Jan 28 22:58:29 euridica kernel:  [<fd9c32c4>] ipv4_conntrack_local+0x53/0x5b [nf_conntrack_ipv4]
Jan 28 22:58:29 euridica kernel:  [<c02f2286>] nf_iterate+0x36/0x67
Jan 28 22:58:29 euridica kernel:  [<c02f241b>] nf_hook_slow+0x52/0xbe
Jan 28 22:58:29 euridica kernel:  [<c02fd280>] ip_queue_xmit+0x3d1/0x420
Jan 28 22:58:29 euridica kernel:  [<c030b9b9>] tcp_transmit_skb+0x78b/0x7ca
Jan 28 22:58:29 euridica kernel:  [<c030d452>] __tcp_push_pending_frames+0x732/0x811
Jan 28 22:58:29 euridica kernel:  [<c030e3de>] tcp_send_fin+0x146/0x150
Jan 28 22:58:29 euridica kernel:  [<c03042c9>] tcp_close+0x22b/0x556
Jan 28 22:58:29 euridica kernel:  [<c031d595>] inet_release+0x43/0x49
Jan 28 22:58:29 euridica kernel:  [<c02d74c8>] sock_release+0x17/0x9d
Jan 28 22:58:29 euridica kernel:  [<c02d7843>] sock_close+0x2d/0x33
Jan 28 22:58:29 euridica kernel:  [<c0178a0a>] __fput+0xca/0x198
Jan 28 22:58:29 euridica kernel:  [<c0178c44>] fput+0x32/0x36
Jan 28 22:58:29 euridica kernel:  [<c017606a>] filp_close+0x54/0x5c
Jan 28 22:58:29 euridica kernel:  [<c0124e72>] put_files_struct+0x7d/0xba
Jan 28 22:58:29 euridica kernel:  [<c012625e>] do_exit+0x30d/0x8e6
Jan 28 22:58:29 euridica kernel:  [<c01268b9>] sys_exit_group+0x0/0x11
Jan 28 22:58:29 euridica kernel:  [<c012f177>] get_signal_to_deliver+0x734/0x760
Jan 28 22:58:29 euridica kernel:  [<c010391e>] do_notify_resume+0x94/0x749
Jan 28 22:58:29 euridica kernel:  [<c01044a1>] work_notifysig+0x13/0x1a
Jan 28 22:58:29 euridica kernel:  =======================

http://www.stardust.webpages.pl/files/tbf/euridica/2.6.20-rc6-mm1/mm-config

Regards,
Michal

-- 
Michal K. K. Piotrowski
LTG - Linux Testers Group
(http://www.stardust.webpages.pl/ltg/)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
  2007-01-28 22:31 ` 2.6.20-rc6-mm1 Michal Piotrowski
@ 2007-01-28 23:10   ` Andrew Morton
  2007-01-29  5:17   ` 2.6.20-rc6-mm1 Herbert Xu
  1 sibling, 0 replies; 14+ messages in thread
From: Andrew Morton @ 2007-01-28 23:10 UTC (permalink / raw)
  To: Michal Piotrowski; +Cc: linux-kernel, netdev, Patrick McHardy

On Sun, 28 Jan 2007 23:31:00 +0100
Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:

> Jan 28 22:58:29 euridica kernel: BUG: using smp_processor_id() in preemptible [00000001] code: yum-updatesd/2846
> Jan 28 22:58:29 euridica kernel: caller is nf_conntrack_in+0x363/0x47f [nf_conntrack]

I'll plug that with this:


diff -puN include/net/netfilter/nf_conntrack.h~netfilter-warning-fix include/net/netfilter/nf_conntrack.h
--- a/include/net/netfilter/nf_conntrack.h~netfilter-warning-fix
+++ a/include/net/netfilter/nf_conntrack.h
@@ -254,7 +254,12 @@ extern atomic_t nf_conntrack_count;
 extern int nf_conntrack_max;
 
 DECLARE_PER_CPU(struct ip_conntrack_stat, nf_conntrack_stat);
-#define NF_CT_STAT_INC(count) (__get_cpu_var(nf_conntrack_stat).count++)
+#define NF_CT_STAT_INC(count)						\
+	do {								\
+		preempt_disable();					\
+		__get_cpu_var(nf_conntrack_stat).count++;		\
+		preempt_enable();					\
+	} while (0);
 
 /* no helper, no nat */
 #define	NF_CT_F_BASIC	0
_

and will let others worry about what the real fix (for 2.6.20, please) is.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
  2007-01-28 22:21   ` [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n) Adrian Bunk
@ 2007-01-28 23:53     ` David Miller
  2007-01-29  0:00       ` Adrian Bunk
  0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2007-01-28 23:53 UTC (permalink / raw)
  To: bunk; +Cc: randy.dunlap, akpm, linux-kernel, netfilter, netdev

From: Adrian Bunk <bunk@stusta.de>
Date: Sun, 28 Jan 2007 23:21:37 +0100

> On Sun, Jan 28, 2007 at 11:41:48AM -0800, Randy Dunlap wrote:
> >...
> > net/built-in.o: In function `q931_help':
> > nf_conntrack_h323_main.c:(.text.q931_help+0x6ad): undefined reference to `ip6_route_output'
> > nf_conntrack_h323_main.c:(.text.q931_help+0x6c3): undefined reference to `ip6_route_output'
> >...
> 
> You didn't send your .config, but it seems you had CONFIG_IPV6=m and
> CONFIG_NF_CONNTRACK_H323=y.
> 
> In this case, the untested patch below should fix it.
> 
> > ~Randy
> 
> cu
> Adrian
> 
> 
> <--  snip  -->
> 
> 
> CONFIG_IPV6=m, CONFIG_NF_CONNTRACK_H323=y results in a compile error.
> 
> Fix this by letting NF_CONNTRACK_H323 depend on (IPV6 || IPV6=n).
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>

Adrian is this the correct way to constrain the selection between
"n" and "m" in this kind of situation?  I thought doing something
like "depends on IPV6" is sufficient to achieve that?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
  2007-01-28 23:53     ` David Miller
@ 2007-01-29  0:00       ` Adrian Bunk
  2007-01-29  0:04         ` David Miller
  0 siblings, 1 reply; 14+ messages in thread
From: Adrian Bunk @ 2007-01-29  0:00 UTC (permalink / raw)
  To: David Miller; +Cc: randy.dunlap, akpm, linux-kernel, netfilter, netdev

On Sun, Jan 28, 2007 at 03:53:48PM -0800, David Miller wrote:
> From: Adrian Bunk <bunk@stusta.de>
> Date: Sun, 28 Jan 2007 23:21:37 +0100
> 
> > On Sun, Jan 28, 2007 at 11:41:48AM -0800, Randy Dunlap wrote:
> > >...
> > > net/built-in.o: In function `q931_help':
> > > nf_conntrack_h323_main.c:(.text.q931_help+0x6ad): undefined reference to `ip6_route_output'
> > > nf_conntrack_h323_main.c:(.text.q931_help+0x6c3): undefined reference to `ip6_route_output'
> > >...
> > 
> > You didn't send your .config, but it seems you had CONFIG_IPV6=m and
> > CONFIG_NF_CONNTRACK_H323=y.
> > 
> > In this case, the untested patch below should fix it.
> > 
> > > ~Randy
> > 
> > cu
> > Adrian
> > 
> > 
> > <--  snip  -->
> > 
> > 
> > CONFIG_IPV6=m, CONFIG_NF_CONNTRACK_H323=y results in a compile error.
> > 
> > Fix this by letting NF_CONNTRACK_H323 depend on (IPV6 || IPV6=n).
> > 
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> Adrian is this the correct way to constrain the selection between
> "n" and "m" in this kind of situation?  I thought doing something
> like "depends on IPV6" is sufficient to achieve that?

"depends on IPV6" would fix the bug - but it would also make 
NF_CONNTRACK_H323 unavailable for all people without IPV6 support in 
their kernel.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
  2007-01-29  0:00       ` Adrian Bunk
@ 2007-01-29  0:04         ` David Miller
  2007-01-29  0:21           ` Adrian Bunk
  0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2007-01-29  0:04 UTC (permalink / raw)
  To: bunk; +Cc: randy.dunlap, akpm, linux-kernel, netfilter, netdev, kaber

From: Adrian Bunk <bunk@stusta.de>
Date: Mon, 29 Jan 2007 01:00:11 +0100

> On Sun, Jan 28, 2007 at 03:53:48PM -0800, David Miller wrote:
> > Adrian is this the correct way to constrain the selection between
> > "n" and "m" in this kind of situation?  I thought doing something
> > like "depends on IPV6" is sufficient to achieve that?
> 
> "depends on IPV6" would fix the bug - but it would also make 
> NF_CONNTRACK_H323 unavailable for all people without IPV6 support in 
> their kernel.

Yes, that is an issue.

I guess with some slightly ugly ifdefs we could support the
whole matrix of possibilities.  But perhaps that's undesirable
for another reason.

Patrick?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
  2007-01-29  0:04         ` David Miller
@ 2007-01-29  0:21           ` Adrian Bunk
  2007-01-29  1:22             ` Randy Dunlap
  2007-01-30 17:13             ` Patrick McHardy
  0 siblings, 2 replies; 14+ messages in thread
From: Adrian Bunk @ 2007-01-29  0:21 UTC (permalink / raw)
  To: David Miller; +Cc: randy.dunlap, akpm, linux-kernel, netfilter, netdev, kaber

On Sun, Jan 28, 2007 at 04:04:42PM -0800, David Miller wrote:
> From: Adrian Bunk <bunk@stusta.de>
> Date: Mon, 29 Jan 2007 01:00:11 +0100
> 
> > On Sun, Jan 28, 2007 at 03:53:48PM -0800, David Miller wrote:
> > > Adrian is this the correct way to constrain the selection between
> > > "n" and "m" in this kind of situation?  I thought doing something
> > > like "depends on IPV6" is sufficient to achieve that?
> > 
> > "depends on IPV6" would fix the bug - but it would also make 
> > NF_CONNTRACK_H323 unavailable for all people without IPV6 support in 
> > their kernel.
> 
> Yes, that is an issue.
> 
> I guess with some slightly ugly ifdefs we could support the
> whole matrix of possibilities.  But perhaps that's undesirable
> for another reason.
>...

This depends on what NF_CONNTRACK_H323=y, IPV6=m is supposed to be:
- not allowed (NF_CONNTRACK_H323 must be modular) or
- NF_CONNTRACK_H323 can only be used for IPV4

My patch implements the first case.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
  2007-01-29  0:21           ` Adrian Bunk
@ 2007-01-29  1:22             ` Randy Dunlap
  2007-01-30 17:13             ` Patrick McHardy
  1 sibling, 0 replies; 14+ messages in thread
From: Randy Dunlap @ 2007-01-29  1:22 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: David Miller, akpm, linux-kernel, netfilter, netdev, kaber

Adrian Bunk wrote:
> On Sun, Jan 28, 2007 at 04:04:42PM -0800, David Miller wrote:
>> From: Adrian Bunk <bunk@stusta.de>
>> Date: Mon, 29 Jan 2007 01:00:11 +0100
>>
>>> On Sun, Jan 28, 2007 at 03:53:48PM -0800, David Miller wrote:
>>>> Adrian is this the correct way to constrain the selection between
>>>> "n" and "m" in this kind of situation?  I thought doing something
>>>> like "depends on IPV6" is sufficient to achieve that?
>>> "depends on IPV6" would fix the bug - but it would also make 
>>> NF_CONNTRACK_H323 unavailable for all people without IPV6 support in 
>>> their kernel.
>> Yes, that is an issue.
>>
>> I guess with some slightly ugly ifdefs we could support the
>> whole matrix of possibilities.  But perhaps that's undesirable
>> for another reason.
>> ...
> 
> This depends on what NF_CONNTRACK_H323=y, IPV6=m is supposed to be:
> - not allowed (NF_CONNTRACK_H323 must be modular) or
> - NF_CONNTRACK_H323 can only be used for IPV4
> 
> My patch implements the first case.

Sorry for the slow reponse.  This bug only came up due to my
bad gfs2/dlm patch, which Adrian has now corrected, so I think
you can just drop this patch.  It now builds for me with only
Adrian's gfs2/dlm patch applied.

-- 
~Randy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
  2007-01-28 22:31 ` 2.6.20-rc6-mm1 Michal Piotrowski
  2007-01-28 23:10   ` 2.6.20-rc6-mm1 Andrew Morton
@ 2007-01-29  5:17   ` Herbert Xu
  2007-01-29  5:29     ` 2.6.20-rc6-mm1 Herbert Xu
  1 sibling, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2007-01-29  5:17 UTC (permalink / raw)
  To: Michal Piotrowski; +Cc: akpm, linux-kernel, netdev

Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:

> Jan 28 22:58:29 euridica kernel: BUG: using smp_processor_id() in preemptible [00000001] code: yum-updatesd/2846
> Jan 28 22:58:29 euridica kernel: caller is nf_conntrack_in+0x363/0x47f [nf_conntrack]
> Jan 28 22:58:29 euridica kernel:  [<c01053c6>] show_trace_log_lvl+0x1a/0x2f
> Jan 28 22:58:29 euridica kernel:  [<c0105ad6>] show_trace+0x12/0x14
> Jan 28 22:58:29 euridica kernel:  [<c0105b98>] dump_stack+0x16/0x18
> Jan 28 22:58:29 euridica kernel:  [<c0207803>] debug_smp_processor_id+0xb3/0xc8
> Jan 28 22:58:29 euridica kernel:  [<fdbf8ad0>] nf_conntrack_in+0x363/0x47f [nf_conntrack]
> Jan 28 22:58:29 euridica kernel:  [<fd9c32c4>] ipv4_conntrack_local+0x53/0x5b [nf_conntrack_ipv4]
> Jan 28 22:58:29 euridica kernel:  [<c02f2286>] nf_iterate+0x36/0x67
> Jan 28 22:58:29 euridica kernel:  [<c02f241b>] nf_hook_slow+0x52/0xbe

This shouldn't have happened.  nf_hook_slow calls nf_iterate and
therefore everything under it with preemption disabled.  So something
must've reenabled it before hitting nf_conntrack_in.

Cheers,
-- 
Visit Openswan at http://www.openswan.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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
  2007-01-29  5:17   ` 2.6.20-rc6-mm1 Herbert Xu
@ 2007-01-29  5:29     ` Herbert Xu
  2007-01-29  6:43       ` 2.6.20-rc6-mm1 Andrew Morton
  0 siblings, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2007-01-29  5:29 UTC (permalink / raw)
  To: Michal Piotrowski; +Cc: akpm, linux-kernel, netdev

On Mon, Jan 29, 2007 at 04:17:44PM +1100, Herbert Xu wrote:
> Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
> 
> > Jan 28 22:58:29 euridica kernel: BUG: using smp_processor_id() in preemptible [00000001] code: yum-updatesd/2846
> > Jan 28 22:58:29 euridica kernel: caller is nf_conntrack_in+0x363/0x47f [nf_conntrack]
> > Jan 28 22:58:29 euridica kernel:  [<c01053c6>] show_trace_log_lvl+0x1a/0x2f
> > Jan 28 22:58:29 euridica kernel:  [<c0105ad6>] show_trace+0x12/0x14
> > Jan 28 22:58:29 euridica kernel:  [<c0105b98>] dump_stack+0x16/0x18
> > Jan 28 22:58:29 euridica kernel:  [<c0207803>] debug_smp_processor_id+0xb3/0xc8
> > Jan 28 22:58:29 euridica kernel:  [<fdbf8ad0>] nf_conntrack_in+0x363/0x47f [nf_conntrack]
> > Jan 28 22:58:29 euridica kernel:  [<fd9c32c4>] ipv4_conntrack_local+0x53/0x5b [nf_conntrack_ipv4]
> > Jan 28 22:58:29 euridica kernel:  [<c02f2286>] nf_iterate+0x36/0x67
> > Jan 28 22:58:29 euridica kernel:  [<c02f241b>] nf_hook_slow+0x52/0xbe
> 
> This shouldn't have happened.  nf_hook_slow calls nf_iterate and
> therefore everything under it with preemption disabled.  So something
> must've reenabled it before hitting nf_conntrack_in.

Does mm now have the preemptible RCU stuff? If so that would certainly
explain this.

IIRC Ingo had made fixes for the networking stack in his rt tree since
the networking code assumes in lots of places that rcu_read_lock
disables preemption.

Cheers,
-- 
Visit Openswan at http://www.openswan.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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
  2007-01-29  5:29     ` 2.6.20-rc6-mm1 Herbert Xu
@ 2007-01-29  6:43       ` Andrew Morton
  2007-01-29  7:21         ` 2.6.20-rc6-mm1 Herbert Xu
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2007-01-29  6:43 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Michal Piotrowski, linux-kernel, netdev, Dipankar Sarma,
	Paul E. McKenney

On Mon, 29 Jan 2007 16:29:29 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Mon, Jan 29, 2007 at 04:17:44PM +1100, Herbert Xu wrote:
> > Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
> > 
> > > Jan 28 22:58:29 euridica kernel: BUG: using smp_processor_id() in preemptible [00000001] code: yum-updatesd/2846
> > > Jan 28 22:58:29 euridica kernel: caller is nf_conntrack_in+0x363/0x47f [nf_conntrack]
> > > Jan 28 22:58:29 euridica kernel:  [<c01053c6>] show_trace_log_lvl+0x1a/0x2f
> > > Jan 28 22:58:29 euridica kernel:  [<c0105ad6>] show_trace+0x12/0x14
> > > Jan 28 22:58:29 euridica kernel:  [<c0105b98>] dump_stack+0x16/0x18
> > > Jan 28 22:58:29 euridica kernel:  [<c0207803>] debug_smp_processor_id+0xb3/0xc8
> > > Jan 28 22:58:29 euridica kernel:  [<fdbf8ad0>] nf_conntrack_in+0x363/0x47f [nf_conntrack]
> > > Jan 28 22:58:29 euridica kernel:  [<fd9c32c4>] ipv4_conntrack_local+0x53/0x5b [nf_conntrack_ipv4]
> > > Jan 28 22:58:29 euridica kernel:  [<c02f2286>] nf_iterate+0x36/0x67
> > > Jan 28 22:58:29 euridica kernel:  [<c02f241b>] nf_hook_slow+0x52/0xbe
> > 
> > This shouldn't have happened.  nf_hook_slow calls nf_iterate and
> > therefore everything under it with preemption disabled.  So something
> > must've reenabled it before hitting nf_conntrack_in.
> 
> Does mm now have the preemptible RCU stuff? If so that would certainly
> explain this.

It does,

> IIRC Ingo had made fixes for the networking stack in his rt tree since
> the networking code assumes in lots of places that rcu_read_lock
> disables preemption.

oh.  We'd better find those fixes then.  I wonder what other code made that
(rather hacky) assumption?  I guess we have enough debug stuff in there to
find out..

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
  2007-01-29  6:43       ` 2.6.20-rc6-mm1 Andrew Morton
@ 2007-01-29  7:21         ` Herbert Xu
  2007-01-29  8:35           ` 2.6.20-rc6-mm1 Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Herbert Xu @ 2007-01-29  7:21 UTC (permalink / raw)
  To: Andrew Morton, Ingo Molnar, David S. Miller
  Cc: Michal Piotrowski, linux-kernel, netdev, Dipankar Sarma,
	Paul E. McKenney

On Sun, Jan 28, 2007 at 10:43:12PM -0800, Andrew Morton wrote:
>
> > IIRC Ingo had made fixes for the networking stack in his rt tree since
> > the networking code assumes in lots of places that rcu_read_lock
> > disables preemption.
> 
> oh.  We'd better find those fixes then.  I wonder what other code made that
> (rather hacky) assumption?  I guess we have enough debug stuff in there to
> find out..

Actually, maybe I was confusing this with the fixes Ingo had for
local_bh_disable vs. preemption in the -rt tree.  Ingo, do you
have preemptible RCU support in your -rt tree and if so did you
have to fix the networking stack to behave correctly with it?

It could also be that the fixes for local_bh_disable also masked
any problems that would trigger under preemptible RCU.

Cheers,
-- 
Visit Openswan at http://www.openswan.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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.20-rc6-mm1
  2007-01-29  7:21         ` 2.6.20-rc6-mm1 Herbert Xu
@ 2007-01-29  8:35           ` Ingo Molnar
  0 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2007-01-29  8:35 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Andrew Morton, David S. Miller, Michal Piotrowski, linux-kernel,
	netdev, Dipankar Sarma, Paul E. McKenney


* Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Actually, maybe I was confusing this with the fixes Ingo had for 
> local_bh_disable vs. preemption in the -rt tree.  Ingo, do you have 
> preemptible RCU support in your -rt tree and if so did you have to fix 
> the networking stack to behave correctly with it?

yeah, -rt is the main prototyping tree for PREEMPT_RCU, and it has been 
included in -rt for 1.5 years or so. There were only some small things 
here and there, but with Paul's latest design i dont remember anything 
but the occasional place that needs to be marked raw_smp_processor_id(). 
CONFIG_DEBUG_PREEMPT ought to catch those. I dont remember any big 
breakage.

i've just reviewed all changes to net/* in -rt, and the changes below 
are the ones that seem to be upstream-relevant.

	Ingo

Index: linux/net/ipv4/multipath_wrandom.c
===================================================================
--- linux.orig/net/ipv4/multipath_wrandom.c
+++ linux/net/ipv4/multipath_wrandom.c
@@ -301,6 +301,7 @@ static void wrandom_flush(void)
 	for (i = 0; i < MULTIPATH_STATE_SIZE; ++i) {
 		struct multipath_route *r;
 
+		rcu_read_lock();
 		spin_lock_bh(&state[i].lock);
 		list_for_each_entry_rcu(r, &state[i].head, list) {
 			struct multipath_dest *d;
@@ -315,6 +316,7 @@ static void wrandom_flush(void)
 		}
 
 		spin_unlock_bh(&state[i].lock);
+		rcu_read_unlock();
 	}
 }
 
Index: linux/net/ipv4/netfilter/arp_tables.c
===================================================================
--- linux.orig/net/ipv4/netfilter/arp_tables.c
+++ linux/net/ipv4/netfilter/arp_tables.c
@@ -245,7 +245,7 @@ unsigned int arpt_do_table(struct sk_buf
 
 	read_lock_bh(&table->lock);
 	private = table->private;
-	table_base = (void *)private->entries[smp_processor_id()];
+	table_base = (void *)private->entries[raw_smp_processor_id()];
 	e = get_entry(table_base, private->hook_entry[hook]);
 	back = get_entry(table_base, private->underflow[hook]);
 
@@ -955,7 +955,7 @@ static int do_add_counters(void __user *
 
 	i = 0;
 	/* Choose the copy that is on our node */
-	loc_cpu_entry = private->entries[smp_processor_id()];
+	loc_cpu_entry = private->entries[raw_smp_processor_id()];
 	ARPT_ENTRY_ITERATE(loc_cpu_entry,
 			   private->size,
 			   add_counter_to_entry,
Index: linux/net/ipv4/netfilter/ip_tables.c
===================================================================
--- linux.orig/net/ipv4/netfilter/ip_tables.c
+++ linux/net/ipv4/netfilter/ip_tables.c
@@ -246,7 +246,7 @@ ipt_do_table(struct sk_buff **pskb,
 	read_lock_bh(&table->lock);
 	IP_NF_ASSERT(table->valid_hooks & (1 << hook));
 	private = table->private;
-	table_base = (void *)private->entries[smp_processor_id()];
+	table_base = (void *)private->entries[raw_smp_processor_id()];
 	e = get_entry(table_base, private->hook_entry[hook]);
 
 	/* For return from builtin chain */
Index: linux/net/ipv4/tcp.c
===================================================================
--- linux.orig/net/ipv4/tcp.c
+++ linux/net/ipv4/tcp.c
@@ -1138,10 +1138,10 @@ int tcp_recvmsg(struct kiocb *iocb, stru
 	preempt_disable();
 	if ((len > sysctl_tcp_dma_copybreak) && !(flags & MSG_PEEK) &&
 	    !sysctl_tcp_low_latency && __get_cpu_var(softnet_data).net_dma) {
-		preempt_enable_no_resched();
+		preempt_enable();
 		tp->ucopy.pinned_list = dma_pin_iovec_pages(msg->msg_iov, len);
 	} else
-		preempt_enable_no_resched();
+		preempt_enable();
 #endif
 
 	do {
Index: linux/net/ipv6/netfilter/ip6_tables.c
===================================================================
--- linux.orig/net/ipv6/netfilter/ip6_tables.c
+++ linux/net/ipv6/netfilter/ip6_tables.c
@@ -282,7 +282,7 @@ ip6t_do_table(struct sk_buff **pskb,
 	read_lock_bh(&table->lock);
 	private = table->private;
 	IP_NF_ASSERT(table->valid_hooks & (1 << hook));
-	table_base = (void *)private->entries[smp_processor_id()];
+	table_base = (void *)private->entries[raw_smp_processor_id()];
 	e = get_entry(table_base, private->hook_entry[hook]);
 
 	/* For return from builtin chain */
@@ -1097,7 +1097,7 @@ do_add_counters(void __user *user, unsig
 
 	i = 0;
 	/* Choose the copy that is on our node */
-	loc_cpu_entry = private->entries[smp_processor_id()];
+	loc_cpu_entry = private->entries[raw_smp_processor_id()];
 	IP6T_ENTRY_ITERATE(loc_cpu_entry,
 			  private->size,
 			  add_counter_to_entry,
Index: linux/net/xfrm/xfrm_user.c
===================================================================
--- linux.orig/net/xfrm/xfrm_user.c
+++ linux/net/xfrm/xfrm_user.c
@@ -1273,13 +1273,12 @@ static int xfrm_get_policy(struct sk_buf
 		xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, tmp.security, delete);
 		security_xfrm_policy_free(&tmp);
 	}
-	if (delete)
-		xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
-			       AUDIT_MAC_IPSEC_DELSPD, (xp) ? 1 : 0, xp, NULL);
-
 	if (xp == NULL)
 		return -ENOENT;
 
+	xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
+		       AUDIT_MAC_IPSEC_DELSPD, delete, xp, NULL);
+
 	if (!delete) {
 		struct sk_buff *resp_skb;
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n)
  2007-01-29  0:21           ` Adrian Bunk
  2007-01-29  1:22             ` Randy Dunlap
@ 2007-01-30 17:13             ` Patrick McHardy
  1 sibling, 0 replies; 14+ messages in thread
From: Patrick McHardy @ 2007-01-30 17:13 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: randy.dunlap, akpm, netdev, linux-kernel, netfilter, David Miller

Adrian Bunk wrote:
> On Sun, Jan 28, 2007 at 04:04:42PM -0800, David Miller wrote:
> 
>>From: Adrian Bunk <bunk@stusta.de>
>>Date: Mon, 29 Jan 2007 01:00:11 +0100
>>
>>>"depends on IPV6" would fix the bug - but it would also make 
>>>NF_CONNTRACK_H323 unavailable for all people without IPV6 support in 
>>>their kernel.
>>
>>Yes, that is an issue.
>>
>>I guess with some slightly ugly ifdefs we could support the
>>whole matrix of possibilities.  But perhaps that's undesirable
>>for another reason.
>>...
> 
> 
> This depends on what NF_CONNTRACK_H323=y, IPV6=m is supposed to be:
> - not allowed (NF_CONNTRACK_H323 must be modular) or
> - NF_CONNTRACK_H323 can only be used for IPV4
> 
> My patch implements the first case.


Unfortunately a few ifdefs aren't enough to support IPV6=m,
NF_CONNTRACK_H323=y. For now I think Adrian's patch is the
best solution (IPV6=m isn't that useful anyway since it will
normally get loaded automatically when the first program
attempts to open an AF_INET6 socket and can't be unloaded),
but I'll look into moving the route lookup to the netfilter
AF ops in 2.6.21 so we can also support that configuration.


Acked-by: Patrick McHardy <kaber@trash.net>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-01-30 17:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20070127234928.64d8e437.akpm@osdl.org>
     [not found] ` <20070128114148.b8067721.randy.dunlap@oracle.com>
2007-01-28 22:21   ` [2.6 patch] NF_CONNTRACK_H323 must depend on (IPV6 || IPV6=n) Adrian Bunk
2007-01-28 23:53     ` David Miller
2007-01-29  0:00       ` Adrian Bunk
2007-01-29  0:04         ` David Miller
2007-01-29  0:21           ` Adrian Bunk
2007-01-29  1:22             ` Randy Dunlap
2007-01-30 17:13             ` Patrick McHardy
2007-01-28 22:31 ` 2.6.20-rc6-mm1 Michal Piotrowski
2007-01-28 23:10   ` 2.6.20-rc6-mm1 Andrew Morton
2007-01-29  5:17   ` 2.6.20-rc6-mm1 Herbert Xu
2007-01-29  5:29     ` 2.6.20-rc6-mm1 Herbert Xu
2007-01-29  6:43       ` 2.6.20-rc6-mm1 Andrew Morton
2007-01-29  7:21         ` 2.6.20-rc6-mm1 Herbert Xu
2007-01-29  8:35           ` 2.6.20-rc6-mm1 Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).