[dpdk-dev] [PATCH] KNI: use a memzone pool for KNI alloc/release

[Marc Sune]

Hi Helin,

inline and snipped. Let me know after reading this mail if you believe I 
can already submit the v2 with the changes you suggested.

On 09/10/14 10:57, Zhang, Helin wrote:
> [snip]
>>>> I don't think the approach of pre-allocating on the first
>>>> rte_kni_alloc() would work (I already discarded this approach before
>>>> implementing the patch), because this would imply we need a define of
>>>> #define MAX_KNI_IFACES during compilation time of DPDK, and the
>>>> pre-allocation is highly dependent on the amount of hugepages memory
>>>> you have and the usage of the KNI interfaces the applications wants to do.
>>>> We can easily end up with DPDK users having to tweak the default
>>>> MAX_KNI_IFACES before compiling DPDK every time, which is definetely
>>>> not desirable IMHO.
>>> Your idea is good! My point is it possible to avoid adding new
>>> interface, then no changes are needed in user app.
>> I see the current approach the most clean and comprehensive (from the
>> perspective of the user of the library) approach. Do you have any other
>> proposal? I am open to discuss and eventually implement it if it turns out to be
>> better.
> How about add a new compile config item in config files? I still think we should avoid adding more interfaces if possible. :)

In my original answer to your comment here cited starting by "I don't 
think the approach of pre-allocating on the first rte_kni_alloc()..." I 
explain why I think this is not a good idea.

A config.g #define approach would be highly dependent on hugepages 
memory size and the usage the applications wants to do with KNI 
interfaces. Specially due to the former, I don't think it is a good 
idea. DPDK doesn't force any user to edit manually the config.h AFAIK, 
unless you want to do some performance optimizations or debug. And I 
think it is a good approach and I would like to keep it and not break it 
with this patch

Any parameter that depends on DPDK applications so far, so really out of 
the scope of DPDK itself (like the size of the pool parameter is), is 
handled via an API call. So I see rte_kni_init() as the natural way to 
do so, specially by the fact that rte_kni_close() API call already exists.

>>>> For rte_kni_close(), the pool is static (incl. the slot struct), and
>>>> the memzones cannot be unreserved, hence there is nothing AFAIU to
>>>> de-initialize; what do you mean specifically?
>>> You can see that rte_kni_close() will be called in XEN (#ifdef
>>> RTE_LIBRTE_XEN_DOM0), XEN support is different from standard Linux
>> support.
>>
>> OK it is called, but what is the (extra) state that I should de-initialize that is
>> coming from this patch? I cannot see any state I've added I have to de-initialize
>> here.
> Just suggest you think about that. maybe nothing needs to be added there. :)

I will definitely double-check before submitting v2.

Thanks for the suggestions
Marc