From: Sabrina Dubroca <sd@queasysnail.net>
To: Ben Greear <greearb@candelatech.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>,
Ben Hutchings <ben@decadent.org.uk>,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
akpm@linux-foundation.org,
"David S. Miller" <davem@davemloft.net>,
Vijay Pandurangan <vijayp@vijayp.ca>,
Cong Wang <cwang@twopensource.com>,
netdev@vger.kernel.org, Evan Jones <ej@evanjones.ca>,
Nicolas Dichtel <nicolas.dichtel@6wind.com>,
Phil Sutter <phil@nwl.cc>,
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>,
xiyou.wangcong@gmail.com
Subject: Re: [PATCH 3.2 085/115] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.
Date: Thu, 28 Apr 2016 12:29:53 +0200 [thread overview]
Message-ID: <20160428102953.GA7656@bistromath.localdomain> (raw)
In-Reply-To: <572155F4.10405@candelatech.com>
Hello,
2016-04-27, 17:14:44 -0700, Ben Greear wrote:
> On 04/27/2016 05:00 PM, Hannes Frederic Sowa wrote:
> > Hi Ben,
> >
> > On Wed, Apr 27, 2016, at 20:07, Ben Hutchings wrote:
> > > On Wed, 2016-04-27 at 08:59 -0700, Ben Greear wrote:
> > > > On 04/26/2016 04:02 PM, Ben Hutchings wrote:
> > > > >
> > > > > 3.2.80-rc1 review patch. If anyone has any objections, please let me know.
> > > > I would be careful about this. It causes regressions when sending
> > > > PACKET_SOCKET buffers from user-space to veth devices.
> > > >
> > > > There was a proposed upstream fix for the regression, but it has not gone
> > > > into the tree as far as I know.
> > > >
> > > > http://www.spinics.net/lists/netdev/msg370436.html
> > > [...]
> > >
> > > OK, I'll drop this for now.
> >
> > The fall out from not having this patch is in my opinion a bigger
> > fallout than not having this patch. This patch fixes silent data
> > corruption vs. the problem Ben Greear is talking about, which might not
> > be that a common usage.
> >
> > What do others think?
> >
> > Bye,
> > Hannes
> >
>
> This patch from Cong Wang seems to fix the regression for me, I think it should be added and
> tested in the main tree, and then apply them to stable as a pair.
>
> http://dmz2.candelatech.com/?p=linux-4.4.dev.y/.git;a=commitdiff;h=8153e983c0e5eba1aafe1fc296248ed2a553f1ac;hp=454b07405d694dad52e7f41af5816eed0190da8a
Actually, no, this is not really a regression.
If you capture packets on a device with checksum offloading enabled,
the TCP/UDP checksum isn't filled. veth also behaves that way. What
the "veth: don't modify ip_summed" patch does is enable proper
checksum validation on veth. This really was a bug in veth.
Cong's patch would also break cases where we choose to inject packets
with invalid checksums, and they would now be accepted as correct.
Your use case is invalid, it just happened to work because of a
bug. If you want the stack to fill checksums so that you want capture
and reinject packets, you have to disable checksum offloading (or
compute the checksum yourself in userspace).
Thanks.
--
Sabrina
next prev parent reply other threads:[~2016-04-28 10:29 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <lsq.1461711744.351546278@decadent.org.uk>
2016-04-26 23:02 ` [PATCH 3.2 085/115] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good Ben Hutchings
2016-04-27 15:59 ` Ben Greear
2016-04-27 18:07 ` Ben Hutchings
2016-04-28 0:00 ` Hannes Frederic Sowa
2016-04-28 0:14 ` Ben Greear
2016-04-28 10:29 ` Sabrina Dubroca [this message]
2016-04-28 13:45 ` Ben Greear
2016-04-30 19:18 ` Ben Hutchings
2016-04-30 18:33 ` Ben Hutchings
2016-04-30 19:40 ` Ben Greear
2016-04-30 19:54 ` Tom Herbert
2016-04-30 20:59 ` Ben Greear
2016-04-30 21:13 ` Vijay Pandurangan
2016-04-30 21:29 ` Ben Greear
2016-04-30 21:36 ` Vijay Pandurangan
2016-04-30 21:52 ` Ben Greear
2016-04-30 22:01 ` Vijay Pandurangan
2016-04-30 22:43 ` Ben Greear
2016-05-01 5:30 ` [PATCH 3.2 085/115] veth: don???t " Willy Tarreau
2016-05-13 16:57 ` Ben Greear
2016-05-13 18:21 ` David Miller
2016-05-13 18:23 ` Ben Greear
2016-04-30 22:42 ` [PATCH 3.2 085/115] veth: don’t " Tom Herbert
2016-04-30 20:15 ` Vijay Pandurangan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160428102953.GA7656@bistromath.localdomain \
--to=sd@queasysnail.net \
--cc=akpm@linux-foundation.org \
--cc=ben@decadent.org.uk \
--cc=cwang@twopensource.com \
--cc=davem@davemloft.net \
--cc=ej@evanjones.ca \
--cc=greearb@candelatech.com \
--cc=hannes@stressinduktion.org \
--cc=linux-kernel@vger.kernel.org \
--cc=makita.toshiaki@lab.ntt.co.jp \
--cc=netdev@vger.kernel.org \
--cc=nicolas.dichtel@6wind.com \
--cc=phil@nwl.cc \
--cc=stable@vger.kernel.org \
--cc=vijayp@vijayp.ca \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).