patch 'test/service: fix spurious failures by extending timeout' has been queued to stable release 20.11.7

[luca.boccassi at gmail.com]

Hi,

FYI, your patch has been queued to stable release 20.11.7

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/05/22. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/kevintraynor/dpdk-stable

This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable/commit/fa67204c3d10bb105b0a801a0601e26f0c255b19

Thanks.

Luca Boccassi

---
>From fa67204c3d10bb105b0a801a0601e26f0c255b19 Mon Sep 17 00:00:00 2001
From: Harry van Haaren <harry.van.haaren at intel.com>
Date: Thu, 6 Oct 2022 12:52:47 +0000
Subject: [PATCH] test/service: fix spurious failures by extending timeout
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

[ upstream commit 6caffbb319ce8847696b9da1b05df8b631390835 ]

This commit extends the timeout for service_may_be_active()
from 100ms to 1000ms. Local testing on a idle and loaded system
(compiling DPDK with all cores) always completes after 1 ms.

The wait time for a service-lcore to finish is also extended
from 100ms to 1000ms.

The same timeout waiting code was duplicated in two tests, and
is now refactored to a standalone function avoiding duplication.

Reported-by: David Marchand <david.marchand at redhat.com>
Suggested-by: Mattias Rönnblom <mattias.ronnblom at ericsson.com>
Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
Acked-by: Morten Brørup <mb at smartsharesystems.com>
Reviewed-by: Mattias Rönnblom <mattias.ronnblom at ericsson.com>
---
 app/test/test_service_cores.c | 49 ++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
index 0aee8c04e3..4f4f450df1 100644
--- a/app/test/test_service_cores.c
+++ b/app/test/test_service_cores.c
@@ -22,6 +22,7 @@ static uint64_t service_tick;
 static uint32_t service_remote_launch_flag;
 
 #define SERVICE_DELAY 1
+#define TIMEOUT_MS 1000
 
 #define DUMMY_SERVICE_NAME "dummy_service"
 #define MT_SAFE_SERVICE_NAME "mt_safe_service"
@@ -119,15 +120,15 @@ unregister_all(void)
 	return TEST_SUCCESS;
 }
 
-/* Wait until service lcore not active, or for 100x SERVICE_DELAY */
+/* Wait until service lcore not active, or for TIMEOUT_MS */
 static void
 wait_slcore_inactive(uint32_t slcore_id)
 {
 	int i;
 
 	for (i = 0; rte_service_lcore_may_be_active(slcore_id) == 1 &&
-			i < 100; i++)
-		rte_delay_ms(SERVICE_DELAY);
+			i < TIMEOUT_MS; i++)
+		rte_delay_ms(1);
 }
 
 /* register a single dummy service */
@@ -903,12 +904,25 @@ service_lcore_start_stop(void)
 	return unregister_all();
 }
 
+static int
+service_ensure_stopped_with_timeout(uint32_t sid)
+{
+	/* give the service time to stop running */
+	int i;
+	for (i = 0; i < TIMEOUT_MS; i++) {
+		if (!rte_service_may_be_active(sid))
+			break;
+		rte_delay_ms(1);
+	}
+
+	return rte_service_may_be_active(sid);
+}
+
 /* stop a service and wait for it to become inactive */
 static int
 service_may_be_active(void)
 {
 	const uint32_t sid = 0;
-	int i;
 
 	/* expected failure cases */
 	TEST_ASSERT_EQUAL(-EINVAL, rte_service_may_be_active(10000),
@@ -928,19 +942,11 @@ service_may_be_active(void)
 	TEST_ASSERT_EQUAL(1, service_lcore_running_check(),
 			"Service core expected to poll service but it didn't");
 
-	/* stop the service */
+	/* stop the service, and wait for not-active with timeout */
 	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0),
 			"Error: Service stop returned non-zero");
-
-	/* give the service 100ms to stop running */
-	for (i = 0; i < 100; i++) {
-		if (!rte_service_may_be_active(sid))
-			break;
-		rte_delay_ms(SERVICE_DELAY);
-	}
-
-	TEST_ASSERT_EQUAL(0, rte_service_may_be_active(sid),
-			  "Error: Service not stopped after 100ms");
+	TEST_ASSERT_EQUAL(0, service_ensure_stopped_with_timeout(sid),
+			  "Error: Service not stopped after timeout period.");
 
 	return unregister_all();
 }
@@ -954,7 +960,6 @@ service_active_two_cores(void)
 		return TEST_SKIPPED;
 
 	const uint32_t sid = 0;
-	int i;
 
 	uint32_t lcore = rte_get_next_lcore(/* start core */ -1,
 					    /* skip main */ 1,
@@ -984,16 +989,8 @@ service_active_two_cores(void)
 	/* stop the service */
 	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0),
 			"Error: Service stop returned non-zero");
-
-	/* give the service 100ms to stop running */
-	for (i = 0; i < 100; i++) {
-		if (!rte_service_may_be_active(sid))
-			break;
-		rte_delay_ms(SERVICE_DELAY);
-	}
-
-	TEST_ASSERT_EQUAL(0, rte_service_may_be_active(sid),
-			  "Error: Service not stopped after 100ms");
+	TEST_ASSERT_EQUAL(0, service_ensure_stopped_with_timeout(sid),
+			  "Error: Service not stopped after timeout period.");
 
 	return unregister_all();
 }
-- 
2.34.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2022-11-03 09:27:29.483709777 +0000
+++ 0068-test-service-fix-spurious-failures-by-extending-time.patch	2022-11-03 09:27:25.489424686 +0000
@@ -1 +1 @@
-From 6caffbb319ce8847696b9da1b05df8b631390835 Mon Sep 17 00:00:00 2001
+From fa67204c3d10bb105b0a801a0601e26f0c255b19 Mon Sep 17 00:00:00 2001
@@ -8,0 +9,2 @@
+[ upstream commit 6caffbb319ce8847696b9da1b05df8b631390835 ]
+
@@ -29 +31 @@
-index 359b6dcd8b..637fcd7cf9 100644
+index 0aee8c04e3..4f4f450df1 100644
@@ -40 +42 @@
-@@ -123,15 +124,15 @@ unregister_all(void)
+@@ -119,15 +120,15 @@ unregister_all(void)
@@ -59 +61 @@
-@@ -921,12 +922,25 @@ service_lcore_start_stop(void)
+@@ -903,12 +904,25 @@ service_lcore_start_stop(void)
@@ -86 +88 @@
-@@ -946,19 +960,11 @@ service_may_be_active(void)
+@@ -928,19 +942,11 @@ service_may_be_active(void)
@@ -109 +111 @@
-@@ -972,7 +978,6 @@ service_active_two_cores(void)
+@@ -954,7 +960,6 @@ service_active_two_cores(void)
@@ -117 +119 @@
-@@ -1002,16 +1007,8 @@ service_active_two_cores(void)
+@@ -984,16 +989,8 @@ service_active_two_cores(void)