[ovs-dev] [PATCH 1/2] datapath: compat: introduce ovs_iptunnel_handle_offloads()

[Pravin Shelar]

On Sun, Dec 21, 2014 at 2:56 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Fri, Dec 19, 2014 at 7:24 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> handle offload code is replicated for different tunneling protocols
>> define compat function to simplify the code.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>
> I got some compiler errors with this patch (on 3.13):
>   CC [M]  /home/jesse/openvswitch/datapath/linux/vxlan.o
> /home/jesse/openvswitch/datapath/linux/ip_tunnels_core.c: In function
> ‘ovs_iptunnel_handle_offloads’:
> /home/jesse/openvswitch/datapath/linux/ip_tunnels_core.c:155:3: error:
> implicit declaration of function ‘OVS_GSO_CB’
> [-Werror=implicit-function-declaration]
>    OVS_GSO_CB(skb)->fix_segment = fix_segment;
>    ^
> /home/jesse/openvswitch/datapath/linux/ip_tunnels_core.c:155:18:
> error: invalid type argument of ‘->’ (have ‘int’)
>    OVS_GSO_CB(skb)->fix_segment = fix_segment;
>                   ^
> cc1: some warnings being treated as errors
>
ok.

>> diff --git a/datapath/linux/compat/ip_tunnels_core.c b/datapath/linux/compat/ip_tunnels_core.c
>> index e71ba4e..7606ad6 100644
>> --- a/datapath/linux/compat/ip_tunnels_core.c
>> +++ b/datapath/linux/compat/ip_tunnels_core.c
>> +struct sk_buff *ovs_iptunnel_handle_offloads(struct sk_buff *skb,
>> +                                             bool csum_help,
>> +                                            void (*fix_segment)(struct sk_buff *))
>> +{
>> +       int err;
>> +
>> +       if (skb_is_encapsulated(skb)) {
>> +               err = -ENOSYS;
>> +               goto error;
>> +       }
>> +
>> +       /* XXX: synchronize inner header for compat and non compat code so that
>> +        * we can do it here.
>> +        */
>> +
>> +       /* skb_reset_inner_headers(skb); */
>
> What is the issue with resetting the inner headers here?
>
I wrote above comment, I guess it is not very clear.

Problem is current code does not set inner-header in handle offload
call, some times handle_offload() is called after pushing outer
header. I am planning on sending another patch to fix this issue.


>> +       /* OVS compat code does not maintain encapsulation bit.
>> +        * skb->encapsulation = 1; */
>> +
>> +       if (skb_is_gso(skb)) {
>> +               err = skb_unclone(skb, GFP_ATOMIC);
>> +               if (unlikely(err))
>> +                       goto error;
>
> Is it necessary to unclone here? This looks like a new addition.

its not necessary. I will remove it.