* [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal @ 2023-08-07 6:18 Julien Hascoet 2023-08-07 7:26 ` David Marchand 0 siblings, 1 reply; 4+ messages in thread From: Julien Hascoet @ 2023-08-07 6:18 UTC (permalink / raw) To: dev [-- Attachment #1: Type: text/plain, Size: 1514 bytes --] Hello, from my understanding after debugging, in test_refcnt_iter the return value of rte_ring_enqueue is not checked; leading to lack of expected mbufs at the end checks. Here is some fix proposal that seems to work after running endurance tests for several days: diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index b4f436b5e2..8a5d26e4f6 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -1033,12 +1033,17 @@ 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) { + /* retry in case of failure */ + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) + ; + } } else { while (ref-- != 0) { rte_pktmbuf_refcnt_update(m, 1); - rte_ring_enqueue(refcnt_mbuf_ring, m); + /* retry in case of failure */ + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) + ; } } rte_pktmbuf_free(m); Can you confirm ? Thank, Julien Hascoet [-- Attachment #2: Type: text/html, Size: 6035 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal 2023-08-07 6:18 [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal Julien Hascoet @ 2023-08-07 7:26 ` David Marchand 2023-08-07 8:02 ` Julien Hascoet 0 siblings, 1 reply; 4+ messages in thread From: David Marchand @ 2023-08-07 7:26 UTC (permalink / raw) To: Julien Hascoet, Olivier Matz; +Cc: dev Hello Julien, On Mon, Aug 7, 2023 at 8:19 AM Julien Hascoet <jhascoet@kalrayinc.com> wrote: > from my understanding after debugging, in test_refcnt_iter the return value of rte_ring_enqueue is not checked; leading to lack of expected mbufs at the end checks. > > Here is some fix proposal that seems to work after running endurance tests for several days: > > diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c > index b4f436b5e2..8a5d26e4f6 100644 > --- a/app/test/test_mbuf.c > +++ b/app/test/test_mbuf.c > @@ -1033,12 +1033,17 @@ 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) { > + /* retry in case of failure */ > + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) > + ; > + } > } else { > while (ref-- != 0) { > rte_pktmbuf_refcnt_update(m, 1); > - rte_ring_enqueue(refcnt_mbuf_ring, m); > + /* retry in case of failure */ > + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) > + ; > } > } > rte_pktmbuf_free(m); > > Can you confirm ? This analysis looks correct (though failing to enqueue in this unit test seems strange to me). Could you send a fix with a Fixes: line in the commitlog, and copying the maintainer? Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal 2023-08-07 7:26 ` David Marchand @ 2023-08-07 8:02 ` Julien Hascoet 2023-08-08 7:03 ` David Marchand 0 siblings, 1 reply; 4+ messages in thread From: Julien Hascoet @ 2023-08-07 8:02 UTC (permalink / raw) To: David Marchand, Olivier Matz; +Cc: dev [-- Attachment #1.1: Type: text/plain, Size: 2267 bytes --] Here is a patch, I let you amend it if needed as I'm a beginner in the dpdk project Thanks ________________________________ De : David Marchand <david.marchand@redhat.com> Envoyé : lundi 7 août 2023 09:26 À : Julien Hascoet <jhascoet@kalrayinc.com>; Olivier Matz <olivier.matz@6wind.com> Cc : dev@dpdk.org <dev@dpdk.org> Objet : Re: [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal Hello Julien, On Mon, Aug 7, 2023 at 8:19 AM Julien Hascoet <jhascoet@kalrayinc.com> wrote: > from my understanding after debugging, in test_refcnt_iter the return value of rte_ring_enqueue is not checked; leading to lack of expected mbufs at the end checks. > > Here is some fix proposal that seems to work after running endurance tests for several days: > > diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c > index b4f436b5e2..8a5d26e4f6 100644 > --- a/app/test/test_mbuf.c > +++ b/app/test/test_mbuf.c > @@ -1033,12 +1033,17 @@ 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) { > + /* retry in case of failure */ > + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) > + ; > + } > } else { > while (ref-- != 0) { > rte_pktmbuf_refcnt_update(m, 1); > - rte_ring_enqueue(refcnt_mbuf_ring, m); > + /* retry in case of failure */ > + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) > + ; > } > } > rte_pktmbuf_free(m); > > Can you confirm ? This analysis looks correct (though failing to enqueue in this unit test seems strange to me). Could you send a fix with a Fixes: line in the commitlog, and copying the maintainer? Thanks. -- David Marchand [-- Attachment #1.2: Type: text/html, Size: 6166 bytes --] [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-app-fix-silent-enqueue-fail-in-test_mbuf-test_refcnt.patch --] [-- Type: text/x-patch; name=0001-app-fix-silent-enqueue-fail-in-test_mbuf-test_refcnt.patch, Size: 1218 bytes --] From 6b57da16f3eca121c91ac0c250c93d4d017eea55 Mon Sep 17 00:00:00 2001 From: jhascoet <jhascoet@kalray.eu> Date: Mon, 7 Aug 2023 09:54:30 +0200 Subject: [PATCH] app: fix silent enqueue fail in test_mbuf test_refcnt_iter In case of ring full state, we retry the enqueue operation in order to avoid mbuf loss. Fixes: af75078fece ("first public release") --- app/test/test_mbuf.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index efac01806b..be114e3302 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -1033,12 +1033,17 @@ 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) { + /* retry in case of failure */ + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) + ; + } } else { while (ref-- != 0) { rte_pktmbuf_refcnt_update(m, 1); - rte_ring_enqueue(refcnt_mbuf_ring, m); + /* retry in case of failure */ + while (rte_ring_enqueue(refcnt_mbuf_ring, m) != 0) + ; } } rte_pktmbuf_free(m); -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal 2023-08-07 8:02 ` Julien Hascoet @ 2023-08-08 7:03 ` David Marchand 0 siblings, 0 replies; 4+ messages in thread From: David Marchand @ 2023-08-08 7:03 UTC (permalink / raw) To: Julien Hascoet; +Cc: Olivier Matz, dev Hello Julien, On Mon, Aug 7, 2023 at 10:02 AM Julien Hascoet <jhascoet@kalrayinc.com> wrote: > > Here is a patch, I let you amend it if needed as I'm a beginner in the dpdk project There are a few issues with the way you sent it. This patch was sent as an attachment while the patch content should be sent inline. It is also missing a Signed-off-by (see Developer's Certificate of Origin). The better is for you to read the contributing guide https://doc.dpdk.org/guides/contributing/patches.html with a focus on 8.5, 8.8 and 8.12 sections. Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-08 7:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-07 6:18 [TEST] dpdk/app/test/test_mbuf.c test_refcnt_mbuf instability + fix proposal Julien Hascoet 2023-08-07 7:26 ` David Marchand 2023-08-07 8:02 ` Julien Hascoet 2023-08-08 7:03 ` David Marchand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).