[dpdk-dev,v5,1/2] eal: allow user to override default pool handle
Checks
Commit Message
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-name=""`.
Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
v4 --> v5:
- Renamed mbuf_pool_name to mbuf_pool_ops_name, same change reflected across
patch in respective areas. (Suggested by Olivier).
v3 --> v4:
- Removed RTE_MBUF_XX_POOL_NAMESIZE macro. (Suggested by Olivier)
- Replaced char * with const char * for 'mbuf_pool_name' var. (Olivier)
- Added eal arg info in release guide.
doc/guides/freebsd_gsg/build_sample_apps.rst | 3 +++
doc/guides/linux_gsg/build_sample_apps.rst | 3 +++
doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
lib/librte_eal/bsdapp/eal/eal.c | 10 ++++++++++
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++
lib/librte_eal/common/eal_common_options.c | 3 +++
lib/librte_eal/common/eal_internal_cfg.h | 2 +-
lib/librte_eal/common/eal_options.h | 2 ++
lib/librte_eal/common/include/rte_eal.h | 9 +++++++++
lib/librte_eal/linuxapp/eal/eal.c | 11 +++++++++++
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++
lib/librte_mbuf/rte_mbuf.c | 5 +++--
12 files changed, 63 insertions(+), 3 deletions(-)
Comments
On Sun, Oct 01, 2017 at 02:44:39PM +0530, 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-name=""`.
>
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
01/10/2017 11:14, Santosh Shukla:
> --- 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_NAME, 1, NULL, OPT_MBUF_POOL_OPS_NAME_NUM},
> {0, 0, NULL, 0 }
> };
I think the options were sorted alphabetically.
[...]
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -82,7 +82,7 @@ struct internal_config {
> volatile enum rte_intr_mode vfio_intr_mode;
> const char *hugefile_prefix; /**< the base filename of hugetlbfs files */
> const char *hugepage_dir; /**< specific hugetlbfs directory to use */
> -
> + const char *mbuf_pool_ops_name; /**< mbuf pool ops name */
Why this config is not stored in mbuf.c?
On Friday 06 October 2017 05:59 AM, Thomas Monjalon wrote:
> 01/10/2017 11:14, Santosh Shukla:
>> --- 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_NAME, 1, NULL, OPT_MBUF_POOL_OPS_NAME_NUM},
>> {0, 0, NULL, 0 }
>> };
> I think the options were sorted alphabetically.
This is most logical comment so far I got from you.
Yes' will do. posting v6. Thanks.
> [...]
>> --- a/lib/librte_eal/common/eal_internal_cfg.h
>> +++ b/lib/librte_eal/common/eal_internal_cfg.h
>> @@ -82,7 +82,7 @@ struct internal_config {
>> volatile enum rte_intr_mode vfio_intr_mode;
>> const char *hugefile_prefix; /**< the base filename of hugetlbfs files */
>> const char *hugepage_dir; /**< specific hugetlbfs directory to use */
>> -
>> + const char *mbuf_pool_ops_name; /**< mbuf pool ops name */
> Why this config is not stored in mbuf.c?
>
Why the config not stored for vfio? hugepage? etc..in that case applicable too.
This is correct place to keep for now, unless as discussed in dpdksummit about eal
parsing abstraction approach.. plugin style approach so that each module has its own
parser. till then It should sit here like other, Its blocker for external-mempool
in general case: Where users are forced to hard-code their handle in _OPS_DEFAULT_=.
v6:
- Include v5 minor comment Thomas.
Patches rebased on upstream tip / commit
id:16bbc98a3e6305766c79fb42e55ca05b323d6e4d
v5:
- Includes v4 minor review comment.
Patches rebased on upstream tip / commit id:5dce9fcdb2
v4:
- Includes v3 review coment changes.
Patches rebased on 06791a4bce: ethdev: get the supported pools for a port
v3:
- Rebased on top of v17.11-rc0
- Updated version.map entry to v17.11.
v2:
DPDK has support for hw and sw mempool. Those mempool
can work optimal for specific PMD's.
Example:
sw ring based PMD for Intel NICs.
HW mempool manager dpaa2 for dpaa2 PMD.
HW mempool manager fpa for octeontx PMD.
There could be a use-case where different vendor NIC's used
on the same platform and User like to configure mempool in such a way that
each of those NIC's should use their preferred mempool(For performance reasons).
Current mempool infrastrucure don't support such use-case.
This patchset tries to address that problem in 2 steps:
0) Allowing user to dynamically configure mempool handle by
passing pool handle as eal arg to `--mbuf-pool-ops-name=<pool-handle>`.
1) Allowing PMD's to advertise their pool capability to an application.
From an application point of view:
- The application must ask to PMD about supported pools.
- PMD will advertise his pool capability in priority order
'0' - Best match mempool ops for _that_ port (highest priority)
'1' - PMD support this mempool ops.
- Based on those i/p from PMD, application to chose pool-hdl and
do pool create.
Change History:
v5 --> v6:
- Added Olivier Acked-by in series.
- Arranged alphbetical order for 'OPT_MBUF_POOL_OPS_NAME' (Suggested By Thomas)
- Placed alphabetical order for 'rte_eth_dev_pool_ops_supported' in version.map
file.
v5 --> v4:
- Renamed mbuf_pool_name to mbuf_pool_ops_name in [01/02]. (Suggested by
Olivier)
- Incorporated review comment suggested by Olivier for [02/02].
Refer specific patch for v5 change history.
v3 --> v4:
- Removed RTE_MBUF_XXX_POOL_NAMESIZE macro and replaced mbuf_pool_name
with const. (Suggested by Olivier)
- Added eal arg info in doc guide.
- Removed _preferred_pool() api and replaced with
rte_eth_dev_pools_ops_supported() api (Suggested by Olivier)
v2 --> v3:
- Changed version.map from DPDK_17.08 to DPDK_17.11.
v1 --> v2:
- Renamed rte_eal_get_mempool_name to rte_eal_mbuf_default_mempool_ops().
(suggested by Olivier)
- Renamed _get_preferred_pool to _get_preferred_pool_ops().
- Updated API description and changes return val from -EINVAL to -ENOTSUP.
(Suggested by Olivier)
* Specific details on v1-->v2 change summary described in each patch.
Checkpatch status:
- None.
Work History:
* Refer [1] for v1.
Thanks.
[1] http://dpdk.org/ml/archives/dev/2017-June/067022.html
Santosh Shukla (2):
eal: allow user to override default pool handle
ethdev: get the supported pool for a port
doc/guides/freebsd_gsg/build_sample_apps.rst | 3 +++
doc/guides/linux_gsg/build_sample_apps.rst | 3 +++
doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
lib/librte_eal/bsdapp/eal/eal.c | 10 ++++++++++
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 7 +++++++
lib/librte_eal/common/eal_common_options.c | 3 +++
lib/librte_eal/common/eal_internal_cfg.h | 2 +-
lib/librte_eal/common/eal_options.h | 2 ++
lib/librte_eal/common/include/rte_eal.h | 9 +++++++++
lib/librte_eal/linuxapp/eal/eal.c | 11 +++++++++++
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 7 +++++++
lib/librte_ether/rte_ethdev.c | 18 ++++++++++++++++++
lib/librte_ether/rte_ethdev.h | 24 ++++++++++++++++++++++++
lib/librte_ether/rte_ethdev_version.map | 1 +
lib/librte_mbuf/rte_mbuf.c | 5 +++--
15 files changed, 106 insertions(+), 3 deletions(-)
06/10/2017 05:31, santosh:
>
> On Friday 06 October 2017 05:59 AM, Thomas Monjalon wrote:
> > 01/10/2017 11:14, Santosh Shukla:
> >> --- 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_NAME, 1, NULL, OPT_MBUF_POOL_OPS_NAME_NUM},
> >> {0, 0, NULL, 0 }
> >> };
> > I think the options were sorted alphabetically.
>
> This is most logical comment so far I got from you.
I will imagine you did not really write this.
> Yes' will do. posting v6. Thanks.
>
> > [...]
> >> --- a/lib/librte_eal/common/eal_internal_cfg.h
> >> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> >> @@ -82,7 +82,7 @@ struct internal_config {
> >> volatile enum rte_intr_mode vfio_intr_mode;
> >> const char *hugefile_prefix; /**< the base filename of hugetlbfs files */
> >> const char *hugepage_dir; /**< specific hugetlbfs directory to use */
> >> -
> >> + const char *mbuf_pool_ops_name; /**< mbuf pool ops name */
> > Why this config is not stored in mbuf.c?
> >
> Why the config not stored for vfio? hugepage? etc..in that case applicable too.
All other configs are related to EAL features.
> This is correct place to keep for now, unless as discussed in dpdksummit about eal
> parsing abstraction approach.. plugin style approach so that each module has its own
> parser. till then It should sit here like other, Its blocker for external-mempool
> in general case: Where users are forced to hard-code their handle in _OPS_DEFAULT_=.
You probably missed that mbuf is not part of EAL.
I'm not talking about parsers, just where to save a variable.
You store mbuf info in EAL, and later, call
rte_eal_mbuf_default_mempool_ops() from mbuf lib.
It would be saner to directly save it in mbuf lib.
But Olivier agreed to save mbuf config in EAL.
I won't discuss it anymore if you don't want to change.
> Santosh Shukla (2):
> eal: allow user to override default pool handle
> ethdev: get the supported pool for a port
I had not reviewed it before yesterday.
It seems functionnally ready.
I have some reserves about naming and placing but I could try
to address them later.
Applied, thanks
@@ -163,6 +163,9 @@ Other options, specific to Linux and are not supported under FreeBSD are as foll
* ``--huge-dir``:
The directory where hugetlbfs is mounted.
+* ``mbuf-pool-ops-name``:
+ Pool ops name for mbuf to use.
+
* ``--file-prefix``:
The prefix text used for hugepage filenames.
@@ -157,6 +157,9 @@ The EAL options are as follows:
* ``--huge-dir``:
The directory where hugetlbfs is mounted.
+* ``mbuf-pool-ops-name``:
+ Pool ops name for mbuf to use.
+
* ``--file-prefix``:
The prefix text used for hugepage filenames.
@@ -110,6 +110,10 @@ See the DPDK Getting Started Guides for more information on these options.
Specify the directory where the hugetlbfs is mounted.
+* ``mbuf-pool-ops-name``:
+
+ Pool ops name for mbuf to use.
+
* ``--proc-type``
Set the type of the current process.
@@ -112,6 +112,13 @@ struct internal_config internal_config;
/* used by rte_rdtsc() */
int rte_cycles_vmware_tsc_map;
+/* Return mbuf pool ops name */
+const char *
+rte_eal_mbuf_default_mempool_ops(void)
+{
+ return internal_config.mbuf_pool_ops_name;
+}
+
/* Return a pointer to the configuration structure */
struct rte_config *
rte_eal_get_configuration(void)
@@ -385,6 +392,9 @@ eal_parse_args(int argc, char **argv)
continue;
switch (opt) {
+ case OPT_MBUF_POOL_OPS_NAME_NUM:
+ internal_config.mbuf_pool_ops_name = optarg;
+ break;
case 'h':
eal_usage(prgname);
exit(EXIT_SUCCESS);
@@ -238,3 +238,10 @@ EXPERIMENTAL {
rte_service_start_with_defaults;
} DPDK_17.08;
+
+DPDK_17.11 {
+ global:
+
+ rte_eal_mbuf_default_mempool_ops;
+
+} DPDK_17.08;
@@ -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_NAME, 1, NULL, OPT_MBUF_POOL_OPS_NAME_NUM},
{0, 0, NULL, 0 }
};
@@ -220,6 +221,7 @@ eal_reset_internal_config(struct internal_config *internal_cfg)
#endif
internal_cfg->vmware_tsc_map = 0;
internal_cfg->create_uio_dev = 0;
+ internal_cfg->mbuf_pool_ops_name = RTE_MBUF_DEFAULT_MEMPOOL_OPS;
}
static int
@@ -1309,5 +1311,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_NAME" Pool ops name for mbuf to use\n"
"\n", RTE_MAX_LCORE);
}
@@ -82,7 +82,7 @@ struct internal_config {
volatile enum rte_intr_mode vfio_intr_mode;
const char *hugefile_prefix; /**< the base filename of hugetlbfs files */
const char *hugepage_dir; /**< specific hugetlbfs directory to use */
-
+ const char *mbuf_pool_ops_name; /**< mbuf pool ops name */
unsigned num_hugepage_sizes; /**< how many sizes on this system */
struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
};
@@ -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_NAME "mbuf-pool-ops-name"
+ OPT_MBUF_POOL_OPS_NAME_NUM,
OPT_LONG_MAX_NUM
};
@@ -287,6 +287,15 @@ static inline int rte_gettid(void)
return RTE_PER_LCORE(_thread_id);
}
+/**
+ * Get default pool ops name for mbuf
+ *
+ * @return
+ * returns default pool ops name.
+ */
+const char *
+rte_eal_mbuf_default_mempool_ops(void);
+
#define RTE_INIT(func) \
static void __attribute__((constructor, used)) func(void)
@@ -121,6 +121,13 @@ struct internal_config internal_config;
/* used by rte_rdtsc() */
int rte_cycles_vmware_tsc_map;
+/* Return mbuf pool ops name */
+const char *
+rte_eal_mbuf_default_mempool_ops(void)
+{
+ return internal_config.mbuf_pool_ops_name;
+}
+
/* Return a pointer to the configuration structure */
struct rte_config *
rte_eal_get_configuration(void)
@@ -610,6 +617,10 @@ eal_parse_args(int argc, char **argv)
internal_config.create_uio_dev = 1;
break;
+ case OPT_MBUF_POOL_OPS_NAME_NUM:
+ internal_config.mbuf_pool_ops_name = optarg;
+ break;
+
default:
if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
RTE_LOG(ERR, EAL, "Option %c is not supported "
@@ -243,3 +243,10 @@ EXPERIMENTAL {
rte_service_start_with_defaults;
} DPDK_17.08;
+
+DPDK_17.11 {
+ global:
+
+ rte_eal_mbuf_default_mempool_ops;
+
+} DPDK_17.08;
@@ -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_ops_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_ops_name = rte_eal_mbuf_default_mempool_ops();
+ ret = rte_mempool_set_ops_byname(mp, mp_ops_name, NULL);
if (ret != 0) {
RTE_LOG(ERR, MBUF, "error setting mempool handler\n");
rte_mempool_free(mp);