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
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
Hi Morten, Thanks for this cleanup. Please see some comments below. On Wed, Sep 16, 2020 at 12:40:13PM +0200, 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. > > 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. I think this change is not related. This makes me think that a deprecation notice could be done for the old names without the RTE_ prefix, to be removed in 21.11. > 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) I don't see why this #if is needed. Wouldn't it work to have only one function definition with the __rte_unused attribute? > +/** > + * 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(). > + * Maybe indicate that these checks are only done when debug is on. > + * @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) It looks that MBUF_RAW_ALLOC_CHECK() is also used in drivers/net/sfc, I think it should be updated too. > > /** > * 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 I don't really like talking about "NULL". What do you think instead of this wording? /** Uninitialized or unspecified port */ > +/** 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 > Thanks, Olivier
Hi Morten,
Any update about this patch please?
07/10/2020 11:16, Olivier Matz:
> Hi Morten,
>
> Thanks for this cleanup. Please see some comments below.
>
> On Wed, Sep 16, 2020 at 12:40:13PM +0200, 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.
> >
> > 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.
>
> I think this change is not related.
>
> This makes me think that a deprecation notice could be done for the
> old names without the RTE_ prefix, to be removed in 21.11.
>
>
> > 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)
>
> I don't see why this #if is needed. Wouldn't it work to have only
> one function definition with the __rte_unused attribute?
>
> > +/**
> > + * 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().
> > + *
>
> Maybe indicate that these checks are only done when debug is on.
>
> > + * @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)
>
> It looks that MBUF_RAW_ALLOC_CHECK() is also used in drivers/net/sfc,
> I think it should be updated too.
>
> >
> > /**
> > * 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
>
> I don't really like talking about "NULL". What do you think instead of
> this wording?
>
> /** Uninitialized or unspecified port */
>
> > +/** For backwards compatibility. */
> > +#define MBUF_INVALID_PORT RTE_MBUF_PORT_INVALID
> >
> > /**
> > * A macro that points to an offset into the data in the mbuf.
>
> Thanks,
> Olivier
>
> From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Tuesday, October 20, 2020 1:56 PM > > Hi Morten, > Any update about this patch please? Thomas, v4 contains all the modifications suggested by Olivier: https://patchwork.dpdk.org/patch/82198/ Except a depreciation notice about the old names without RTE_. I'm not sure about the formal requirements for depreciation notices. If there are no formal requirements, please feel free to add a notice when merging. 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. > > 07/10/2020 11:16, Olivier Matz: > > Hi Morten, > > > > Thanks for this cleanup. Please see some comments below. > > > > On Wed, Sep 16, 2020 at 12:40:13PM +0200, 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. > > > > > > 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. > > > > I think this change is not related. > > > > This makes me think that a deprecation notice could be done for the > > old names without the RTE_ prefix, to be removed in 21.11. > > > > > > > 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) > > > > I don't see why this #if is needed. Wouldn't it work to have only > > one function definition with the __rte_unused attribute? > > > > > +/** > > > + * 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(). > > > + * > > > > Maybe indicate that these checks are only done when debug is on. > > > > > + * @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) > > > > It looks that MBUF_RAW_ALLOC_CHECK() is also used in drivers/net/sfc, > > I think it should be updated too. > > > > > > > > /** > > > * 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 > > > > I don't really like talking about "NULL". What do you think instead of > > this wording? > > > > /** Uninitialized or unspecified port */ > > > > > +/** For backwards compatibility. */ > > > +#define MBUF_INVALID_PORT RTE_MBUF_PORT_INVALID > > > > > > /** > > > * A macro that points to an offset into the data in the mbuf. > > > > Thanks, > > Olivier > > >