[ovs-dev] [PATCH 03/12] datapath: compat: Introduce dst-cache for tunnels

[Jesse Gross]

On Tue, May 24, 2016 at 12:26 PM, pravin shelar <pshelar at ovn.org> wrote:
> On Mon, May 23, 2016 at 11:09 AM, Jesse Gross <jesse at kernel.org> wrote:
>> On Sun, May 22, 2016 at 9:36 PM, pravin shelar <pshelar at ovn.org> wrote:
>>> On Fri, May 20, 2016 at 6:46 PM, Jesse Gross <jesse at kernel.org> wrote:
>>>> On Wed, May 4, 2016 at 4:35 PM, Pravin B Shelar <pshelar at ovn.org> wrote:
>>>>> diff --git a/datapath/linux/compat/include/net/dst_cache.h b/datapath/linux/compat/include/net/dst_cache.h
>>>>> new file mode 100644
>>>>> index 0000000..5904f2c
>>>>> --- /dev/null
>>>>> +++ b/datapath/linux/compat/include/net/dst_cache.h
>>>> [...]
>>>>> +#define ovs_dst_cache_get dst_cache_get
>>>>> +struct dst_entry *ovs_dst_cache_get(struct dst_cache *dst_cache);
>>>>
>>>> I believe these are all straight backports of what is upstream. Given
>>>> that, maybe rpl_ is more appropriate than ovs_? Usually, the ovs_
>>>> functions are ones that have required different functionality in some
>>>> way.
>>>>
>>> Since whole dst-cache module is backport, I used ovs_ prefix. But I am
>>> fine with rpm_ prefix also.
>>
>> I guess rpl_ still seems a little more appropriate but there isn't
>> really that much of a distinction. (I guess we could just settle on
>> one or the other for all functions going forward.)
>>
>>>>> diff --git a/datapath/linux/compat/utils.c b/datapath/linux/compat/utils.c
>>>>> index 0ee6e80..7008ecf 100644
>>>>> --- a/datapath/linux/compat/utils.c
>>>>> +++ b/datapath/linux/compat/utils.c
>>>>> @@ -65,3 +65,28 @@ bool rpl___net_get_random_once(void *buf, int nbytes, bool *done,
>>>>>  EXPORT_SYMBOL_GPL(rpl___net_get_random_once);
>>>>>
>>>>>  #endif
>>>>> +
>>>>> +#ifdef NEED_ALLOC_PERCPU_GFP
>>>>> +void __percpu *__alloc_percpu_gfp(size_t size, size_t align, gfp_t gfp)
>>>>
>>>> It looks like on 4.6 at least, this is just a call to pcpu_alloc(),
>>>> which exists back to 3.10. Is there a reason that we need something
>>>> more?
>>>
>>> I am not sure which 4.6 are looking at; But the release kernel used gfp version.
>>> http://lxr.free-electrons.com/source/net/core/dst_cache.c#L144
>>
>> I actually meant the implementation of __alloc_precpu_gfp() rather
>> than the caller. It seems like it is just a very simple call to
>> pcpu_alloc():
>> http://lxr.free-electrons.com/source/mm/percpu.c#L1065
>
> pcpu_alloc() does not take gfp flags, so it can not allocate the
> object in atomic context. Therefore it can not be just call to older
> pcpu_alloc().

OK, I see.