[ovs-dev] [PATCH 11/12] datapath: compat: Add support for IPv6 UDP tunnel segmentation.

[pravin shelar]

On Wed, May 25, 2016 at 4:36 PM, Jesse Gross <jesse at kernel.org> wrote:
> On Wed, May 4, 2016 at 4:35 PM, Pravin B Shelar <pshelar at ovn.org> wrote:
>> diff --git a/datapath/linux/compat/include/net/ipv6.h b/datapath/linux/compat/include/net/ipv6.h
>> index 5cc5b6e..6faafb0 100644
>> --- a/datapath/linux/compat/include/net/ipv6.h
>> +++ b/datapath/linux/compat/include/net/ipv6.h
>> @@ -81,4 +81,9 @@ static inline __be32 ip6_make_flowinfo(unsigned int tclass, __be32 flowlabel)
>>         return htonl(tclass << IPV6_TCLASS_SHIFT) | flowlabel;
>>  }
>>
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
>> +#define ip6_local_out rpl_ip6_local_out
>> +int rpl_ip6_local_out(struct sk_buff *skb);
>> +#endif
>> +
>>  #endif
>
> The IPv4 version is defined in gso.h and the implementations of both
> functions are in gso.c. Should we move both definitions to one header
> or another?
>
I will move ipv4 function declarations to ipv4 header. I think we can
keep all definitions in gso.c.

>> +void ovs_udp_csum_gso(struct sk_buff *skb)
>> +{
>> +       int udp_offset = skb_transport_offset(skb);
>> +
>> +       ovs_udp_gso(skb);
>> +
>> +       if (!OVS_GSO_CB(skb)->ipv6) {
>> +               struct iphdr *iph = ip_hdr(skb);
>> +
>> +               /* csum segment if tunnel sets skb with csum. The cleanest way
>> +                * to do this just to set it up from scratch. */
>> +               skb->ip_summed = CHECKSUM_NONE;
>> +               udp_set_csum(false, skb, iph->saddr, iph->daddr,
>> +                               skb->len - udp_offset);
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +       } else {
>> +               struct ipv6hdr *ip6h;
>> +
>> +               ip6h = ipv6_hdr(skb);
>> +               udp6_set_csum(false, skb, &ip6h->saddr, &ip6h->daddr,
>> +                             skb->len - udp_offset);
>
> It seems like there is an inconsistency between IPv4 and IPv6 when it
> comes to setting skb->ip_summed = CHECKSUM_NONE. Presumably, this
> isn't really necessary since we are passing in 0 for feature flags to
> skb_gso_segment().

ok. I will fix the inconsistency.