Subject: Re: BUG: warning at net/core/dev.c:1171/skb_checksum_help() 2.6.18-rc3

From: Patrick McHardy <[email-masked]>
Date: Thu, 03 Aug 2006 11:32:32 +0200

Herbert Xu wrote:
> On Mon, Jul 31, 2006 at 09:30:50PM +1000, herbert wrote:
> 
>>>diff --git a/net/ipv4/netfilter/ip_nat_core.c b/net/ipv4/netfilter/ip_nat_core.c
>>>index 1741d55..731efbb 100644
>>>--- a/net/ipv4/netfilter/ip_nat_core.c
>>>+++ b/net/ipv4/netfilter/ip_nat_core.c
>>>@@ -443,7 +443,9 @@ int ip_nat_icmp_reply_translation(struct
>>> 
>>> 	/* We're actually going to mangle it beyond trivial checksum
>>> 	   adjustment, so make sure the current checksum is correct. */
>>>-	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY) {
>>>+
>>>+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY &&
>>>+	    (*pskb)->ip_summed != CHECKSUM_PARTIAL) {
>>> 		hdrlen = (*pskb)->nh.iph->ihl * 4;
>>> 		if ((u16)csum_fold(skb_checksum(*pskb, hdrlen,
>>> 						(*pskb)->len - hdrlen, 0)))
> 
> 
> Actually, we could drop this chunk of code altogether.
> 
> The reason is that if the packet comes in with the correct checksum,
> it'll go out of NAT with the correct checksum.  If it came in with
> the wrong checksum, it'll go out with the wrong checksum.
> 
> We let TCP packets with incorrect checksums pass through NAT, so why
> not do the same here?
> 
> After all, we're here to do NAT, not verify checksums.  We charge extra
> for that :)

The checksum is verified here because a full checksum update is done
later in that function and we don't want to accidentally fix up
broken checksums.