[PATCH v6 20/23] mbuf: remove and stop using rte marker fields

[David Marchand]

On Tue, Feb 27, 2024 at 6:44 AM Tyler Retzlaff
<roretzla at linux.microsoft.com> wrote:
>
> RTE_MARKER typedefs are a GCC extension unsupported by MSVC. Remove
> RTE_MARKER fields from rte_mbuf struct.
>
> Maintain alignment of fields after removed cacheline1 marker by placing
> C11 alignas(RTE_CACHE_LINE_MIN_SIZE).
>
> Update implementation of rte_mbuf_prefetch_part1() and
> rte_mbuf_prefetch_part2() inline functions calculate pointer for
> prefetch of cachline0 and cachline1 without using removed markers.
>
> Update static_assert of rte_mbuf struct fields to reference data_off and
> packet_type fields that occupy the original offsets of the marker
> fields.
>
> Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> ---
>  doc/guides/rel_notes/release_24_03.rst |  9 ++++++++
>  lib/mbuf/rte_mbuf.h                    |  4 ++--
>  lib/mbuf/rte_mbuf_core.h               | 39 +++++++++++++---------------------
>  3 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst
> index 879bb49..67750f2 100644
> --- a/doc/guides/rel_notes/release_24_03.rst
> +++ b/doc/guides/rel_notes/release_24_03.rst
> @@ -156,6 +156,15 @@ Removed Items
>    The application reserved statically defined logtypes ``RTE_LOGTYPE_USER1..RTE_LOGTYPE_USER8``
>    are still defined.
>
> +* mbuf: ``RTE_MARKER`` fields ``cacheline0`` ``cacheline1``
> +  ``rx_descriptor_fields1`` and ``RTE_MARKER64`` field ``rearm_data``
> +  have been removed from ``struct rte_mbuf``.
> +  Prefetch of ``cacheline0`` and ``cacheline1`` may be achieved through
> +  ``rte_mbuf_prefetch_part1()`` and ``rte_mbuf_prefetch_part2()`` inline
> +  functions respectively.
> +  Access to ``rearm_data`` and ``rx_descriptor_fields1`` should be
> +  through new inline functions ``rte_mbuf_rearm_data()`` and
> +  ``rte_mbuf_rx_descriptor_fields1()`` respectively.
>
>  API Changes
>  -----------
> diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
> index aa7495b..61cda20 100644
> --- a/lib/mbuf/rte_mbuf.h
> +++ b/lib/mbuf/rte_mbuf.h
> @@ -108,7 +108,7 @@
>  static inline void
>  rte_mbuf_prefetch_part1(struct rte_mbuf *m)
>  {
> -       rte_prefetch0(&m->cacheline0);
> +       rte_prefetch0(m);
>  }
>
>  /**
> @@ -126,7 +126,7 @@
>  rte_mbuf_prefetch_part2(struct rte_mbuf *m)
>  {
>  #if RTE_CACHE_LINE_SIZE == 64
> -       rte_prefetch0(&m->cacheline1);
> +       rte_prefetch0(RTE_PTR_ADD(m, RTE_CACHE_LINE_MIN_SIZE));
>  #else
>         RTE_SET_USED(m);
>  #endif
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index 36551c2..4e06f15 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -18,6 +18,7 @@
>
>  #include <assert.h>
>  #include <stddef.h>
> +#include <stdalign.h>
>  #include <stdint.h>
>
>  #include <rte_common.h>
> @@ -467,8 +468,6 @@ enum {
>   * The generic rte_mbuf, containing a packet mbuf.
>   */
>  struct rte_mbuf {
> -       RTE_MARKER cacheline0;
> -
>         void *buf_addr;           /**< Virtual address of segment buffer. */
>  #if RTE_IOVA_IN_MBUF
>         /**
> @@ -495,7 +494,6 @@ struct rte_mbuf {
>          * To obtain a pointer to rearm_data use the rte_mbuf_rearm_data()
>          * accessor instead of directly referencing through the data_off field.
>          */
> -       RTE_MARKER64 rearm_data;
>         uint16_t data_off;

One subtile change of removing the marker is that fields may not be
aligned as before.

#if RTE_IOVA_IN_MBUF
        /**
         * Physical address of segment buffer.
         * This field is undefined if the build is configured to use only
         * virtual address as IOVA (i.e. RTE_IOVA_IN_MBUF is 0).
         * Force alignment to 8-bytes, so as to ensure we have the exact
         * same mbuf cacheline0 layout for 32-bit and 64-bit. This makes
         * working on vector drivers easier.
         */
        rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t));
#else
        /**
         * Next segment of scattered packet.
         * This field is valid when physical address field is undefined.
         * Otherwise next pointer in the second cache line will be used.
         */
        struct rte_mbuf *next;
#endif

When building ! RTE_IOVA_IN_MBUF on a 32 bits arch, the next pointer
is not force aligned to 64bits.
Which has a cascade effect on data_off alignement.

In file included from ../lib/mbuf/rte_mbuf_core.h:19,
                 from ../lib/mbuf/rte_mbuf.h:42,
                 from ../lib/mbuf/rte_mbuf_dyn.c:18:
../lib/mbuf/rte_mbuf_core.h:676:1: error: static assertion failed: "data_off"
  676 | static_assert(!(offsetof(struct rte_mbuf, data_off) !=
      | ^~~~~~~~~~~~~


I hope reviewers pay attention to the alignment changes when removing
those markers.
This is not trivial to catch in the CI.


-- 
David Marchand