RE: [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Ivan Malov [mailto:ivan.malov@arknetworks.am]
> Sent: Sunday, 20 July 2025 00.30
> 
> Hi Morten,
> 
> On Sat, 19 Jul 2025, Morten Brørup wrote:
> 
> >> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> >> Sent: Saturday, 19 July 2025 12.23
> >>
> >> Sanity checking a reinitialized mbuf (a.k.a. raw mbuf) has been
> >> refactored
> >> to follow the same design pattern as sanity checking a normal mbuf,
> and
> >> now depends on RTE_LIBRTE_MBUF_DEBUG instead of RTE_ENABLE_ASSERT.
> >>
> >> The details of the changes are as follows:
> >>
> >> Non-inlined functions rte_mbuf_raw_sanity_check() and
> >> rte_mbuf_raw_check()
> >> have been added.
> >> They do for a reinitialized mbuf what rte_mbuf_sanity_check() and
> >> rte_mbuf_check() do for a normal mbuf.
> >> They basically check the same conditions as
> >> __rte_mbuf_raw_sanity_check()
> >> previously did, but use "if (!condition) rte_panic(message)" instead
> of
> >> RTE_ASSSERT(), so they don't depend on RTE_ENABLE_ASSERT.
> >>
> >> The inline function __rte_mbuf_raw_sanity_check() has been replaced
> >> by the new macro __rte_mbuf_raw_sanity_check_mp(), which either calls
> >> rte_mbuf_raw_sanity_check() or does nothing, depending on
> >> RTE_LIBRTE_MBUF_DEBUG, just like the __rte_mbuf_sanity_check() macro
> >> does
> >> for a normal mbuf.
> >>
> >> Note that the new macro __rte_mbuf_raw_sanity_check_mp() takes an
> >> optional
> >> mempool parameter to verify that the mbuf belongs to the expected
> mbuf
> >> pool.
> >> This addition is mainly relevant for sanity checking reinitialized
> mbufs
> >> freed directly into a given mempool by a PMD when using
> >> RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE.
> >>
> >> The macro __rte_mbuf_raw_sanity_check() has been kept for backwards
> API
> >> compatibility.
> >> It simply calls __rte_mbuf_raw_sanity_check_mp() without specifying a
> >> mempool.
> >>
> >> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > Building this with RTE_LIBRTE_MBUF_DEBUG 1 in rte_config.h spews out
> warnings like:
> 
> Oddly, for me, building DPDK this way does not result in similar
> warnings.
> OS: Debian 12, gcc (Debian 12.2.0-14) 12.2.0, meson 1.0.1, ninja 1.11.1.
> 
> May be I'm missing something.

Build commands:
meson setup -Dplatform=generic -Dmax_numa_nodes=1 -Dcheck_includes=true build
ninja -C build

OS: Ubuntu 24.04.2 LTS (GNU/Linux 6.8.0-60-generic x86_64), gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0, meson 1.3.2, ninja 1.11.1.

diff --git a/config/rte_config.h b/config/rte_config.h
index 05344e2619..f1ad7f09de 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -64,6 +64,7 @@

 /* mbuf defines */
 #define RTE_MBUF_DEFAULT_MEMPOOL_OPS "ring_mp_mc"
+#define RTE_LIBRTE_MBUF_DEBUG 1 /* default not set */

 /* ether defines */
 #define RTE_MAX_QUEUES_PER_PORT 1024

> 
> Thank you.
> 
> >
> > ../lib/mbuf/rte_mbuf.h: In function ‘rte_mbuf_raw_free’:
> > ../lib/mbuf/rte_mbuf.h:700:9: warning: ‘rte_mbuf_raw_sanity_check’ is
> deprecated: Symbol is not yet part of stable ABI [-Wdeprecated-
> declarations]
> >  700 |         __rte_mbuf_raw_sanity_check_mp(m, NULL);
> >      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../lib/mbuf/rte_mbuf.h:542:1: note: declared here
> >  542 | rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const
> struct rte_mempool *mp);
> >      | ^~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Exporting the new APIs as non-experimental (i.e. using
> RTE_EXPORT_SYMBOL() instead of RTE_EXPORT_EXPERIMENTAL_SYMBOL() in the C
> file, and omitting __rte_experimental in the header file, as shown
> below) works, but is that acceptable?
> >
> > Or is there a better way of avoiding these warnings?
> >
> >> ---
> >>  lib/mbuf/rte_mbuf.c |  61 ++++++++++++++++++++++++++
> >>  lib/mbuf/rte_mbuf.h | 104 ++++++++++++++++++++++++++++--------------
> --
> >>  2 files changed, 127 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> >> index 9e7731a8a2..ff4db73b50 100644
> >> --- a/lib/mbuf/rte_mbuf.c
> >> +++ b/lib/mbuf/rte_mbuf.c
> >> @@ -373,6 +373,67 @@ rte_pktmbuf_pool_create_extbuf(const char *name,
> >> unsigned int n,
> >>  	return mp;
> >>  }
> >>
> >> +/* do some sanity checks on a reinitialized mbuf: panic if it fails
> */
> >> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_mbuf_raw_sanity_check, 25.11)
> >
> > +// RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_mbuf_raw_sanity_check, 25.11)
> > +RTE_EXPORT_SYMBOL(rte_mbuf_raw_sanity_check)
> >
> >> +void
> >> +rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const struct
> >> rte_mempool *mp)
> >> +{
> >> +	const char *reason;
> >> +
> >> +	if (rte_mbuf_raw_check(m, mp, &reason))
> >> +		rte_panic("%s\n", reason);
> >> +}
> >> +
> >> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_mbuf_raw_check, 25.11)
> >
> > +// RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_mbuf_raw_check, 25.11)
> > +RTE_EXPORT_SYMBOL(rte_mbuf_raw_check)
> >
> >> +int rte_mbuf_raw_check(const struct rte_mbuf *m, const struct
> >> rte_mempool *mp,
> >> +		   const char **reason)
> >> +{
> >> +	/* check sanity */
> >> +	if (rte_mbuf_check(m, 0, reason) == -1)
> >> +		return -1;
> >> +
> >> +	/* check initialized */
> >> +	if (rte_mbuf_refcnt_read(m) != 1) {
> >> +		*reason = "uninitialized ref cnt";
> >> +		return -1;
> >> +	}
> >> +	if (m->next != NULL) {
> >> +		*reason = "uninitialized next ptr";
> >> +		return -1;
> >> +	}
> >> +	if (m->nb_segs != 1) {
> >> +		*reason = "uninitialized nb_segs";
> >> +		return -1;
> >> +	}
> >> +	if (RTE_MBUF_CLONED(m)) {
> >> +		*reason = "cloned";
> >> +		return -1;
> >> +	}
> >> +	if (RTE_MBUF_HAS_EXTBUF(m)) {
> >> +		if (!RTE_MBUF_HAS_PINNED_EXTBUF(m)) {
> >> +			*reason = "external buffer not pinned";
> >> +			return -1;
> >> +		}
> >> +
> >> +		uint16_t cnt = rte_mbuf_ext_refcnt_read(m->shinfo);
> >> +		if ((cnt == 0) || (cnt == UINT16_MAX)) {
> >> +			*reason = "pinned external buffer bad ref cnt";
> >> +			return -1;
> >> +		}
> >> +		if (cnt != 1) {
> >> +			*reason = "pinned external buffer uninitialized ref
> >> cnt";
> >> +			return -1;
> >> +		}
> >> +	}
> >> +
> >> +	if (mp != NULL && m->pool != mp) {
> >> +		*reason = "wrong mbuf pool";
> >> +		return -1;
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  /* do some sanity checks on a mbuf: panic if it fails */
> >>  RTE_EXPORT_SYMBOL(rte_mbuf_sanity_check)
> >>  void
> >> diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
> >> index 06ab7502a5..2621cc385c 100644
> >> --- a/lib/mbuf/rte_mbuf.h
> >> +++ b/lib/mbuf/rte_mbuf.h
> >> @@ -339,11 +339,17 @@ rte_pktmbuf_priv_flags(struct rte_mempool *mp)
> >>
> >>  #ifdef RTE_LIBRTE_MBUF_DEBUG
> >>
> >> +/**  check reinitialized mbuf type in debug mode */
> >> +#define __rte_mbuf_raw_sanity_check_mp(m, mp)
> >> rte_mbuf_raw_sanity_check(m, mp)
> >> +
> >>  /**  check mbuf type in debug mode */
> >>  #define __rte_mbuf_sanity_check(m, is_h) rte_mbuf_sanity_check(m,
> is_h)
> >>
> >>  #else /*  RTE_LIBRTE_MBUF_DEBUG */
> >>
> >> +/**  check reinitialized mbuf type in debug mode */
> >> +#define __rte_mbuf_raw_sanity_check_mp(m, mp) do { } while (0)
> >> +
> >>  /**  check mbuf type in debug mode */
> >>  #define __rte_mbuf_sanity_check(m, is_h) do { } while (0)
> >>
> >> @@ -513,6 +519,54 @@ rte_mbuf_ext_refcnt_update(struct
> >> rte_mbuf_ext_shared_info *shinfo,
> >>  } while (0)
> >>
> >>
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: This API may change, or be removed, without
> prior
> >> notice.
> >> + *
> >> + * Sanity checks on a reinitialized mbuf.
> >> + *
> >> + * Check the consistency of the given reinitialized mbuf.
> >> + * The function will cause a panic if corruption is detected.
> >> + *
> >> + * Check that the mbuf is properly reinitialized (refcnt=1,
> next=NULL,
> >> + * nb_segs=1), as done by rte_pktmbuf_prefree_seg().
> >> + *
> >> + * @param m
> >> + *   The mbuf to be checked.
> >> + * @param mp
> >> + *   The mempool to which the mbuf belongs.
> >> + *   NULL if unknown, not to be checked.
> >> + */
> >> +__rte_experimental
> >
> > +// __rte_experimental
> >
> >> +void
> >> +rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const struct
> >> rte_mempool *mp);
> >> +
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: This API may change, or be removed, without
> prior
> >> notice.
> >> + *
> >> + * Sanity checks on a reinitialized mbuf.
> >> + *
> >> + * Almost like rte_mbuf_raw_sanity_check(), but this function gives
> the
> >> reason
> >> + * if corruption is detected rather than panic.
> >> + *
> >> + * @param m
> >> + *   The mbuf to be checked.
> >> + * @param mp
> >> + *   The mempool to which the mbuf belongs.
> >> + *   NULL if unknown, not to be checked.
> >> + * @param reason
> >> + *   A reference to a string pointer where to store the reason why a
> >> mbuf is
> >> + *   considered invalid.
> >> + * @return
> >> + *   - 0 if no issue has been found, reason is left untouched.
> >> + *   - -1 if a problem is detected, reason then points to a string
> >> describing
> >> + *     the reason why the mbuf is deemed invalid.
> >> + */
> >> +__rte_experimental
> >
> > +// __rte_experimental
> >
> >> +int rte_mbuf_raw_check(const struct rte_mbuf *m, const struct
> >> rte_mempool *mp,
> >> +		   const char **reason);
> >> +
> >>  /**
> >>   * Sanity checks on an mbuf.
> >>   *
> >> @@ -550,33 +604,11 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m,
> >> int is_header);
> >>  int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
> >>  		   const char **reason);
> >>
> >> -/**
> >> - * Sanity checks on a reinitialized mbuf in debug mode.
> >> - *
> >> - * Check the consistency of the given reinitialized mbuf.
> >> - * The function will cause a panic if corruption is detected.
> >> - *
> >> - * Check that the mbuf is properly reinitialized (refcnt=1,
> next=NULL,
> >> - * nb_segs=1), as done by rte_pktmbuf_prefree_seg().
> >> - *
> >> - * @param m
> >> - *   The mbuf to be checked.
> >> - */
> >> -static __rte_always_inline void
> >> -__rte_mbuf_raw_sanity_check(__rte_unused const struct rte_mbuf *m)
> >> -{
> >> -	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);
> >> -	RTE_ASSERT(m->next == NULL);
> >> -	RTE_ASSERT(m->nb_segs == 1);
> >> -	RTE_ASSERT(!RTE_MBUF_CLONED(m));
> >> -	RTE_ASSERT(!RTE_MBUF_HAS_EXTBUF(m) ||
> >> -			(RTE_MBUF_HAS_PINNED_EXTBUF(m) &&
> >> -			rte_mbuf_ext_refcnt_read(m->shinfo) == 1));
> >> -	__rte_mbuf_sanity_check(m, 0);
> >> -}
> >> +/** For backwards compatibility. */
> >> +#define __rte_mbuf_raw_sanity_check(m)
> >> __rte_mbuf_raw_sanity_check_mp(m, NULL)
> >>
> >>  /** For backwards compatibility. */
> >> -#define MBUF_RAW_ALLOC_CHECK(m) __rte_mbuf_raw_sanity_check(m)
> >> +#define MBUF_RAW_ALLOC_CHECK(m) __rte_mbuf_raw_sanity_check_mp(m,
> NULL)
> >>
> >>  /**
> >>   * Allocate an uninitialized mbuf from mempool *mp*.
> >> @@ -606,7 +638,7 @@ static inline struct rte_mbuf
> >> *rte_mbuf_raw_alloc(struct rte_mempool *mp)
> >>
> >>  	if (rte_mempool_get(mp, &ret.ptr) < 0)
> >>  		return NULL;
> >> -	__rte_mbuf_raw_sanity_check(ret.m);
> >> +	__rte_mbuf_raw_sanity_check_mp(ret.m, mp);
> >>  	return ret.m;
> >>  }
> >>
> >> @@ -644,7 +676,7 @@ rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp,
> >> struct rte_mbuf **mbufs, unsigne
> >>  	int rc = rte_mempool_get_bulk(mp, (void **)mbufs, count);
> >>  	if (likely(rc == 0))
> >>  		for (unsigned int idx = 0; idx < count; idx++)
> >> -			__rte_mbuf_raw_sanity_check(mbufs[idx]);
> >> +			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], mp);
> >>  	return rc;
> >>  }
> >>
> >> @@ -665,7 +697,7 @@ rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp,
> >> struct rte_mbuf **mbufs, unsigne
> >>  static __rte_always_inline void
> >>  rte_mbuf_raw_free(struct rte_mbuf *m)
> >>  {
> >> -	__rte_mbuf_raw_sanity_check(m);
> >> +	__rte_mbuf_raw_sanity_check_mp(m, NULL);
> >>  	rte_mempool_put(m->pool, m);
> >>  }
> >>
> >> @@ -696,12 +728,8 @@ __rte_experimental
> >>  static __rte_always_inline void
> >>  rte_mbuf_raw_free_bulk(struct rte_mempool *mp, struct rte_mbuf
> **mbufs,
> >> unsigned int count)
> >>  {
> >> -	for (unsigned int idx = 0; idx < count; idx++) {
> >> -		const struct rte_mbuf *m = mbufs[idx];
> >> -		RTE_ASSERT(m != NULL);
> >> -		RTE_ASSERT(m->pool == mp);
> >> -		__rte_mbuf_raw_sanity_check(m);
> >> -	}
> >> +	for (unsigned int idx = 0; idx < count; idx++)
> >> +		__rte_mbuf_raw_sanity_check_mp(mbufs[idx], mp);
> >>
> >>  	rte_mempool_put_bulk(mp, (void **)mbufs, count);
> >>  }
> >> @@ -1021,22 +1049,22 @@ static inline int
> rte_pktmbuf_alloc_bulk(struct
> >> rte_mempool *pool,
> >>  	switch (count % 4) {
> >>  	case 0:
> >>  		while (idx != count) {
> >> -			__rte_mbuf_raw_sanity_check(mbufs[idx]);
> >> +			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
> >>  			rte_pktmbuf_reset(mbufs[idx]);
> >>  			idx++;
> >>  			/* fall-through */
> >>  	case 3:
> >> -			__rte_mbuf_raw_sanity_check(mbufs[idx]);
> >> +			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
> >>  			rte_pktmbuf_reset(mbufs[idx]);
> >>  			idx++;
> >>  			/* fall-through */
> >>  	case 2:
> >> -			__rte_mbuf_raw_sanity_check(mbufs[idx]);
> >> +			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
> >>  			rte_pktmbuf_reset(mbufs[idx]);
> >>  			idx++;
> >>  			/* fall-through */
> >>  	case 1:
> >> -			__rte_mbuf_raw_sanity_check(mbufs[idx]);
> >> +			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
> >>  			rte_pktmbuf_reset(mbufs[idx]);
> >>  			idx++;
> >>  			/* fall-through */
> >> --
> >> 2.43.0
> >
> >