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

[Marc Sune <marc.sune@bisdn.de>]

Hi Helin,

On 10/10/14 07:25, Zhang, Helin wrote:
> Hi Marc
>
> More comments added.
>
>> [snip]
>>>>>> 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.
> I understood your concern. It is not bad of adding a config item in config files
> (config/config_linux), as it already has a lot of compile time configurations in them.
> For a specific platform, the maximum number of KNI interfaces should be fixed,
> and no need to be changed frequently.
rte_kni_init() should be staying. Actually the asymmetry of the API 
nowadays (no rte_kni_init, because fd is created during first alloc but 
an rte_kni_close) looks weird to me. Just an aside question, not related 
to this patch, why was the KNI fd not closed in the last rte_kni_release 
to be consistent?
>
>> 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
> No need to edit config.h, just modify config/config_linux or config/config_bsd.
This is what I meant, all the config_*.h
>> 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.
> I agree that your solution is good, I am just thinking if we can make less changes
> for API level.

I can understand the reluctance for adding new API calls, but, let me 
double check, as I am not sure you understood my point:

If we set it in the config_*.h, and we set MAX_NUM_OF_KNI to a value 
whatever, 8, 16... 128..., it is quite possible that a lot of users of 
DPDK that will use the KNI (only those) get run-time errors since the 
memzones cannot be pre-allocated (out of memory). Memzones are 
preallocated at rte_kni_init() (or at first alloc, as per what you are 
suggesting). Moreover, the user would have to go and change (e.g. 
reduce) the MAX_NUM_OF_KNI in the config_*.h and recompile DPDK. I don't 
think that's what we want.

Marc