[dpdk-dev] [PATCH 0/1] mbuf: minor cleanup

[dpdk-dev] [PATCH 0/1] mbuf: minor cleanup

[Morten Brørup]

The mbuf header files had some commenting style errors that affected the
API documentation.
Also, the RTE_ prefix was missing on a macro and a definition.

This cleanup was also discussed here:
http://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35C6111E@smartserver.smartshare.dk/

Please note:
This patch does not touch the offload and attachment flags that are missing
the RTE_ prefix. I would rather leave that to another patch from someone who
can also write a devtools script to help converting drivers and applications
to the new names.


Morten Brørup (1):
  mbuf: minor cleanup

 lib/librte_mbuf/rte_mbuf.h      | 47 ++++++++++++++++++++-------------
 lib/librte_mbuf/rte_mbuf_core.h |  9 ++++---
 2 files changed, 35 insertions(+), 21 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 1/1] mbuf: minor cleanup

[Morten Brørup]

The mbuf header files had some commenting style errors that affected the
API documentation.
Also, the RTE_ prefix was missing on a macro and a definition.

Note: This patch does not touch the offload and attachment flags that are
also missing the RTE_ prefix.

Changes only affecting documentation:
* Removed the MBUF_INVALID_PORT definition from rte_mbuf.h; it is
  already defined in rte_mbuf_core.h.
  This removal also reestablished the description of the
  rte_pktmbuf_reset() function.
* Corrected the comment related to RTE_MBUF_MAX_NB_SEGS.
* Corrected the comment related to PKT_TX_QINQ_PKT.

Changes regarding missing RTE_ prefix:
* Converted the MBUF_RAW_ALLOC_CHECK() macro to an
  __rte_mbuf_raw_sanity_check() inline function.
  Added backwards compatible macro with the original name.
* Renamed the MBUF_INVALID_PORT definition to RTE_MBUF_PORT_INVALID.
  Added backwards compatible definition with the original name.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/librte_mbuf/rte_mbuf.h      | 47 ++++++++++++++++++++-------------
 lib/librte_mbuf/rte_mbuf_core.h |  9 ++++---
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 7259575a7..ec48da67d 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -554,12 +554,28 @@ __rte_experimental
 int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
 		   const char **reason);
 
-#define MBUF_RAW_ALLOC_CHECK(m) do {				\
-	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);		\
-	RTE_ASSERT((m)->next == NULL);				\
-	RTE_ASSERT((m)->nb_segs == 1);				\
-	__rte_mbuf_sanity_check(m, 0);				\
-} 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.
+ */
+static __rte_always_inline void
+__rte_mbuf_raw_sanity_check(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_mbuf_sanity_check(m, 0);
+}
+/** For backwards compatibility. */
+#define MBUF_RAW_ALLOC_CHECK(m) __rte_mbuf_raw_sanity_check(m)
 
 /**
  * Allocate an uninitialized mbuf from mempool *mp*.
@@ -586,7 +602,7 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 
 	if (rte_mempool_get(mp, (void **)&m) < 0)
 		return NULL;
-	MBUF_RAW_ALLOC_CHECK(m);
+	__rte_mbuf_raw_sanity_check(m);
 	return m;
 }
 
@@ -609,10 +625,7 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 {
 	RTE_ASSERT(!RTE_MBUF_CLONED(m) &&
 		  (!RTE_MBUF_HAS_EXTBUF(m) || RTE_MBUF_HAS_PINNED_EXTBUF(m)));
-	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);
-	RTE_ASSERT(m->next == NULL);
-	RTE_ASSERT(m->nb_segs == 1);
-	__rte_mbuf_sanity_check(m, 0);
+	__rte_mbuf_raw_sanity_check(m);
 	rte_mempool_put(m->pool, m);
 }
 
@@ -858,8 +871,6 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
  * @param m
  *   The packet mbuf to be reset.
  */
-#define MBUF_INVALID_PORT UINT16_MAX
-
 static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 {
 	m->next = NULL;
@@ -868,7 +879,7 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 	m->vlan_tci = 0;
 	m->vlan_tci_outer = 0;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 
 	m->ol_flags &= EXT_ATTACHED_MBUF;
 	m->packet_type = 0;
@@ -931,22 +942,22 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
index 8cd7137ac..4ac5609e3 100644
--- a/lib/librte_mbuf/rte_mbuf_core.h
+++ b/lib/librte_mbuf/rte_mbuf_core.h
@@ -272,7 +272,7 @@ extern "C" {
  * mbuf 'vlan_tci' & 'vlan_tci_outer' must be valid when this flag is set.
  */
 #define PKT_TX_QINQ        (1ULL << 49)
-/* this old name is deprecated */
+/** This old name is deprecated. */
 #define PKT_TX_QINQ_PKT    PKT_TX_QINQ
 
 /**
@@ -686,7 +686,7 @@ struct rte_mbuf_ext_shared_info {
 	};
 };
 
-/**< Maximum number of nb_segs allowed. */
+/** Maximum number of nb_segs allowed. */
 #define RTE_MBUF_MAX_NB_SEGS	UINT16_MAX
 
 /**
@@ -714,7 +714,10 @@ struct rte_mbuf_ext_shared_info {
 #define RTE_MBUF_DIRECT(mb) \
 	(!((mb)->ol_flags & (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF)))
 
-#define MBUF_INVALID_PORT UINT16_MAX
+/** NULL value for the uint16_t port type. */
+#define RTE_MBUF_PORT_INVALID UINT16_MAX
+/** For backwards compatibility. */
+#define MBUF_INVALID_PORT RTE_MBUF_PORT_INVALID
 
 /**
  * A macro that points to an offset into the data in the mbuf.
-- 
2.17.1

[dpdk-dev] [PATCH v3] mbuf: minor cleanup

[Morten Brørup]

The mbuf header files had some commenting style errors that affected the
API documentation.
Also, the RTE_ prefix was missing on a macro and a definition.

Note: This patch does not touch the offload and attachment flags that are
also missing the RTE_ prefix.

Changes only affecting documentation:
* Removed the MBUF_INVALID_PORT definition from rte_mbuf.h; it is
  already defined in rte_mbuf_core.h.
  This removal also reestablished the description of the
  rte_pktmbuf_reset() function.
* Corrected the comment related to RTE_MBUF_MAX_NB_SEGS.
* Corrected the comment related to PKT_TX_QINQ_PKT.

Changes regarding missing RTE_ prefix:
* Converted the MBUF_RAW_ALLOC_CHECK() macro to an
  __rte_mbuf_raw_sanity_check() inline function.
  Added backwards compatible macro with the original name.
* Renamed the MBUF_INVALID_PORT definition to RTE_MBUF_PORT_INVALID.
  Added backwards compatible definition with the original name.

v2:
* Use RTE_MBUF_PORT_INVALID instead of MBUF_INVALID_PORT in rte_mbuf.c.

v3:
* The functions/macros used in __rte_mbuf_raw_sanity_check() require
  RTE_ENABLE_ASSERT or RTE_LIBRTE_MBUF_DEBUG, or they don't use the mbuf
  parameter, which generates a compiler waning. So mark the mbuf parameter
  __rte_unused if none of them are defined.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 doc/guides/rel_notes/deprecation.rst |  7 ----
 lib/librte_mbuf/rte_mbuf.c           |  4 +-
 lib/librte_mbuf/rte_mbuf.h           | 55 +++++++++++++++++++---------
 lib/librte_mbuf/rte_mbuf_core.h      |  9 +++--
 4 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 279eccb04..88d7d0761 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -294,13 +294,6 @@ Deprecation Notices
   - https://patches.dpdk.org/patch/71457/
   - https://patches.dpdk.org/patch/71456/
 
-* rawdev: The rawdev APIs which take a device-specific structure as
-  parameter directly, or indirectly via a "private" pointer inside another
-  structure, will be modified to take an additional parameter of the
-  structure size. The affected APIs will include ``rte_rawdev_info_get``,
-  ``rte_rawdev_configure``, ``rte_rawdev_queue_conf_get`` and
-  ``rte_rawdev_queue_setup``.
-
 * acl: ``RTE_ACL_CLASSIFY_NUM`` enum value will be removed.
   This enum value is not used inside DPDK, while it prevents to add new
   classify algorithms without causing an ABI breakage.
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 8a456e5e6..53a015311 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -104,7 +104,7 @@ rte_pktmbuf_init(struct rte_mempool *mp,
 	/* init some constant fields */
 	m->pool = mp;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 	rte_mbuf_refcnt_set(m, 1);
 	m->next = NULL;
 }
@@ -207,7 +207,7 @@ __rte_pktmbuf_init_extmem(struct rte_mempool *mp,
 	/* init some constant fields */
 	m->pool = mp;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 	m->ol_flags = EXT_ATTACHED_MBUF;
 	rte_mbuf_refcnt_set(m, 1);
 	m->next = NULL;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 7259575a7..406d3abb2 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -554,12 +554,36 @@ __rte_experimental
 int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
 		   const char **reason);
 
-#define MBUF_RAW_ALLOC_CHECK(m) do {				\
-	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);		\
-	RTE_ASSERT((m)->next == NULL);				\
-	RTE_ASSERT((m)->nb_segs == 1);				\
-	__rte_mbuf_sanity_check(m, 0);				\
-} while (0)
+#if defined(RTE_ENABLE_ASSERT) || defined(RTE_LIBRTE_MBUF_DEBUG)
+/**
+ * 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.
+ */
+static __rte_always_inline void
+__rte_mbuf_raw_sanity_check(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_mbuf_sanity_check(m, 0);
+}
+#else
+static __rte_always_inline void
+__rte_mbuf_raw_sanity_check(const struct rte_mbuf *m __rte_unused)
+{
+    /* Nothing here. */
+}
+#endif
+/** For backwards compatibility. */
+#define MBUF_RAW_ALLOC_CHECK(m) __rte_mbuf_raw_sanity_check(m)
 
 /**
  * Allocate an uninitialized mbuf from mempool *mp*.
@@ -586,7 +610,7 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 
 	if (rte_mempool_get(mp, (void **)&m) < 0)
 		return NULL;
-	MBUF_RAW_ALLOC_CHECK(m);
+	__rte_mbuf_raw_sanity_check(m);
 	return m;
 }
 
@@ -609,10 +633,7 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 {
 	RTE_ASSERT(!RTE_MBUF_CLONED(m) &&
 		  (!RTE_MBUF_HAS_EXTBUF(m) || RTE_MBUF_HAS_PINNED_EXTBUF(m)));
-	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);
-	RTE_ASSERT(m->next == NULL);
-	RTE_ASSERT(m->nb_segs == 1);
-	__rte_mbuf_sanity_check(m, 0);
+	__rte_mbuf_raw_sanity_check(m);
 	rte_mempool_put(m->pool, m);
 }
 
@@ -858,8 +879,6 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
  * @param m
  *   The packet mbuf to be reset.
  */
-#define MBUF_INVALID_PORT UINT16_MAX
-
 static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 {
 	m->next = NULL;
@@ -868,7 +887,7 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 	m->vlan_tci = 0;
 	m->vlan_tci_outer = 0;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 
 	m->ol_flags &= EXT_ATTACHED_MBUF;
 	m->packet_type = 0;
@@ -931,22 +950,22 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
index 8cd7137ac..4ac5609e3 100644
--- a/lib/librte_mbuf/rte_mbuf_core.h
+++ b/lib/librte_mbuf/rte_mbuf_core.h
@@ -272,7 +272,7 @@ extern "C" {
  * mbuf 'vlan_tci' & 'vlan_tci_outer' must be valid when this flag is set.
  */
 #define PKT_TX_QINQ        (1ULL << 49)
-/* this old name is deprecated */
+/** This old name is deprecated. */
 #define PKT_TX_QINQ_PKT    PKT_TX_QINQ
 
 /**
@@ -686,7 +686,7 @@ struct rte_mbuf_ext_shared_info {
 	};
 };
 
-/**< Maximum number of nb_segs allowed. */
+/** Maximum number of nb_segs allowed. */
 #define RTE_MBUF_MAX_NB_SEGS	UINT16_MAX
 
 /**
@@ -714,7 +714,10 @@ struct rte_mbuf_ext_shared_info {
 #define RTE_MBUF_DIRECT(mb) \
 	(!((mb)->ol_flags & (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF)))
 
-#define MBUF_INVALID_PORT UINT16_MAX
+/** NULL value for the uint16_t port type. */
+#define RTE_MBUF_PORT_INVALID UINT16_MAX
+/** For backwards compatibility. */
+#define MBUF_INVALID_PORT RTE_MBUF_PORT_INVALID
 
 /**
  * A macro that points to an offset into the data in the mbuf.
-- 
2.17.1

[dpdk-dev] [PATCH v4] mbuf: minor cleanup

[Morten Brørup]

The mbuf header files had some commenting style errors that affected the
API documentation.
Also, the RTE_ prefix was missing on a macro and a definition.

Note: This patch does not touch the offload and attachment flags that are
also missing the RTE_ prefix.

Changes only affecting documentation:
* Removed the MBUF_INVALID_PORT definition from rte_mbuf.h; it is
  already defined in rte_mbuf_core.h.
  This removal also reestablished the description of the
  rte_pktmbuf_reset() function.
* Corrected the comment related to RTE_MBUF_MAX_NB_SEGS.
* Corrected the comment related to PKT_TX_QINQ_PKT.

Changes regarding missing RTE_ prefix:
* Converted the MBUF_RAW_ALLOC_CHECK() macro to an
  __rte_mbuf_raw_sanity_check() inline function.
  Added backwards compatible macro with the original name.
* Renamed the MBUF_INVALID_PORT definition to RTE_MBUF_PORT_INVALID.
  Added backwards compatible definition with the original name.

v2:
* Use RTE_MBUF_PORT_INVALID instead of MBUF_INVALID_PORT in rte_mbuf.c.

v3:
* The functions/macros used in __rte_mbuf_raw_sanity_check() require
  RTE_ENABLE_ASSERT or RTE_LIBRTE_MBUF_DEBUG, or they don't use the mbuf
  parameter, which generates a compiler waning. So mark the mbuf parameter
  __rte_unused if none of them are defined.

v4:
* Removed compile time variants of __rte_mbuf_raw_sanity_check(); keeping
  the one variant where the mbuf parameter is marked __rte_unused.
* Reflect in the __rte_mbuf_raw_sanity_check() description headline that
  it is for debug mode.
* Improved the description of RTE_MBUF_PORT_INVALID.
* Updated sfc driver to use __rte_mbuf_raw_sanity_check() instead of
  MBUF_RAW_ALLOC_CHECK().

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 drivers/net/sfc/sfc_ef100_rx.c     |  6 ++--
 drivers/net/sfc/sfc_ef10_essb_rx.c |  4 +--
 drivers/net/sfc/sfc_ef10_rx.c      |  4 +--
 drivers/net/sfc/sfc_rx.c           |  2 +-
 lib/librte_mbuf/rte_mbuf.c         |  4 +--
 lib/librte_mbuf/rte_mbuf.h         | 48 +++++++++++++++++++-----------
 lib/librte_mbuf/rte_mbuf_core.h    |  9 ++++--
 7 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c
index 5e761601b..c1c56d0e7 100644
--- a/drivers/net/sfc/sfc_ef100_rx.c
+++ b/drivers/net/sfc/sfc_ef100_rx.c
@@ -168,7 +168,7 @@ sfc_ef100_rx_qrefill(struct sfc_ef100_rxq *rxq)
 			struct sfc_ef100_rx_sw_desc *rxd;
 			rte_iova_t phys_addr;
 
-			MBUF_RAW_ALLOC_CHECK(m);
+			__rte_mbuf_raw_sanity_check(m);
 
 			SFC_ASSERT((id & ~ptr_mask) == 0);
 			rxd = &rxq->sw_ring[id];
@@ -481,7 +481,7 @@ sfc_ef100_rx_process_ready_pkts(struct sfc_ef100_rxq *rxq,
 		rxq->ready_pkts--;
 
 		pkt = sfc_ef100_rx_next_mbuf(rxq);
-		MBUF_RAW_ALLOC_CHECK(pkt);
+		__rte_mbuf_raw_sanity_check(pkt);
 
 		RTE_BUILD_BUG_ON(sizeof(pkt->rearm_data[0]) !=
 				 sizeof(rxq->rearm_data));
@@ -505,7 +505,7 @@ sfc_ef100_rx_process_ready_pkts(struct sfc_ef100_rxq *rxq,
 			struct rte_mbuf *seg;
 
 			seg = sfc_ef100_rx_next_mbuf(rxq);
-			MBUF_RAW_ALLOC_CHECK(seg);
+			__rte_mbuf_raw_sanity_check(seg);
 
 			seg->data_off = RTE_PKTMBUF_HEADROOM;
 
diff --git a/drivers/net/sfc/sfc_ef10_essb_rx.c b/drivers/net/sfc/sfc_ef10_essb_rx.c
index 97c81c823..b167e01c5 100644
--- a/drivers/net/sfc/sfc_ef10_essb_rx.c
+++ b/drivers/net/sfc/sfc_ef10_essb_rx.c
@@ -125,7 +125,7 @@ sfc_ef10_essb_next_mbuf(const struct sfc_ef10_essb_rxq *rxq,
 	struct rte_mbuf *m;
 
 	m = (struct rte_mbuf *)((uintptr_t)mbuf + rxq->buf_stride);
-	MBUF_RAW_ALLOC_CHECK(m);
+	__rte_mbuf_raw_sanity_check(m);
 	return m;
 }
 
@@ -136,7 +136,7 @@ sfc_ef10_essb_mbuf_by_index(const struct sfc_ef10_essb_rxq *rxq,
 	struct rte_mbuf *m;
 
 	m = (struct rte_mbuf *)((uintptr_t)mbuf + idx * rxq->buf_stride);
-	MBUF_RAW_ALLOC_CHECK(m);
+	__rte_mbuf_raw_sanity_check(m);
 	return m;
 }
 
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index a43554a13..0a1bc1878 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -148,7 +148,7 @@ sfc_ef10_rx_qrefill(struct sfc_ef10_rxq *rxq)
 			struct sfc_ef10_rx_sw_desc *rxd;
 			rte_iova_t phys_addr;
 
-			MBUF_RAW_ALLOC_CHECK(m);
+			__rte_mbuf_raw_sanity_check(m);
 
 			SFC_ASSERT((id & ~ptr_mask) == 0);
 			rxd = &rxq->sw_ring[id];
@@ -297,7 +297,7 @@ sfc_ef10_rx_process_event(struct sfc_ef10_rxq *rxq, efx_qword_t rx_ev,
 		rxd = &rxq->sw_ring[pending++ & ptr_mask];
 		m = rxd->mbuf;
 
-		MBUF_RAW_ALLOC_CHECK(m);
+		__rte_mbuf_raw_sanity_check(m);
 
 		m->data_off = RTE_PKTMBUF_HEADROOM;
 		rte_pktmbuf_data_len(m) = seg_len;
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index de0773b8a..3415dcad7 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -109,7 +109,7 @@ sfc_efx_rx_qrefill(struct sfc_efx_rxq *rxq)
 		     ++i, id = (id + 1) & rxq->ptr_mask) {
 			m = objs[i];
 
-			MBUF_RAW_ALLOC_CHECK(m);
+			__rte_mbuf_raw_sanity_check(m);
 
 			rxd = &rxq->sw_desc[id];
 			rxd->mbuf = m;
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 8a456e5e6..53a015311 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -104,7 +104,7 @@ rte_pktmbuf_init(struct rte_mempool *mp,
 	/* init some constant fields */
 	m->pool = mp;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 	rte_mbuf_refcnt_set(m, 1);
 	m->next = NULL;
 }
@@ -207,7 +207,7 @@ __rte_pktmbuf_init_extmem(struct rte_mempool *mp,
 	/* init some constant fields */
 	m->pool = mp;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 	m->ol_flags = EXT_ATTACHED_MBUF;
 	rte_mbuf_refcnt_set(m, 1);
 	m->next = NULL;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index a1414ed7c..f2cd395d2 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -540,12 +540,29 @@ __rte_experimental
 int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
 		   const char **reason);
 
-#define MBUF_RAW_ALLOC_CHECK(m) do {				\
-	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);		\
-	RTE_ASSERT((m)->next == NULL);				\
-	RTE_ASSERT((m)->nb_segs == 1);				\
-	__rte_mbuf_sanity_check(m, 0);				\
-} while (0)
+/**
+ * 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_mbuf_sanity_check(m, 0);
+}
+
+/** For backwards compatibility. */
+#define MBUF_RAW_ALLOC_CHECK(m) __rte_mbuf_raw_sanity_check(m)
 
 /**
  * Allocate an uninitialized mbuf from mempool *mp*.
@@ -572,7 +589,7 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
 
 	if (rte_mempool_get(mp, (void **)&m) < 0)
 		return NULL;
-	MBUF_RAW_ALLOC_CHECK(m);
+	__rte_mbuf_raw_sanity_check(m);
 	return m;
 }
 
@@ -595,10 +612,7 @@ rte_mbuf_raw_free(struct rte_mbuf *m)
 {
 	RTE_ASSERT(!RTE_MBUF_CLONED(m) &&
 		  (!RTE_MBUF_HAS_EXTBUF(m) || RTE_MBUF_HAS_PINNED_EXTBUF(m)));
-	RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);
-	RTE_ASSERT(m->next == NULL);
-	RTE_ASSERT(m->nb_segs == 1);
-	__rte_mbuf_sanity_check(m, 0);
+	__rte_mbuf_raw_sanity_check(m);
 	rte_mempool_put(m->pool, m);
 }
 
@@ -844,8 +858,6 @@ static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
  * @param m
  *   The packet mbuf to be reset.
  */
-#define MBUF_INVALID_PORT UINT16_MAX
-
 static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 {
 	m->next = NULL;
@@ -854,7 +866,7 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 	m->vlan_tci = 0;
 	m->vlan_tci_outer = 0;
 	m->nb_segs = 1;
-	m->port = MBUF_INVALID_PORT;
+	m->port = RTE_MBUF_PORT_INVALID;
 
 	m->ol_flags &= EXT_ATTACHED_MBUF;
 	m->packet_type = 0;
@@ -917,22 +929,22 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 	switch (count % 4) {
 	case 0:
 		while (idx != count) {
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 3:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 2:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
 	case 1:
-			MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+			__rte_mbuf_raw_sanity_check(mbufs[idx]);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
 			/* fall-through */
diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
index 065d87d28..f68d467bd 100644
--- a/lib/librte_mbuf/rte_mbuf_core.h
+++ b/lib/librte_mbuf/rte_mbuf_core.h
@@ -279,7 +279,7 @@ extern "C" {
  * mbuf 'vlan_tci' & 'vlan_tci_outer' must be valid when this flag is set.
  */
 #define PKT_TX_QINQ        (1ULL << 49)
-/* this old name is deprecated */
+/** This old name is deprecated. */
 #define PKT_TX_QINQ_PKT    PKT_TX_QINQ
 
 /**
@@ -679,7 +679,7 @@ struct rte_mbuf_ext_shared_info {
 	uint16_t refcnt;
 };
 
-/**< Maximum number of nb_segs allowed. */
+/** Maximum number of nb_segs allowed. */
 #define RTE_MBUF_MAX_NB_SEGS	UINT16_MAX
 
 /**
@@ -707,7 +707,10 @@ struct rte_mbuf_ext_shared_info {
 #define RTE_MBUF_DIRECT(mb) \
 	(!((mb)->ol_flags & (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF)))
 
-#define MBUF_INVALID_PORT UINT16_MAX
+/** Uninitialized or unspecified port. */
+#define RTE_MBUF_PORT_INVALID UINT16_MAX
+/** For backwards compatibility. */
+#define MBUF_INVALID_PORT RTE_MBUF_PORT_INVALID
 
 /**
  * A macro that points to an offset into the data in the mbuf.
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4] mbuf: minor cleanup

[Olivier Matz]

On Mon, Oct 26, 2020 at 03:42:01PM +0100, Morten Brørup wrote:
> The mbuf header files had some commenting style errors that affected the
> API documentation.
> Also, the RTE_ prefix was missing on a macro and a definition.
> 
> Note: This patch does not touch the offload and attachment flags that are
> also missing the RTE_ prefix.
> 
> Changes only affecting documentation:
> * Removed the MBUF_INVALID_PORT definition from rte_mbuf.h; it is
>   already defined in rte_mbuf_core.h.
>   This removal also reestablished the description of the
>   rte_pktmbuf_reset() function.
> * Corrected the comment related to RTE_MBUF_MAX_NB_SEGS.
> * Corrected the comment related to PKT_TX_QINQ_PKT.
> 
> Changes regarding missing RTE_ prefix:
> * Converted the MBUF_RAW_ALLOC_CHECK() macro to an
>   __rte_mbuf_raw_sanity_check() inline function.
>   Added backwards compatible macro with the original name.
> * Renamed the MBUF_INVALID_PORT definition to RTE_MBUF_PORT_INVALID.
>   Added backwards compatible definition with the original name.
> 
> v2:
> * Use RTE_MBUF_PORT_INVALID instead of MBUF_INVALID_PORT in rte_mbuf.c.
> 
> v3:
> * The functions/macros used in __rte_mbuf_raw_sanity_check() require
>   RTE_ENABLE_ASSERT or RTE_LIBRTE_MBUF_DEBUG, or they don't use the mbuf
>   parameter, which generates a compiler waning. So mark the mbuf parameter
>   __rte_unused if none of them are defined.
> 
> v4:
> * Removed compile time variants of __rte_mbuf_raw_sanity_check(); keeping
>   the one variant where the mbuf parameter is marked __rte_unused.
> * Reflect in the __rte_mbuf_raw_sanity_check() description headline that
>   it is for debug mode.
> * Improved the description of RTE_MBUF_PORT_INVALID.
> * Updated sfc driver to use __rte_mbuf_raw_sanity_check() instead of
>   MBUF_RAW_ALLOC_CHECK().
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks!

Re: [dpdk-dev] [PATCH v4] mbuf: minor cleanup

[Thomas Monjalon]

05/11/2020 14:36, Olivier Matz:
> On Mon, Oct 26, 2020 at 03:42:01PM +0100, Morten Brørup wrote:
> > The mbuf header files had some commenting style errors that affected the
> > API documentation.
> > Also, the RTE_ prefix was missing on a macro and a definition.
> > 
> > Note: This patch does not touch the offload and attachment flags that are
> > also missing the RTE_ prefix.
> > 
> > Changes only affecting documentation:
> > * Removed the MBUF_INVALID_PORT definition from rte_mbuf.h; it is
> >   already defined in rte_mbuf_core.h.
> >   This removal also reestablished the description of the
> >   rte_pktmbuf_reset() function.
> > * Corrected the comment related to RTE_MBUF_MAX_NB_SEGS.
> > * Corrected the comment related to PKT_TX_QINQ_PKT.
> > 
> > Changes regarding missing RTE_ prefix:
> > * Converted the MBUF_RAW_ALLOC_CHECK() macro to an
> >   __rte_mbuf_raw_sanity_check() inline function.
> >   Added backwards compatible macro with the original name.
> > * Renamed the MBUF_INVALID_PORT definition to RTE_MBUF_PORT_INVALID.
> >   Added backwards compatible definition with the original name.
> > 
> > v2:
> > * Use RTE_MBUF_PORT_INVALID instead of MBUF_INVALID_PORT in rte_mbuf.c.
> > 
> > v3:
> > * The functions/macros used in __rte_mbuf_raw_sanity_check() require
> >   RTE_ENABLE_ASSERT or RTE_LIBRTE_MBUF_DEBUG, or they don't use the mbuf
> >   parameter, which generates a compiler waning. So mark the mbuf parameter
> >   __rte_unused if none of them are defined.
> > 
> > v4:
> > * Removed compile time variants of __rte_mbuf_raw_sanity_check(); keeping
> >   the one variant where the mbuf parameter is marked __rte_unused.
> > * Reflect in the __rte_mbuf_raw_sanity_check() description headline that
> >   it is for debug mode.
> > * Improved the description of RTE_MBUF_PORT_INVALID.
> > * Updated sfc driver to use __rte_mbuf_raw_sanity_check() instead of
> >   MBUF_RAW_ALLOC_CHECK().
> > 
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Thanks!

Applied, thanks