On 02/19/2014 05:25 PM, Patrick McHardy wrote: > On Wed, Feb 19, 2014 at 05:12:13PM +0100, Nikolay Aleksandrov wrote: >> On 02/17/2014 07:46 PM, Patrick McHardy wrote: >>>>> >>>>> We need to take care of checksumming. Shouldn't be too hard to get *most* >>>>> cases right using the existing checksum helpers. >>>>> >>>> Yes, but would you like to do that in here or have a separate op which >>>> is configurable i.e. you can set what to calculate and where to put it, >>>> or would you prefer to make it automatic based on what we're changing here ? >>> >>> Something here is a lot cheaper since we can use incremental checksumming. >>> That won't be possible somewhere else, additionally we'd have to check >>> the checksum before recalculating it to make sure we don't fix up bad >>> packets. So yes, I think it should be done here. >>> >> Okay, I've been working on this today and have gotten to a point where it works :) >> But I have a few questions, currently I've made it so the header that you modify >> is the only one that gets its checksum updated (with the exception of pseudo >> header if the address gets written to), I recompute the changed bytes one at a >> time because of the freedom of offset and length. Now this works fine and I can >> mangle the ip/ip6 headers or the transport headers (tcp/udp, I'll look into >> adding sctp as well), but there's a problem with the cross-header writing i.e. >> if a write spills from the network header to the transport header, currently I >> only update the network header part (correctly, only up to the bytes that were >> changed inside) and leave the transport header broken. >> This also applies to the LL header, if a write spills into the network header >> then we'll have a broken packet. >> >> Would you like me to add additional logic to support the cross-header writes or >> leave it as it is ? > > I think that should be fine. > > Regarding the one byte at a time - update, how about rounding down to the > next multiple of four, adjusting the replacement data accordingly and using > csum_replace4() instead? > I was hoping you wouldn't say that :-) (joking) Yep, I know, I'll do it. Thanks again -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
