From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hemant Agrawal Subject: Re: [PATCH v3 1/2] eal: allow user to override default pool handle Date: Thu, 7 Sep 2017 14:55:41 +0530 Message-ID: <8db675e6-9918-288d-d4e5-14b4b22c9ef7@nxp.com> References: <20170720070613.18211-2-santosh.shukla@caviumnetworks.com> <20170815080717.9413-1-santosh.shukla@caviumnetworks.com> <20170815080717.9413-2-santosh.shukla@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , To: Santosh Shukla , , Return-path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0054.outbound.protection.outlook.com [104.47.32.54]) by dpdk.org (Postfix) with ESMTP id A2168968 for ; Thu, 7 Sep 2017 11:25:47 +0200 (CEST) In-Reply-To: <20170815080717.9413-2-santosh.shukla@caviumnetworks.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 8/15/2017 1:37 PM, Santosh Shukla wrote: > DPDK has support for both sw and hw mempool and > currently user is limited to use ring_mp_mc pool. > In case user want to use other pool handle, > need to update config RTE_MEMPOOL_OPS_DEFAULT, then > build and run with desired pool handle. > > Introducing eal option to override default pool handle. > > Now user can override the RTE_MEMPOOL_OPS_DEFAULT by passing > pool handle to eal `--mbuf-pool-ops=""`. > > Signed-off-by: Santosh Shukla > --- > v2 --> v3: > - Updated version.map entry to v17.11. > > v1 --> v2: > (Changes per review feedback from Olivier) > - Renamed rte_eal_get_mempool_name to rte_eal_mbuf_default_mempool_ops(). > - Added support in bsdapp too. > > Few changes considered while working on v2 like: > - Choosing eal arg name 'mbuf-pool-ops' out of 3 proposed names by Olivier. > Initialy thought to use eal arg 'mbuf-default-mempool-ops' But > since its way too long and user may find difficult to use it. That's why > chose 'mbuf-pool-ops', As that name very close to api name and #define. > - Adding new RTE_ for NAMESIZE in rte_eal.h called > 'RTE_MBUF_DEFAULT_MEMPOOL_OPS'. > > lib/librte_eal/bsdapp/eal/eal.c | 17 +++++++++++++++++ > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++ > lib/librte_eal/common/eal_common_options.c | 5 +++++ > lib/librte_eal/common/eal_internal_cfg.h | 1 + > lib/librte_eal/common/eal_options.h | 2 ++ > lib/librte_eal/common/include/rte_eal.h | 11 +++++++++++ > lib/librte_eal/linuxapp/eal/eal.c | 18 ++++++++++++++++++ > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++ > lib/librte_mbuf/rte_mbuf.c | 5 +++-- > 9 files changed, 71 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c > index 5fa598842..1def123be 100644 > --- a/lib/librte_eal/bsdapp/eal/eal.c > +++ b/lib/librte_eal/bsdapp/eal/eal.c > @@ -112,6 +112,13 @@ struct internal_config internal_config; > /* used by rte_rdtsc() */ > int rte_cycles_vmware_tsc_map; > > +/* Return mbuf pool name */ > +const char * > +rte_eal_mbuf_default_mempool_ops(void) > +{ > + return internal_config.mbuf_pool_name; > +} > + > /* Return a pointer to the configuration structure */ > struct rte_config * > rte_eal_get_configuration(void) > @@ -385,6 +392,16 @@ eal_parse_args(int argc, char **argv) > continue; > > switch (opt) { > + case OPT_MBUF_POOL_OPS_NUM: > + ret = snprintf(internal_config.mbuf_pool_name, > + sizeof(internal_config.mbuf_pool_name), > + "%s", optarg); > + if (ret < 0 || (uint32_t)ret >= > + sizeof(internal_config.mbuf_pool_name)) { > + ret = -1; > + goto out; > + } > + break; > case 'h': > eal_usage(prgname); > exit(EXIT_SUCCESS); > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > index aac6fd776..172a28bbf 100644 > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > @@ -237,3 +237,10 @@ EXPERIMENTAL { > rte_service_unregister; > > } DPDK_17.08; > + > +DPDK_17.11 { > + global: > + > + rte_eal_mbuf_default_mempool_ops; > + > +} DPDK_17.08; > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > index 1da185e59..78db639e2 100644 > --- a/lib/librte_eal/common/eal_common_options.c > +++ b/lib/librte_eal/common/eal_common_options.c > @@ -98,6 +98,7 @@ eal_long_options[] = { > {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM }, > {OPT_VMWARE_TSC_MAP, 0, NULL, OPT_VMWARE_TSC_MAP_NUM }, > {OPT_XEN_DOM0, 0, NULL, OPT_XEN_DOM0_NUM }, > + {OPT_MBUF_POOL_OPS, 1, NULL, OPT_MBUF_POOL_OPS_NUM}, > {0, 0, NULL, 0 } > }; > > @@ -220,6 +221,9 @@ eal_reset_internal_config(struct internal_config *internal_cfg) > #endif > internal_cfg->vmware_tsc_map = 0; > internal_cfg->create_uio_dev = 0; > + snprintf(internal_cfg->mbuf_pool_name, > + sizeof(internal_cfg->mbuf_pool_name), "%s", > + RTE_MBUF_DEFAULT_MEMPOOL_OPS); > } > > static int > @@ -1309,5 +1313,6 @@ eal_common_usage(void) > " --"OPT_NO_PCI" Disable PCI\n" > " --"OPT_NO_HPET" Disable HPET\n" > " --"OPT_NO_SHCONF" No shared config (mmap'd files)\n" > + " --"OPT_MBUF_POOL_OPS" Pool ops name for mbuf to use\n" > "\n", RTE_MAX_LCORE); > } > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h > index 7b7e8c887..e8a770a42 100644 > --- a/lib/librte_eal/common/eal_internal_cfg.h > +++ b/lib/librte_eal/common/eal_internal_cfg.h > @@ -83,6 +83,7 @@ struct internal_config { > const char *hugefile_prefix; /**< the base filename of hugetlbfs files */ > const char *hugepage_dir; /**< specific hugetlbfs directory to use */ > > + char mbuf_pool_name[RTE_MBUF_POOL_OPS_NAMESIZE]; /**< mbuf pool name */ > unsigned num_hugepage_sizes; /**< how many sizes on this system */ > struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES]; > }; > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h > index 439a26104..9c893aa2d 100644 > --- a/lib/librte_eal/common/eal_options.h > +++ b/lib/librte_eal/common/eal_options.h > @@ -83,6 +83,8 @@ enum { > OPT_VMWARE_TSC_MAP_NUM, > #define OPT_XEN_DOM0 "xen-dom0" > OPT_XEN_DOM0_NUM, > +#define OPT_MBUF_POOL_OPS "mbuf-pool-ops" > + OPT_MBUF_POOL_OPS_NUM, > OPT_LONG_MAX_NUM > }; > > diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h > index 0e7363d77..fcd212843 100644 > --- a/lib/librte_eal/common/include/rte_eal.h > +++ b/lib/librte_eal/common/include/rte_eal.h > @@ -54,6 +54,8 @@ extern "C" { > > /* Maximum thread_name length. */ > #define RTE_MAX_THREAD_NAME_LEN 16 > +/* Maximum length of mbuf pool ops name. */ > +#define RTE_MBUF_POOL_OPS_NAMESIZE 32 > > /** > * The lcore role (used in RTE or not). > @@ -287,6 +289,15 @@ static inline int rte_gettid(void) > return RTE_PER_LCORE(_thread_id); > } > > +/** > + * Ops to get default pool name for mbuf > + * > + * @return > + * returns default pool name. > + */ > +const char * > +rte_eal_mbuf_default_mempool_ops(void); > + > #define RTE_INIT(func) \ > static void __attribute__((constructor, used)) func(void) > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c > index 48f12f44c..99c2a14d4 100644 > --- a/lib/librte_eal/linuxapp/eal/eal.c > +++ b/lib/librte_eal/linuxapp/eal/eal.c > @@ -121,6 +121,13 @@ struct internal_config internal_config; > /* used by rte_rdtsc() */ > int rte_cycles_vmware_tsc_map; > > +/* Return mbuf pool name */ > +const char * > +rte_eal_mbuf_default_mempool_ops(void) > +{ > + return internal_config.mbuf_pool_name; > +} > + > /* Return a pointer to the configuration structure */ > struct rte_config * > rte_eal_get_configuration(void) > @@ -610,6 +617,17 @@ eal_parse_args(int argc, char **argv) > internal_config.create_uio_dev = 1; > break; > > + case OPT_MBUF_POOL_OPS_NUM: > + ret = snprintf(internal_config.mbuf_pool_name, > + sizeof(internal_config.mbuf_pool_name), > + "%s", optarg); > + if (ret < 0 || (uint32_t)ret >= > + sizeof(internal_config.mbuf_pool_name)) { > + ret = -1; > + goto out; > + } > + break; > + > default: > if (opt < OPT_LONG_MIN_NUM && isprint(opt)) { > RTE_LOG(ERR, EAL, "Option %c is not supported " > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > index 3a8f15406..5b48fcd37 100644 > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > @@ -242,3 +242,10 @@ EXPERIMENTAL { > rte_service_unregister; > > } DPDK_17.08; > + > +DPDK_17.11 { > + global: > + > + rte_eal_mbuf_default_mempool_ops; > + > +} DPDK_17.08; > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index 26a62b8e1..e1fc90ef3 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -157,6 +157,7 @@ rte_pktmbuf_pool_create(const char *name, unsigned n, > { > struct rte_mempool *mp; > struct rte_pktmbuf_pool_private mbp_priv; > + const char *mp_name; > unsigned elt_size; > int ret; > > @@ -176,8 +177,8 @@ rte_pktmbuf_pool_create(const char *name, unsigned n, > if (mp == NULL) > return NULL; > > - ret = rte_mempool_set_ops_byname(mp, > - RTE_MBUF_DEFAULT_MEMPOOL_OPS, NULL); > + mp_name = rte_eal_mbuf_default_mempool_ops(); > + ret = rte_mempool_set_ops_byname(mp, mp_name, NULL); > if (ret != 0) { > RTE_LOG(ERR, MBUF, "error setting mempool handler\n"); > rte_mempool_free(mp); > I guess you will take care of Olivier's comments. With that, Acked-by: Hemant Agrawal