Zero checksum in netconsole/netdump packets

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.

@@ -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);

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

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!

@@ -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));
 

From: Chris Lalancette <[email protected]>
Date: Tue, 07 Nov 2006 08:33:00 -0500

Looks good, applied, thanks Chris.

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 <[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;

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

Best regards,

Krzysztof Olędzki