Re: [PATCH net v2 3/4] ipv6: no CHECKSUM_PARTIAL on MSG_MORE corked sockets
From: Hannes Frederic Sowa <hidden>
Date: 2015-10-27 21:42:46
Hi Tom, On Tue, Oct 27, 2015, at 20:19, Hannes Frederic Sowa wrote:
On Tue, Oct 27, 2015, at 19:37, Tom Herbert wrote:quoted
On Tue, Oct 27, 2015 at 11:29 AM, Hannes Frederic Sowa [off-list ref] wrote:quoted
On Tue, Oct 27, 2015, at 18:32, Tom Herbert wrote:quoted
On Tue, Oct 27, 2015 at 9:44 AM, Hannes Frederic Sowa [off-list ref] wrote:quoted
On Tue, Oct 27, 2015, at 17:36, Tom Herbert wrote:> > - if (cork->length + length > maxnonfragsize - headersize) {quoted
quoted
+ if (cork->length + length > maxnonfragsize - headersize) { emsgsize: - ipv6_local_error(sk, EMSGSIZE, fl6, - mtu - headersize + - sizeof(struct ipv6hdr)); - return -EMSGSIZE; - } + ipv6_local_error(sk, EMSGSIZE, fl6, + mtu - headersize + + sizeof(struct ipv6hdr)); + return -EMSGSIZE; } + /* CHECKSUM_PARTIAL only with no extension headers and whenNo, please don't do this. CHECKSUM_PARTIAL should work with extension headers as defined, so this is just disabling otherwise valid and useful functionality. If (some) drivers have problems with this they need to be identified and fixed.I don't understand. The old code already didn't allow the use of opt_flen with CHECKSUM_PARTIAL.Then that's a problem with the old code :-). Is there any other reason that we can't use CHECKSUM_PARTIAL with extension headers other than lack of correct driver support?The lack of correct driver support is a big bumper, but as I wrote, I don't see a reason to not lift this restriction in net-next. I proposed a new feature flag, or by looking at your series, we could probably use the extension header okay field for that.Okay, but why bother doing this for net? This problem has obviously existed for a while, and even if the restriction is maintained here there are still other paths that don't go through ip_append_data that could trip the bug. Also, drivers are welcome to fix their issues in net I believe.I even don't know if it could be a hardware issue. Also I don't want to break people's communication with a patch. IMHO without the WARN_ON_ONCEs, which I agreed to remove, I currently don't see any problem for net. You don't agree on a netdev-feature flag, indicating the driver is okay with hardware checksumming and extension headers? We could add this to net-next pretty fast, I think. It does not require people to revert this patch in case their driver misbehaves and we don't get a fix for it, soon. Also what should we do if the driver simply does not support extension headers + checksum offloading? Completely kill checksum offloading for IPv6?
I posted v3 just now. I would like to let David consider it for net inclusion. We can work on how to lift this limitation then in net-next, okay? I am currently in favor of a new netdev-feature. What do you think? Your RFC series could help here, too. Thanks, Hannes