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 289357FDA for ; Fri, 10 Oct 2014 10:54:44 +0200 (CEST) Received: from [192.168.43.175] (77.Red-176-80-0.dynamicIP.rima-tde.net [176.80.0.77]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx.bisdn.de (Postfix) with ESMTPSA id EAF87A2700; Fri, 10 Oct 2014 11:02:08 +0200 (CEST) Message-ID: <5437A0A0.3040208@bisdn.de> Date: Fri, 10 Oct 2014 11:02:24 +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> <54377EB7.6080300@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 08:54:44 -0000 Helin n 10/10/14 09:35, Zhang, Helin wrote: > Hi Marc > >> [snip] >> 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? > Initially there was no rte_kni_close() which was later added for supporting XEN > specifically. > If KNI needs an init function, it might need to be put in rte_eal_init() like other > modules do. Well, calling rte_kni_init() always allocates memzones that if the user(=DPDK user, not the user of the final application) doesn't need to use KNI is wasted memory. I would not put it there, unless we add a flag in rte_eal_init() to enable/disable KNI subsystem and tune the pre-allocation of max_num_ifaces in runtime, without the necessity to recompile DPDK. More below. > >>>> 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. > Actually the end user doesn't know the mapping between hugepage memory size > and the maximum number of KNI. > Right? Giving 1G memory, end user doesn't know > how many KNI interfaces it can support. So the maximum number of KNI interfaces > still needs to be tuned. I don't get what you mean. Using rte_kni_init you can be sure that the application will be able to pre-allocate the necessary memory to not fail during the life-time of an application or die otherwise. If it cannot, the user knows what to do => increase memory. The user doesn't have to start tweaking and playing with DPDK config_*.h files, neither get errors at runtime, neither has to recompile DPDK if suddenly a certain application wants to be able to create 64 KNI interfaces instead of 8. The creation of all KNI interfaces at boot time is only one use case, a degenerate case, which is I guess what you still have in mind. Our application (and others as a general case) may create, destroy, create, destroy during the (big eventually) lifetime of the application. You don't want the application to fail during operation due to an improper compile flags of DPDK (the define that you propose of MAX_NUM_IFACES). The user needs to be able to control how many memzones are pre-allocated, and this has a direct relation with the amount of hugepages memory available. I have the patch v2 with your comments sorted out except this difference of opinion with rte_kni_init. I can circulate it and comment on it. I would also like to know if anyone else besides Helin&I in the mailing list has any opinion on this. thanks Marc