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

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

[Morten Brørup]

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>
---
 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)
+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)
+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
+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
+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

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

[Morten Brørup]

> 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:

../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

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

[Ivan Malov]

[-- Attachment #1: Type: text/plain, Size: 11960 bytes --]

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.

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
>
>

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

[Morten Brørup]

> 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
> >
> >

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

[Ivan Malov]

[-- Attachment #1: Type: text/plain, Size: 14166 bytes --]

On Sun, 20 Jul 2025, Morten Brørup wrote:

>> 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

Thanks, with 'check_includes=true', the warnings show up indeed.

Forgive me being unoriginal, but, given the fact the whole RTE_LIBRTE_MBUF_DEBUG
section in 'rte_mbuf.h' is based on the two new experimental APIs, how about
adding a check to the '#ifdef RTE_LIBRTE_MBUF_DEBUG' part, something like

#ifndef ALLOW_EXPERIMENTAL_API
#error "For RTE_LIBRTE_MBUF_DEBUG, one shall also enable ALLOW_EXPERIMENTAL_API"
#endif

or an equivalent check expressed in meson + a reminder to remove when stable?

For me, with ALLOW_EXPERIMENTAL_API defined alongside RTE_LIBRTE_MBUF_DEBUG,
the code builds seemingly without warnings.

Thank you.

> 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
>>>
>>>
>

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

[Morten Brørup]

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_ASSERT(), 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.

Since the new - experimental - function rte_mbuf_raw_sanity_check() is
called when RTE_LIBRTE_MBUF_DEBUG is defined, RTE_LIBRTE_MBUF_DEBUG now
requires ALLOW_EXPERIMENTAL_API until the function's experimental status
is removed.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v2:
* Added explicit build check for ALLOW_EXPERIMENTAL_API being enabled when
  RTE_LIBRTE_MBUF_DEBUG is enabled, with descriptive error message if not
  so. (Ivan Malov)
* Fixed typo in patch description.
---
 lib/mbuf/rte_mbuf.c |  61 +++++++++++++++++++++++++
 lib/mbuf/rte_mbuf.h | 109 +++++++++++++++++++++++++++++---------------
 2 files changed, 132 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)
+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)
+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..8344f60bc7 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -339,11 +339,22 @@ rte_pktmbuf_priv_flags(struct rte_mempool *mp)
 
 #ifdef RTE_LIBRTE_MBUF_DEBUG
 
+#ifndef ALLOW_EXPERIMENTAL_API
+/* Because rte_mbuf_raw_sanity_check() is an experimental API. */
+#error "RTE_LIBRTE_MBUF_DEBUG requires ALLOW_EXPERIMENTAL_API."
+#endif
+
+/**  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 +524,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
+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
+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 +609,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 +643,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 +681,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 +702,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 +733,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 +1054,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

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

[Stephen Hemminger]

On Sat, 19 Jul 2025 16:14:42 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

Good to see this a not inline.

> 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?

These can be exported without being experimental.
Experimental is optional for new symbols, to allow for future API changes.
But this has been around for a long time.