On Wed, 9 Apr 2025 23:22:50 +0000
"Lombardo, Ed" <ed.lomba...@netscout.com> wrote:
> Hi,
> I just finished modifying and testing our application to just do transmit of
> packets received on an NIC interface and let the rte_eth_tx_burst () free the
> mbuf and all works fine for both traffic types. This proves to me that my
> implementation of processing the packets and queueing them to tx ring and
> transmit from the tx ring is not buggy, which I had carefully verified in gdb
> early on. I still believe there is a problem with our application with many
> threads that can do rte_pktmbuf_free() on the same mbuf.
>
> I added these lines in my driver source file:
> #define RTE_LITRTE_MBUF_DEBUG 1
> #define RTE_LIBRTE_MEMPOOL_DEBUG 1
> #define RTE_ENABLE_ASSERT 1
>
> I don't see any asserts occur during my tx packet testing.
>
> The dpdk header files show the Atomic ifdef checks
> rte_build_config.h:#define RTE_MBUF_REFCNT_ATOMIC
> rte_mbuf_core.h: * or non-atomic) is controlled by
> the RTE_MBUF_REFCNT_ATOMIC flag.
> rte_mbuf.h:#ifdef RTE_MBUF_REFCNT_ATOMIC
> rte_mbuf.h:#else /* ! RTE_MBUF_REFCNT_ATOMIC */
> rte_mbuf.h:#endif /* RTE_MBUF_REFCNT_ATOMIC */
>
> I verified in building our application with DPDK rte_mbuf.h header file that
> the atomic functions for mbuf refcnt read/writes are turned ON. I added junk
> characters, and the compiler spotted syntax errors.
>
> So, I am back to the question as to why I get mbuf issue with multiple
> threads processing the same mbuf?
>
> Any more suggestions.
>
> Thanks,
> Ed
>
> -----Original Message-----
> From: Stephen Hemminger <step...@networkplumber.org>
> Sent: Wednesday, April 9, 2025 12:24 PM
> To: Lombardo, Ed <ed.lomba...@netscout.com>
> Cc: Dmitry Kozlyuk <dmitry.kozl...@gmail.com>; users@dpdk.org
> Subject: Re: mbuf refcnt issue
>
> External Email: This message originated outside of NETSCOUT. Do not click
> links or open attachments unless you recognize the sender and know the
> content is safe.
>
> On Wed, 9 Apr 2025 04:46:09 +0000
> "Lombardo, Ed" <ed.lomba...@netscout.com> wrote:
>
> > Hi Stephen,
> > I am looking a the rte_mbuf.h file for rte_pktmbuf_free() and it is not
> > clear to me that it checks if the mbuf refcnt is 1 before decrementing it
> > and allowing the mbuf and segments (if any) to be returned to free pool.
> >
> > Could my application issue be I have tx threads that transmit packets and
> > does rte_pktmbuf_free(), while one other thread will perform
> > rte_pktmbuf_free() on the same mbuf? I ensured I bump the mbuf refcnt to 2
> > before other threads can process the same mbuf.
> >
> > Thanks,
> > Ed
>
> It doesn't need to check refcnt there. The check is done later (since mbuf
> can be multi segment).
>
> rte_pktmbuf_free
> -> rte_pktmbuf_free_seg
> -> rte_pktmbuf_prefree_seg
>
> static __rte_always_inline struct rte_mbuf * rte_pktmbuf_prefree_seg(struct
> rte_mbuf *m) {
> __rte_mbuf_sanity_check(m, 0);
>
> if (likely(rte_mbuf_refcnt_read(m) == 1)) {
> normal fast path. breaks the chain.
> } else if (__rte_mbuf_refcnt_update(m, -1) == 0) {
> refcnt > 1 logic
>
> Note, the refcnt doesn't always go to zero when the mbuf is put back in the
> pool.
> The refcnt for a freed mbuf (in the pool) doesn't matter, it is free, it is
> dead.
> The refcnt is reset to 1 when mbuf is extracted from the pool.
>
>
>
You might find something by poisoning the mbuf when it is freed, so that any
attempt to use
the data would get junk. Also, add check at start of rte_pktmbuf_free_seg() to
catch dup free.
Something like this, but only compile tested.
It will break some of the functional tests, because they tend to make bogus
dummy mbufs.
diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 06ab7502a5..6088b34506 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -1423,10 +1423,11 @@ static inline int
__rte_pktmbuf_pinned_extbuf_decref(struct rte_mbuf *m)
static __rte_always_inline struct rte_mbuf *
rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
{
- __rte_mbuf_sanity_check(m, 0);
- if (likely(rte_mbuf_refcnt_read(m) == 1)) {
+ __rte_mbuf_sanity_check(m, 0);
+ unsigned int refcnt = rte_mbuf_refcnt_read(m);
+ if (likely(refcnt == 1)) {
if (!RTE_MBUF_DIRECT(m)) {
rte_pktmbuf_detach(m);
if (RTE_MBUF_HAS_EXTBUF(m) &&
@@ -1435,13 +1436,15 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
return NULL;
}
+ m->refcnt = 0;
if (m->next != NULL)
m->next = NULL;
if (m->nb_segs != 1)
m->nb_segs = 1;
return m;
-
+ } else if (unlikely(refcnt == 0 || refcnt >= UINT16_MAX - 1)) {
+ rte_panic("mbuf refcnt underflow %u\n", refcnt);
} else if (__rte_mbuf_refcnt_update(m, -1) == 0) {
if (!RTE_MBUF_DIRECT(m)) {
@@ -1452,6 +1455,7 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
return NULL;
}
+ m->refcnt = 0;
if (m->next != NULL)
m->next = NULL;
if (m->nb_segs != 1)
