From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Sune Subject: Re: [PATCH] KNI: use a memzone pool for KNI alloc/release Date: Fri, 10 Oct 2014 08:37:43 +0200 Message-ID: <54377EB7.6080300@bisdn.de> References: <1411985756-2744-1-git-send-email-marc.sune@bisdn.de> <543633B5.3020101@bisdn.de> <54363ED5.9060304@bisdn.de> <54364B1F.8030605@bisdn.de> <54366044.8020507@bisdn.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "dev-VfR2kkLFssw@public.gmane.org" To: "Zhang, Helin" Return-path: In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" 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