On Fri, Jul 8, 2016 at 1:49 PM, Jesse Gross <je...@kernel.org> wrote:
> On Fri, Jul 8, 2016 at 9:19 AM, Pravin B Shelar <pshe...@ovn.org> wrote:
>> diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
>> index 0f2b09a..5441973 100644
>> --- a/datapath/linux/compat/gso.h
>> +++ b/datapath/linux/compat/gso.h
>> @@ -23,6 +23,11 @@ struct ovs_gso_cb {
>> #ifndef HAVE_INNER_NETWORK_HEADER
>> unsigned int inner_network_header;
>> #endif
>> +#ifndef HAVE_NDO_FILL_METADATA_DST
>> + /* Keep original tunnel info during userspace action execution. */
>> + struct metadata_dst *tmp;
>
> Can you give this a little bit more descriptive name? I always find
> 'tmp' a little too vague.
>
>> +#define SKB_SETUP_FILL_METADATA_DST(skb) { \
>> + struct metadata_dst *new_md_dst; \
>> + struct metadata_dst *md_dst; \
>> + int md_size; \
>> + \
>> + SKB_RESTORE_FILL_METADATA_DST(skb); \
>> + new_md_dst = kmalloc(sizeof(struct metadata_dst) + 256, GFP_ATOMIC);
>> \
>
> As I mentioned in the other patch, I think it would be good to zero
> out this newly allocated memory to avoid any possible subtle problems.
>
> But otherwise I am happy with this:
> Acked-by: Jesse Gross <je...@kernel.org>
>
I noticed a missing NULL check in this patch. So I added null check
here and added memset to metadata_dst_alloc(). With these two changes
I pushed patch series to master.
Thanks for all review.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
