From patchwork Fri Jul 24 13:45:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 74763 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 78D86A0526; Fri, 24 Jul 2020 15:44:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D313A1C032; Fri, 24 Jul 2020 15:44:06 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 754501C01E for ; Fri, 24 Jul 2020 15:44:03 +0200 (CEST) IronPort-SDR: pHoUJHFGXreQnKwgSYzfOgcRN6isb6sFqfcSnmM5lfw3D+CpUDG01FwJ7fA1deFFmj8T9WkNHb 3ieQeT+pnScA== X-IronPort-AV: E=McAfee;i="6000,8403,9691"; a="148199446" X-IronPort-AV: E=Sophos;i="5.75,390,1589266800"; d="scan'208";a="148199446" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jul 2020 06:44:03 -0700 IronPort-SDR: uA8cCaOFFwDIImYfFz+xtqFbYniHSEDq/blEhh42rT/NeUHj7FVxmbOd75a/ZGDVQTFIuGGYzG DAP37u5N2How== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,390,1589266800"; d="scan'208";a="488744171" Received: from silpixa00399779.ir.intel.com (HELO silpixa00399779.ger.corp.intel.com) ([10.237.222.209]) by fmsmga006.fm.intel.com with ESMTP; 24 Jul 2020 06:44:00 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: david.marchand@redhat.com, igor.romanov@oktetlabs.ru, honnappa.nagarahalli@arm.com, ferruh.yigit@intel.com, nd@arm.com, aconole@redhat.com, l.wojciechow@partner.samsung.com, phil.yang@arm.com, Harry van Haaren Date: Fri, 24 Jul 2020 14:45:06 +0100 Message-Id: <20200724134506.11959-2-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200724134506.11959-1-harry.van.haaren@intel.com> References: <20200724124503.96282-1-harry.van.haaren@intel.com> <20200724134506.11959-1-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v5 2/2] test/service: fix race condition on stopping lcore X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" This commit fixes a potential race condition in the tests where the lcore running a service would increment a counter that was already reset by the test-suite thread. The resulting race-condition incremented value could cause CI failures, as indicated by DPDK's CI. This patch fixes the race-condition by making use of the added rte_service_lcore_active() API, which indicates when a service-core is no longer in the service-core polling loop. The unit test makes use of the above function to detect when all statistics increments are done in the service-core thread, and then the unit test continues finalizing and checking state. Fixes: f28f3594ded2 ("service: add attribute API") Reported-by: David Marchand Signed-off-by: Harry van Haaren Reviewed-by: Phil Yang Reviewed-by: Honnappa Nagarahalli --- v4: - Update test to new _may_be_ style API (Honnappa) - Add reviewed by from ML v3: - Refactor while() to for() to simplify (Harry) - Use SERVICE_DELAY instead of magic const 1 (Phil) - Add Phil's reviewed by tag from ML v2: Thanks for discussion on v1, this v2 fixup for the CI including previous feedback on ML. --- app/test/test_service_cores.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c index ef1d8fcb9..8bc1d9913 100644 --- a/app/test/test_service_cores.c +++ b/app/test/test_service_cores.c @@ -362,6 +362,9 @@ service_lcore_attr_get(void) "Service core add did not return zero"); TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(id, slcore_id, 1), "Enabling valid service and core failed"); + /* Ensure service is not active before starting */ + TEST_ASSERT_EQUAL(0, rte_service_lcore_may_be_active(slcore_id), + "Not-active service core reported as active"); TEST_ASSERT_EQUAL(0, rte_service_lcore_start(slcore_id), "Starting service core failed"); @@ -382,7 +385,22 @@ service_lcore_attr_get(void) lcore_attr_id, &lcore_attr_value), "Invalid lcore attr didn't return -EINVAL"); - rte_service_lcore_stop(slcore_id); + /* Ensure service is active */ + TEST_ASSERT_EQUAL(1, rte_service_lcore_may_be_active(slcore_id), + "Active service core reported as not-active"); + + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(id, slcore_id, 0), + "Disabling valid service and core failed"); + TEST_ASSERT_EQUAL(0, rte_service_lcore_stop(slcore_id), + "Failed to stop service lcore"); + + /* Wait until service lcore not active, or for 100x SERVICE_DELAY */ + for (int i = 0; rte_service_lcore_may_be_active(slcore_id) == 1 && + i < 100; i++) + rte_delay_ms(SERVICE_DELAY); + + TEST_ASSERT_EQUAL(0, rte_service_lcore_may_be_active(slcore_id), + "Service lcore not stopped after waiting."); TEST_ASSERT_EQUAL(0, rte_service_lcore_attr_reset_all(slcore_id), "Valid lcore_attr_reset_all() didn't return success");