Re: [dpdk-dev] [PATCH] mempool: enforce valid flags at creation

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 10/12/21 10:28 AM, David Marchand wrote:
> If we do not enforce valid flags are passed by an application, this
> application might face issues in the future when we add more flags.

Thanks. I'd even consider it as a bug and the fix to be
backported.

> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

A nit below, other than that:

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[snip]

> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index c5f859ae71..a2a78125f4 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c
> @@ -777,6 +777,13 @@ rte_mempool_cache_free(struct rte_mempool_cache *cache)
>  	rte_free(cache);
>  }
>  
> +#define MEMPOOL_KNOWN_FLAGS ( MEMPOOL_F_NO_SPREAD \
> +	| MEMPOOL_F_NO_CACHE_ALIGN \
> +	| MEMPOOL_F_SP_PUT \
> +	| MEMPOOL_F_SC_GET \
> +	| MEMPOOL_F_POOL_CREATED \
> +	| MEMPOOL_F_NO_IOVA_CONTIG \
> +	)
>  /* create an empty mempool */
>  struct rte_mempool *
>  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
> @@ -806,6 +813,12 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>  			  RTE_CACHE_LINE_MASK) != 0);
>  #endif
>  
> +	/* enforce no unknown flag is passed by the application */
> +	if ((flags & ~MEMPOOL_KNOWN_FLAGS) != 0) {
> +		rte_errno = EINVAL;
> +		return NULL;
> +	}
> +

I think it is better to check arguments in parameters order.
So, it is a bit better to move the check to happen after
cache_size validation below.

>  	mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head, rte_mempool_list);
>  
>  	/* asked for zero items */

[snip]