From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 4BBEC7F91 for ; Fri, 10 Oct 2014 07:18:03 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP; 09 Oct 2014 22:25:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="398016298" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by FMSMGA003.fm.intel.com with ESMTP; 09 Oct 2014 22:18:33 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 9 Oct 2014 22:25:26 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 9 Oct 2014 22:25:26 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.230]) by shsmsx102.ccr.corp.intel.com ([169.254.2.192]) with mapi id 14.03.0195.001; Fri, 10 Oct 2014 13:25:24 +0800 From: "Zhang, Helin" To: Marc Sune Thread-Topic: [dpdk-dev] [PATCH] KNI: use a memzone pool for KNI alloc/release Thread-Index: AQHP285+iUgjNWB580Cf22Ir3DYXhpwnUh3w//+O9YCAAIoo0P//gxyAgACOkED//4AWgAARAUBw//+RKwD//jwMwA== Date: Fri, 10 Oct 2014 05:25:23 +0000 Message-ID: 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: <54366044.8020507@bisdn.de> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 05:18:03 -0000 Hi Marc More comments added. > -----Original Message----- > From: Marc Sune [mailto:marc.sune@bisdn.de] > Sent: Thursday, October 9, 2014 6:16 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] KNI: use a memzone pool for KNI alloc/rel= ease >=20 > Hi Helin, >=20 > inline and snipped. Let me know after reading this mail if you believe I = can > already submit the v2 with the changes you suggested. >=20 > 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. :) >=20 > In my original answer to your comment here cited starting by "I don't thi= nk 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 configuratio= ns in them. For a specific platform, the maximum number of KNI interfaces should be fix= ed, and no need to be changed frequently. >=20 > A config.g #define approach would be highly dependent on hugepages memory > size and the usage the applications wants to do with KNI interfaces. Spec= ially > due to the former, I don't think it is a good idea. DPDK doesn't force an= y 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 lik= e 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. >=20 > 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. >=20 > >>>> 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. :) >=20 > I will definitely double-check before submitting v2. >=20 > Thanks for the suggestions > Marc Regards, Helin