app: enhance error check if enqueue fail in test_mbuf test_refcnt_iter

Message ID 20230824073718.718872-1-jhascoet@kalray.eu (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series app: enhance error check if enqueue fail in test_mbuf test_refcnt_iter |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

jhascoet Aug. 24, 2023, 7:37 a.m. UTC
From: Julien Hascoet <ju.hascoet@gmail.com>

As the ring is big enough by construction, the ring
enqueue ops cannot fail; therefore, we check and panic
on it as soon as it is detected.

Signed-off-by: Julien Hascoet <ju.hascoet@gmail.com>
---
 app/test/test_mbuf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
  

Comments

Stephen Hemminger Oct. 4, 2024, 9:18 p.m. UTC | #1
On Thu, 24 Aug 2023 09:37:18 +0200
jhascoet <ju.hascoet@gmail.com> wrote:

> From: Julien Hascoet <ju.hascoet@gmail.com>
> 
> As the ring is big enough by construction, the ring
> enqueue ops cannot fail; therefore, we check and panic
> on it as soon as it is detected.
> 
> Signed-off-by: Julien Hascoet <ju.hascoet@gmail.com>
> ---

As per earlier patch in the email thread.
This is a test for something that should never happen.
Adding that test code is good and bad. Good, tests should never assume
code works. Bad, it creates more paths and complexity in the test code.

If the test was broken, and enqueue fails, it would fail in later checks
because of unfreed mbuf's.

Bottom line: lets skip this patch, it only happened to the submitter
because of other problems in their environment.
  

Patch

diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index d7393df7eb..c94dd3749e 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -1033,12 +1033,15 @@  test_refcnt_iter(unsigned int lcore, unsigned int iter,
 		tref += ref;
 		if ((ref & 1) != 0) {
 			rte_pktmbuf_refcnt_update(m, ref);
-			while (ref-- != 0)
-				rte_ring_enqueue(refcnt_mbuf_ring, m);
+			while (ref-- != 0) {
+				if (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0)
+					goto fail_enqueue;
+			}
 		} else {
 			while (ref-- != 0) {
 				rte_pktmbuf_refcnt_update(m, 1);
-				rte_ring_enqueue(refcnt_mbuf_ring, m);
+				if (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0)
+					goto fail_enqueue;
 			}
 		}
 		rte_pktmbuf_free(m);
@@ -1066,6 +1069,10 @@  test_refcnt_iter(unsigned int lcore, unsigned int iter,
 
 	rte_panic("(lcore=%u, iter=%u): after %us only "
 	          "%u of %u mbufs left free\n", lcore, iter, wn, i, n);
+
+fail_enqueue:
+	rte_panic("(lcore=%u, iter=%u): enqueue mbuf %u cannot "
+		"fail since ring is big enough\n", lcore, iter, i);
 }
 
 static int