All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-08 19:44 [PATCH v3 " Edward Cree
@ 2016-01-08 19:47 ` Edward Cree
  2016-01-11 10:09   ` David Laight
  0 siblings, 1 reply; 20+ messages in thread
From: Edward Cree @ 2016-01-08 19:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 net/ipv4/ip_gre.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 04a48c0..8a589d3 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -440,6 +440,20 @@ drop:
 	return 0;
 }
 
+static __sum16 gre_checksum(struct sk_buff *skb)
+{
+	__sum16 csum;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		csum = csum_fold(lco_csum(skb));
+		if (csum == 0)
+			csum = CSUM_MANGLED_0;
+		return csum;
+	} else {
+		return csum_fold(skb_checksum(skb, 0, skb->len, 0));
+	}
+}
+
 static void build_header(struct sk_buff *skb, int hdr_len, __be16 flags,
 			 __be16 proto, __be32 key, __be32 seq)
 {
@@ -467,8 +481,7 @@ static void build_header(struct sk_buff *skb, int hdr_len, __be16 flags,
 		    !(skb_shinfo(skb)->gso_type &
 		      (SKB_GSO_GRE | SKB_GSO_GRE_CSUM))) {
 			*ptr = 0;
-			*(__sum16 *)ptr = csum_fold(skb_checksum(skb, 0,
-								 skb->len, 0));
+			*(__sum16 *)ptr = gre_checksum(skb);
 		}
 	}
 }
@@ -493,7 +506,7 @@ static void __gre_xmit(struct sk_buff *skb, struct net_device *dev,
 static struct sk_buff *gre_handle_offloads(struct sk_buff *skb,
 					   bool csum)
 {
-	return iptunnel_handle_offloads(skb, csum,
+	return iptunnel_handle_offloads(skb, false,
 					csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE);
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* RE: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-08 19:47 ` [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4 Edward Cree
@ 2016-01-11 10:09   ` David Laight
  2016-01-11 13:21     ` Edward Cree
  0 siblings, 1 reply; 20+ messages in thread
From: David Laight @ 2016-01-11 10:09 UTC (permalink / raw)
  To: 'Edward Cree', David Miller
  Cc: netdev@vger.kernel.org, linux-net-drivers@solarflare.com,
	tom@herbertland.com, alexander.duyck@gmail.com

From: Edward Cree
> Sent: 08 January 2016 19:47
...
> +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +		csum = csum_fold(lco_csum(skb));
> +		if (csum == 0)
> +			csum = CSUM_MANGLED_0;
> +		return csum;
> +	} else {
> +		return csum_fold(skb_checksum(skb, 0, skb->len, 0));
> +	}

You see to be worried about csum_fold() returning 0 in one
path, but not in the other.
I'm guessing that 0 can only happen if all the bytes that have
been checksummed are zero.
This is almost certainly never true if any bytes have been summed,
and might be better avoided by initialising any such checksum to
0xffff (instead of 0) much earlier on.

	David


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-11 10:09   ` David Laight
@ 2016-01-11 13:21     ` Edward Cree
  2016-01-11 18:39       ` Alexander Duyck
  0 siblings, 1 reply; 20+ messages in thread
From: Edward Cree @ 2016-01-11 13:21 UTC (permalink / raw)
  To: David Laight, David Miller
  Cc: netdev@vger.kernel.org, linux-net-drivers@solarflare.com,
	tom@herbertland.com, alexander.duyck@gmail.com

On 11/01/16 10:09, David Laight wrote:
> From: Edward Cree
>> Sent: 08 January 2016 19:47
> ...
>> +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> +		csum = csum_fold(lco_csum(skb));
>> +		if (csum == 0)
>> +			csum = CSUM_MANGLED_0;
>> +		return csum;
>> +	} else {
>> +		return csum_fold(skb_checksum(skb, 0, skb->len, 0));
>> +	}
> You see to be worried about csum_fold() returning 0 in one
> path, but not in the other.
> I'm guessing that 0 can only happen if all the bytes that have
> been checksummed are zero.
csum_fold complements, so if the sum comes to (say) 0x1fffe, it will return ~0xffff which is 0.

Next version of patch will mangle 0 for both branches.

-ed

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-11 13:21     ` Edward Cree
@ 2016-01-11 18:39       ` Alexander Duyck
  2016-01-11 19:02         ` Edward Cree
  2016-01-20 19:11         ` Rustad, Mark D
  0 siblings, 2 replies; 20+ messages in thread
From: Alexander Duyck @ 2016-01-11 18:39 UTC (permalink / raw)
  To: Edward Cree
  Cc: David Laight, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, tom@herbertland.com

On Mon, Jan 11, 2016 at 5:21 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 11/01/16 10:09, David Laight wrote:
>> From: Edward Cree
>>> Sent: 08 January 2016 19:47
>> ...
>>> +    if (skb->ip_summed == CHECKSUM_PARTIAL) {
>>> +            csum = csum_fold(lco_csum(skb));
>>> +            if (csum == 0)
>>> +                    csum = CSUM_MANGLED_0;
>>> +            return csum;
>>> +    } else {
>>> +            return csum_fold(skb_checksum(skb, 0, skb->len, 0));
>>> +    }
>> You see to be worried about csum_fold() returning 0 in one
>> path, but not in the other.
>> I'm guessing that 0 can only happen if all the bytes that have
>> been checksummed are zero.
> csum_fold complements, so if the sum comes to (say) 0x1fffe, it will return ~0xffff which is 0.
>
> Next version of patch will mangle 0 for both branches.

Actually you may want to go the other way on that.  If they weren't
flipping the checksum value for GRE before why should we start doing
that now?  I'm pretty sure the checksum mangling is a very UDP centric
thing.  There is no need to introduce it to other protocols.

- Alex

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-11 18:39       ` Alexander Duyck
@ 2016-01-11 19:02         ` Edward Cree
  2016-01-20 19:11         ` Rustad, Mark D
  1 sibling, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-01-11 19:02 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: David Laight, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, tom@herbertland.com

On 11/01/16 18:39, Alexander Duyck wrote:
> On Mon, Jan 11, 2016 at 5:21 AM, Edward Cree <ecree@solarflare.com> wrote:
>> csum_fold complements, so if the sum comes to (say) 0x1fffe, it will return ~0xffff which is 0.
>>
>> Next version of patch will mangle 0 for both branches.
> Actually you may want to go the other way on that.  If they weren't
> flipping the checksum value for GRE before why should we start doing
> that now?  I'm pretty sure the checksum mangling is a very UDP centric
> thing.  There is no need to introduce it to other protocols.
Good catch, you're quite right.

-ed

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-11 18:39       ` Alexander Duyck
  2016-01-11 19:02         ` Edward Cree
@ 2016-01-20 19:11         ` Rustad, Mark D
  2016-01-20 19:35           ` Alexander Duyck
  1 sibling, 1 reply; 20+ messages in thread
From: Rustad, Mark D @ 2016-01-20 19:11 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Edward Cree, David Laight, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, tom@herbertland.com

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

Alexander Duyck <alexander.duyck@gmail.com> wrote:

> Actually you may want to go the other way on that.  If they weren't
> flipping the checksum value for GRE before why should we start doing
> that now?  I'm pretty sure the checksum mangling is a very UDP centric
> thing.  There is no need to introduce it to other protocols.

If different checksum representations are needed, then there really should  
be an explicit indication of whether it is a UDP-style checksum or other in  
the skb I would think rather than guessing it based on the offset. Of  
course it would be convenient if all the protocols that use a one's  
complement checksum would tolerate the UDP representation. I have a long  
(and now old) history working with real one's complement machines, and so I  
would want to believe that any correct implementation would tolerate it,  
but I don't know for a fact that they do.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-20 19:11         ` Rustad, Mark D
@ 2016-01-20 19:35           ` Alexander Duyck
  2016-01-20 19:58             ` Tom Herbert
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Duyck @ 2016-01-20 19:35 UTC (permalink / raw)
  To: Rustad, Mark D
  Cc: Edward Cree, David Laight, David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, tom@herbertland.com

On Wed, Jan 20, 2016 at 11:11 AM, Rustad, Mark D
<mark.d.rustad@intel.com> wrote:
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
>> Actually you may want to go the other way on that.  If they weren't
>> flipping the checksum value for GRE before why should we start doing
>> that now?  I'm pretty sure the checksum mangling is a very UDP centric
>> thing.  There is no need to introduce it to other protocols.
>
>
> If different checksum representations are needed, then there really should
> be an explicit indication of whether it is a UDP-style checksum or other in
> the skb I would think rather than guessing it based on the offset. Of course
> it would be convenient if all the protocols that use a one's complement
> checksum would tolerate the UDP representation. I have a long (and now old)
> history working with real one's complement machines, and so I would want to
> believe that any correct implementation would tolerate it, but I don't know
> for a fact that they do.

The only reason why UDP does the bit flip is because it has reserved a
checksum of 0 as a special value.  For the checksum math itself either
0xFFFF or 0 are interchangeable.  The only time they would make any
difference would be if we had a value of 0 that we were checksumming,
but since that is not the case the values will always end up
converging back onto 0xFFFF as the final result in the case of a
correct checksum.

- Alex

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-20 19:35           ` Alexander Duyck
@ 2016-01-20 19:58             ` Tom Herbert
  2016-01-20 21:13               ` Alexander Duyck
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Herbert @ 2016-01-20 19:58 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Rustad, Mark D, Edward Cree, David Laight, David Miller,
	netdev@vger.kernel.org, linux-net-drivers@solarflare.com

On Wed, Jan 20, 2016 at 11:35 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Wed, Jan 20, 2016 at 11:11 AM, Rustad, Mark D
> <mark.d.rustad@intel.com> wrote:
>> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>
>>> Actually you may want to go the other way on that.  If they weren't
>>> flipping the checksum value for GRE before why should we start doing
>>> that now?  I'm pretty sure the checksum mangling is a very UDP centric
>>> thing.  There is no need to introduce it to other protocols.
>>
>>
>> If different checksum representations are needed, then there really should
>> be an explicit indication of whether it is a UDP-style checksum or other in
>> the skb I would think rather than guessing it based on the offset. Of course
>> it would be convenient if all the protocols that use a one's complement
>> checksum would tolerate the UDP representation. I have a long (and now old)
>> history working with real one's complement machines, and so I would want to
>> believe that any correct implementation would tolerate it, but I don't know
>> for a fact that they do.
>
> The only reason why UDP does the bit flip is because it has reserved a
> checksum of 0 as a special value.  For the checksum math itself either
> 0xFFFF or 0 are interchangeable.  The only time they would make any
> difference would be if we had a value of 0 that we were checksumming,
> but since that is not the case the values will always end up
> converging back onto 0xFFFF as the final result in the case of a
> correct checksum.
>
0xffff is mathematically equivalent to 0x0 for checksum. I would
rather always flip 0 to 0xffff in LCO rather than adding an explicit
indication (i.e. another flag) in SKB that it has a UDP checksum.

Tom

> - Alex

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-20 19:58             ` Tom Herbert
@ 2016-01-20 21:13               ` Alexander Duyck
  2016-01-20 23:34                 ` Rustad, Mark D
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Duyck @ 2016-01-20 21:13 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Rustad, Mark D, Edward Cree, David Laight, David Miller,
	netdev@vger.kernel.org, linux-net-drivers@solarflare.com

On Wed, Jan 20, 2016 at 11:58 AM, Tom Herbert <tom@herbertland.com> wrote:
> On Wed, Jan 20, 2016 at 11:35 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Wed, Jan 20, 2016 at 11:11 AM, Rustad, Mark D
>> <mark.d.rustad@intel.com> wrote:
>>> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>>
>>>> Actually you may want to go the other way on that.  If they weren't
>>>> flipping the checksum value for GRE before why should we start doing
>>>> that now?  I'm pretty sure the checksum mangling is a very UDP centric
>>>> thing.  There is no need to introduce it to other protocols.
>>>
>>>
>>> If different checksum representations are needed, then there really should
>>> be an explicit indication of whether it is a UDP-style checksum or other in
>>> the skb I would think rather than guessing it based on the offset. Of course
>>> it would be convenient if all the protocols that use a one's complement
>>> checksum would tolerate the UDP representation. I have a long (and now old)
>>> history working with real one's complement machines, and so I would want to
>>> believe that any correct implementation would tolerate it, but I don't know
>>> for a fact that they do.
>>
>> The only reason why UDP does the bit flip is because it has reserved a
>> checksum of 0 as a special value.  For the checksum math itself either
>> 0xFFFF or 0 are interchangeable.  The only time they would make any
>> difference would be if we had a value of 0 that we were checksumming,
>> but since that is not the case the values will always end up
>> converging back onto 0xFFFF as the final result in the case of a
>> correct checksum.
>>
> 0xffff is mathematically equivalent to 0x0 for checksum. I would
> rather always flip 0 to 0xffff in LCO rather than adding an explicit
> indication (i.e. another flag) in SKB that it has a UDP checksum.

There isn't any need to add such an indication, nor do we need to
start bitflipping the return value from csum_fold in all cases.  I
think there was just some confusion about UDP checksums vs GRE or TCP
checksums.

I'd say we are better off keeping this simple.  The original patch
just needs to drop the check for the resultant checksum being 0 since
that is not needed for GRE.

- Alex

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-01-20 21:13               ` Alexander Duyck
@ 2016-01-20 23:34                 ` Rustad, Mark D
  0 siblings, 0 replies; 20+ messages in thread
From: Rustad, Mark D @ 2016-01-20 23:34 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Tom Herbert, Edward Cree, David Laight, David Miller,
	netdev@vger.kernel.org, linux-net-drivers@solarflare.com

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

Alexander Duyck <alexander.duyck@gmail.com> wrote:

> There isn't any need to add such an indication, nor do we need to
> start bitflipping the return value from csum_fold in all cases.  I
> think there was just some confusion about UDP checksums vs GRE or TCP
> checksums.

Yeah. I think I finally got there. The naive software methods will never  
generate a true 0 unless everything was zero. Real one's complement  
machines did addition in terms of subtraction so that 1 + -1 would never  
produce a -0, only a normal 0. Of course a simple adder would produce a -0,  
making it impossible to get back to a normal 0.

> I'd say we are better off keeping this simple.  The original patch
> just needs to drop the check for the resultant checksum being 0 since
> that is not needed for GRE.

I'm all in favor of simple. I had just started to worry about a possible  
change in behavior that might have interoperability problems with some  
implementations. I wonder if any implementation ever did the addition by  
subtraction, but also failed to make 0 compare equal to -0? I guess if they  
knew enough to do the former, they should not have blown the latter.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 net-next 0/8] Local Checksum Offload
@ 2016-02-05 20:39 Edward Cree
  2016-02-05 20:40 ` [PATCH net-next 1/8] net: local checksum offload for encapsulation Edward Cree
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:39 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

Tested with VxLAN, GRE and FOU-IPIP tunnels.  Not tested with GENEVE,
 because iproute2 doesn't support enabling checksums on GENEVE tunnels.
Also tested VxLAN with IPv6 (as both inner and outer protocol).

Changes from v3:
 * Fixed inverted checksum values introduced in v3.
 * Don't mangle zero checksums in GRE.
 * Clear skb->encapsulation in iptunnel_handle_offloads when not using
   CHECKSUM_PARTIAL, lest drivers incorrectly interpret that as a request
   for inner checksum offload.

Changes from v2:
 * Added support for IPv4 GRE.
 * Split out 'always set up for checksum offload' into its own patch.
 * Removed csum_help from iptunnel_handle_offloads.
 * Rewrote LCO callers to only fold once.
 * Simplified nocheck handling.

Changes from v1:
 * Enabled support in more encapsulation protocols.
   I think it now covers everything except GRE.
 * Wrote up some documentation covering TX checksum offload, LCO and RCO.

Edward Cree (8):
  net: local checksum offload for encapsulation
  net: udp: always set up for CHECKSUM_PARTIAL offload
  net: enable LCO for udp_tunnel_handle_offloads() users
  net: vxlan: enable local checksum offload
  fou: enable LCO in FOU and GUE
  net: gre: Implement LCO for GRE over IPv4
  net: ip_tunnel: remove 'csum_help' argument to
    iptunnel_handle_offloads
  Documentation/networking: add checksum-offloads.txt to explain LCO

 Documentation/networking/00-INDEX              |   2 +
 Documentation/networking/checksum-offloads.txt | 119 +++++++++++++++++++++++++
 drivers/net/vxlan.c                            |  18 ++--
 include/linux/skbuff.h                         |  26 ++++++
 include/net/ip_tunnels.h                       |   3 +-
 include/net/udp_tunnel.h                       |   2 +-
 net/ipv4/fou.c                                 |  14 ++-
 net/ipv4/ip_gre.c                              |  17 +++-
 net/ipv4/ip_tunnel_core.c                      |  22 ++---
 net/ipv4/ipip.c                                |   2 +-
 net/ipv4/udp.c                                 |  28 ++----
 net/ipv6/ip6_checksum.c                        |  23 ++---
 net/ipv6/sit.c                                 |   4 +-
 net/netfilter/ipvs/ip_vs_xmit.c                |   6 +-
 14 files changed, 201 insertions(+), 85 deletions(-)
 create mode 100644 Documentation/networking/checksum-offloads.txt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH net-next 1/8] net: local checksum offload for encapsulation
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
@ 2016-02-05 20:40 ` Edward Cree
  2016-02-05 20:41 ` [PATCH net-next 2/8] net: udp: always set up for CHECKSUM_PARTIAL offload Edward Cree
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:40 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

The arithmetic properties of the ones-complement checksum mean that a
 correctly checksummed inner packet, including its checksum, has a ones
 complement sum depending only on whatever value was used to initialise
 the checksum field before checksumming (in the case of TCP and UDP,
 this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
 CHECKSUM_PARTIAL, we can compute the outer checksum based only on the
 packed data not covered by the inner checksum, and the initial value of
 the inner checksum field.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 include/linux/skbuff.h    | 24 ++++++++++++++++++++++++
 net/ipv4/ip_tunnel_core.c | 10 +++++-----
 net/ipv4/udp.c            | 20 ++++++++++----------
 net/ipv6/ip6_checksum.c   | 14 +++++++-------
 4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 11f935c..3e9eb52 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3683,5 +3683,29 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
 	return hdr_len + skb_gso_transport_seglen(skb);
 }
 
+/* Local Checksum Offload.
+ * Compute outer checksum based on the assumption that the
+ * inner checksum will be offloaded later.
+ * Fill in outer checksum adjustment (e.g. with sum of outer
+ * pseudo-header) before calling.
+ * Also ensure that inner checksum is in linear data area.
+ */
+static inline __wsum lco_csum(struct sk_buff *skb)
+{
+	char *inner_csum_field;
+	__wsum csum;
+
+	/* Start with complement of inner checksum adjustment */
+	inner_csum_field = skb->data + skb_checksum_start_offset(skb) +
+				skb->csum_offset;
+	csum = ~csum_unfold(*(__force __sum16 *)inner_csum_field);
+	/* Add in checksum of our headers (incl. outer checksum
+	 * adjustment filled in by caller)
+	 */
+	csum = skb_checksum(skb, 0, skb_checksum_start_offset(skb), csum);
+	/* The result is the checksum from skb->data to end of packet */
+	return csum;
+}
+
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 859d415..d74ce93 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -166,20 +166,20 @@ struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb,
 		return skb;
 	}
 
-	/* If packet is not gso and we are resolving any partial checksum,
+	/* If packet is not gso and we are not offloading inner checksum,
 	 * clear encapsulation flag. This allows setting CHECKSUM_PARTIAL
 	 * on the outer header without confusing devices that implement
 	 * NETIF_F_IP_CSUM with encapsulation.
 	 */
-	if (csum_help)
-		skb->encapsulation = 0;
-
 	if (skb->ip_summed == CHECKSUM_PARTIAL && csum_help) {
+		skb->encapsulation = 0;
 		err = skb_checksum_help(skb);
 		if (unlikely(err))
 			goto error;
-	} else if (skb->ip_summed != CHECKSUM_PARTIAL)
+	} else if (skb->ip_summed != CHECKSUM_PARTIAL) {
 		skb->ip_summed = CHECKSUM_NONE;
+		skb->encapsulation = 0;
+	}
 
 	return skb;
 error:
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index be0b218..005280d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -848,16 +848,18 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb,
 {
 	struct udphdr *uh = udp_hdr(skb);
 
-	if (nocheck)
+	if (nocheck) {
 		uh->check = 0;
-	else if (skb_is_gso(skb))
+	} else if (skb_is_gso(skb)) {
 		uh->check = ~udp_v4_check(len, saddr, daddr, 0);
-	else if (skb_dst(skb) && skb_dst(skb)->dev &&
-		 (skb_dst(skb)->dev->features &
-		  (NETIF_F_IP_CSUM | NETIF_F_HW_CSUM))) {
-
-		BUG_ON(skb->ip_summed == CHECKSUM_PARTIAL);
-
+	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		uh->check = 0;
+		uh->check = udp_v4_check(len, saddr, daddr, lco_csum(skb));
+		if (uh->check == 0)
+			uh->check = CSUM_MANGLED_0;
+	} else if (skb_dst(skb) && skb_dst(skb)->dev &&
+		   (skb_dst(skb)->dev->features &
+		    (NETIF_F_IP_CSUM | NETIF_F_HW_CSUM))) {
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct udphdr, check);
@@ -865,8 +867,6 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb,
 	} else {
 		__wsum csum;
 
-		BUG_ON(skb->ip_summed == CHECKSUM_PARTIAL);
-
 		uh->check = 0;
 		csum = skb_checksum(skb, 0, len, 0);
 		uh->check = udp_v4_check(len, saddr, daddr, csum);
diff --git a/net/ipv6/ip6_checksum.c b/net/ipv6/ip6_checksum.c
index 9a4d732..4924bd7 100644
--- a/net/ipv6/ip6_checksum.c
+++ b/net/ipv6/ip6_checksum.c
@@ -98,11 +98,13 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb,
 		uh->check = 0;
 	else if (skb_is_gso(skb))
 		uh->check = ~udp_v6_check(len, saddr, daddr, 0);
-	else if (skb_dst(skb) && skb_dst(skb)->dev &&
-		 (skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) {
-
-		BUG_ON(skb->ip_summed == CHECKSUM_PARTIAL);
-
+	else if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		uh->check = 0;
+		uh->check = udp_v6_check(len, saddr, daddr, lco_csum(skb));
+		if (uh->check == 0)
+			uh->check = CSUM_MANGLED_0;
+	} else if (skb_dst(skb) && skb_dst(skb)->dev &&
+		   (skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) {
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct udphdr, check);
@@ -110,8 +112,6 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb,
 	} else {
 		__wsum csum;
 
-		BUG_ON(skb->ip_summed == CHECKSUM_PARTIAL);
-
 		uh->check = 0;
 		csum = skb_checksum(skb, 0, len, 0);
 		uh->check = udp_v6_check(len, saddr, daddr, csum);

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 2/8] net: udp: always set up for CHECKSUM_PARTIAL offload
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
  2016-02-05 20:40 ` [PATCH net-next 1/8] net: local checksum offload for encapsulation Edward Cree
@ 2016-02-05 20:41 ` Edward Cree
  2016-02-05 20:41 ` [PATCH net-next 3/8] net: enable LCO for udp_tunnel_handle_offloads() users Edward Cree
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

If the dst device doesn't support it, it'll get fixed up later anyway
 by validate_xmit_skb().  Also, this allows us to take advantage of LCO
 to avoid summing the payload multiple times.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 net/ipv4/udp.c          | 14 +-------------
 net/ipv6/ip6_checksum.c | 13 +------------
 2 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 005280d..c6bca27 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -857,23 +857,11 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb,
 		uh->check = udp_v4_check(len, saddr, daddr, lco_csum(skb));
 		if (uh->check == 0)
 			uh->check = CSUM_MANGLED_0;
-	} else if (skb_dst(skb) && skb_dst(skb)->dev &&
-		   (skb_dst(skb)->dev->features &
-		    (NETIF_F_IP_CSUM | NETIF_F_HW_CSUM))) {
+	} else {
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct udphdr, check);
 		uh->check = ~udp_v4_check(len, saddr, daddr, 0);
-	} else {
-		__wsum csum;
-
-		uh->check = 0;
-		csum = skb_checksum(skb, 0, len, 0);
-		uh->check = udp_v4_check(len, saddr, daddr, csum);
-		if (uh->check == 0)
-			uh->check = CSUM_MANGLED_0;
-
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
 }
 EXPORT_SYMBOL(udp_set_csum);
diff --git a/net/ipv6/ip6_checksum.c b/net/ipv6/ip6_checksum.c
index 4924bd7..8f92058 100644
--- a/net/ipv6/ip6_checksum.c
+++ b/net/ipv6/ip6_checksum.c
@@ -103,22 +103,11 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb,
 		uh->check = udp_v6_check(len, saddr, daddr, lco_csum(skb));
 		if (uh->check == 0)
 			uh->check = CSUM_MANGLED_0;
-	} else if (skb_dst(skb) && skb_dst(skb)->dev &&
-		   (skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) {
+	} else {
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct udphdr, check);
 		uh->check = ~udp_v6_check(len, saddr, daddr, 0);
-	} else {
-		__wsum csum;
-
-		uh->check = 0;
-		csum = skb_checksum(skb, 0, len, 0);
-		uh->check = udp_v6_check(len, saddr, daddr, csum);
-		if (uh->check == 0)
-			uh->check = CSUM_MANGLED_0;
-
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
 }
 EXPORT_SYMBOL(udp6_set_csum);

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 3/8] net: enable LCO for udp_tunnel_handle_offloads() users
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
  2016-02-05 20:40 ` [PATCH net-next 1/8] net: local checksum offload for encapsulation Edward Cree
  2016-02-05 20:41 ` [PATCH net-next 2/8] net: udp: always set up for CHECKSUM_PARTIAL offload Edward Cree
@ 2016-02-05 20:41 ` Edward Cree
  2016-02-05 20:42 ` [PATCH net-next 4/8] net: vxlan: enable local checksum offload Edward Cree
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

The only protocol affected at present is Geneve.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 include/net/udp_tunnel.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index cca2ad3..734c156 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -103,7 +103,8 @@ static inline struct sk_buff *udp_tunnel_handle_offloads(struct sk_buff *skb,
 {
 	int type = udp_csum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
 
-	return iptunnel_handle_offloads(skb, udp_csum, type);
+	/* As we're a UDP tunnel, we support LCO, so don't need csum_help */
+	return iptunnel_handle_offloads(skb, false, type);
 }
 
 static inline void udp_tunnel_gro_complete(struct sk_buff *skb, int nhoff)

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 4/8] net: vxlan: enable local checksum offload
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
                   ` (2 preceding siblings ...)
  2016-02-05 20:41 ` [PATCH net-next 3/8] net: enable LCO for udp_tunnel_handle_offloads() users Edward Cree
@ 2016-02-05 20:42 ` Edward Cree
  2016-02-05 20:42 ` [PATCH net-next 5/8] fou: enable LCO in FOU and GUE Edward Cree
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 drivers/net/vxlan.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 6543918..7299e5f 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1706,10 +1706,8 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 		if (csum_start <= VXLAN_MAX_REMCSUM_START &&
 		    !(csum_start & VXLAN_RCO_SHIFT_MASK) &&
 		    (skb->csum_offset == offsetof(struct udphdr, check) ||
-		     skb->csum_offset == offsetof(struct tcphdr, check))) {
-			udp_sum = false;
+		     skb->csum_offset == offsetof(struct tcphdr, check)))
 			type |= SKB_GSO_TUNNEL_REMCSUM;
-		}
 	}
 
 	skb_scrub_packet(skb, xnet);
@@ -1731,7 +1729,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 		goto err;
 	}
 
-	skb = iptunnel_handle_offloads(skb, udp_sum, type);
+	skb = iptunnel_handle_offloads(skb, false, type);
 	if (IS_ERR(skb)) {
 		err = -EINVAL;
 		goto err;
@@ -1763,8 +1761,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
 
 	udp_tunnel6_xmit_skb(dst, sk, skb, dev, saddr, daddr, prio,
-			     ttl, src_port, dst_port,
-			     !!(vxflags & VXLAN_F_UDP_ZERO_CSUM6_TX));
+			     ttl, src_port, dst_port, !udp_sum);
 	return 0;
 err:
 	dst_release(dst);
@@ -1791,10 +1788,8 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 		if (csum_start <= VXLAN_MAX_REMCSUM_START &&
 		    !(csum_start & VXLAN_RCO_SHIFT_MASK) &&
 		    (skb->csum_offset == offsetof(struct udphdr, check) ||
-		     skb->csum_offset == offsetof(struct tcphdr, check))) {
-			udp_sum = false;
+		     skb->csum_offset == offsetof(struct tcphdr, check)))
 			type |= SKB_GSO_TUNNEL_REMCSUM;
-		}
 	}
 
 	min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
@@ -1812,7 +1807,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 	if (WARN_ON(!skb))
 		return -ENOMEM;
 
-	skb = iptunnel_handle_offloads(skb, udp_sum, type);
+	skb = iptunnel_handle_offloads(skb, false, type);
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);
 
@@ -1842,8 +1837,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
 
 	udp_tunnel_xmit_skb(rt, sk, skb, src, dst, tos, ttl, df,
-			    src_port, dst_port, xnet,
-			    !(vxflags & VXLAN_F_UDP_CSUM));
+			    src_port, dst_port, xnet, !udp_sum);
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 5/8] fou: enable LCO in FOU and GUE
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
                   ` (3 preceding siblings ...)
  2016-02-05 20:42 ` [PATCH net-next 4/8] net: vxlan: enable local checksum offload Edward Cree
@ 2016-02-05 20:42 ` Edward Cree
  2016-02-05 20:42 ` [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4 Edward Cree
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 net/ipv4/fou.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 976f0dc..dac1874 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -774,7 +774,6 @@ static void fou_build_udp(struct sk_buff *skb, struct ip_tunnel_encap *e,
 	uh->dest = e->dport;
 	uh->source = sport;
 	uh->len = htons(skb->len);
-	uh->check = 0;
 	udp_set_csum(!(e->flags & TUNNEL_ENCAP_FLAG_CSUM), skb,
 		     fl4->saddr, fl4->daddr, skb->len);
 
@@ -784,11 +783,11 @@ static void fou_build_udp(struct sk_buff *skb, struct ip_tunnel_encap *e,
 int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 		     u8 *protocol, struct flowi4 *fl4)
 {
-	bool csum = !!(e->flags & TUNNEL_ENCAP_FLAG_CSUM);
-	int type = csum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+	int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM ? SKB_GSO_UDP_TUNNEL_CSUM :
+						       SKB_GSO_UDP_TUNNEL;
 	__be16 sport;
 
-	skb = iptunnel_handle_offloads(skb, csum, type);
+	skb = iptunnel_handle_offloads(skb, false, type);
 
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);
@@ -804,8 +803,8 @@ EXPORT_SYMBOL(fou_build_header);
 int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 		     u8 *protocol, struct flowi4 *fl4)
 {
-	bool csum = !!(e->flags & TUNNEL_ENCAP_FLAG_CSUM);
-	int type = csum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+	int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM ? SKB_GSO_UDP_TUNNEL_CSUM :
+						       SKB_GSO_UDP_TUNNEL;
 	struct guehdr *guehdr;
 	size_t hdrlen, optlen = 0;
 	__be16 sport;
@@ -814,7 +813,6 @@ int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 
 	if ((e->flags & TUNNEL_ENCAP_FLAG_REMCSUM) &&
 	    skb->ip_summed == CHECKSUM_PARTIAL) {
-		csum = false;
 		optlen += GUE_PLEN_REMCSUM;
 		type |= SKB_GSO_TUNNEL_REMCSUM;
 		need_priv = true;
@@ -822,7 +820,7 @@ int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 
 	optlen += need_priv ? GUE_LEN_PRIV : 0;
 
-	skb = iptunnel_handle_offloads(skb, csum, type);
+	skb = iptunnel_handle_offloads(skb, false, type);
 
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
                   ` (4 preceding siblings ...)
  2016-02-05 20:42 ` [PATCH net-next 5/8] fou: enable LCO in FOU and GUE Edward Cree
@ 2016-02-05 20:42 ` Edward Cree
  2016-02-05 20:42 ` [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle_offloads Edward Cree
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 net/ipv4/ip_gre.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 7c51c4e..9b31532 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -440,6 +440,17 @@ drop:
 	return 0;
 }
 
+static __sum16 gre_checksum(struct sk_buff *skb)
+{
+	__wsum csum;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		csum = lco_csum(skb);
+	else
+		csum = skb_checksum(skb, 0, skb->len, 0);
+	return csum_fold(csum);
+}
+
 static void build_header(struct sk_buff *skb, int hdr_len, __be16 flags,
 			 __be16 proto, __be32 key, __be32 seq)
 {
@@ -467,8 +478,7 @@ static void build_header(struct sk_buff *skb, int hdr_len, __be16 flags,
 		    !(skb_shinfo(skb)->gso_type &
 		      (SKB_GSO_GRE | SKB_GSO_GRE_CSUM))) {
 			*ptr = 0;
-			*(__sum16 *)ptr = csum_fold(skb_checksum(skb, 0,
-								 skb->len, 0));
+			*(__sum16 *)ptr = gre_checksum(skb);
 		}
 	}
 }
@@ -493,7 +503,7 @@ static void __gre_xmit(struct sk_buff *skb, struct net_device *dev,
 static struct sk_buff *gre_handle_offloads(struct sk_buff *skb,
 					   bool csum)
 {
-	return iptunnel_handle_offloads(skb, csum,
+	return iptunnel_handle_offloads(skb, false,
 					csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE);
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle_offloads
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
                   ` (5 preceding siblings ...)
  2016-02-05 20:42 ` [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4 Edward Cree
@ 2016-02-05 20:42 ` Edward Cree
  2016-02-05 20:43 ` [PATCH net-next 8/8] Documentation/networking: add checksum-offloads.txt to explain LCO Edward Cree
  2016-02-11 12:29 ` [PATCH v4 net-next 0/8] Local Checksum Offload David Miller
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

All users now pass false, so we can remove it, and remove the code that
 was conditional upon it.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 drivers/net/vxlan.c             |  4 ++--
 include/net/ip_tunnels.h        |  3 +--
 include/net/udp_tunnel.h        |  3 +--
 net/ipv4/fou.c                  |  4 ++--
 net/ipv4/ip_gre.c               |  3 +--
 net/ipv4/ip_tunnel_core.c       | 18 ++++++------------
 net/ipv4/ipip.c                 |  2 +-
 net/ipv6/sit.c                  |  4 ++--
 net/netfilter/ipvs/ip_vs_xmit.c |  6 ++----
 9 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 7299e5f..a5c0363 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1729,7 +1729,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 		goto err;
 	}
 
-	skb = iptunnel_handle_offloads(skb, false, type);
+	skb = iptunnel_handle_offloads(skb, type);
 	if (IS_ERR(skb)) {
 		err = -EINVAL;
 		goto err;
@@ -1807,7 +1807,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 	if (WARN_ON(!skb))
 		return -ENOMEM;
 
-	skb = iptunnel_handle_offloads(skb, false, type);
+	skb = iptunnel_handle_offloads(skb, type);
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);
 
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 6db96ea..bc439f3 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -279,8 +279,7 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
 struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
 					     gfp_t flags);
 
-struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb, bool gre_csum,
-					 int gso_type_mask);
+struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb, int gso_type_mask);
 
 static inline void iptunnel_xmit_stats(struct net_device *dev, int pkt_len)
 {
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 734c156..97f5adb 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -103,8 +103,7 @@ static inline struct sk_buff *udp_tunnel_handle_offloads(struct sk_buff *skb,
 {
 	int type = udp_csum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
 
-	/* As we're a UDP tunnel, we support LCO, so don't need csum_help */
-	return iptunnel_handle_offloads(skb, false, type);
+	return iptunnel_handle_offloads(skb, type);
 }
 
 static inline void udp_tunnel_gro_complete(struct sk_buff *skb, int nhoff)
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index dac1874..88dab0c 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -787,7 +787,7 @@ int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 						       SKB_GSO_UDP_TUNNEL;
 	__be16 sport;
 
-	skb = iptunnel_handle_offloads(skb, false, type);
+	skb = iptunnel_handle_offloads(skb, type);
 
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);
@@ -820,7 +820,7 @@ int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 
 	optlen += need_priv ? GUE_LEN_PRIV : 0;
 
-	skb = iptunnel_handle_offloads(skb, false, type);
+	skb = iptunnel_handle_offloads(skb, type);
 
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 9b31532..65748db 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -503,8 +503,7 @@ static void __gre_xmit(struct sk_buff *skb, struct net_device *dev,
 static struct sk_buff *gre_handle_offloads(struct sk_buff *skb,
 					   bool csum)
 {
-	return iptunnel_handle_offloads(skb, false,
-					csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE);
+	return iptunnel_handle_offloads(skb, csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE);
 }
 
 static struct rtable *gre_get_rt(struct sk_buff *skb,
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index d74ce93..a6e58b6 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -148,7 +148,6 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
 EXPORT_SYMBOL_GPL(iptunnel_metadata_reply);
 
 struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb,
-					 bool csum_help,
 					 int gso_type_mask)
 {
 	int err;
@@ -166,18 +165,13 @@ struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb,
 		return skb;
 	}
 
-	/* If packet is not gso and we are not offloading inner checksum,
-	 * clear encapsulation flag. This allows setting CHECKSUM_PARTIAL
-	 * on the outer header without confusing devices that implement
-	 * NETIF_F_IP_CSUM with encapsulation.
-	 */
-	if (skb->ip_summed == CHECKSUM_PARTIAL && csum_help) {
-		skb->encapsulation = 0;
-		err = skb_checksum_help(skb);
-		if (unlikely(err))
-			goto error;
-	} else if (skb->ip_summed != CHECKSUM_PARTIAL) {
+	if (skb->ip_summed != CHECKSUM_PARTIAL) {
 		skb->ip_summed = CHECKSUM_NONE;
+		/* We clear encapsulation here to prevent badly-written
+		 * drivers potentially deciding to offload an inner checksum
+		 * if we set CHECKSUM_PARTIAL on the outer header.
+		 * This should go away when the drivers are all fixed.
+		 */
 		skb->encapsulation = 0;
 	}
 
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 4044da6..6ec5b42 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -219,7 +219,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(skb->protocol != htons(ETH_P_IP)))
 		goto tx_error;
 
-	skb = iptunnel_handle_offloads(skb, false, SKB_GSO_IPIP);
+	skb = iptunnel_handle_offloads(skb, SKB_GSO_IPIP);
 	if (IS_ERR(skb))
 		goto out;
 
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 2066d1c..9a6b407 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -911,7 +911,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
 		goto tx_error;
 	}
 
-	skb = iptunnel_handle_offloads(skb, false, SKB_GSO_SIT);
+	skb = iptunnel_handle_offloads(skb, SKB_GSO_SIT);
 	if (IS_ERR(skb)) {
 		ip_rt_put(rt);
 		goto out;
@@ -1000,7 +1000,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 	const struct iphdr  *tiph = &tunnel->parms.iph;
 
-	skb = iptunnel_handle_offloads(skb, false, SKB_GSO_IPIP);
+	skb = iptunnel_handle_offloads(skb, SKB_GSO_IPIP);
 	if (IS_ERR(skb))
 		goto out;
 
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 3264cb49..a3f5cd9 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -1019,8 +1019,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	if (IS_ERR(skb))
 		goto tx_error;
 
-	skb = iptunnel_handle_offloads(
-		skb, false, __tun_gso_type_mask(AF_INET, cp->af));
+	skb = iptunnel_handle_offloads(skb, __tun_gso_type_mask(AF_INET, cp->af));
 	if (IS_ERR(skb))
 		goto tx_error;
 
@@ -1112,8 +1111,7 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	if (IS_ERR(skb))
 		goto tx_error;
 
-	skb = iptunnel_handle_offloads(
-		skb, false, __tun_gso_type_mask(AF_INET6, cp->af));
+	skb = iptunnel_handle_offloads(skb, __tun_gso_type_mask(AF_INET6, cp->af));
 	if (IS_ERR(skb))
 		goto tx_error;
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 8/8] Documentation/networking: add checksum-offloads.txt to explain LCO
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
                   ` (6 preceding siblings ...)
  2016-02-05 20:42 ` [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle_offloads Edward Cree
@ 2016-02-05 20:43 ` Edward Cree
  2016-02-11 12:29 ` [PATCH v4 net-next 0/8] Local Checksum Offload David Miller
  8 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2016-02-05 20:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 Documentation/networking/00-INDEX              |   2 +
 Documentation/networking/checksum-offloads.txt | 119 +++++++++++++++++++++++++
 include/linux/skbuff.h                         |   2 +
 3 files changed, 123 insertions(+)
 create mode 100644 Documentation/networking/checksum-offloads.txt

diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
index df27a1a..415154a 100644
--- a/Documentation/networking/00-INDEX
+++ b/Documentation/networking/00-INDEX
@@ -44,6 +44,8 @@ can.txt
 	- documentation on CAN protocol family.
 cdc_mbim.txt
 	- 3G/LTE USB modem (Mobile Broadband Interface Model)
+checksum-offloads.txt
+	- Explanation of checksum offloads; LCO, RCO
 cops.txt
 	- info on the COPS LocalTalk Linux driver
 cs89x0.txt
diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
new file mode 100644
index 0000000..de2a327
--- /dev/null
+++ b/Documentation/networking/checksum-offloads.txt
@@ -0,0 +1,119 @@
+Checksum Offloads in the Linux Networking Stack
+
+
+Introduction
+============
+
+This document describes a set of techniques in the Linux networking stack
+ to take advantage of checksum offload capabilities of various NICs.
+
+The following technologies are described:
+ * TX Checksum Offload
+ * LCO: Local Checksum Offload
+ * RCO: Remote Checksum Offload
+
+Things that should be documented here but aren't yet:
+ * RX Checksum Offload
+ * CHECKSUM_UNNECESSARY conversion
+
+
+TX Checksum Offload
+===================
+
+The interface for offloading a transmit checksum to a device is explained
+ in detail in comments near the top of include/linux/skbuff.h.
+In brief, it allows to request the device fill in a single ones-complement
+ checksum defined by the sk_buff fields skb->csum_start and
+ skb->csum_offset.  The device should compute the 16-bit ones-complement
+ checksum (i.e. the 'IP-style' checksum) from csum_start to the end of the
+ packet, and fill in the result at (csum_start + csum_offset).
+Because csum_offset cannot be negative, this ensures that the previous
+ value of the checksum field is included in the checksum computation, thus
+ it can be used to supply any needed corrections to the checksum (such as
+ the sum of the pseudo-header for UDP or TCP).
+This interface only allows a single checksum to be offloaded.  Where
+ encapsulation is used, the packet may have multiple checksum fields in
+ different header layers, and the rest will have to be handled by another
+ mechanism such as LCO or RCO.
+No offloading of the IP header checksum is performed; it is always done in
+ software.  This is OK because when we build the IP header, we obviously
+ have it in cache, so summing it isn't expensive.  It's also rather short.
+The requirements for GSO are more complicated, because when segmenting an
+ encapsulated packet both the inner and outer checksums may need to be
+ edited or recomputed for each resulting segment.  See the skbuff.h comment
+ (section 'E') for more details.
+
+A driver declares its offload capabilities in netdev->hw_features; see
+ Documentation/networking/netdev-features for more.  Note that a device
+ which only advertises NETIF_F_IP[V6]_CSUM must still obey the csum_start
+ and csum_offset given in the SKB; if it tries to deduce these itself in
+ hardware (as some NICs do) the driver should check that the values in the
+ SKB match those which the hardware will deduce, and if not, fall back to
+ checksumming in software instead (with skb_checksum_help or one of the
+ skb_csum_off_chk* functions as mentioned in include/linux/skbuff.h).  This
+ is a pain, but that's what you get when hardware tries to be clever.
+
+The stack should, for the most part, assume that checksum offload is
+ supported by the underlying device.  The only place that should check is
+ validate_xmit_skb(), and the functions it calls directly or indirectly.
+ That function compares the offload features requested by the SKB (which
+ may include other offloads besides TX Checksum Offload) and, if they are
+ not supported or enabled on the device (determined by netdev->features),
+ performs the corresponding offload in software.  In the case of TX
+ Checksum Offload, that means calling skb_checksum_help(skb).
+
+
+LCO: Local Checksum Offload
+===========================
+
+LCO is a technique for efficiently computing the outer checksum of an
+ encapsulated datagram when the inner checksum is due to be offloaded.
+The ones-complement sum of a correctly checksummed TCP or UDP packet is
+ equal to the sum of the pseudo header, because everything else gets
+ 'cancelled out' by the checksum field.  This is because the sum was
+ complemented before being written to the checksum field.
+More generally, this holds in any case where the 'IP-style' ones complement
+ checksum is used, and thus any checksum that TX Checksum Offload supports.
+That is, if we have set up TX Checksum Offload with a start/offset pair, we
+ know that _after the device has filled in that checksum_, the ones
+ complement sum from csum_start to the end of the packet will be equal to
+ _whatever value we put in the checksum field beforehand_.  This allows us
+ to compute the outer checksum without looking at the payload: we simply
+ stop summing when we get to csum_start, then add the 16-bit word at
+ (csum_start + csum_offset).
+Then, when the true inner checksum is filled in (either by hardware or by
+ skb_checksum_help()), the outer checksum will become correct by virtue of
+ the arithmetic.
+
+LCO is performed by the stack when constructing an outer UDP header for an
+ encapsulation such as VXLAN or GENEVE, in udp_set_csum().  Similarly for
+ the IPv6 equivalents, in udp6_set_csum().
+It is also performed when constructing an IPv4 GRE header, in
+ net/ipv4/ip_gre.c:build_header().  It is *not* currently performed when
+ constructing an IPv6 GRE header; the GRE checksum is computed over the
+ whole packet in net/ipv6/ip6_gre.c:ip6gre_xmit2(), but it should be
+ possible to use LCO here as IPv6 GRE still uses an IP-style checksum.
+All of the LCO implementations use a helper function lco_csum(), in
+ include/linux/skbuff.h.
+
+LCO can safely be used for nested encapsulations; in this case, the outer
+ encapsulation layer will sum over both its own header and the 'middle'
+ header.  This does mean that the 'middle' header will get summed multiple
+ times, but there doesn't seem to be a way to avoid that without incurring
+ bigger costs (e.g. in SKB bloat).
+
+
+RCO: Remote Checksum Offload
+============================
+
+RCO is a technique for eliding the inner checksum of an encapsulated
+ datagram, allowing the outer checksum to be offloaded.  It does, however,
+ involve a change to the encapsulation protocols, which the receiver must
+ also support.  For this reason, it is disabled by default.
+RCO is detailed in the following Internet-Drafts:
+https://tools.ietf.org/html/draft-herbert-remotecsumoffload-00
+https://tools.ietf.org/html/draft-herbert-vxlan-rco-00
+In Linux, RCO is implemented individually in each encapsulation protocol,
+ and most tunnel types have flags controlling its use.  For instance, VXLAN
+ has the flag VXLAN_F_REMCSUM_TX (per struct vxlan_rdst) to indicate that
+ RCO should be used when transmitting to a given remote destination.
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3e9eb52..61b8cef 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3686,6 +3686,8 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
 /* Local Checksum Offload.
  * Compute outer checksum based on the assumption that the
  * inner checksum will be offloaded later.
+ * See Documentation/networking/checksum-offloads.txt for
+ * explanation of how this works.
  * Fill in outer checksum adjustment (e.g. with sum of outer
  * pseudo-header) before calling.
  * Also ensure that inner checksum is in linear data area.

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 net-next 0/8] Local Checksum Offload
  2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
                   ` (7 preceding siblings ...)
  2016-02-05 20:43 ` [PATCH net-next 8/8] Documentation/networking: add checksum-offloads.txt to explain LCO Edward Cree
@ 2016-02-11 12:29 ` David Miller
  8 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2016-02-11 12:29 UTC (permalink / raw)
  To: ecree; +Cc: netdev, linux-net-drivers, tom, alexander.duyck

From: Edward Cree <ecree@solarflare.com>
Date: Fri, 5 Feb 2016 20:39:34 +0000

> Tested with VxLAN, GRE and FOU-IPIP tunnels.  Not tested with GENEVE,
>  because iproute2 doesn't support enabling checksums on GENEVE tunnels.
> Also tested VxLAN with IPv6 (as both inner and outer protocol).

This gets rejects when I try to apply it, please respin.

Thanks!

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-02-11 12:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-05 20:39 [PATCH v4 net-next 0/8] Local Checksum Offload Edward Cree
2016-02-05 20:40 ` [PATCH net-next 1/8] net: local checksum offload for encapsulation Edward Cree
2016-02-05 20:41 ` [PATCH net-next 2/8] net: udp: always set up for CHECKSUM_PARTIAL offload Edward Cree
2016-02-05 20:41 ` [PATCH net-next 3/8] net: enable LCO for udp_tunnel_handle_offloads() users Edward Cree
2016-02-05 20:42 ` [PATCH net-next 4/8] net: vxlan: enable local checksum offload Edward Cree
2016-02-05 20:42 ` [PATCH net-next 5/8] fou: enable LCO in FOU and GUE Edward Cree
2016-02-05 20:42 ` [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4 Edward Cree
2016-02-05 20:42 ` [PATCH net-next 7/8] net: ip_tunnel: remove 'csum_help' argument to iptunnel_handle_offloads Edward Cree
2016-02-05 20:43 ` [PATCH net-next 8/8] Documentation/networking: add checksum-offloads.txt to explain LCO Edward Cree
2016-02-11 12:29 ` [PATCH v4 net-next 0/8] Local Checksum Offload David Miller
  -- strict thread matches above, loose matches on Subject: below --
2016-01-08 19:44 [PATCH v3 " Edward Cree
2016-01-08 19:47 ` [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4 Edward Cree
2016-01-11 10:09   ` David Laight
2016-01-11 13:21     ` Edward Cree
2016-01-11 18:39       ` Alexander Duyck
2016-01-11 19:02         ` Edward Cree
2016-01-20 19:11         ` Rustad, Mark D
2016-01-20 19:35           ` Alexander Duyck
2016-01-20 19:58             ` Tom Herbert
2016-01-20 21:13               ` Alexander Duyck
2016-01-20 23:34                 ` Rustad, Mark D

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.