From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EA117A0C43; Mon, 18 Oct 2021 10:37:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D72DC40141; Mon, 18 Oct 2021 10:37:33 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 3D4A54003C for ; Mon, 18 Oct 2021 10:37:33 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 9BB597F530; Mon, 18 Oct 2021 11:37:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 9BB597F530 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1634546252; bh=IgloFhziJZ/7xPeuBRSEZXWW4j3SNACVTDuG6zRMxQQ=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=bfLZ02j/3qioK4uzyB8AQaRKcupSRoK0BZzkG39qwzS86+9v9Lux7XcNHI1cdG89g xUW9gahQEtLJ7A0Jx34hvOvOt3cKHTWYYU8428ePPMABiDpNmSMZ/q/bbyfBQq3YR0 m89F1FqNrJ6qyLLBCh+JILlFwPgVRWgELEoYWRDs= To: David Marchand , Olivier Matz Cc: Dmitry Kozlyuk , Ray Kinsella , Stephen Hemminger , dev@dpdk.org References: <20211018082635.2054-1-david.marchand@redhat.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <79f4f029-ca13-6048-396c-232fe5ef5fb7@oktetlabs.ru> Date: Mon, 18 Oct 2021 11:37:32 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20211018082635.2054-1-david.marchand@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] mempool: accept user flags only X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list 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 10/18/21 11:26 AM, David Marchand wrote: > As reported by Dmitry, MEMPOOL_F_POOL_CREATED is a flag only manipulated > internally. > This flag is not supposed to be requested from an application and would > probably result in an incorrect behavior if an application did pass it. > > Other internal flags may be introduced later. > > Rework the check and export a mask of valid user flags for use in the > unit test. > > Fixes: b240af8b10f9 ("mempool: enforce valid flags at creation") > > Reported-by: Dmitry Kozlyuk > Signed-off-by: David Marchand > --- > app/test/test_mempool.c | 8 ++++---- > lib/mempool/rte_mempool.c | 11 ++--------- > lib/mempool/rte_mempool.h | 9 +++++++++ > 3 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c > index 66bc8d86b7..ba05742f76 100644 > --- a/app/test/test_mempool.c > +++ b/app/test/test_mempool.c > @@ -205,15 +205,15 @@ static int test_mempool_creation_with_exceeded_cache_size(void) > return 0; > } > > -static int test_mempool_creation_with_unknown_flag(void) > +static int test_mempool_creation_with_invalid_flags(void) > { > struct rte_mempool *mp_cov; > > - mp_cov = rte_mempool_create("test_mempool_unknown_flag", MEMPOOL_SIZE, > + mp_cov = rte_mempool_create("test_mempool_invalid_flags", MEMPOOL_SIZE, > MEMPOOL_ELT_SIZE, 0, 0, > NULL, NULL, > NULL, NULL, > - SOCKET_ID_ANY, MEMPOOL_F_NO_IOVA_CONTIG << 1); > + SOCKET_ID_ANY, ~MEMPOOL_VALID_USER_FLAGS); > > if (mp_cov != NULL) { > rte_mempool_free(mp_cov); > @@ -653,7 +653,7 @@ test_mempool(void) > if (test_mempool_creation_with_exceeded_cache_size() < 0) > GOTO_ERR(ret, err); > > - if (test_mempool_creation_with_unknown_flag() < 0) > + if (test_mempool_creation_with_invalid_flags() < 0) > GOTO_ERR(ret, err); > > if (test_mempool_same_name_twice_creation() < 0) > diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c > index 607419ccaf..7f92d79c89 100644 > --- a/lib/mempool/rte_mempool.c > +++ b/lib/mempool/rte_mempool.c > @@ -777,13 +777,6 @@ 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, > @@ -828,8 +821,8 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size, > return NULL; > } > > - /* enforce no unknown flag is passed by the application */ > - if ((flags & ~MEMPOOL_KNOWN_FLAGS) != 0) { > + /* enforce only user flags are passed by the application */ > + if ((flags & ~MEMPOOL_VALID_USER_FLAGS) != 0) { > rte_errno = EINVAL; > return NULL; > } > diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h > index 88bcbc51ef..d2bf2843f7 100644 > --- a/lib/mempool/rte_mempool.h > +++ b/lib/mempool/rte_mempool.h > @@ -258,6 +258,15 @@ struct rte_mempool { > #define MEMPOOL_F_POOL_CREATED 0x0010 /**< Internal: pool is created. */ > #define MEMPOOL_F_NO_IOVA_CONTIG 0x0020 /**< Don't need IOVA contiguous objs. */ > > +/** > + * This macro lists all the mempool flags an application may request. > + */ > +#define MEMPOOL_VALID_USER_FLAGS (MEMPOOL_F_NO_SPREAD \ I think RTE_ prefix is missing here since it is in a public header now. > + | MEMPOOL_F_NO_CACHE_ALIGN \ > + | MEMPOOL_F_SP_PUT \ > + | MEMPOOL_F_SC_GET \ > + | MEMPOOL_F_NO_IOVA_CONTIG \ > + ) > /** > * @internal When debug is enabled, store some statistics. > * > Should we make a patch to add defines with RTE_ prefix and add a deprecation for old flags without RTE_ prefix? Olivier, what do you think? If you have time to care about it, it would be great. (I'm unfamiliar with coccinelle yet).