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.
> 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.
> 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.
> 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?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
