[PATCH v4 01/18] mbuf: deprecate GCC marker in rte mbuf struct

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> Sent: Sunday, 18 February 2024 16.22
> 
> 18/02/2024 14:07, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > > 15/02/2024 07:21, Tyler Retzlaff:
> > > > --- 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?
> 
> Yes it is an API breakage.
> Do we prefer waiting 24.11 for poisoning this macro?

Personally, I generally don't mind API breakages, assuming that they are visible and thus don't silently introduce bugs.
The distro people might have a different opinion.