On Sun, Dec 21, 2014 at 2:56 PM, Jesse Gross <je...@nicira.com> wrote:
> On Fri, Dec 19, 2014 at 7:24 PM, Pravin B Shelar <pshe...@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 <pshe...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
