On Tue, Jul 26, 2016 at 12:30 PM, pravin shelar <pshe...@ovn.org> wrote: > On Tue, Jul 26, 2016 at 11:14 AM, Jesse Gross <je...@kernel.org> wrote: >> On Tue, Jul 26, 2016 at 10:56 AM, pravin shelar <pshe...@ovn.org> wrote: >>> On Tue, Jul 26, 2016 at 10:06 AM, Jesse Gross <je...@kernel.org> wrote: >>>> On Mon, Jul 25, 2016 at 5:49 PM, Pravin B Shelar <pshe...@ovn.org> wrote: >>>>> OVS compat layer can handle tunnel GSO packets. but it does >>>>> keep skb encapsulation on for packet handled in GSO. This can >>>>> confuse some NIC drivers. I have seen this issue on intel devices: >>>>> >>>>> i40e 0000:42:00.0: TX driver issue detected, PF reset issued >>>>> >>>>> Following patch resets this bit in case compat layer handles the packet. >>>>> >>>>> VMware-BZ: 1698877 >>>>> Signed-off-by: Pravin B Shelar <pshe...@ovn.org> >>>> >>>> In upstream, this is done as part of the GSO code (for example, in >>>> __skb_udp_tunnel_segment()) so that probably makes more sense and is >>>> safer if this is GSO specific. There is already code in >>>> ovs_iptunnel_handle_offloads() that will clear the encapsulation bit >>>> in the case of checksum offload on the outer header. >>>> >>> ovs_iptunnel_handle_offloads() is not equivalent to >>> skb_udp_tunnel_segment(). If handle-offload clear the bit it would not >>> be possible to check the encapsulation bit after handle-offload call >>> in tunnel implementation. At this point this bit is not checked, so it >>> is not issue. But that would be different behavior compared to >>> upstream. >> >> I was actually referring to existing behavior in >> ovs_iptunnel_handle_offlads(). Here is the code that I was talking >> about: >> >> #if LINUX_VERSION_CODE >= KERNEL_VERSION(3,8,0) >> /* If packet is not gso and we are resolving any partial 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; >> #endif >> >> In the case of outer checksums being enabled and offloaded and no GSO, >> we will have cleared skb->encapsulation bit already, so there should >> be no confusion for the NIC driver. As a result, this seems like a >> GSO-only problem and we can just mirror what upstream does and clear >> the bit in the GSO code. I'm a little bit nervous about >> indiscriminately clearing it in all cases, since it seems like it is >> possible that we will accidentally do it in some case where we are >> trying to use more of the network stack. >> > > So you are fine with clearing the bit in ip_local_out() but only for > GSO packets.
Effectively, yes, just in a different spot. I think inside tnl_skb_gso_segment() would make the most sense. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
