From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 9FD09914 for ; Wed, 9 Mar 2016 12:44:45 +0100 (CET) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 256D2C057EC9; Wed, 9 Mar 2016 11:44:45 +0000 (UTC) Received: from sopuli.koti.laiskiainen.org (vpn1-4-189.ams2.redhat.com [10.36.4.189]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u29Bihjc025528; Wed, 9 Mar 2016 06:44:44 -0500 To: "Hunt, David" , dev@dpdk.org References: <1455634095-4183-1-git-send-email-david.hunt@intel.com> <1457517037-71693-1-git-send-email-david.hunt@intel.com> <1457517037-71693-4-git-send-email-david.hunt@intel.com> <56E000FC.2040204@redhat.com> <56E00B45.9030503@intel.com> From: Panu Matilainen Message-ID: <56E00CAB.90100@redhat.com> Date: Wed, 9 Mar 2016 13:44:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56E00B45.9030503@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Subject: Re: [dpdk-dev] [PATCH v3 3/4] mempool: allow rte_pktmbuf_pool_create switch between memool handlers 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: Wed, 09 Mar 2016 11:44:46 -0000 On 03/09/2016 01:38 PM, Hunt, David wrote: > Hi Panu, > > On 3/9/2016 10:54 AM, Panu Matilainen wrote: >> On 03/09/2016 11:50 AM, David Hunt wrote: >>> If the user wants to have rte_pktmbuf_pool_create() use an external >>> mempool >>> handler, they define RTE_MEMPOOL_HANDLER_NAME to be the name of the >>> mempool handler they wish to use, and change RTE_MEMPOOL_HANDLER_EXT >>> to 'y' >>> >>> Signed-off-by: David Hunt >>> --- >>> config/common_base | 2 ++ >>> lib/librte_mbuf/rte_mbuf.c | 8 ++++++++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/config/common_base b/config/common_base >>> index 1af28c8..9d70cf4 100644 >>> --- a/config/common_base >>> +++ b/config/common_base >>> @@ -350,6 +350,8 @@ CONFIG_RTE_RING_PAUSE_REP_COUNT=0 >>> CONFIG_RTE_LIBRTE_MEMPOOL=y >>> CONFIG_RTE_MEMPOOL_CACHE_MAX_SIZE=512 >>> CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n >>> +CONFIG_RTE_MEMPOOL_HANDLER_EXT=n >>> +CONFIG_RTE_MEMPOOL_HANDLER_NAME="custom_handler" >>> >>> # >>> # Compile librte_mbuf >>> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c >>> index c18b438..42b0cd1 100644 >>> --- a/lib/librte_mbuf/rte_mbuf.c >>> +++ b/lib/librte_mbuf/rte_mbuf.c >>> @@ -167,10 +167,18 @@ rte_pktmbuf_pool_create(const char *name, >>> unsigned n, >>> mbp_priv.mbuf_data_room_size = data_room_size; >>> mbp_priv.mbuf_priv_size = priv_size; >>> >>> +#ifdef RTE_MEMPOOL_HANDLER_EXT >>> + return rte_mempool_create_ext(name, n, elt_size, >>> + cache_size, sizeof(struct rte_pktmbuf_pool_private), >>> + rte_pktmbuf_pool_init, &mbp_priv, rte_pktmbuf_init, NULL, >>> + socket_id, 0, >>> + RTE_MEMPOOL_HANDLER_NAME); >>> +#else >>> return rte_mempool_create(name, n, elt_size, >>> cache_size, sizeof(struct rte_pktmbuf_pool_private), >>> rte_pktmbuf_pool_init, &mbp_priv, rte_pktmbuf_init, NULL, >>> socket_id, 0); >>> +#endif >>> } >>> >>> /* do some sanity checks on a mbuf: panic if it fails */ >>> >> >> This kind of thing really has to be run-time configurable, not a >> library build-time option. >> >> - Panu - > > Interesting point. I was attempting to minimise the amount of > application code changes. The problem with such build options is that the feature is for all practical purposes unusable in a distro setting where DPDK is just another shared library used by multiple applications. > Would you prefer if I took out that change, and added a new > rte_pktmbuf_pool_create_ext() function which tool an extra parameter as > the mempool handler name to use? > > /* helper to create a mbuf pool using external mempool handler */ > struct rte_mempool * > rte_pktmbuf_pool_create_ext(const char *name, unsigned n, > unsigned cache_size, uint16_t priv_size, uint16_t data_room_size, > int socket_id, const char *handler_name) > > That way we could leave the old rte_pktmbuf_pool_create() exactly as it > is, and any apps that wanted to use an > external handler could call rte_pktmbuf_pool_create_ext() > I could do this easily enough for v4 (which I hope to get out later today)? Yes, that's the way to do it. Thanks. - Panu - > Thanks, > David. > > > > >