Thread (23 messages) 23 messages, 3 authors, 2015-10-28

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

Thanks,
Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help