Zero checksum in netconsole/netdump packets

Zero checksum in netconsole/netdump packets

[Chris Lalancette]

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

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 <clalance@redhat.com>

[-- Attachment #2: linux-2.6.19-rc4-netpoll-udp-checksum.patch --]
[-- Type: text/x-patch, Size: 598 bytes --]

--- linux-2.6/net/core/netpoll.c.orig	2006-11-06 18:16:58.000000000 -0500
+++ linux-2.6/net/core/netpoll.c	2006-11-06 18:31:20.000000000 -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

[David Miller]

From: Chris Lalancette <clalance@redhat.com>
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.

Re: Zero checksum in netconsole/netdump packets

[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. 
<snip>
|       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 <clalance@redhat.com>
|  
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;

Re: Zero checksum in netconsole/netdump packets

[Krzysztof Oledzki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 664 bytes --]



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.
> <snip>
> |       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 <clalance@redhat.com>
> |
> 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

[Chris Lalancette]

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

David Miller wrote:
> From: Chris Lalancette <clalance@redhat.com>
> 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 <clalance@redhat.com>

[-- Attachment #2: linux-2.6.19-rc4-netpoll-udp-checksum.patch --]
[-- Type: text/x-patch, Size: 576 bytes --]

--- linux-2.6/net/core/netpoll.c.orig	2006-11-06 18:16:58.000000000 -0500
+++ linux-2.6/net/core/netpoll.c	2006-11-07 08:16:29.000000000 -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

[David Miller]

From: Chris Lalancette <clalance@redhat.com>
Date: Tue, 07 Nov 2006 08:33:00 -0500

> David Miller wrote:
> > From: Chris Lalancette <clalance@redhat.com>
> > 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 <clalance@redhat.com>

Looks good, applied, thanks Chris.