From 8fd107123aaa5bcbafcba9c9f76c0fcff16ed784 Mon Sep 17 00:00:00 2001 From: Olivier Matz Date: Wed, 26 Jan 2022 14:36:35 +0100 Subject: [PATCH] mempool: fix mempool drivers usage in secondary When the mempool drivers in the primary applications are not loaded in the same order than in the secondary application, the behavior is undefined because the mempool->ops_index corresponds to different drivers. This patch adds a table in shared memory, which associates an index to a mempool driver. This table is filled by the primary process, and accessed by secondary processes when mempool ops are registered. Unfortunatly, rte_mempool_register_ops() cannot be called too early (in a constructor), because it needs to access to access to the shared memory. A previous tentative to solve this issue was introducing a new init level. This patch instead uses a vdev for each mempool driver, and take advantage of the probing method which is called by EAL at the right moment. The RTE_MEMPOOL_REGISTER_OPS() macro is moved in a new file, because it includes some files that are part of "driver_sdk_headers". There is no ABI change from the application perspective, but after this change, the mempool drivers have to be recompiled. Link: http://inbox.dpdk.org/dev/1586787714-13890-1-git-send-email-xiangxia.m.yue@gmail.com/ Signed-off-by: Olivier Matz --- drivers/mempool/bucket/rte_mempool_bucket.c | 1 + drivers/mempool/cnxk/cn10k_mempool_ops.c | 1 + drivers/mempool/cnxk/cn9k_mempool_ops.c | 1 + drivers/mempool/dpaa/dpaa_mempool.c | 1 + drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 1 + drivers/mempool/meson.build | 2 +- .../mempool/octeontx/rte_mempool_octeontx.c | 1 + drivers/mempool/ring/rte_mempool_ring.c | 1 + drivers/mempool/stack/rte_mempool_stack.c | 1 + lib/mempool/meson.build | 4 + lib/mempool/rte_mempool.h | 16 ++- lib/mempool/rte_mempool_driver.h | 32 +++++ lib/mempool/rte_mempool_ops.c | 113 ++++++++++++++++-- lib/mempool/version.map | 3 + 14 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 lib/mempool/rte_mempool_driver.h diff --git a/drivers/mempool/bucket/rte_mempool_bucket.c b/drivers/mempool/bucket/rte_mempool_bucket.c index c0b480bfc7..651e6278a3 100644 --- a/drivers/mempool/bucket/rte_mempool_bucket.c +++ b/drivers/mempool/bucket/rte_mempool_bucket.c @@ -14,6 +14,7 @@ #include #include #include +#include #include /* diff --git a/drivers/mempool/cnxk/cn10k_mempool_ops.c b/drivers/mempool/cnxk/cn10k_mempool_ops.c index ba826f0f01..53445baee8 100644 --- a/drivers/mempool/cnxk/cn10k_mempool_ops.c +++ b/drivers/mempool/cnxk/cn10k_mempool_ops.c @@ -3,6 +3,7 @@ */ #include +#include #include "roc_api.h" #include "cnxk_mempool.h" diff --git a/drivers/mempool/cnxk/cn9k_mempool_ops.c b/drivers/mempool/cnxk/cn9k_mempool_ops.c index b7967f8085..9af37b8d42 100644 --- a/drivers/mempool/cnxk/cn9k_mempool_ops.c +++ b/drivers/mempool/cnxk/cn9k_mempool_ops.c @@ -3,6 +3,7 @@ */ #include +#include #include "roc_api.h" #include "cnxk_mempool.h" diff --git a/drivers/mempool/dpaa/dpaa_mempool.c b/drivers/mempool/dpaa/dpaa_mempool.c index 32639a3bfd..8c4efb63d2 100644 --- a/drivers/mempool/dpaa/dpaa_mempool.c +++ b/drivers/mempool/dpaa/dpaa_mempool.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include diff --git a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c index 56c629c681..44be10bf8f 100644 --- a/drivers/mempool/dpaa2/dpaa2_hw_mempool.c +++ b/drivers/mempool/dpaa2/dpaa2_hw_mempool.c @@ -22,6 +22,7 @@ #include #include #include "rte_dpaa2_mempool.h" +#include #include "fslmc_vfio.h" #include diff --git a/drivers/mempool/meson.build b/drivers/mempool/meson.build index dc88812585..29d7db649c 100644 --- a/drivers/mempool/meson.build +++ b/drivers/mempool/meson.build @@ -11,6 +11,6 @@ drivers = [ 'stack', ] -std_deps = ['mempool'] +std_deps = ['mempool', 'bus_vdev'] log_prefix = 'mempool' diff --git a/drivers/mempool/octeontx/rte_mempool_octeontx.c b/drivers/mempool/octeontx/rte_mempool_octeontx.c index f4de1c8412..90ebb492f1 100644 --- a/drivers/mempool/octeontx/rte_mempool_octeontx.c +++ b/drivers/mempool/octeontx/rte_mempool_octeontx.c @@ -4,6 +4,7 @@ #include #include +#include #include #include diff --git a/drivers/mempool/ring/rte_mempool_ring.c b/drivers/mempool/ring/rte_mempool_ring.c index c6aa935eea..30bf19de08 100644 --- a/drivers/mempool/ring/rte_mempool_ring.c +++ b/drivers/mempool/ring/rte_mempool_ring.c @@ -8,6 +8,7 @@ #include #include #include +#include static int common_ring_mp_enqueue(struct rte_mempool *mp, void * const *obj_table, diff --git a/drivers/mempool/stack/rte_mempool_stack.c b/drivers/mempool/stack/rte_mempool_stack.c index 1476905227..3322dbd059 100644 --- a/drivers/mempool/stack/rte_mempool_stack.c +++ b/drivers/mempool/stack/rte_mempool_stack.c @@ -4,6 +4,7 @@ #include #include +#include #include static int diff --git a/lib/mempool/meson.build b/lib/mempool/meson.build index b8aaa00694..8113dfdae4 100644 --- a/lib/mempool/meson.build +++ b/lib/mempool/meson.build @@ -20,4 +20,8 @@ headers = files( 'rte_mempool_trace.h', 'rte_mempool_trace_fp.h', ) +driver_sdk_headers += files( + 'rte_mempool_driver.h', +) + deps += ['ring', 'telemetry'] diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h index 3ada37cb86..ffc8c5b205 100644 --- a/lib/mempool/rte_mempool.h +++ b/lib/mempool/rte_mempool.h @@ -684,7 +684,11 @@ struct rte_mempool_ops { */ struct rte_mempool_ops_table { rte_spinlock_t sl; /**< Spinlock for add/delete. */ - uint32_t num_ops; /**< Number of used ops structs in the table. */ + /** + * Number of used ops structs in the table. Note: in a secondary + * process, some ops_index can be higher than num_ops. + */ + uint32_t num_ops; /** * Storage for all possible ops structs. */ @@ -918,8 +922,14 @@ rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, * Pointer to an ops structure to register. * @return * - >=0: Success; return the index of the ops struct in the table. - * - -EINVAL - some missing callbacks while registering ops struct. - * - -ENOSPC - the maximum number of ops structs has been reached. + * - -EINVAL: some missing callbacks while registering ops struct. + * - -ENOSPC: the maximum number of ops structs has been reached, or + * the maximum number of memzones has already been allocated + * - -ENOMEM: no appropriate memory area found in which to create memzone + * - -EXISTS: the mempool ops are already registered (primary process only) + * - -ENOENT: the mempool ops are not registered on primary process + * (secondary process only) + * - -ENAMETOOLONG: the name of the mempool ops is too long */ int rte_mempool_register_ops(const struct rte_mempool_ops *ops); diff --git a/lib/mempool/rte_mempool_driver.h b/lib/mempool/rte_mempool_driver.h new file mode 100644 index 0000000000..6298a17b1a --- /dev/null +++ b/lib/mempool/rte_mempool_driver.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2022 6WIND S.A. + */ + +#ifndef _RTE_MEMPOOL_DRIVER_H_ +#define _RTE_MEMPOOL_DRIVER_H_ + +#include +#include +#include + +/** + * Macro to statically register the ops of a mempool handler. + * Note that the rte_mempool_register_ops fails silently here when + * more than RTE_MEMPOOL_MAX_OPS_IDX is registered. + */ +#define RTE_MEMPOOL_REGISTER_OPS(name) \ + static int \ + mempool_ops_probe_##name(struct rte_vdev_device *vdev) \ + { \ + RTE_SET_USED(vdev); \ + rte_mempool_register_ops(&name); \ + return 0; \ + } \ + static struct rte_vdev_driver drv_##name = { \ + .probe = mempool_ops_probe_##name, \ + .remove = NULL, \ + }; \ + RTE_PMD_REGISTER_VDEV(mempool_##name, drv_##name); \ + RTE_PMD_AUTOLOAD_VDEV(mempool_##name##0) + +#endif /* _RTE_MEMPOOL_DRIVER_H_ */ diff --git a/lib/mempool/rte_mempool_ops.c b/lib/mempool/rte_mempool_ops.c index 2d36dee8f0..8753b52642 100644 --- a/lib/mempool/rte_mempool_ops.c +++ b/lib/mempool/rte_mempool_ops.c @@ -9,31 +9,77 @@ #include #include #include -#include #include "rte_mempool_trace.h" +#define RTE_MEMPOOL_OPS_MZNAME "rte_mempool_ops" + /* indirect jump table to support external memory pools. */ struct rte_mempool_ops_table rte_mempool_ops_table = { .sl = RTE_SPINLOCK_INITIALIZER, .num_ops = 0 }; +/* shared mempool ops table, for multiprocess support */ +struct rte_mempool_shared_ops_table { + size_t count; + struct { + char name[RTE_MEMPOOL_OPS_NAMESIZE]; + } ops[RTE_MEMPOOL_MAX_OPS_IDX]; +}; +static struct rte_mempool_shared_ops_table *shared_ops_table; + +/* Allocate and initialize the shared memory. */ +static int +rte_mempool_ops_shm_init(void) +{ + const struct rte_memzone *mz; + static int saved_ret = -EAGAIN; + + if (saved_ret != -EAGAIN) + return saved_ret; + + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + mz = rte_memzone_reserve(RTE_MEMPOOL_OPS_MZNAME, + sizeof(*shared_ops_table), + SOCKET_ID_ANY, 0); + if (mz == NULL) { + saved_ret = -rte_errno; + return saved_ret; + } + shared_ops_table = mz->addr; + memset(shared_ops_table, 0, sizeof(*shared_ops_table)); + } else { + mz = rte_memzone_lookup(RTE_MEMPOOL_OPS_MZNAME); + if (mz == NULL) { + saved_ret = -ENOENT; + return saved_ret; + } + shared_ops_table = mz->addr; + } + + saved_ret = 0; + return saved_ret; +} + /* add a new ops struct in rte_mempool_ops_table, return its index. */ int rte_mempool_register_ops(const struct rte_mempool_ops *h) { struct rte_mempool_ops *ops; int16_t ops_index; + unsigned int i; + int ret; rte_spinlock_lock(&rte_mempool_ops_table.sl); - if (rte_mempool_ops_table.num_ops >= - RTE_MEMPOOL_MAX_OPS_IDX) { + ret = rte_mempool_ops_shm_init(); + if (ret < 0) { rte_spinlock_unlock(&rte_mempool_ops_table.sl); RTE_LOG(ERR, MEMPOOL, - "Maximum number of mempool ops structs exceeded\n"); - return -ENOSPC; + "Failed to register shared memzone for mempool ops: %s\n", + strerror(-ret)); + return ret; } if (h->alloc == NULL || h->enqueue == NULL || @@ -46,13 +92,50 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h) if (strlen(h->name) >= sizeof(ops->name) - 1) { rte_spinlock_unlock(&rte_mempool_ops_table.sl); - RTE_LOG(DEBUG, EAL, "%s(): mempool_ops <%s>: name too long\n", + RTE_LOG(ERR, MEMPOOL, "%s(): mempool_ops <%s>: name too long\n", __func__, h->name); - rte_errno = EEXIST; - return -EEXIST; + rte_errno = ENAMETOOLONG; + return -ENAMETOOLONG; + } + + /* lookup for ops in shared memory */ + for (i = 0; i < RTE_MEMPOOL_MAX_OPS_IDX; i++) { + if (!strcmp(h->name, shared_ops_table->ops[i].name)) + break; + } + + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + if (i < RTE_MEMPOOL_MAX_OPS_IDX) { + rte_spinlock_unlock(&rte_mempool_ops_table.sl); + RTE_LOG(ERR, MEMPOOL, + "%s(): mempool_ops <%s>: already registered\n", + __func__, h->name); + rte_errno = EEXIST; + return -EEXIST; + } + ops_index = shared_ops_table->count; + } else { + if (i == RTE_MEMPOOL_MAX_OPS_IDX) { + rte_spinlock_unlock(&rte_mempool_ops_table.sl); + RTE_LOG(ERR, MEMPOOL, + "%s(): mempool_ops <%s>: not registered in primary process\n", + __func__, h->name); + rte_errno = ENOENT; + return -ENOENT; + } + ops_index = i; } - ops_index = rte_mempool_ops_table.num_ops++; + if (ops_index >= RTE_MEMPOOL_MAX_OPS_IDX) { + rte_spinlock_unlock(&rte_mempool_ops_table.sl); + RTE_LOG(ERR, MEMPOOL, + "Maximum number of mempool ops structs exceeded\n"); + rte_errno = ENOSPC; + return -ENOSPC; + } + + /* update local table */ + rte_mempool_ops_table.num_ops++; ops = &rte_mempool_ops_table.ops[ops_index]; strlcpy(ops->name, h->name, sizeof(ops->name)); ops->alloc = h->alloc; @@ -65,6 +148,13 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h) ops->get_info = h->get_info; ops->dequeue_contig_blocks = h->dequeue_contig_blocks; + /* update shared memory on primary process */ + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + strlcpy(shared_ops_table->ops[ops_index].name, h->name, + sizeof(ops->name)); + shared_ops_table->count++; + } + rte_spinlock_unlock(&rte_mempool_ops_table.sl); return ops_index; @@ -171,9 +261,8 @@ rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, if (mp->flags & RTE_MEMPOOL_F_POOL_CREATED) return -EEXIST; - for (i = 0; i < rte_mempool_ops_table.num_ops; i++) { - if (!strcmp(name, - rte_mempool_ops_table.ops[i].name)) { + for (i = 0; i < RTE_MEMPOOL_MAX_OPS_IDX; i++) { + if (!strcmp(name, rte_mempool_ops_table.ops[i].name)) { ops = &rte_mempool_ops_table.ops[i]; break; } diff --git a/lib/mempool/version.map b/lib/mempool/version.map index b67d7aace7..0b8bba8764 100644 --- a/lib/mempool/version.map +++ b/lib/mempool/version.map @@ -71,4 +71,7 @@ INTERNAL { # added in 21.11 rte_mempool_event_callback_register; rte_mempool_event_callback_unregister; + + # added in 22.03 + rte_mempool_ops_shm_init; }; -- 2.30.2