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 B6C7745BD4; Fri, 25 Oct 2024 11:48:19 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 83FC740156; Fri, 25 Oct 2024 11:48:19 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id DAFA64003C for ; Fri, 25 Oct 2024 11:48:17 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D808D339; Fri, 25 Oct 2024 02:48:46 -0700 (PDT) Received: from [10.1.35.137] (unknown [10.1.35.137]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 242243F71E; Fri, 25 Oct 2024 02:48:15 -0700 (PDT) Message-ID: <8ddcca82-5f6e-46c5-90af-87db626188db@foss.arm.com> Date: Fri, 25 Oct 2024 10:46:49 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] test/bitops: check worker lcore availability To: David Marchand , dev@dpdk.org Cc: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Tyler Retzlaff References: <20241011152533.3189097-1-david.marchand@redhat.com> <20241025073926.43621-1-david.marchand@redhat.com> Content-Language: en-US From: Jack Bond-Preston In-Reply-To: <20241025073926.43621-1-david.marchand@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 Acked-by: Jack Bond-Preston On 25/10/2024 08:39, David Marchand wrote: > Coverity is not able to understand that having 2 lcores means that > rte_get_next_lcore(-1, 0, 1) can't return RTE_MAX_LCORE. > Add a check. > > Coverity issue: 445382, 445383, 445384, 445387, 445389, 445391 > Fixes: 35326b61aecb ("bitops: add atomic bit operations in new API") > > Signed-off-by: David Marchand > --- > Changes since v2: > - added a helper, > - prefer RTE_VERIFY(), > > Note: > - a better fix would be to check lcore id validity in the EAL launch API, > but it requires inspecting all functions and it could result in some > API change, so sending this as a simple fix for now, > > --- > app/test/test_bitops.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/app/test/test_bitops.c b/app/test/test_bitops.c > index 681e984037..78a7df6bb1 100644 > --- a/app/test/test_bitops.c > +++ b/app/test/test_bitops.c > @@ -13,6 +13,17 @@ > #include > #include "test.h" > > +static unsigned int > +get_worker_lcore(void) > +{ > + unsigned int lcore_id = rte_get_next_lcore(-1, 1, 0); > + > + /* avoid checkers (like Coverity) false positives */ > + RTE_VERIFY(lcore_id < RTE_MAX_LCORE); > + > + return lcore_id; > +} > + > #define GEN_TEST_BIT_ACCESS(test_name, set_fun, clear_fun, assign_fun, flip_fun, test_fun, size, \ > mod) \ > static int \ > @@ -158,7 +169,7 @@ test_bit_atomic_parallel_assign ## size(void) \ > printf("Need multiple cores to run parallel test.\n"); \ > return TEST_SKIPPED; \ > } \ > - worker_lcore_id = rte_get_next_lcore(-1, 1, 0); \ > + worker_lcore_id = get_worker_lcore(); \ > lmain.bit = rte_rand_max(size); \ > do { \ > lworker.bit = rte_rand_max(size); \ > @@ -217,7 +228,7 @@ test_bit_atomic_parallel_test_and_modify ## size(void) \ > printf("Need multiple cores to run parallel test.\n"); \ > return TEST_SKIPPED; \ > } \ > - worker_lcore_id = rte_get_next_lcore(-1, 1, 0); \ > + worker_lcore_id = get_worker_lcore(); \ > int rc = rte_eal_remote_launch(run_parallel_test_and_modify ## size, &lworker, \ > worker_lcore_id); \ > TEST_ASSERT(rc == 0, "Worker thread launch failed"); \ > @@ -266,7 +277,7 @@ test_bit_atomic_parallel_flip ## size(void) \ > printf("Need multiple cores to run parallel test.\n"); \ > return TEST_SKIPPED; \ > } \ > - worker_lcore_id = rte_get_next_lcore(-1, 1, 0); \ > + worker_lcore_id = get_worker_lcore(); \ > int rc = rte_eal_remote_launch(run_parallel_flip ## size, &lworker, worker_lcore_id); \ > TEST_ASSERT(rc == 0, "Worker thread launch failed"); \ > run_parallel_flip ## size(&lmain); \