[PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4

[Edward Cree]

Signed-off-by: Edward Cree 
---
 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);
 }
 

Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4

[Tom Herbert]

On Wed, Jan 20, 2016 at 11:35 AM, Alexander Duyck
 wrote:
> On Wed, Jan 20, 2016 at 11:11 AM, Rustad, Mark D
>  wrote:
>> Alexander Duyck  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
> 0x 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 0x as the final result in the case of a
> correct checksum.
>
0x is mathematically equivalent to 0x0 for checksum. I would
rather always flip 0 to 0x in LCO rather than adding an explicit
indication (i.e. another flag) in SKB that it has a UDP checksum.

Tom

> - Alex

Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4

[Alexander Duyck]

On Wed, Jan 20, 2016 at 11:11 AM, Rustad, Mark D
 wrote:
> Alexander Duyck  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
0x 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 0x as the final result in the case of a
correct checksum.

- Alex

Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4

[Rustad, Mark D]

Alexander Duyck  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.


--
Mark Rustad, Networking Division, Intel Corporation


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4

[Alexander Duyck]

On Wed, Jan 20, 2016 at 11:58 AM, Tom Herbert  wrote:
> On Wed, Jan 20, 2016 at 11:35 AM, Alexander Duyck
>  wrote:
>> On Wed, Jan 20, 2016 at 11:11 AM, Rustad, Mark D
>>  wrote:
>>> Alexander Duyck  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
>> 0x 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 0x as the final result in the case of a
>> correct checksum.
>>
> 0x is mathematically equivalent to 0x0 for checksum. I would
> rather always flip 0 to 0x 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

Re: [PATCH net-next 6/8] net: gre: Implement LCO for GRE over IPv4

[Rustad, Mark D]

Alexander Duyck  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.


--
Mark Rustad, Networking Division, Intel Corporation


signature.asc
Description: Message signed with OpenPGP using GPGMail