Zero checksum in netconsole/netdump packets

2006-11-06 Thread Chris Lalancette
Hello,
  I was reading some tcpdump's of netdump traffic today, and I realized 
that all of the packets that go from the crashing machine to the netdump server 
have a zero checksum.  Looking at the code, it looks like netconsole/netdump 
use the function netpoll_send_udp to send out the packets.  However, in 
netdump_send_udp, the checksum is set to 0, and never seems to be computed.  Is 
this intentional, or just an oversight?  I would think that we would always 
want to compute the UDP checksum, but there might be something I am 
overlooking.  Incidentally, it seems like the only user of netpoll_send_udp is 
netconsole (and netdump in RedHat kernels).
 Assuming that this is just an oversight, attached is a simple patch to 
compute the UDP checksum in netpoll_send_udp.

Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>
--- linux-2.6/net/core/netpoll.c.orig	2006-11-06 18:16:58.0 -0500
+++ linux-2.6/net/core/netpoll.c	2006-11-06 18:31:20.0 -0500
@@ -356,6 +356,10 @@ void netpoll_send_udp(struct netpoll *np
 	put_unaligned(htonl(np->remote_ip), &(iph->daddr));
 	iph->check= ip_fast_csum((unsigned char *)iph, iph->ihl);
 
+	udph->check = csum_tcpudp_magic(iph->saddr, iph->daddr, udp_len,
+	IPPROTO_UDP,
+	csum_partial((unsigned char *)udph, udp_len, 0));
+
 	eth = (struct ethhdr *) skb_push(skb, ETH_HLEN);
 	skb->mac.raw = skb->data;
 	skb->protocol = eth->h_proto = htons(ETH_P_IP);


Re: Zero checksum in netconsole/netdump packets

2006-11-06 Thread David Miller
From: Chris Lalancette <[EMAIL PROTECTED]>
Date: Mon, 06 Nov 2006 18:40:59 -0500

>  Assuming that this is just an oversight, attached is a simple
>  patch to compute the UDP checksum in netpoll_send_udp.

If the resulting checksum is zero, you should set it to
all 1's, like the real UDP code does.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Zero checksum in netconsole/netdump packets

2006-11-07 Thread Gerrit Renker
Quoting Chris Lalancette:
|  Hello,
|   I realized that all of the packets that go from the crashing machine to 
the netdump server have a zero checksum. 

|   Assuming that this is just an oversight, attached is a simple patch to 
compute the UDP checksum in netpoll_send_udp.
|  
|  Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>
|  
RFC 768 allows to not compute the checksum by leaving uh->check at 0 - hence it 
is not illegal.
But without David's suggestion the code is not valid, since otherwise there is 
no way of 
distinguishing a computed `0' from an ignored `0' field:
 if ( udph->check == 0 )
udph->check = -1;
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Zero checksum in netconsole/netdump packets

2006-11-07 Thread Krzysztof Oledzki



On Tue, 7 Nov 2006, Gerrit Renker wrote:


Quoting Chris Lalancette:
|  Hello,
|   I realized that all of the packets that go from the crashing machine to 
the netdump server have a zero checksum.

|   Assuming that this is just an oversight, attached is a simple patch to 
compute the UDP checksum in netpoll_send_udp.
|
|  Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>
|
RFC 768 allows to not compute the checksum by leaving uh->check at 0 - hence it 
is not illegal.


BTW: leaving UDP checksum at 0 is only valid for IPv4, with IPv6 we _have 
to_ compute a checksum.


Best regards,

Krzysztof Olędzki

Re: Zero checksum in netconsole/netdump packets

2006-11-07 Thread Chris Lalancette

David Miller wrote:

From: Chris Lalancette <[EMAIL PROTECTED]>
Date: Mon, 06 Nov 2006 18:40:59 -0500


 Assuming that this is just an oversight, attached is a simple
 patch to compute the UDP checksum in netpoll_send_udp.


If the resulting checksum is zero, you should set it to
all 1's, like the real UDP code does.


David,
Ah, thanks.  Forgot about that.  I re-spun the patch with the change 
(attached).  I also moved the UDP checksum calculation up to where the rest of 
the UDP header setup is, to make it more consistent.

Thanks again for the comments!

Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>
--- linux-2.6/net/core/netpoll.c.orig	2006-11-06 18:16:58.0 -0500
+++ linux-2.6/net/core/netpoll.c	2006-11-07 08:16:29.0 -0500
@@ -340,6 +340,12 @@ void netpoll_send_udp(struct netpoll *np
 	udph->dest = htons(np->remote_port);
 	udph->len = htons(udp_len);
 	udph->check = 0;
+	udph->check = csum_tcpudp_magic(htonl(np->local_ip),
+	htonl(np->remote_ip),
+	udp_len, IPPROTO_UDP,
+	csum_partial((unsigned char *)udph, udp_len, 0));
+	if (udph->check == 0)
+		udph->check = -1;
 
 	skb->nh.iph = iph = (struct iphdr *)skb_push(skb, sizeof(*iph));
 


Re: Zero checksum in netconsole/netdump packets

2006-11-07 Thread David Miller
From: Chris Lalancette <[EMAIL PROTECTED]>
Date: Tue, 07 Nov 2006 08:33:00 -0500

> David Miller wrote:
> > From: Chris Lalancette <[EMAIL PROTECTED]>
> > Date: Mon, 06 Nov 2006 18:40:59 -0500
> > 
> >>  Assuming that this is just an oversight, attached is a simple
> >>  patch to compute the UDP checksum in netpoll_send_udp.
> > 
> > If the resulting checksum is zero, you should set it to
> > all 1's, like the real UDP code does.
> 
> David,
>  Ah, thanks.  Forgot about that.  I re-spun the patch with the change 
> (attached).  I also moved the UDP checksum calculation up to where the rest 
> of the UDP header setup is, to make it more consistent.
> 
> Thanks again for the comments!
> 
> Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>

Looks good, applied, thanks Chris.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html