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

[Patrick McHardy <kaber@trash.net>]

Herbert Xu wrote:
> On Thu, Aug 03, 2006 at 11:29:41AM +0200, Patrick McHardy wrote:
> 
>>>>+u_int16_t nf_proto_csum_update(struct sk_buff *skb,
>>>>+			       u_int32_t oldval, u_int32_t newval,
>>>>+			       u_int16_t csum, int pseudohdr)
>>>>+{
>>>>+	if (skb->ip_summed != CHECKSUM_PARTIAL) {
>>>>+		csum = nf_csum_update(oldval, newval, csum);
>>>>+		if (skb->ip_summed == CHECKSUM_COMPLETE && pseudohdr)
>>>
>>>
>>>Shouldn't that be !pseudohdr?
>>
>>No, if the changed data is part of the pseudo-hdr, we need to update
>>skb->csum so the skb passes later checksum checks.
> 
> 
> Are you sure? If ip_summed is CHECKSUM_COMPLETE then skb->csum is the
> checksum of the payload *without* the pseudo header.

The pseudohdr is included indirectly through the tcp/udp checksum.

>>>This is a 32-bit quantity so nf_csum_update should eat a 32-bit quantity
>>>as well.  Also, this checksum is not inverted so you need
>>>
>>>			skb->csum = ~nf_csum_update(oldval, newval, ~skb->csum);
>>
>>I'll change it to 32-bit and try the inversion, but it works fine this
>>way.
> 
> 
> It'll break for e1000 at least since it puts the checksum into the
> high 16 bits of skb->csum (on i386).

Yes, the 32-bit thing is a bug, I meant it works fine without inverting
the checksum.