From: Joseph Gasparakis <joseph.gasparakis@intel.com>
To: Or Gerlitz <or.gerlitz@gmail.com>
Cc: Tom Herbert <therbert@google.com>,
Joseph Gasparakis <joseph.gasparakis@intel.com>,
Or Gerlitz <ogerlitz@mellanox.com>,
David Miller <davem@davemloft.net>,
Linux Netdev List <netdev@vger.kernel.org>,
Jerry Chu <hkchu@google.com>, Eric Dumazet <edumazet@google.com>
Subject: Re: [PATCH 2/3] net: UDP gro_receive accept csum=0
Date: Thu, 13 Feb 2014 16:04:02 -0800 (PST) [thread overview]
Message-ID: <alpine.LFD.2.03.1402131600120.8285@intel.com> (raw)
In-Reply-To: <CAJZOPZKRiR-XZo1b9-TQG9ULpVhL31bTBD+4Z7T+ODZcPX_L2g@mail.gmail.com>
On Thu, 13 Feb 2014, Or Gerlitz wrote:
> On Fri, Feb 14, 2014 at 12:27 AM, Tom Herbert <therbert@google.com> wrote:
>
> >> [...] this is according to the conventions set by
> >> 0afb166 vxlan: Add capability of Rx checksum offload for inner packet
> >> 6a674e9 net: Add support for hardware-offloaded encapsulation
> >> B/c after finalizing the GRO work and decapsulating, skb injected up into
> >> the TCP stack with ip_summed equals to CHECKSUM_NONE are rejected. If >> this assumption is wrong, maybe we can remove testing the ip_summed field
> >> here altogether?
>
> > If I'm interpreting the semantics correctly, when skb->encapsulation
> > is set the ip_summed is valid for both the inner and outer header
> > (e.g. CHECKSUM_COMPLETE is always assumed okay for both layers). If
> > skb->encapsulation is not set then ip_summed is only valid for outer header.
>
> Yep, I think this would be correct interpertation, Joseph, agree?
Agreed.
>
> > So then the patch is broken in the case that encap is not set,
> > ip_summed is CHECKSUM_UNNECESSARY, csum == 0, and we need to
> > validate the inner checksum.
>
> Just to make sure, by "the patch" you refer to your patch or the current code?
>
> > But even worse, is there a fundamental issue where udp4_csum_init is able
> > to change ip_summed to be CHECKSUM_UNNECESSARY (either check == 0
> > or ip_summed == CHECKSUM_UNNECESSARY) regardless of
> > skb->encapsulation, sending the packet into encap_rcv which could
> > ultimately incorrectly apply ip_summed on the inner TCP/UDP packet?
>
> By fundamental you mean performance issue or functionality issue (bug) or both?
>
I would expect the check to be for ip_summed == CHECKSUM_UNNECESSARY. This
was the original thought behind commit:
0afb166 vxlan: Add capability of Rx checksum offload for inner packet
next prev parent reply other threads:[~2014-02-13 23:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-11 17:43 [PATCH 2/3] net: UDP gro_receive accept csum=0 Tom Herbert
2014-02-13 8:06 ` Or Gerlitz
2014-02-13 22:27 ` Tom Herbert
2014-02-13 22:50 ` Or Gerlitz
2014-02-14 0:04 ` Joseph Gasparakis [this message]
2014-02-14 0:59 ` Tom Herbert
2014-02-14 18:34 ` Joseph Gasparakis
2014-02-14 23:54 ` Tom Herbert
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=alpine.LFD.2.03.1402131600120.8285@intel.com \
--to=joseph.gasparakis@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkchu@google.com \
--cc=netdev@vger.kernel.org \
--cc=ogerlitz@mellanox.com \
--cc=or.gerlitz@gmail.com \
--cc=therbert@google.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).