DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf
@ 2025-07-19 10:23 Morten Brørup
  2025-07-19 14:14 ` Morten Brørup
  0 siblings, 1 reply; 5+ messages in thread
From: Morten Brørup @ 2025-07-19 10:23 UTC (permalink / raw)
  To: dev, Thomas Monjalon, Stephen Hemminger, Bruce Richardson,
	Konstantin Ananyev, Andrew Rybchenko
  Cc: 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf
  2025-07-19 10:23 [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf Morten Brørup
@ 2025-07-19 14:14 ` Morten Brørup
  2025-07-19 22:30   ` Ivan Malov
  0 siblings, 1 reply; 5+ messages in thread
From: Morten Brørup @ 2025-07-19 14:14 UTC (permalink / raw)
  To: dev, Thomas Monjalon, Stephen Hemminger, Bruce Richardson,
	Konstantin Ananyev, Andrew Rybchenko

> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf
  2025-07-19 14:14 ` Morten Brørup
@ 2025-07-19 22:30   ` Ivan Malov
  2025-07-20  9:25     ` Morten Brørup
  0 siblings, 1 reply; 5+ messages in thread
From: Ivan Malov @ 2025-07-19 22:30 UTC (permalink / raw)
  To: Morten Brørup
  Cc: dev, Thomas Monjalon, Stephen Hemminger, Bruce Richardson,
	Konstantin Ananyev, Andrew Rybchenko

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf
  2025-07-19 22:30   ` Ivan Malov
@ 2025-07-20  9:25     ` Morten Brørup
  2025-07-20 13:09       ` Ivan Malov
  0 siblings, 1 reply; 5+ messages in thread
From: Morten Brørup @ 2025-07-20  9:25 UTC (permalink / raw)
  To: Ivan Malov
  Cc: dev, Thomas Monjalon, Stephen Hemminger, Bruce Richardson,
	Konstantin Ananyev, Andrew Rybchenko

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf
  2025-07-20  9:25     ` Morten Brørup
@ 2025-07-20 13:09       ` Ivan Malov
  0 siblings, 0 replies; 5+ messages in thread
From: Ivan Malov @ 2025-07-20 13:09 UTC (permalink / raw)
  To: Morten Brørup
  Cc: dev, Thomas Monjalon, Stephen Hemminger, Bruce Richardson,
	Konstantin Ananyev, Andrew Rybchenko

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-07-20 13:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-19 10:23 [PATCH] mbuf: de-inline sanity checking a reinitialized mbuf Morten Brørup
2025-07-19 14:14 ` Morten Brørup
2025-07-19 22:30   ` Ivan Malov
2025-07-20  9:25     ` Morten Brørup
2025-07-20 13:09       ` Ivan Malov

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