| Message ID | 1457517037-71693-4-git-send-email-david.hunt@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 1EB14377E; Wed, 9 Mar 2016 10:51:16 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id C1B562E81 for <dev@dpdk.org>; Wed, 9 Mar 2016 10:51:14 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 09 Mar 2016 01:51:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,310,1455004800"; d="scan'208";a="920053039" Received: from sie-lab-214-251.ir.intel.com (HELO silpixa373510.ir.intel.com) ([10.237.214.251]) by fmsmga001.fm.intel.com with ESMTP; 09 Mar 2016 01:51:13 -0800 From: David Hunt <david.hunt@intel.com> To: dev@dpdk.org Date: Wed, 9 Mar 2016 09:50:36 +0000 Message-Id: <1457517037-71693-4-git-send-email-david.hunt@intel.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1457517037-71693-1-git-send-email-david.hunt@intel.com> References: <1455634095-4183-1-git-send-email-david.hunt@intel.com> <1457517037-71693-1-git-send-email-david.hunt@intel.com> Subject: [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 <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Hunt, David
March 9, 2016, 9:50 a.m. UTC
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 <david.hunt@intel.com>
---
config/common_base | 2 ++
lib/librte_mbuf/rte_mbuf.c | 8 ++++++++
2 files changed, 10 insertions(+)
Comments
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 <david.hunt@intel.com> > --- > 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 -
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 <david.hunt@intel.com> >> --- >> 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.
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 <david.hunt@intel.com> >>> --- >>> 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. > > > > >
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 */