On Tue, Oct 27, 2015 at 2:42 PM, Hannes Frederic Sowa <han...@stressinduktion.org> wrote: > 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: >> > On Tue, Oct 27, 2015 at 11:29 AM, Hannes Frederic Sowa >> > <han...@stressinduktion.org> wrote: >> > > On Tue, Oct 27, 2015, at 18:32, Tom Herbert wrote: >> > >> On Tue, Oct 27, 2015 at 9:44 AM, Hannes Frederic Sowa >> > >> <han...@stressinduktion.org> wrote: >> > >> > >> > >> > >> > >> > On Tue, Oct 27, 2015, at 17:36, Tom Herbert wrote:> > - >> > >> > if >> > >> > (cork->length + length > maxnonfragsize - headersize) { >> > >> >> > + 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 when >> > >> >> >> > >> >> No, 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. > I really do not like the feature flag, it's just a bandaid over the real problem-- in fact my goal is to eliminate NETIF_F_IP{V6}_CSUM and just have NETIF_F_HW_CSUM. I will repost the helper patches, but we really do need to start fixing this stuff in the drivers instead of more hacking in the stack.
Tom > Thanks, > Hannes > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html