On Tue, Jul 5, 2016 at 4:40 PM, Jesse Gross <je...@kernel.org> wrote:
> On Fri, Jul 1, 2016 at 5:58 PM, Pravin B Shelar <pshe...@ovn.org> wrote:
>> diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
>> index db1c713..a7229c8 100644
>> --- a/datapath/linux/compat/geneve.c
>> +++ b/datapath/linux/compat/geneve.c
>> @@ -578,6 +574,7 @@ static int geneve_build_skb(struct rtable *rt, struct
>> sk_buff *skb,
>> return 0;
>>
>> free_rt:
>> + kfree_skb(skb);
>> ip_rt_put(rt);
>> return err;
>> }
>
> It looks like this replaces the upstream tx_error changes in this
> patch with the kfree_skb() here. Can we follow the original pattern?
>
ok.
>> diff --git a/datapath/linux/compat/include/net/udp_tunnel.h
>> b/datapath/linux/compat/include/net/udp_tunnel.h
>> index 85aed98..65adc02 100644
>> --- a/datapath/linux/compat/include/net/udp_tunnel.h
>> +++ b/datapath/linux/compat/include/net/udp_tunnel.h
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,7,0)
>> +/* this is to handle the return type change in handle-offload
>> + * functions.
>> + */
>> +static inline int
>> +rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum,
>> + bool is_vxlan)
>> +{
>> + if (skb_is_gso(skb) && skb_is_encapsulated(skb)) {
>> + kfree_skb(skb);
>> + return -ENOSYS;
>> + }
>> + return udp_tunnel_handle_offloads(skb, udp_csum);
>> +}
>> +
> [...]
>> +static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb,
>> + bool udp_csum,
>> + bool is_vxlan)
>> {
>> + int type = 0;
>> +
>> void (*fix_segment)(struct sk_buff *);
>>
>> if (skb_is_gso(skb) && skb_is_encapsulated(skb)) {
>> kfree_skb(skb);
>> - return ERR_PTR(-ENOSYS);
>> + return -ENOSYS;
>> }
>
> In both of these cases, I don't think that we should free the skb on
> error any more.
>
right.
>> diff --git a/datapath/linux/compat/utils.c b/datapath/linux/compat/utils.c
>> index 7113e09..7008ecf 100644
>> --- a/datapath/linux/compat/utils.c
>> +++ b/datapath/linux/compat/utils.c
>> @@ -72,9 +72,6 @@ void __percpu *__alloc_percpu_gfp(size_t size, size_t
>> align, gfp_t gfp)
>> void __percpu *p;
>> int i;
>>
>> - /* older kernel do not allow all GFP flags, specifically atomic
>> - * allocation.
>> - */
>> if (gfp & ~(GFP_KERNEL | __GFP_ZERO))
>> return NULL;
>> p = __alloc_percpu(size, align);
>
> It looks like this should be part of another patch?
yes, removed it.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
