netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steffen Klassert <steffen.klassert@secunet.com>
To: Ansis Atteka <ansisatteka@gmail.com>
Cc: Ansis Atteka <aatteka@ovn.org>, <netdev@vger.kernel.org>
Subject: Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets
Date: Tue, 18 Apr 2017 11:09:19 +0200	[thread overview]
Message-ID: <20170418090919.GD30104@secunet.com> (raw)
In-Reply-To: <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com>

On Thu, Apr 13, 2017 at 07:45:08PM -0700, Ansis Atteka wrote:
> On 11 April 2017 at 00:07, Steffen Klassert <steffen.klassert@secunet.com>
> wrote:
> >
> > What's wrong with the checksum provided by the GSO layer and
> > why we have to do this unconditionally here?
> >
> 
> I believe with "GSO layer" you meant the skb_gso_segment() function
> invocation from xfrm_output_gso()?
> 
> If so, then the problem with that is that the list of the skb's returned by
> that function could be in CHECKSUM_PARTIAL state if skb came from a UDP
> tunnel such as Geneve:

This should not happen. We don't announce checksum capabilities,
so the GSO layer should generate the full checksum.

__skb_udp_tunnel_segment() and udp4_ufo_fragment() add the
NETIF_F_HW_CSUM flag to features. This is certainly wrong
if the packet undergoes an IPsec transformation.

I don't have a testcase for this, so not sure if this is
your problem. Could you try the (untested) patch below?

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index b2be1d9..cc0c89c 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -74,7 +74,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
 	 * outer one so strip the existing checksum feature flags and
 	 * instead set the flag based on our outer checksum offload value.
 	 */
-	if (remcsum || ufo) {
+	if ((remcsum || ufo) && !(skb_dst(skb) && dst_xfrm(skb_dst(skb)))) {
 		features &= ~NETIF_F_CSUM_MASK;
 		if (!need_csum || offload_csum)
 			features |= NETIF_F_HW_CSUM;
@@ -238,7 +238,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
 	 * due to the fact that we have already done the checksum in
 	 * software prior to segmenting the frame.
 	 */
-	if (!skb->encap_hdr_csum)
+	if (!skb->encap_hdr_csum && !(skb_dst(skb) && dst_xfrm(skb_dst(skb))))
 		features |= NETIF_F_HW_CSUM;
 
 	/* Fragment the skb. IP headers of the fragments are updated in

  parent reply	other threads:[~2017-04-18  9:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-10 18:42 [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets Ansis Atteka
2017-04-11  7:07 ` Steffen Klassert
     [not found]   ` <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com>
2017-04-14  2:54     ` Ansis Atteka
2017-04-18  9:09     ` Steffen Klassert [this message]
2017-04-19  2:10       ` Ansis Atteka
2017-04-20  9:47         ` Steffen Klassert
2017-04-21 21:45           ` Ansis Atteka
2017-04-27  9:04             ` Steffen Klassert

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=20170418090919.GD30104@secunet.com \
    --to=steffen.klassert@secunet.com \
    --cc=aatteka@ovn.org \
    --cc=ansisatteka@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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).