On Fri, May 20, 2016 at 5:53 PM, Jesse Gross <je...@kernel.org> wrote:
> On Wed, May 4, 2016 at 4:35 PM, Pravin B Shelar <pshe...@ovn.org> wrote:
>> Following patch backports updated iptunnel pull function.
>> Also brings in following upstream fix:
>>
>> commit a09a4c8dd1ec7f830e1fb9e59eb72bddc965d168
>> Author: Jesse Gross <je...@kernel.org>
>> Date: Sat Mar 19 09:32:02 2016 -0700
>>
>> tunnels: Remove encapsulation offloads on decap.
>>
>> If a packet is either locally encapsulated or processed through GRO
>> it is marked with the offloads that it requires. However, when it is
>> decapsulated these tunnel offload indications are not removed. This
>> means that if we receive an encapsulated TCP packet, aggregate it with
>> GRO, decapsulate, and retransmit the resulting frame on a NIC that does
>> not support encapsulation, we won't be able to take advantage of hardware
>> offloads even though it is just a simple TCP packet at this point.
>>
>> This fixes the problem by stripping off encapsulation offload indications
>> when packets are decapsulated.
>>
>> The performance impacts of this bug are significant. In a test where a
>> Geneve encapsulated TCP stream is sent to a hypervisor, GRO'ed,
>> decapsulated,
>> and bridged to a VM performance is improved by 60% (5Gbps->8Gbps) as a
>> result of avoiding unnecessary segmentation at the VM tap interface.
>>
>> Reported-by: Ramu Ramamurthy <srama...@linux.vnet.ibm.com>
>> Fixes: 68c33163 ("v4 GRE: Add TCP segmentation offload for GRE")
>> Signed-off-by: Jesse Gross <je...@kernel.org>
>> Signed-off-by: David S. Miller <da...@davemloft.net>
>>
>> Signed-off-by: Pravin B Shelar <pshe...@ovn.org>
>
> I think in order for this bug fix to be fully useful, we'll need to
> bump up the range of kernels where we backport tunnel implementations
> to < 4.7. That's kind of annoying but it's probably important since it
> comes up in common OVS use cases.
>
ok.
>> diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
>> index f38d4a3..0f6bda0 100644
>> --- a/datapath/linux/compat/geneve.c
>> +++ b/datapath/linux/compat/geneve.c
>> @@ -247,7 +247,7 @@ static int geneve_udp_encap_recv(struct sock *sk, struct
>> sk_buff *skb)
>>
>> opts_len = geneveh->opt_len * 4;
>> if (iptunnel_pull_header(skb, GENEVE_BASE_HLEN + opts_len,
>> - htons(ETH_P_TEB)))
>> + htons(ETH_P_TEB), false))
>
> Even though these devices are created by OVS, I think it's possible
> that someone could move them to another namespace - the same as with
> the upstream version.
>
Later patch does bring in the change. To check namespace here in this
patch I will need to pull in lot of unrelated code. so I will keep it
as it is in this patch.
>> static struct gre_cisco_protocol __rcu *gre_cisco_proto;
>> diff --git a/datapath/linux/compat/include/linux/skbuff.h
>> b/datapath/linux/compat/include/linux/skbuff.h
>> index 376dfda..03ea707 100644
>> --- a/datapath/linux/compat/include/linux/skbuff.h
>> +++ b/datapath/linux/compat/include/linux/skbuff.h
>> +#ifndef HAVE_SKB_CLEAR_HASH_IF_NOT_L4
>> +static inline void skb_clear_hash_if_not_l4(struct sk_buff *skb)
>> +{
>> + skb_clear_hash(skb);
>> +}
>
> 3.10 actually has skb->l4_rxhash, so we should be able to implement
> this fully even on kernels that don't otherwise support it.
>
ok.
>> diff --git a/datapath/linux/compat/ip_tunnels_core.c
>> b/datapath/linux/compat/ip_tunnels_core.c
>> index 0858d02..2d91abb 100644
>> --- a/datapath/linux/compat/ip_tunnels_core.c
>> +++ b/datapath/linux/compat/ip_tunnels_core.c
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,7,0)
> [...]
>> +static inline int iptunnel_pull_offloads(struct sk_buff *skb)
>
> Should this go in the header file to be consistent with upstream?
sure, I moved it to the header file.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
