| Message ID | 54E5ED9D.8080206@mellanox.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Feb 19, 2015 at 6:05 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote: > Hi, > > It seems that the OVS GRE code lacks handling of offloads (e.g to come into > play with NICs that support NVGRE). > > I assume we need to place a call to iptunnel_handle_offloads before invoking > iptunnel_xmit, agree? so ~the quick patch below should do the work? I wasn't > sure how to set the value of the 2nd param for iptunnel_handle_offloads(). I don't think that this is the issue. __build_header() calls gre_handle_offloads() which should do all of this work already. I did notice that skb_set_inner_protocol() is not being called because it is in the wrong place in the GRE encapsulation code. It is currently in ip_gre.c():__gre_xmit(), if that line was moved to gre_demux.c:gre_build_header() then it would be used by all callers. One other thing that is potentially an issue for offloads is that all of the encapsulations call vlan_hwaccel_push_inside() after the respective handle_offloads code. This doesn't seem right as it could affect the layer pointers, although it only likely matters in a minority of cases where the inner MAC pointer is used. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 19, 2015 at 7:01 PM, Jesse Gross <jesse@nicira.com> wrote: > On Thu, Feb 19, 2015 at 6:05 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote: >> It seems that the OVS GRE code lacks handling of offloads (e.g to come into >> play with NICs that support NVGRE). >> >> I assume we need to place a call to iptunnel_handle_offloads before invoking >> iptunnel_xmit, agree? so ~the quick patch below should do the work? I wasn't >> sure how to set the value of the 2nd param for iptunnel_handle_offloads(). > > I don't think that this is the issue. __build_header() calls > gre_handle_offloads() which should do all of this work already. yep... gre_handle_offloads() is called, so something else goes wrong here. > I did notice that skb_set_inner_protocol() is not being called because > it is in the wrong place in the GRE encapsulation code. It is > currently in ip_gre.c():__gre_xmit(), if that line was moved to > gre_demux.c:gre_build_header() then it would be used by all callers. Oh, thanks, I'll add that and see if / how much does it help. > One other thing that is potentially an issue for offloads is that all > of the encapsulations call vlan_hwaccel_push_inside() after the > respective handle_offloads code. This doesn't seem right as it could > affect the layer pointers, although it only likely matters in a > minority of cases where the inner MAC pointer is used. mmm, I don't think we're failing on that now, but can look this up too. Did you ever tested Linux OVS/NGRE with a NIC that can do offloads for this type of traffic? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 19, 2015 at 11:54 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote: > On Thu, Feb 19, 2015 at 7:01 PM, Jesse Gross <jesse@nicira.com> wrote: >> On Thu, Feb 19, 2015 at 6:05 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote: > >>> It seems that the OVS GRE code lacks handling of offloads (e.g to come into >>> play with NICs that support NVGRE). >>> >>> I assume we need to place a call to iptunnel_handle_offloads before invoking >>> iptunnel_xmit, agree? so ~the quick patch below should do the work? I wasn't >>> sure how to set the value of the 2nd param for iptunnel_handle_offloads(). >> >> I don't think that this is the issue. __build_header() calls >> gre_handle_offloads() which should do all of this work already. > > yep... gre_handle_offloads() is called, so something else goes wrong here. > >> I did notice that skb_set_inner_protocol() is not being called because >> it is in the wrong place in the GRE encapsulation code. It is >> currently in ip_gre.c():__gre_xmit(), if that line was moved to >> gre_demux.c:gre_build_header() then it would be used by all callers. > > Oh, thanks, I'll add that and see if / how much does it help. > >> One other thing that is potentially an issue for offloads is that all >> of the encapsulations call vlan_hwaccel_push_inside() after the >> respective handle_offloads code. This doesn't seem right as it could >> affect the layer pointers, although it only likely matters in a >> minority of cases where the inner MAC pointer is used. > > mmm, I don't think we're failing on that now, but can look this up too. > > > Did you ever tested Linux OVS/NGRE with a NIC that can do offloads for > this type of traffic? I have not personally tried it. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c index f17ac96..524825f 100644 --- a/net/openvswitch/vport-gre.c +++ b/net/openvswitch/vport-gre.c @@ -187,6 +187,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb) htons(IP_DF) : 0; skb->ignore_df = 1; + + skb = iptunnel_handle_offloads(skb, false, SKB_GSO_GRE); + if (IS_ERR(skb)) + goto err_free_rt; return iptunnel_xmit(skb->sk, rt, skb, fl.saddr,
Hi, It seems that the OVS GRE code lacks handling of offloads (e.g to come into play with NICs that support NVGRE). I assume we need to place a call to iptunnel_handle_offloads before invoking iptunnel_xmit, agree? so ~the quick patch below should do the work? I wasn't sure how to set the value of the 2nd param for iptunnel_handle_offloads(). Or. tun_key->ipv4_dst, IPPROTO_GRE, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html