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 5A2C81396 for ; Mon, 3 Oct 2016 17:49:56 +0200 (CEST) Received: from lfbn-1-5996-232.w90-110.abo.wanadoo.fr ([90.110.195.232] helo=[192.168.1.13]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1br5Y3-0006hp-5E; Mon, 03 Oct 2016 17:53:00 +0200 To: Hemant Agrawal , dev@dpdk.org References: <1474292567-21912-1-git-send-email-olivier.matz@6wind.com> Cc: jerin.jacob@caviumnetworks.com, david.hunt@intel.com From: Olivier Matz Message-ID: Date: Mon, 3 Oct 2016 17:49:44 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC 0/7] changing mbuf pool handler 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: Mon, 03 Oct 2016 15:49:56 -0000 Hi Hemant, Thank you for your feedback. On 09/22/2016 01:52 PM, Hemant Agrawal wrote: > Hi Olivier > > On 9/19/2016 7:12 PM, Olivier Matz wrote: >> Hello, >> >> Following discussion from [1] ("usages issue with external mempool"). >> >> This is a tentative to make the mempool_ops feature introduced >> by David Hunt [2] more widely used by applications. >> >> It applies on top of a minor fix in mbuf lib [3]. >> >> To sumarize the needs (please comment if I did not got it properly): >> >> - new hw-assisted mempool handlers will soon be introduced >> - to make use of it, the new mempool API [4] (rte_mempool_create_empty, >> rte_mempool_populate, ...) has to be used >> - the legacy mempool API (rte_mempool_create) does not allow to change >> the mempool ops. The default is "ring_p_c" depending on >> flags. >> - the mbuf helper (rte_pktmbuf_pool_create) does not allow to change >> them either, and the default is RTE_MBUF_DEFAULT_MEMPOOL_OPS >> ("ring_mp_mc") >> - today, most (if not all) applications and examples use either >> rte_pktmbuf_pool_create or rte_mempool_create to create the mbuf >> pool, making it difficult to take advantage of this feature with >> existing apps. >> >> My initial idea was to deprecate both rte_pktmbuf_pool_create() and >> rte_mempool_create(), forcing the applications to use the new API, which >> is more flexible. But after digging a bit, it appeared that >> rte_mempool_create() is widely used, and not only for mbufs. Deprecating >> it would have a big impact on applications, and replacing it with the >> new API would be overkill in many use-cases. > > I agree with the proposal. > >> >> So I finally tried the following approach (inspired from a suggestion >> Jerin [5]): >> >> - add a new mempool_ops parameter to rte_pktmbuf_pool_create(). This >> unfortunatelly breaks the API, but I implemented an ABI compat layer. >> If the patch is accepted, we could discuss how to announce/schedule >> the API change. >> - update the applications and documentation to prefer >> rte_pktmbuf_pool_create() as much as possible >> - update most used examples (testpmd, l2fwd, l3fwd) to add a new command >> line argument to select the mempool handler >> >> I hope the external applications would then switch to >> rte_pktmbuf_pool_create(), since it supports most of the use-cases (even >> priv_size != 0, since we can call rte_mempool_obj_iter() after) . >> > > I will still prefer if you can add the "rte_mempool_obj_cb_t *obj_cb, > void *obj_cb_arg" into "rte_pktmbuf_pool_create". This single > consolidated wrapper will almost make it certain that applications will > not try to use rte_mempool_create for packet buffers. The patch changes the example applications. I'm not sure I understand why adding these arguments would force application to not use rte_mempool_create() for packet buffers. Do you have a application in mind? For the mempool_ops parameter, we must pass it at init because we need to know the mempool handler before populating the pool. For object initialization, it can be done after, so I thought it was better to reduce the number of arguments to avoid to fall in the mempool_create() syndrom :) Any other opinions? Regards, Olivier