> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Sunday, 18 February 2024 13.40
>
> 15/02/2024 07:21, Tyler Retzlaff:
> > Provide a macro that allows conditional expansion of RTE_MARKER
> fields
> > to empty to allow rte_mbuf to be used with MSVC. It is proposed that
> > we announce the fields to be __rte_deprecated (currently disabled).
> >
> > Introduce C11 anonymous unions to permit aliasing of well-known
> > offsets by name into the rte_mbuf structure by a *new* name and to
> > provide padding for cache alignment.
> >
> > Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com>
> > ---
> > doc/guides/rel_notes/deprecation.rst | 20 ++
> > lib/eal/include/rte_common.h | 6 +
> > lib/mbuf/rte_mbuf_core.h | 375 +++++++++++++++++++------
> ----------
> > 3 files changed, 233 insertions(+), 168 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > index 10630ba..8594255 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -17,6 +17,26 @@ Other API and ABI deprecation notices are to be
> posted below.
> > Deprecation Notices
> > -------------------
> >
> > +* mbuf: Named zero sized fields of type ``RTE_MARKER`` and
> ``RTE_MARKER64``
> > + will be removed from ``struct rte_mbuf`` and replaced with new
> fields
> > + in anonymous unions.
> > +
> > + The names of the fields impacted are:
> > +
> > + old name new name
> > +
> > + ``cacheline0`` ``mbuf_cachelin0``
> > + ``rearm_data`` ``mbuf_rearm_data``
> > + ``rx_descriptor_fields1`` ``mbuf_rx_descriptor_fields1``
> > + ``cacheline1`` ``mbuf_cachelin1``
> > +
> > + Contributions to DPDK should immediately start using the new
> names,
> > + applications should adapt to new names as soon as possible as the
> > + old names will be removed in a future DPDK release.
> > +
> > + Note: types of the new names are not API compatible with the old
> and
> > + some code conversion is required to maintain correct behavior.
> > +
> > * build: The ``enable_kmods`` option is deprecated and will be
> removed in a future release.
> > Setting/clearing the option has no impact on the build.
> > Instead, kernel modules will be always built for OS's where out-
> of-tree kernel modules
> > diff --git a/lib/eal/include/rte_common.h
> b/lib/eal/include/rte_common.h
> > index d7d6390..af73f67 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -582,6 +582,12 @@ static void
> __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
> > /** Marker for 8B alignment in a structure. */
> > __extension__ typedef uint64_t RTE_MARKER64[0];
> >
> > +#define __rte_marker(type, name) type name /* __rte_deprecated */
> > +
> > +#else
> > +
> > +#define __rte_marker(type, name)
> > +
> > #endif
> >
> > /*********** Macros for calculating min and max **********/
> > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> > index 5688683..9e9590b 100644
> > --- a/lib/mbuf/rte_mbuf_core.h
> > +++ b/lib/mbuf/rte_mbuf_core.h
> > @@ -16,7 +16,10 @@
> > * New fields and flags should fit in the "dynamic space".
> > */
> >
> > +#include <assert.h>
> > +#include <stdalign.h>
> > #include <stdint.h>
> > +#include <stddef.h>
> >
> > #include <rte_byteorder.h>
> > #include <rte_stdatomic.h>
> > @@ -464,204 +467,240 @@ enum {
> > * The generic rte_mbuf, containing a packet mbuf.
> > */
> > struct rte_mbuf {
> > - RTE_MARKER cacheline0;
> > -
> > - void *buf_addr; /**< Virtual address of segment buffer.
> */
> > + __rte_marker(RTE_MARKER, cacheline0);
>
> You don't need to keep the first argument.
> This would be simpler:
> __rte_marker(cacheline0);
> You just need to create 2 functions: __rte_marker and __rte_marker64.
>
> You should replace all occurrences of RTE_MARKER in DPDK in one patch,
> and mark RTE_MARKER as deprecated (use #pragma GCC poison)
I like this suggestion.
However, some applications might use RTE_MARKER in their own structures.
Wouldn't it be considered API breakage to mark RTE_MARKER as deprecated?
>
> > + union {
> > + char mbuf_cacheline0[RTE_CACHE_LINE_MIN_SIZE];
> > + __extension__
> > + struct {
> > + void *buf_addr; /**< Virtual address of
> segment buffer.
>
> I think it is ugly.
>
> Changing mbuf API is a serious issue.
