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 490C22B96 for ; Wed, 9 Mar 2016 12:39:03 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 09 Mar 2016 03:38:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,311,1455004800"; d="scan'208";a="62770376" Received: from dhunt5-mobl.ger.corp.intel.com (HELO [10.237.220.68]) ([10.237.220.68]) by fmsmga004.fm.intel.com with ESMTP; 09 Mar 2016 03:38:45 -0800 To: Panu Matilainen , 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> From: "Hunt, David" Message-ID: <56E00B45.9030503@intel.com> Date: Wed, 9 Mar 2016 11:38:45 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56E000FC.2040204@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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:39:04 -0000 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. 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)? Thanks, David.