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.
-
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

Reply via email to