Hi,
FYI, your patch has been queued to LTS release 18.11.11
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/24/20. 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-queue
This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable-queue/commit/4852ea2b3d0cbd694b1515fb3bc58a1cc3320a02
Thanks.
Kevin.
---
>From 4852ea2b3d0cbd694b1515fb3bc58a1cc3320a02 Mon Sep 17 00:00:00 2001
From: Lukasz Wojciechowski <l.wojciechow at partner.samsung.com>
Date: Sat, 17 Oct 2020 05:06:52 +0200
Subject: [PATCH] test/distributor: fix lcores statistics
[ upstream commit 2dfdfcb404493a781d152afbc85a2a8a5b90580b ]
Statistics of handled packets are cleared and read on main lcore,
while they are increased in workers handlers on different lcores.
Without synchronization occasionally showed invalid values.
This patch uses atomic mechanisms to synchronize.
Relaxed memory model is used.
Fixes: c3eabff124e6 ("distributor: add unit tests")
Signed-off-by: Lukasz Wojciechowski <l.wojciechow at partner.samsung.com>
Acked-by: David Hunt <david.hunt at intel.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
---
test/test/test_distributor.c | 39 +++++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/test/test/test_distributor.c b/test/test/test_distributor.c
index d13e38f062..d8e007c47b 100644
--- a/test/test/test_distributor.c
+++ b/test/test/test_distributor.c
@@ -44,5 +44,6 @@ total_packet_count(void)
unsigned i, count = 0;
for (i = 0; i < worker_idx; i++)
- count += worker_stats[i].handled_packets;
+ count += __atomic_load_n(&worker_stats[i].handled_packets,
+ __ATOMIC_RELAXED);
return count;
}
@@ -52,5 +53,8 @@ static inline void
clear_packet_count(void)
{
- memset(&worker_stats, 0, sizeof(worker_stats));
+ unsigned int i;
+ for (i = 0; i < RTE_MAX_LCORE; i++)
+ __atomic_store_n(&worker_stats[i].handled_packets, 0,
+ __ATOMIC_RELAXED);
}
@@ -130,5 +134,6 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
for (i = 0; i < rte_lcore_count() - 1; i++)
printf("Worker %u handled %u packets\n", i,
- worker_stats[i].handled_packets);
+ __atomic_load_n(&worker_stats[i].handled_packets,
+ __ATOMIC_RELAXED));
printf("Sanity test with all zero hashes done.\n");
@@ -155,5 +160,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
for (i = 0; i < rte_lcore_count() - 1; i++)
printf("Worker %u handled %u packets\n", i,
- worker_stats[i].handled_packets);
+ __atomic_load_n(
+ &worker_stats[i].handled_packets,
+ __ATOMIC_RELAXED));
printf("Sanity test with two hash values done\n");
}
@@ -181,5 +188,6 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
for (i = 0; i < rte_lcore_count() - 1; i++)
printf("Worker %u handled %u packets\n", i,
- worker_stats[i].handled_packets);
+ __atomic_load_n(&worker_stats[i].handled_packets,
+ __ATOMIC_RELAXED));
printf("Sanity test with non-zero hashes done\n");
@@ -273,10 +281,12 @@ handle_work_with_free_mbufs(void *arg)
num = rte_distributor_get_pkt(d, id, buf, NULL, 0);
while (!quit) {
- worker_stats[id].handled_packets += num;
+ __atomic_fetch_add(&worker_stats[id].handled_packets, num,
+ __ATOMIC_RELAXED);
for (i = 0; i < num; i++)
rte_pktmbuf_free(buf[i]);
num = rte_distributor_get_pkt(d, id, buf, NULL, 0);
}
- worker_stats[id].handled_packets += num;
+ __atomic_fetch_add(&worker_stats[id].handled_packets, num,
+ __ATOMIC_RELAXED);
rte_distributor_return_pkt(d, id, buf, num);
return 0;
@@ -347,5 +357,6 @@ handle_work_for_shutdown_test(void *arg)
* for zero_quit */
while (!quit && !(id == zero_id && zero_quit)) {
- worker_stats[id].handled_packets += num;
+ __atomic_fetch_add(&worker_stats[id].handled_packets, num,
+ __ATOMIC_RELAXED);
num = rte_distributor_get_pkt(d, id, buf, NULL, 0);
@@ -357,6 +368,7 @@ handle_work_for_shutdown_test(void *arg)
zero_id = __atomic_load_n(&zero_idx, __ATOMIC_ACQUIRE);
}
- worker_stats[id].handled_packets += num;
+ __atomic_fetch_add(&worker_stats[id].handled_packets, num,
+ __ATOMIC_RELAXED);
if (id == zero_id) {
rte_distributor_return_pkt(d, id, NULL, 0);
@@ -371,5 +383,6 @@ handle_work_for_shutdown_test(void *arg)
while (!quit) {
- worker_stats[id].handled_packets += num;
+ __atomic_fetch_add(&worker_stats[id].handled_packets,
+ num, __ATOMIC_RELAXED);
num = rte_distributor_get_pkt(d, id, buf, NULL, 0);
}
@@ -437,5 +450,6 @@ sanity_test_with_worker_shutdown(struct worker_params *wp,
for (i = 0; i < rte_lcore_count() - 1; i++)
printf("Worker %u handled %u packets\n", i,
- worker_stats[i].handled_packets);
+ __atomic_load_n(&worker_stats[i].handled_packets,
+ __ATOMIC_RELAXED));
if (total_packet_count() != BURST * 2) {
@@ -497,5 +511,6 @@ test_flush_with_worker_shutdown(struct worker_params *wp,
for (i = 0; i < rte_lcore_count() - 1; i++)
printf("Worker %u handled %u packets\n", i,
- worker_stats[i].handled_packets);
+ __atomic_load_n(&worker_stats[i].handled_packets,
+ __ATOMIC_RELAXED));
if (total_packet_count() != BURST) {
--
2.26.2
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2020-11-18 16:33:38.438541184 +0000
+++ 0025-test-distributor-fix-lcores-statistics.patch 2020-11-18 16:33:37.929215062 +0000
@@ -1 +1 @@
-From 2dfdfcb404493a781d152afbc85a2a8a5b90580b Mon Sep 17 00:00:00 2001
+From 4852ea2b3d0cbd694b1515fb3bc58a1cc3320a02 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 2dfdfcb404493a781d152afbc85a2a8a5b90580b ]
+
@@ -14 +15,0 @@
-Cc: stable at dpdk.org
@@ -20 +21 @@
- app/test/test_distributor.c | 39 +++++++++++++++++++++++++------------
+ test/test/test_distributor.c | 39 +++++++++++++++++++++++++-----------
@@ -23,4 +24,4 @@
-diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
-index ec1fe348ba..4343efed14 100644
---- a/app/test/test_distributor.c
-+++ b/app/test/test_distributor.c
+diff --git a/test/test/test_distributor.c b/test/test/test_distributor.c
+index d13e38f062..d8e007c47b 100644
+--- a/test/test/test_distributor.c
++++ b/test/test/test_distributor.c
@@ -85 +86 @@
-@@ -348,5 +358,6 @@ handle_work_for_shutdown_test(void *arg)
+@@ -347,5 +357,6 @@ handle_work_for_shutdown_test(void *arg)
@@ -93 +94 @@
-@@ -358,6 +369,7 @@ handle_work_for_shutdown_test(void *arg)
+@@ -357,6 +368,7 @@ handle_work_for_shutdown_test(void *arg)
@@ -102 +103 @@
-@@ -372,5 +384,6 @@ handle_work_for_shutdown_test(void *arg)
+@@ -371,5 +383,6 @@ handle_work_for_shutdown_test(void *arg)
@@ -110 +111 @@
-@@ -438,5 +451,6 @@ sanity_test_with_worker_shutdown(struct worker_params *wp,
+@@ -437,5 +450,6 @@ sanity_test_with_worker_shutdown(struct worker_params *wp,
@@ -118 +119 @@
-@@ -498,5 +512,6 @@ test_flush_with_worker_shutdown(struct worker_params *wp,
+@@ -497,5 +511,6 @@ test_flush_with_worker_shutdown(struct worker_params *wp,