Re: udp.c

Re: udp.c

[Herbert Xu]

David S. Miller <davem@davemloft.net> wrote:
> From: Jesper Juhl <jesper.juhl@gmail.com>
> Date: Mon, 13 Jun 2005 19:23:36 +0200
> 
>> Why not remove the function and audit the code for users (and if any,
>> remove them)...? Let's get rid of it instead of having a function sit
>> around the only purpose of which is to BUG();
> 
> Then if someone breaks that invariant, we'll never find out.
> 
> The code is staying, sorry.

It'll dump the stack anyway if we just make it a NULL pointer.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

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
--
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -201,11 +201,6 @@ fail:
 	return 1;
 }
 
-static void udp_v4_hash(struct sock *sk)
-{
-	BUG();
-}
-
 static void udp_v4_unhash(struct sock *sk)
 {
 	write_lock_bh(&udp_hash_lock);
@@ -1370,7 +1365,7 @@ struct proto udp_prot = {
 	.recvmsg =	udp_recvmsg,
 	.sendpage =	udp_sendpage,
 	.backlog_rcv =	udp_queue_rcv_skb,
-	.hash =		udp_v4_hash,
+	/* .hash is intentionally left NULL */
 	.unhash =	udp_v4_unhash,
 	.get_port =	udp_v4_get_port,
 	.obj_size =	sizeof(struct udp_sock),
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -135,11 +135,6 @@ fail:
 	return 1;
 }
 
-static void udp_v6_hash(struct sock *sk)
-{
-	BUG();
-}
-
 static void udp_v6_unhash(struct sock *sk)
 {
  	write_lock_bh(&udp_hash_lock);
@@ -1048,7 +1043,7 @@ struct proto udpv6_prot = {
 	.sendmsg =	udpv6_sendmsg,
 	.recvmsg =	udpv6_recvmsg,
 	.backlog_rcv =	udpv6_queue_rcv_skb,
-	.hash =		udp_v6_hash,
+	/* .hash is intentionally left NULL */
 	.unhash =	udp_v6_unhash,
 	.get_port =	udp_v6_get_port,
 	.obj_size =	sizeof(struct udp6_sock),

Re: udp.c

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 14 Jun 2005 07:42:52 +1000

> It'll dump the stack anyway if we just make it a NULL pointer.

Some platforms don't handle that very cleanly, for example
it may be necessary to have something mapped at page zero
for one reason or another.

Re: udp.c

[Herbert Xu]

On Mon, Jun 13, 2005 at 02:57:16PM -0700, David S. Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue, 14 Jun 2005 07:42:52 +1000
> 
> > It'll dump the stack anyway if we just make it a NULL pointer.
> 
> Some platforms don't handle that very cleanly, for example
> it may be necessary to have something mapped at page zero
> for one reason or another.

Are there any existing platforms that do that in kernel mode?
-- 
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

Re: udp.c

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 14 Jun 2005 09:04:22 +1000

> On Mon, Jun 13, 2005 at 02:57:16PM -0700, David S. Miller wrote:
> > From: Herbert Xu <herbert@gondor.apana.org.au>
> > Date: Tue, 14 Jun 2005 07:42:52 +1000
> > 
> > > It'll dump the stack anyway if we just make it a NULL pointer.
> > 
> > Some platforms don't handle that very cleanly, for example
> > it may be necessary to have something mapped at page zero
> > for one reason or another.
> 
> Are there any existing platforms that do that in kernel mode?

X86 did, especially during bootup, for a long time.

I know the highly optimized sparc64 instruction TLB miss handler
doesn't handle this properly and this usually hangs the machine.
I've put some checks in there that tries to handle it properly,
but there are still some cases that pass through.

Re: udp.c

[Andi Kleen]

"David S. Miller" <davem@davemloft.net> writes:

> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue, 14 Jun 2005 09:04:22 +1000
>
>> On Mon, Jun 13, 2005 at 02:57:16PM -0700, David S. Miller wrote:
>> > From: Herbert Xu <herbert@gondor.apana.org.au>
>> > Date: Tue, 14 Jun 2005 07:42:52 +1000
>> > 
>> > > It'll dump the stack anyway if we just make it a NULL pointer.
>> > 
>> > Some platforms don't handle that very cleanly, for example
>> > it may be necessary to have something mapped at page zero
>> > for one reason or another.
>> 
>> Are there any existing platforms that do that in kernel mode?
>
> X86 did, especially during bootup, for a long time.

Still does, as does x86-64, but actually it could be changed now using
change_page_attr (and then later undoing it). Is it worth it?

-Andi

Re: udp.c

[David S. Miller]

From: Andi Kleen <ak@muc.de>
Date: Tue, 14 Jun 2005 01:53:56 +0200

> Still does, as does x86-64, but actually it could be changed now using
> change_page_attr (and then later undoing it). Is it worth it?

A lot of bootup OOPS's have occured in the past and
were never discovered until someone on a non-x86
platform tested the patch.

Re: udp.c

[Andi Kleen]

On Mon, Jun 13, 2005 at 05:00:45PM -0700, David S. Miller wrote:
> From: Andi Kleen <ak@muc.de>
> Date: Tue, 14 Jun 2005 01:53:56 +0200
> 
> > Still does, as does x86-64, but actually it could be changed now using
> > change_page_attr (and then later undoing it). Is it worth it?
> 
> A lot of bootup OOPS's have occured in the past and
> were never discovered until someone on a non-x86
> platform tested the patch.

Ok I can fix it, but only reasonably after mem_init (at least without
hacking up change_page_attr a lot to deal with bootmem).
Is that still worth it? 

-Andi

Re: udp.c

[David S. Miller]

From: Andi Kleen <ak@muc.de>
Date: 14 Jun 2005 02:12:49 +0200,Tue, 14 Jun 2005 02:12:49 +0200

> Ok I can fix it, but only reasonably after mem_init (at least without
> hacking up change_page_attr a lot to deal with bootmem).
> Is that still worth it? 

I think so.