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