From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx.bisdn.de (mx.bisdn.de [185.27.182.31]) by dpdk.org (Postfix) with ESMTP id DDC757FAC for ; Fri, 10 Oct 2014 08:29:56 +0200 (CEST) Received: from [192.168.1.43] (137.Red-88-11-218.dynamicIP.rima-tde.net [88.11.218.137]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx.bisdn.de (Postfix) with ESMTPSA id 19663A2DF7; Fri, 10 Oct 2014 08:37:20 +0200 (CEST) Message-ID: <54377EB7.6080300@bisdn.de> Date: Fri, 10 Oct 2014 08:37:43 +0200 From: Marc Sune User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131103 Icedove/17.0.10 MIME-Version: 1.0 To: "Zhang, Helin" 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> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] KNI: use a memzone pool for KNI alloc/release X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Oct 2014 06:29:57 -0000 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