[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1445973550.508206.421832305.2B25EDF1@webmail.messagingengine.com>
Date: Tue, 27 Oct 2015 20:19:10 +0100
From: Hannes Frederic Sowa <hannes@...essinduktion.org>
To: Tom Herbert <tom@...bertland.com>
Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Vladislav Yasevich <vyasevich@...il.com>,
Benjamin Coddington <bcodding@...hat.com>
Subject: Re: [PATCH net v2 3/4] ipv6: no CHECKSUM_PARTIAL on MSG_MORE corked
sockets
On Tue, Oct 27, 2015, at 19:37, Tom Herbert wrote:
> On Tue, Oct 27, 2015 at 11:29 AM, Hannes Frederic Sowa
> <hannes@...essinduktion.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
> >> <hannes@...essinduktion.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?
Bye,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists