TX offloads for NVGRE (OVS GRE with inner protocol being TEB)

TX offloads for NVGRE (OVS GRE with inner protocol being TEB)

[Or Gerlitz]

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.

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,
                              tun_key->ipv4_dst, IPPROTO_GRE,

Re: TX offloads for NVGRE (OVS GRE with inner protocol being TEB)

[Jesse Gross]

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.

Re: TX offloads for NVGRE (OVS GRE with inner protocol being TEB)

[Or Gerlitz]

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?

Re: TX offloads for NVGRE (OVS GRE with inner protocol being TEB)

[Jesse Gross]

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.