[PATCH v5 0/3] mbuf: simplify handling of reinitialized mbufs

[PATCH v5 0/3] mbuf: simplify handling of reinitialized mbufs

[Morten Brørup]

This series contains three changes:
- sanity checking a reinitialized mbuf (a.k.a. raw mbuf) was de-inlined
- reinitialized mbuf free and alloc bulk functions were promoted as stable
- resetting fields of reinitialized mbufs was simplified, so fields that
  are already reset are not reset again

Morten Brørup (3):
  mbuf: de-inline sanity checking a reinitialized mbuf
  mbuf: promote raw free and alloc bulk functions as stable
  mbuf: no need to reset all fields on reinitialized mbufs

 lib/mbuf/rte_mbuf.c |  61 +++++++++++++++++++
 lib/mbuf/rte_mbuf.h | 141 ++++++++++++++++++++++++++++----------------
 2 files changed, 150 insertions(+), 52 deletions(-)

-- 
2.43.0

[PATCH v5 1/3] 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.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
v4:
* No changes.
v3:
* Removed experimental status for the new functions.
  Experimental is optional for new symbols, to allow for future API
  changes. But this has been around for a long time. (Stephen Hemminger)
* Consequentially, the added build check for ALLOW_EXPERIMENTAL_API with
  RTE_LIBRTE_MBUF_DEBUG is no longer needed, and was removed.
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 | 96 +++++++++++++++++++++++++++------------------
 2 files changed, 119 insertions(+), 38 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 9e7731a8a2..af39c13cf7 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_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_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..552cda1ae5 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,46 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info *shinfo,
 } while (0)
 
 
+/**
+ * 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.
+ */
+void
+rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const struct rte_mempool *mp);
+
+/**
+ * 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.
+ */
+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 +596,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 +630,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 +668,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 +689,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 +720,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 +1041,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 v5 2/3] promote reinitialized mbuf free and alloc bulk functions as stable

[Morten Brørup]

Ethdev drivers should use these APIs for allocating/freeing mbufs instead
of bypassing the mbuf library and accessing the mempool APIs, so they
cannot be experimental anymore.

Also updated the packet mbuf alloc bulk function to use the reinitialized
mbuf (a.k.a. raw mbuf) alloc bulk function, now that it is stable.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v5:
* Fix compiler warning.
---
 lib/mbuf/rte_mbuf.h | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 552cda1ae5..49c93ab356 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -635,9 +635,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Allocate a bulk of uninitialized mbufs from mempool *mp*.
  *
  * This function can be used by PMDs (especially in Rx functions)
@@ -661,7 +658,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
  *   - 0: Success.
  *   - -ENOENT: Not enough entries in the mempool; no mbufs are retrieved.
  */
-__rte_experimental
 static __rte_always_inline int
 rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -694,9 +690,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Put a bulk of mbufs allocated from the same mempool back into the mempool.
  *
  * The caller must ensure that the mbufs come from the specified mempool,
@@ -716,7 +709,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
  * @param count
  *   Array size.
  */
-__rte_experimental
 static __rte_always_inline void
 rte_mbuf_raw_free_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -1029,7 +1021,7 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	unsigned idx = 0;
 	int rc;
 
-	rc = rte_mempool_get_bulk(pool, (void **)mbufs, count);
+	rc = rte_mbuf_raw_alloc_bulk(pool, mbufs, count);
 	if (unlikely(rc))
 		return rc;
 
@@ -1041,22 +1033,18 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
-- 
2.43.0

[PATCH v5 3/3] mbuf: no need to reset all fields on reinitialized mbufs

[Morten Brørup]

The 'next' and 'nb_segs' fields are already reset on newly allocated
reinitialized mbufs (a.k.a. raw mbufs), so a simpler reset function for
such mbufs was added.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 49c93ab356..3517ca6858 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -954,6 +954,35 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
 					(uint16_t)m->buf_len);
 }
 
+/**
+ * Reset the fields of a packet mbuf to their default values.
+ *
+ * The caller must ensure that the mbuf is direct and properly
+ * reinitialized (refcnt=1, next=NULL, nb_segs=1), as done by
+ * rte_pktmbuf_prefree_seg().
+ *
+ * This function should be used with care, when optimization is required.
+ * For standard needs, prefer rte_pktmbuf_reset().
+ *
+ * @param m
+ *   The packet mbuf to be reset.
+ */
+static inline void rte_mbuf_raw_reset(struct rte_mbuf *m)
+{
+	m->pkt_len = 0;
+	m->tx_offload = 0;
+	m->vlan_tci = 0;
+	m->vlan_tci_outer = 0;
+	m->port = RTE_MBUF_PORT_INVALID;
+
+	m->ol_flags &= RTE_MBUF_F_EXTERNAL;
+	m->packet_type = 0;
+	rte_pktmbuf_reset_headroom(m);
+
+	m->data_len = 0;
+	__rte_mbuf_sanity_check(m, 1);
+}
+
 /**
  * Reset the fields of a packet mbuf to their default values.
  *
@@ -997,7 +1026,7 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 {
 	struct rte_mbuf *m;
 	if ((m = rte_mbuf_raw_alloc(mp)) != NULL)
-		rte_pktmbuf_reset(m);
+		rte_mbuf_raw_reset(m);
 	return m;
 }
 
@@ -1033,19 +1062,19 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 		}
-- 
2.43.0

[PATCH v6 0/3] mbuf: simplify handling of reinitialized mbufs

[Morten Brørup]

This series contains three changes:
- sanity checking a reinitialized mbuf (a.k.a. raw mbuf) was de-inlined
- reinitialized mbuf free and alloc bulk functions were promoted as stable
- resetting fields of reinitialized mbufs was simplified, so fields that
  are already reset are not reset again

Morten Brørup (3):
  mbuf: de-inline sanity checking a reinitialized mbuf
  mbuf: promote raw free and alloc bulk functions as stable
  mbuf: no need to reset all fields on reinitialized mbufs

 lib/mbuf/rte_mbuf.c |  61 +++++++++++++++++++
 lib/mbuf/rte_mbuf.h | 145 ++++++++++++++++++++++++++++----------------
 2 files changed, 154 insertions(+), 52 deletions(-)

-- 
2.43.0

[PATCH v6 1/3] 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.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v3:
* Removed experimental status for the new functions.
  Experimental is optional for new symbols, to allow for future API
  changes. But this has been around for a long time. (Stephen Hemminger)
* Consequentially, the added build check for ALLOW_EXPERIMENTAL_API with
  RTE_LIBRTE_MBUF_DEBUG is no longer needed, and was removed.
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 | 96 +++++++++++++++++++++++++++------------------
 2 files changed, 119 insertions(+), 38 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 9e7731a8a2..af39c13cf7 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_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_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..552cda1ae5 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,46 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info *shinfo,
 } while (0)
 
 
+/**
+ * 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.
+ */
+void
+rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const struct rte_mempool *mp);
+
+/**
+ * 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.
+ */
+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 +596,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 +630,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 +668,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 +689,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 +720,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 +1041,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 v6 1/3] mbuf: de-inline sanity checking a reinitialized mbuf

[Morten Brørup]

Forgot to add the ACKs from previous version of this patch:
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

-Morten

[PATCH v6 2/3] mbuf: promote raw free and alloc bulk functions as stable

[Morten Brørup]

If ethdev drivers should use these APIs for allocating/freeing mbufs
instead of bypassing the mbuf library and accessing the mempool APIs, they
cannot be experimental anymore.

Also updated the packet mbuf alloc bulk function to use the raw alloc bulk
function, now that it is stable.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 552cda1ae5..49c93ab356 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -635,9 +635,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Allocate a bulk of uninitialized mbufs from mempool *mp*.
  *
  * This function can be used by PMDs (especially in Rx functions)
@@ -661,7 +658,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
  *   - 0: Success.
  *   - -ENOENT: Not enough entries in the mempool; no mbufs are retrieved.
  */
-__rte_experimental
 static __rte_always_inline int
 rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -694,9 +690,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Put a bulk of mbufs allocated from the same mempool back into the mempool.
  *
  * The caller must ensure that the mbufs come from the specified mempool,
@@ -716,7 +709,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
  * @param count
  *   Array size.
  */
-__rte_experimental
 static __rte_always_inline void
 rte_mbuf_raw_free_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -1029,7 +1021,7 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	unsigned idx = 0;
 	int rc;
 
-	rc = rte_mempool_get_bulk(pool, (void **)mbufs, count);
+	rc = rte_mbuf_raw_alloc_bulk(pool, mbufs, count);
 	if (unlikely(rc))
 		return rc;
 
@@ -1041,22 +1033,18 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
-- 
2.43.0

[PATCH v6 3/3] mbuf: no need to reset all fields on reinitialized mbufs

[Morten Brørup]

The 'next' and 'nb_segs' fields are already reset on newly allocated
reinitialized mbufs (a.k.a. raw mbufs), so a simpler reset function for
reinitialized mbufs was added.

The 'ol_flags' field must indicate whether the mbuf uses an external
buffer.
Unlike the normal packet mbuf reset function, which gets this information
from the mbuf itself (by masking the RTE_MBUF_F_EXTERNAL flag), the
simpler reset function gets this information from the mempool, thereby
reducing the number of read-modify-writes on the mbuf from two to one.
The remaining read-modify-write on the mbuf in the simpler reset function
is in rte_pktmbuf_reset_headroom(), where the 'buf_len' field is read
before writing the 'data_off' field.
And I am hoping that it will be possible to eliminate that one too.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 49c93ab356..92a1e8ba7e 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -954,6 +954,39 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
 					(uint16_t)m->buf_len);
 }
 
+/**
+ * Reset the fields of a packet mbuf to their default values.
+ *
+ * The caller must ensure that the mbuf comes from the specified mempool,
+ * is direct and properly reinitialized (refcnt=1, next=NULL, nb_segs=1),
+ * as done by rte_pktmbuf_prefree_seg().
+ *
+ * This function should be used with care, when optimization is required.
+ * For standard needs, prefer rte_pktmbuf_reset().
+ *
+ * @param mp
+ *   The mempool to which the mbuf belongs.
+ * @param m
+ *   The packet mbuf to be reset.
+ */
+static inline void rte_mbuf_raw_reset(struct rte_mempool *mp, struct rte_mbuf *m)
+{
+	uint32_t flags = rte_pktmbuf_priv_flags(mp);
+
+	m->pkt_len = 0;
+	m->tx_offload = 0;
+	m->vlan_tci = 0;
+	m->vlan_tci_outer = 0;
+	m->port = RTE_MBUF_PORT_INVALID;
+
+	m->ol_flags = (flags & RTE_PKTMBUF_POOL_F_PINNED_EXT_BUF) ? RTE_MBUF_F_EXTERNAL : 0;
+	m->packet_type = 0;
+	rte_pktmbuf_reset_headroom(m);
+
+	m->data_len = 0;
+	__rte_mbuf_sanity_check(m, 1);
+}
+
 /**
  * Reset the fields of a packet mbuf to their default values.
  *
@@ -997,7 +1030,7 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 {
 	struct rte_mbuf *m;
 	if ((m = rte_mbuf_raw_alloc(mp)) != NULL)
-		rte_pktmbuf_reset(m);
+		rte_mbuf_raw_reset(mp, m);
 	return m;
 }
 
@@ -1033,19 +1066,19 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(pool, mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(pool, mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(pool, mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			rte_pktmbuf_reset(mbufs[idx]);
+			rte_mbuf_raw_reset(pool, mbufs[idx]);
 			idx++;
 			/* fall-through */
 		}
-- 
2.43.0

[PATCH v7 0/3] mbuf: simplify handling of reinitialized mbufs

[Morten Brørup]

This series contains three changes:
- sanity checking a reinitialized mbuf (a.k.a. raw mbuf) was de-inlined
- reinitialized mbuf free and alloc bulk functions were promoted as stable
- resetting fields of reinitialized mbufs was optimized, making the reset
  operation write-only of the mbuf, instead of read-modify-write

Morten Brørup (3):
  mbuf: de-inline sanity checking a reinitialized mbuf
  mbuf: promote raw free and alloc bulk functions as stable
  mbuf: optimize reset of reinitialized mbufs

 lib/mbuf/rte_mbuf.c |  61 +++++++++++++++
 lib/mbuf/rte_mbuf.h | 176 +++++++++++++++++++++++++-------------------
 2 files changed, 162 insertions(+), 75 deletions(-)

-- 
2.43.0

[PATCH v7 1/3] 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.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
v3:
* Removed experimental status for the new functions.
  Experimental is optional for new symbols, to allow for future API
  changes. But this has been around for a long time. (Stephen Hemminger)
* Consequentially, the added build check for ALLOW_EXPERIMENTAL_API with
  RTE_LIBRTE_MBUF_DEBUG is no longer needed, and was removed.
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 | 96 +++++++++++++++++++++++++++------------------
 2 files changed, 119 insertions(+), 38 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 9e7731a8a2..af39c13cf7 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_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_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..552cda1ae5 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,46 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info *shinfo,
 } while (0)
 
 
+/**
+ * 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.
+ */
+void
+rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const struct rte_mempool *mp);
+
+/**
+ * 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.
+ */
+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 +596,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 +630,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 +668,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 +689,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 +720,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 +1041,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 v7 2/3] mbuf: promote raw free and alloc bulk functions as stable

[Morten Brørup]

If ethdev drivers should use these APIs for allocating/freeing mbufs
instead of bypassing the mbuf library and accessing the mempool APIs, they
cannot be experimental anymore.

Also updated the packet mbuf alloc bulk function to use the raw alloc bulk
function, now that it is stable.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 552cda1ae5..49c93ab356 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -635,9 +635,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Allocate a bulk of uninitialized mbufs from mempool *mp*.
  *
  * This function can be used by PMDs (especially in Rx functions)
@@ -661,7 +658,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
  *   - 0: Success.
  *   - -ENOENT: Not enough entries in the mempool; no mbufs are retrieved.
  */
-__rte_experimental
 static __rte_always_inline int
 rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -694,9 +690,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Put a bulk of mbufs allocated from the same mempool back into the mempool.
  *
  * The caller must ensure that the mbufs come from the specified mempool,
@@ -716,7 +709,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
  * @param count
  *   Array size.
  */
-__rte_experimental
 static __rte_always_inline void
 rte_mbuf_raw_free_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -1029,7 +1021,7 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	unsigned idx = 0;
 	int rc;
 
-	rc = rte_mempool_get_bulk(pool, (void **)mbufs, count);
+	rc = rte_mbuf_raw_alloc_bulk(pool, mbufs, count);
 	if (unlikely(rc))
 		return rc;
 
@@ -1041,22 +1033,18 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
-- 
2.43.0

[PATCH v7 3/3] mbuf: optimize reset of reinitialized mbufs

[Morten Brørup]

An optimized function for resetting a bulk of newly allocated
reinitialized mbufs (a.k.a. raw mbufs) was added.

Compared to the normal packet mbuf reset function, it takes advantage of
the following two details:
1. The 'next' and 'nb_segs' fields are already reset, so resetting them
has been omitted.
2. When resetting the mbuf, the 'ol_flags' field must indicate whether the
mbuf uses an external buffer, and the 'data_off' field must not exceed the
data room size when resetting the data offset to include the default
headroom.
Unlike the normal packet mbuf reset function, which reads the mbuf itself
to get the information required for resetting these two fields, this
function gets the information from the mempool.

This makes the function write-only of the mbuf, unlike the normal packet
mbuf reset function, which is read-modify-write of the mbuf.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 74 ++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 49c93ab356..6f37a2e91e 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -954,6 +954,50 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
 					(uint16_t)m->buf_len);
 }
 
+/**
+ * Reset the fields of a bulk of packet mbufs to their default values.
+ *
+ * The caller must ensure that the mbufs come from the specified mempool,
+ * are direct and properly reinitialized (refcnt=1, next=NULL, nb_segs=1),
+ * as done by rte_pktmbuf_prefree_seg().
+ *
+ * This function should be used with care, when optimization is required.
+ * For standard needs, prefer rte_pktmbuf_reset().
+ *
+ * @param mp
+ *   The mempool to which the mbuf belongs.
+ * @param mbufs
+ *   Array of pointers to packet mbufs.
+ *   The array must not contain NULL pointers.
+ * @param count
+ *   Array size.
+ */
+static inline void
+rte_mbuf_raw_reset_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
+{
+	uint64_t ol_flags = (rte_pktmbuf_priv_flags(mp) & RTE_PKTMBUF_POOL_F_PINNED_EXT_BUF) ?
+			RTE_MBUF_F_EXTERNAL : 0;
+	uint16_t data_off = RTE_MIN_T(RTE_PKTMBUF_HEADROOM, rte_pktmbuf_data_room_size(mp),
+			uint16_t);
+
+	for (unsigned int idx = 0; idx < count; idx++) {
+		struct rte_mbuf *m = mbufs[idx];
+
+		m->pkt_len = 0;
+		m->tx_offload = 0;
+		m->vlan_tci = 0;
+		m->vlan_tci_outer = 0;
+		m->port = RTE_MBUF_PORT_INVALID;
+
+		m->ol_flags = ol_flags;
+		m->packet_type = 0;
+		m->data_off = data_off;
+
+		m->data_len = 0;
+		__rte_mbuf_sanity_check(m, 1);
+	}
+}
+
 /**
  * Reset the fields of a packet mbuf to their default values.
  *
@@ -997,7 +1041,7 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 {
 	struct rte_mbuf *m;
 	if ((m = rte_mbuf_raw_alloc(mp)) != NULL)
-		rte_pktmbuf_reset(m);
+		rte_mbuf_raw_reset_bulk(mp, &m, 1);
 	return m;
 }
 
@@ -1018,38 +1062,12 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	 struct rte_mbuf **mbufs, unsigned count)
 {
-	unsigned idx = 0;
 	int rc;
 
 	rc = rte_mbuf_raw_alloc_bulk(pool, (void **)mbufs, count);
 	if (unlikely(rc))
 		return rc;
-
-	/* To understand duff's device on loop unwinding optimization, see
-	 * https://en.wikipedia.org/wiki/Duff's_device.
-	 * Here while() loop is used rather than do() while{} to avoid extra
-	 * check if count is zero.
-	 */
-	switch (count % 4) {
-	case 0:
-		while (idx != count) {
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-	case 3:
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-	case 2:
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-	case 1:
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-		}
-	}
+	rte_mbuf_raw_reset_bulk(pool, mbufs, count);
 	return 0;
 }
 
-- 
2.43.0

[PATCH v8 0/3] mbuf: simplify handling of reinitialized mbufs

[Morten Brørup]

This series contains three changes:
- sanity checking a reinitialized mbuf (a.k.a. raw mbuf) was de-inlined
- reinitialized mbuf free and alloc bulk functions were promoted as stable
- resetting fields of reinitialized mbufs was optimized, making the reset
  operation write-only of the mbuf, instead of read-modify-write

Morten Brørup (3):
  mbuf: de-inline sanity checking a reinitialized mbuf
  mbuf: promote raw free and alloc bulk functions as stable
  mbuf: optimize reset of reinitialized mbufs

 lib/mbuf/rte_mbuf.c |  61 +++++++++++++++
 lib/mbuf/rte_mbuf.h | 176 +++++++++++++++++++++++++-------------------
 2 files changed, 162 insertions(+), 75 deletions(-)

-- 
2.43.0

[PATCH v8 1/3] 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.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
v3:
* Removed experimental status for the new functions.
  Experimental is optional for new symbols, to allow for future API
  changes. But this has been around for a long time. (Stephen Hemminger)
* Consequentially, the added build check for ALLOW_EXPERIMENTAL_API with
  RTE_LIBRTE_MBUF_DEBUG is no longer needed, and was removed.
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 | 96 +++++++++++++++++++++++++++------------------
 2 files changed, 119 insertions(+), 38 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 9e7731a8a2..af39c13cf7 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_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_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..552cda1ae5 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,46 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info *shinfo,
 } while (0)
 
 
+/**
+ * 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.
+ */
+void
+rte_mbuf_raw_sanity_check(const struct rte_mbuf *m, const struct rte_mempool *mp);
+
+/**
+ * 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.
+ */
+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 +596,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 +630,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 +668,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 +689,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 +720,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 +1041,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 v8 1/3] mbuf: de-inline sanity checking a reinitialized mbuf

[Bruce Richardson]

On Sat, Aug 23, 2025 at 06:30:00AM +0000, Morten Brørup wrote:
> 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.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> ---

Looks ok to me, one comment inline below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> v3:
> * Removed experimental status for the new functions.
>   Experimental is optional for new symbols, to allow for future API
>   changes. But this has been around for a long time. (Stephen Hemminger)
> * Consequentially, the added build check for ALLOW_EXPERIMENTAL_API with
>   RTE_LIBRTE_MBUF_DEBUG is no longer needed, and was removed.
> 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 | 96 +++++++++++++++++++++++++++------------------
>  2 files changed, 119 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> index 9e7731a8a2..af39c13cf7 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_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_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..552cda1ae5 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 */

This is in release mode, not debug mode. Comment below seems wrong too.

> +#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,46 @@ rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info *shinfo,
>  } while (0)

<snip>

RE: [PATCH v8 1/3] mbuf: de-inline sanity checking a reinitialized mbuf

[Morten Brørup]

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Thursday, 9 October 2025 18.50
> 
> On Sat, Aug 23, 2025 at 06:30:00AM +0000, Morten Brørup wrote:
> > 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.
> >
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> > ---
> 
> Looks ok to me, one comment inline below.

Thank you for reviewing, Bruce.
Comment replied to inline below. :-)

> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> > v3:
> > * Removed experimental status for the new functions.
> >   Experimental is optional for new symbols, to allow for future API
> >   changes. But this has been around for a long time. (Stephen
> Hemminger)
> > * Consequentially, the added build check for ALLOW_EXPERIMENTAL_API
> with
> >   RTE_LIBRTE_MBUF_DEBUG is no longer needed, and was removed.
> > 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 | 96 +++++++++++++++++++++++++++----------------
> --
> >  2 files changed, 119 insertions(+), 38 deletions(-)
> >
> > diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> > index 9e7731a8a2..af39c13cf7 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_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_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..552cda1ae5 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 */
> 
> This is in release mode, not debug mode. Comment below seems wrong too.

Yes, I noticed the comment was present in both debug and release mode, which I couldn't understand. So I guessed it was for Doxygen or some other parser.
I have seen weird stuff for Doxygen, e.g. "#ifdef __DOXYGEN__" for documenting a function [1], so I didn't attempt to understand the reason for it, but just followed the same pattern.

Also, the comment says that the macro does something in debug mode, so I guess that the comment is not incorrect (but misleading) when describing a release mode macro.
Again, I didn't understand it, so I left it as is.

[1]: https://elixir.bootlin.com/dpdk/v25.07/source/lib/eal/include/generic/rte_memcpy.h#L94

> 
> > +#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,46 @@ rte_mbuf_ext_refcnt_update(struct
> rte_mbuf_ext_shared_info *shinfo,
> >  } while (0)
> 
> <snip>

Re: [PATCH v8 1/3] mbuf: de-inline sanity checking a reinitialized mbuf

[Thomas Monjalon]

09/10/2025 19:12, Morten Brørup:
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > On Sat, Aug 23, 2025 at 06:30:00AM +0000, Morten Brørup wrote:
> > > +/**  check reinitialized mbuf type in debug mode */
> > 
> > This is in release mode, not debug mode. Comment below seems wrong too.
> 
> Yes, I noticed the comment was present in both debug and release mode,
> which I couldn't understand. So I guessed it was for Doxygen or some other parser.
> I have seen weird stuff for Doxygen, e.g. "#ifdef __DOXYGEN__"
> for documenting a function [1],
> so I didn't attempt to understand the reason for it,
> but just followed the same pattern.

Hum, as a maintainer, I would prefer you try to understand, or ask please.
Note: we can use Slack for such questions.

__DOXYGEN__ is defined only by Doxygen,
so any code inside #ifdef __DOXYGEN__ is for documentation only.
It was supposed to be used in lib/eal/include/generic
for functions which are really defined inline per CPU implementation.

RE: [PATCH v8 1/3] mbuf: de-inline sanity checking a reinitialized mbuf

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Thursday, 9 October 2025 19.29
> 
> 09/10/2025 19:12, Morten Brørup:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > On Sat, Aug 23, 2025 at 06:30:00AM +0000, Morten Brørup wrote:
> > > > +/**  check reinitialized mbuf type in debug mode */
> > >
> > > This is in release mode, not debug mode. Comment below seems wrong
> too.
> >
> > Yes, I noticed the comment was present in both debug and release
> mode,
> > which I couldn't understand. So I guessed it was for Doxygen or some
> other parser.
> > I have seen weird stuff for Doxygen, e.g. "#ifdef __DOXYGEN__"
> > for documenting a function [1],
> > so I didn't attempt to understand the reason for it,
> > but just followed the same pattern.
> 
> Hum, as a maintainer, I would prefer you try to understand, or ask
> please.
> Note: we can use Slack for such questions.

Point taken. Good guidance!
I'll try to broaden my scope of knowledge to include preprocessing for Doxygen.

> 
> __DOXYGEN__ is defined only by Doxygen,
> so any code inside #ifdef __DOXYGEN__ is for documentation only.
> It was supposed to be used in lib/eal/include/generic
> for functions which are really defined inline per CPU implementation.
> 
Yes, I get that.
But I don't get - when there are two definitions of a macro - why is the same comment/documentation present in both instances?
And if the instances have different comments/documentation, how is that reflected in the documentation output from Doxygen?

In this specific case, how should the code look to both 1) get the wanted Doxygen documentation output, and 2) have relevant comments inline in the source code for both instances?

[PATCH v8 2/3] mbuf: promote raw free and alloc bulk functions as stable

[Morten Brørup]

If ethdev drivers should use these APIs for allocating/freeing mbufs
instead of bypassing the mbuf library and accessing the mempool APIs, they
cannot be experimental anymore.

Also updated the packet mbuf alloc bulk function to use the raw alloc bulk
function, now that it is stable.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 552cda1ae5..49c93ab356 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -635,9 +635,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Allocate a bulk of uninitialized mbufs from mempool *mp*.
  *
  * This function can be used by PMDs (especially in Rx functions)
@@ -661,7 +658,6 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
  *   - 0: Success.
  *   - -ENOENT: Not enough entries in the mempool; no mbufs are retrieved.
  */
-__rte_experimental
 static __rte_always_inline int
 rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -694,9 +690,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 }
 
 /**
- * @warning
- * @b EXPERIMENTAL: This API may change, or be removed, without prior notice.
- *
  * Put a bulk of mbufs allocated from the same mempool back into the mempool.
  *
  * The caller must ensure that the mbufs come from the specified mempool,
@@ -716,7 +709,6 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
  * @param count
  *   Array size.
  */
-__rte_experimental
 static __rte_always_inline void
 rte_mbuf_raw_free_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
 {
@@ -1029,7 +1021,7 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	unsigned idx = 0;
 	int rc;
 
-	rc = rte_mempool_get_bulk(pool, (void **)mbufs, count);
+	rc = rte_mbuf_raw_alloc_bulk(pool, mbufs, count);
 	if (unlikely(rc))
 		return rc;
 
@@ -1041,22 +1033,18 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			__rte_mbuf_raw_sanity_check_mp(mbufs[idx], pool);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
-- 
2.43.0

Re: [PATCH v8 2/3] mbuf: promote raw free and alloc bulk functions as stable

[Bruce Richardson]

On Sat, Aug 23, 2025 at 06:30:01AM +0000, Morten Brørup wrote:
> If ethdev drivers should use these APIs for allocating/freeing mbufs
> instead of bypassing the mbuf library and accessing the mempool APIs, they
> cannot be experimental anymore.
> 

Not sure this is actually true. AFAIK there is no issue with internal use
of experimental functions (with certain caveats around use in macros or
inlines) since we will fixup any internal issues if the function changes.

However, no issue with making these stable.

> Also updated the packet mbuf alloc bulk function to use the raw alloc bulk
> function, now that it is stable.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v8 3/3] mbuf: optimize reset of reinitialized mbufs

[Morten Brørup]

An optimized function for resetting a bulk of newly allocated
reinitialized mbufs (a.k.a. raw mbufs) was added.

Compared to the normal packet mbuf reset function, it takes advantage of
the following two details:
1. The 'next' and 'nb_segs' fields are already reset, so resetting them
has been omitted.
2. When resetting the mbuf, the 'ol_flags' field must indicate whether the
mbuf uses an external buffer, and the 'data_off' field must not exceed the
data room size when resetting the data offset to include the default
headroom.
Unlike the normal packet mbuf reset function, which reads the mbuf itself
to get the information required for resetting these two fields, this
function gets the information from the mempool.

This makes the function write-only of the mbuf, unlike the normal packet
mbuf reset function, which is read-modify-write of the mbuf.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mbuf/rte_mbuf.h | 74 ++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 49c93ab356..6f37a2e91e 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -954,6 +954,50 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
 					(uint16_t)m->buf_len);
 }
 
+/**
+ * Reset the fields of a bulk of packet mbufs to their default values.
+ *
+ * The caller must ensure that the mbufs come from the specified mempool,
+ * are direct and properly reinitialized (refcnt=1, next=NULL, nb_segs=1),
+ * as done by rte_pktmbuf_prefree_seg().
+ *
+ * This function should be used with care, when optimization is required.
+ * For standard needs, prefer rte_pktmbuf_reset().
+ *
+ * @param mp
+ *   The mempool to which the mbuf belongs.
+ * @param mbufs
+ *   Array of pointers to packet mbufs.
+ *   The array must not contain NULL pointers.
+ * @param count
+ *   Array size.
+ */
+static inline void
+rte_mbuf_raw_reset_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
+{
+	uint64_t ol_flags = (rte_pktmbuf_priv_flags(mp) & RTE_PKTMBUF_POOL_F_PINNED_EXT_BUF) ?
+			RTE_MBUF_F_EXTERNAL : 0;
+	uint16_t data_off = RTE_MIN_T(RTE_PKTMBUF_HEADROOM, rte_pktmbuf_data_room_size(mp),
+			uint16_t);
+
+	for (unsigned int idx = 0; idx < count; idx++) {
+		struct rte_mbuf *m = mbufs[idx];
+
+		m->pkt_len = 0;
+		m->tx_offload = 0;
+		m->vlan_tci = 0;
+		m->vlan_tci_outer = 0;
+		m->port = RTE_MBUF_PORT_INVALID;
+
+		m->ol_flags = ol_flags;
+		m->packet_type = 0;
+		m->data_off = data_off;
+
+		m->data_len = 0;
+		__rte_mbuf_sanity_check(m, 1);
+	}
+}
+
 /**
  * Reset the fields of a packet mbuf to their default values.
  *
@@ -997,7 +1041,7 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 {
 	struct rte_mbuf *m;
 	if ((m = rte_mbuf_raw_alloc(mp)) != NULL)
-		rte_pktmbuf_reset(m);
+		rte_mbuf_raw_reset_bulk(mp, &m, 1);
 	return m;
 }
 
@@ -1018,38 +1062,12 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	 struct rte_mbuf **mbufs, unsigned count)
 {
-	unsigned idx = 0;
 	int rc;
 
 	rc = rte_mbuf_raw_alloc_bulk(pool, mbufs, count);
 	if (unlikely(rc))
 		return rc;
-
-	/* To understand duff's device on loop unwinding optimization, see
-	 * https://en.wikipedia.org/wiki/Duff's_device.
-	 * Here while() loop is used rather than do() while{} to avoid extra
-	 * check if count is zero.
-	 */
-	switch (count % 4) {
-	case 0:
-		while (idx != count) {
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-	case 3:
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-	case 2:
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-	case 1:
-			rte_pktmbuf_reset(mbufs[idx]);
-			idx++;
-			/* fall-through */
-		}
-	}
+	rte_mbuf_raw_reset_bulk(pool, mbufs, count);
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH v8 3/3] mbuf: optimize reset of reinitialized mbufs

[Stephen Hemminger]

On Sat, 23 Aug 2025 06:30:02 +0000
Morten Brørup <mb@smartsharesystems.com> wrote:

> An optimized function for resetting a bulk of newly allocated
> reinitialized mbufs (a.k.a. raw mbufs) was added.
> 
> Compared to the normal packet mbuf reset function, it takes advantage of
> the following two details:
> 1. The 'next' and 'nb_segs' fields are already reset, so resetting them
> has been omitted.

This doesn't matter, since they are on the same cache line the
operation of touching them costs nothing.

Re: [PATCH v8 3/3] mbuf: optimize reset of reinitialized mbufs

[Bruce Richardson]

On Sat, Aug 23, 2025 at 06:30:02AM +0000, Morten Brørup wrote:
> An optimized function for resetting a bulk of newly allocated
> reinitialized mbufs (a.k.a. raw mbufs) was added.
> 
> Compared to the normal packet mbuf reset function, it takes advantage of
> the following two details:
> 1. The 'next' and 'nb_segs' fields are already reset, so resetting them
> has been omitted.
> 2. When resetting the mbuf, the 'ol_flags' field must indicate whether the
> mbuf uses an external buffer, and the 'data_off' field must not exceed the
> data room size when resetting the data offset to include the default
> headroom.
> Unlike the normal packet mbuf reset function, which reads the mbuf itself
> to get the information required for resetting these two fields, this
> function gets the information from the mempool.
> 
> This makes the function write-only of the mbuf, unlike the normal packet
> mbuf reset function, which is read-modify-write of the mbuf.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/mbuf/rte_mbuf.h | 74 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 46 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
> index 49c93ab356..6f37a2e91e 100644
> --- a/lib/mbuf/rte_mbuf.h
> +++ b/lib/mbuf/rte_mbuf.h
> @@ -954,6 +954,50 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
>  					(uint16_t)m->buf_len);
>  }
>  
> +/**
> + * Reset the fields of a bulk of packet mbufs to their default values.
> + *
> + * The caller must ensure that the mbufs come from the specified mempool,
> + * are direct and properly reinitialized (refcnt=1, next=NULL, nb_segs=1),
> + * as done by rte_pktmbuf_prefree_seg().
> + *
> + * This function should be used with care, when optimization is required.
> + * For standard needs, prefer rte_pktmbuf_reset().
> + *
> + * @param mp
> + *   The mempool to which the mbuf belongs.
> + * @param mbufs
> + *   Array of pointers to packet mbufs.
> + *   The array must not contain NULL pointers.
> + * @param count
> + *   Array size.
> + */
> +static inline void
> +rte_mbuf_raw_reset_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs, unsigned int count)
> +{
> +	uint64_t ol_flags = (rte_pktmbuf_priv_flags(mp) & RTE_PKTMBUF_POOL_F_PINNED_EXT_BUF) ?
> +			RTE_MBUF_F_EXTERNAL : 0;
> +	uint16_t data_off = RTE_MIN_T(RTE_PKTMBUF_HEADROOM, rte_pktmbuf_data_room_size(mp),
> +			uint16_t);
> +
> +	for (unsigned int idx = 0; idx < count; idx++) {
> +		struct rte_mbuf *m = mbufs[idx];
> +
> +		m->pkt_len = 0;
> +		m->tx_offload = 0;
> +		m->vlan_tci = 0;
> +		m->vlan_tci_outer = 0;
> +		m->port = RTE_MBUF_PORT_INVALID;

Have you considered doing all initialization using 64-bit stores? It's
generally cheaper to do a single 64-bit store than e.g. set of 16-bit ones.
This also means that we could remove the restriction on having refcnt and
nb_segs already set. As in PMDs, a single store can init data_off, ref_cnt,
nb_segs and port.

Similarly for packet_type and pkt_len, and data_len/vlan_tci and rss fields
etc. For max performance, the whole of the mbuf cleared here can be done in
40 bytes, or 5 64-bit stores. If we do the stores in order, possibly the
compiler can even opportunistically coalesce more stores, so we could even
end up getting 128-bit or larger stores depending on the ISA compiled for.
[Maybe the compiler will do this even if they are not in order, but I'd
like to maximize my chances here! :-)]

/Bruce

> +
> +		m->ol_flags = ol_flags;
> +		m->packet_type = 0;
> +		m->data_off = data_off;
> +
> +		m->data_len = 0;
> +		__rte_mbuf_sanity_check(m, 1);
> +	}
> +}
> +
<snip>

RE: [PATCH v8 3/3] mbuf: optimize reset of reinitialized mbufs

[Morten Brørup]

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Thursday, 9 October 2025 19.15
> 
> On Sat, Aug 23, 2025 at 06:30:02AM +0000, Morten Brørup wrote:
> > An optimized function for resetting a bulk of newly allocated
> > reinitialized mbufs (a.k.a. raw mbufs) was added.
> >
> > Compared to the normal packet mbuf reset function, it takes advantage
> of
> > the following two details:
> > 1. The 'next' and 'nb_segs' fields are already reset, so resetting
> them
> > has been omitted.
> > 2. When resetting the mbuf, the 'ol_flags' field must indicate
> whether the
> > mbuf uses an external buffer, and the 'data_off' field must not
> exceed the
> > data room size when resetting the data offset to include the default
> > headroom.
> > Unlike the normal packet mbuf reset function, which reads the mbuf
> itself
> > to get the information required for resetting these two fields, this
> > function gets the information from the mempool.
> >
> > This makes the function write-only of the mbuf, unlike the normal
> packet
> > mbuf reset function, which is read-modify-write of the mbuf.
> >
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > ---
> >  lib/mbuf/rte_mbuf.h | 74 ++++++++++++++++++++++++++++---------------
> --
> >  1 file changed, 46 insertions(+), 28 deletions(-)
> >
> > diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
> > index 49c93ab356..6f37a2e91e 100644
> > --- a/lib/mbuf/rte_mbuf.h
> > +++ b/lib/mbuf/rte_mbuf.h
> > @@ -954,6 +954,50 @@ static inline void
> rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
> >  					(uint16_t)m->buf_len);
> >  }
> >
> > +/**
> > + * Reset the fields of a bulk of packet mbufs to their default
> values.
> > + *
> > + * The caller must ensure that the mbufs come from the specified
> mempool,
> > + * are direct and properly reinitialized (refcnt=1, next=NULL,
> nb_segs=1),
> > + * as done by rte_pktmbuf_prefree_seg().
> > + *
> > + * This function should be used with care, when optimization is
> required.
> > + * For standard needs, prefer rte_pktmbuf_reset().
> > + *
> > + * @param mp
> > + *   The mempool to which the mbuf belongs.
> > + * @param mbufs
> > + *   Array of pointers to packet mbufs.
> > + *   The array must not contain NULL pointers.
> > + * @param count
> > + *   Array size.
> > + */
> > +static inline void
> > +rte_mbuf_raw_reset_bulk(struct rte_mempool *mp, struct rte_mbuf
> **mbufs, unsigned int count)
> > +{
> > +	uint64_t ol_flags = (rte_pktmbuf_priv_flags(mp) &
> RTE_PKTMBUF_POOL_F_PINNED_EXT_BUF) ?
> > +			RTE_MBUF_F_EXTERNAL : 0;
> > +	uint16_t data_off = RTE_MIN_T(RTE_PKTMBUF_HEADROOM,
> rte_pktmbuf_data_room_size(mp),
> > +			uint16_t);
> > +
> > +	for (unsigned int idx = 0; idx < count; idx++) {
> > +		struct rte_mbuf *m = mbufs[idx];
> > +
> > +		m->pkt_len = 0;
> > +		m->tx_offload = 0;
> > +		m->vlan_tci = 0;
> > +		m->vlan_tci_outer = 0;
> > +		m->port = RTE_MBUF_PORT_INVALID;
> 
> Have you considered doing all initialization using 64-bit stores? It's
> generally cheaper to do a single 64-bit store than e.g. set of 16-bit
> ones.

The code is basically copy-paste from rte_pktmbuf_reset().
I kept it the same way for readability.

> This also means that we could remove the restriction on having refcnt
> and
> nb_segs already set. As in PMDs, a single store can init data_off,
> ref_cnt,
> nb_segs and port.

Yes, I have given the concept a lot of thought already.
If we didn't require mbufs residing in the mempool to have any fields initialized, specifically "next" and "nb_segs", it would improve performance for drivers freeing mbufs back to the mempool, because writing to the mbufs would no longer be required at that point; the mbufs could simply be freed back to the mempool. Instead, we would require the driver to initialize these fields - which it probably does on RX anyway, if it supports segmented packets.
But I consider this concept a major API change, also affecting applications assuming that these fields are initialized when allocating raw mbufs from the mempool. So I haven't pursued it.

> 
> Similarly for packet_type and pkt_len, and data_len/vlan_tci and rss
> fields
> etc. For max performance, the whole of the mbuf cleared here can be
> done in
> 40 bytes, or 5 64-bit stores. If we do the stores in order, possibly
> the
> compiler can even opportunistically coalesce more stores, so we could
> even
> end up getting 128-bit or larger stores depending on the ISA compiled
> for.
> [Maybe the compiler will do this even if they are not in order, but I'd
> like to maximize my chances here! :-)]
> 
> /Bruce
> 
> > +
> > +		m->ol_flags = ol_flags;
> > +		m->packet_type = 0;
> > +		m->data_off = data_off;
> > +
> > +		m->data_len = 0;
> > +		__rte_mbuf_sanity_check(m, 1);
> > +	}
> > +}
> > +
> <snip>

RE: [PATCH v8 0/3] mbuf: simplify handling of reinitialized mbufs

[Morten Brørup]

PING for review of this series.

Venlig hilsen / Kind regards,
-Morten Brørup


> -----Original Message-----
> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> Sent: Saturday, 23 August 2025 08.30
> To: dev@dpdk.org; Thomas Monjalon; Stephen Hemminger; Bruce Richardson;
> Konstantin Ananyev; Andrew Rybchenko; Ivan Malov; Chengwen Feng
> Cc: Morten Brørup
> Subject: [PATCH v8 0/3] mbuf: simplify handling of reinitialized mbufs
> 
> This series contains three changes:
> - sanity checking a reinitialized mbuf (a.k.a. raw mbuf) was de-inlined
> - reinitialized mbuf free and alloc bulk functions were promoted as
> stable
> - resetting fields of reinitialized mbufs was optimized, making the
> reset
>   operation write-only of the mbuf, instead of read-modify-write
> 
> Morten Brørup (3):
>   mbuf: de-inline sanity checking a reinitialized mbuf
>   mbuf: promote raw free and alloc bulk functions as stable
>   mbuf: optimize reset of reinitialized mbufs
> 
>  lib/mbuf/rte_mbuf.c |  61 +++++++++++++++
>  lib/mbuf/rte_mbuf.h | 176 +++++++++++++++++++++++++-------------------
>  2 files changed, 162 insertions(+), 75 deletions(-)
> 
> --
> 2.43.0