> From: Robert Sanford <[email protected]>
>
> rte_mempool_create_empty() adds a mempool to the mempool list before
> the mempool has a store. The store is allocated, and
> RTE_MEMPOOL_F_POOL_CREATED is set, by the first populate call. In
> between, rte_mempool_walk() passes the mempool to its callback, and
> rte_mempool_list_dump() calls rte_mempool_dump() on it, which reads
> the number of objects in the store. With the ring driver this
> dereferences a NULL ring, and the process crashes.
>
> Skip mempools that do not have RTE_MEMPOOL_F_POOL_CREATED set in
> rte_mempool_walk() and rte_mempool_list_dump(), and document it.
NAK to rte_mempool_walk() skipping unpopulated pools. The effect is too broad.
The correct solution is for the relevant callback functions to check
RTE_MEMPOOL_F_POOL_CREATED, if it needs to access the mempool driver.
Instead, please fix rte_mempool_dump(FILE *f, struct rte_mempool *mp) to not
dump the parts accessing the mempool driver when RTE_MEMPOOL_F_POOL_CREATED is
not set. We want to know that the mempool exists, and partial information is
much better than no information.
> The
> populating thread sets the flag without holding a lock, so add a
> release fence before setting it and an acquire fence after checking
> it. A walker that sees the flag then also sees the store.
>
> rte_mempool_lookup() is unchanged. It does not access the store, and
> callers may look up a mempool that is not populated yet.
>
> Fixes: 449c49b93a6b ("mempool: support handler operations")
> Cc: [email protected]
>
> Signed-off-by: Robert Sanford <[email protected]>
> ---
> lib/mempool/rte_mempool.c | 20 +++++++++++++++++++-
> lib/mempool/rte_mempool.h | 3 +++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index d9f339a3c2..7c4d4697c7 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c
> @@ -304,6 +304,12 @@ mempool_ops_alloc_once(struct rte_mempool *mp)
> ret = rte_mempool_ops_alloc(mp);
> if (ret != 0)
> return ret;
> + /*
> + * rte_mempool_walk() and rte_mempool_list_dump() access
> the
> + * store of pools with this flag set. Make the store
> visible
> + * before the flag.
> + */
> + rte_atomic_thread_fence(rte_memory_order_release);
> mp->flags |= RTE_MEMPOOL_F_POOL_CREATED;
> }
> return 0;
> @@ -1412,6 +1418,11 @@ rte_mempool_list_dump(FILE *f)
>
> TAILQ_FOREACH(te, mempool_list, next) {
> mp = (struct rte_mempool *) te->data;
> + /* skip mempools that are not populated yet */
> + if ((mp->flags & RTE_MEMPOOL_F_POOL_CREATED) == 0)
> + continue;
> + /* pairs with the release fence in mempool_ops_alloc_once()
> */
> + rte_atomic_thread_fence(rte_memory_order_acquire);
> rte_mempool_dump(f, mp);
> }
>
> @@ -1460,7 +1471,14 @@ void rte_mempool_walk(void (*func)(struct
> rte_mempool *, void *),
> rte_mcfg_mempool_read_lock();
>
> RTE_TAILQ_FOREACH_SAFE(te, mempool_list, next, tmp_te) {
> - (*func)((struct rte_mempool *) te->data, arg);
> + struct rte_mempool *mp = (struct rte_mempool *) te->data;
> +
> + /* skip mempools that are not populated yet */
> + if ((mp->flags & RTE_MEMPOOL_F_POOL_CREATED) == 0)
> + continue;
> + /* pairs with the release fence in mempool_ops_alloc_once()
> */
> + rte_atomic_thread_fence(rte_memory_order_acquire);
> + (*func)(mp, arg);
> }
>
> rte_mcfg_mempool_read_unlock();
> diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
> index bd58d8a9d5..7782768a2d 100644
> --- a/lib/mempool/rte_mempool.h
> +++ b/lib/mempool/rte_mempool.h
> @@ -1907,6 +1907,8 @@ static inline void *rte_mempool_get_priv(struct
> rte_mempool *mp)
> /**
> * Dump the status of all mempools on the console
> *
> + * Mempools that are not populated yet are skipped.
> + *
> * @param f
> * A pointer to a file for output
> */
> @@ -1949,6 +1951,7 @@ uint32_t rte_mempool_calc_obj_size(uint32_t
> elt_size, uint32_t flags,
> *
> * The mempool list is locked while the iterator function runs, so the
> * iterator function must not create or free a mempool.
> + * Mempools that are not populated yet are skipped.
> *
> * @param func
> * Iterator function
> --
> 2.34.1
