From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 430C7200 for ; Fri, 22 Dec 2017 16:26:32 +0100 (CET) Received: from lfbn-lil-1-110-231.w90-45.abo.wanadoo.fr ([90.45.197.231] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1eSPJX-0006jW-Qa; Fri, 22 Dec 2017 16:32:49 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Fri, 22 Dec 2017 16:26:18 +0100 Date: Fri, 22 Dec 2017 16:26:18 +0100 From: Olivier MATZ To: "Wiles, Keith" Cc: Hemant Agrawal , "dev@dpdk.org" , Jerin Jacob , Neil Horman Message-ID: <20171222152617.2d3r3nlsaspsmz3t@platinum> References: <1513333251-4147-1-git-send-email-hemant.agrawal@nxp.com> <1513334482-4788-1-git-send-email-hemant.agrawal@nxp.com> <52edf0a5-f406-1773-ee52-5ca9ee4cf7fc@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2] doc: announce ABI change for pktmbuf pool create API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Dec 2017 15:26:32 -0000 On Tue, Dec 19, 2017 at 01:41:05PM +0000, Wiles, Keith wrote: > > > > On Dec 18, 2017, at 11:40 PM, Hemant Agrawal wrote: > > > > On 12/18/2017 7:21 PM, Wiles, Keith wrote: > >> > >> > >>> On Dec 15, 2017, at 4:41 AM, Hemant Agrawal wrote: > >>> > >>> Introduce a new argument ops_name in rte_mempool_set_ops_byname > >>> for allowing the application to optionally specify the mempool ops. > >>> > >>> Signed-off-by: Hemant Agrawal > >>> --- > >>> v2: fix checkpatch error > >>> > >>> doc/guides/rel_notes/deprecation.rst | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > >>> index 13e8543..968ca14 100644 > >>> --- a/doc/guides/rel_notes/deprecation.rst > >>> +++ b/doc/guides/rel_notes/deprecation.rst > >>> @@ -53,3 +53,6 @@ Deprecation Notices > >>> > >>> * librte_meter: The API will change to accommodate configuration profiles. > >>> Most of the API functions will have an additional opaque parameter. > >>> + > >>> +* librte_mbuf: a new optional parameter for representing name of mempool_ops > >>> + will be added to the API ``rte_pktmbuf_pool_create``. > >> > >> > >> Sorry, for the late response I was on vacation. > >> > >> My question is why do we need to change rte_pktmbuf_pool_create ABI yet again, why could we not add a new API to just set the name of the pool after it is created. This would allow all current applications to work without any ABI breakage and only require adding a new API call for anyone that wants the name. The rte_pktmbuf_pool_create() routine could assign a default name or some incrementing style name as the default. e.g. ‘pktmbuf_%d’ with a static incrementing variable or whatever you like. > >> > >> Sorry if this was asked and answered before. > >> > > > > I understand the concerns. > > > > However, the new API to just set the name will not work post create. > > rte_pktmbuf_pool_create is a wrapper API, which complete the mempool configuration on the basis default mempool_ops. > > Really can not add the name after the fact, I have not looked, but it seem very odd we can not use the mempool pointer and update the ops_name. What is stopping this from working? Changing the ops name is not possible after the mempool is created. The ops name defines how the objects are stored, we cannot update it once the objects are created. > > The idea proposed is to create pktmbuf pool from a specific mempool (ops_name). > > > > We can leave "rte_pktmbuf_pool_create" as it is. > > and create another similar API with e.g. "rte_pktmbuf_pool_create_specific", which will also take ops_name as argument. (We can combine the internal implementation with NULL ops_name for rte_pktmbuf_pool_create.) > > I would accept this approach over the original patch to change the name of a commonly used API. > > > > This way we will have flexibility for the applications looking for pktmbufs from a specific mempool. > > > > any thoughts? What do you think about this proposition? http://dpdk.org/ml/archives/dev/2017-December/084775.html The application can do: /* override value previously set by eal args, if any */ rte_mbuf_set_user_pool_ops("my-ops"); /* as before, no API change */ rte_pktmbuf_pool_create(...); With this approach, it is less convenients to create several pools with different ops, but I'm not sure there is a use case for that.