[PATCH] mbuf: replace GCC marker extension with C11 anonymous unions

[Morten Brørup]

> From: Tyler Retzlaff [mailto:roretzla at linux.microsoft.com]
> Sent: Wednesday, 31 January 2024 21.46
> 
> On Wed, Jan 31, 2024 at 01:49:34PM +0000, Bruce Richardson wrote:
> > On Tue, Jan 30, 2024 at 03:26:13PM -0800, Tyler Retzlaff wrote:
> > > Replace the use of RTE_MARKER<x> with C11 anonymous unions to
> improve
> > > code portability between toolchains.
> > >
> > > Update use of rte_mbuf rearm_data field in net/ionic, net/sfc and
> > > net/virtio which were accessing field as a zero-length array.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> > > ---
> > >  drivers/net/ionic/ionic_lif.c               |   8 +-
> > >  drivers/net/ionic/ionic_rxtx_sg.c           |   4 +-
> > >  drivers/net/ionic/ionic_rxtx_simple.c       |   2 +-
> > >  drivers/net/sfc/sfc_ef100_rx.c              |   8 +-
> > >  drivers/net/sfc/sfc_ef10_rx.c               |  12 +--
> > >  drivers/net/virtio/virtio_rxtx_packed_avx.h |   8 +-
> > >  lib/mbuf/rte_mbuf_core.h                    | 135 +++++++++++++++-
> ------------
> > >  7 files changed, 94 insertions(+), 83 deletions(-)
> > >
> > <snip>
> > @@ -464,9 +464,10 @@ enum {
> > >   * The generic rte_mbuf, containing a packet mbuf.
> > >   */
> > >  struct rte_mbuf {
> > > -	RTE_MARKER cacheline0;
> > > -
> > > -	void *buf_addr;           /**< Virtual address of segment buffer.
> */
> > > +	union {
> > > +	    void *cacheline0;
> > > +	    void *buf_addr;           /**< Virtual address of segment
> buffer. */
> > > +	};
> >
> > This marker is never used, so we should just look to drop it. I think
> it
> > was originally added to have an equivalent to the cacheline1 marker.
> 
> it's actually got a use in one location.
> 
> rte_mbuf.h:
> 
> static inline void
> rte_mbuf_prefetch_part1(struct rte_mbuf *m)
> {	
>         rte_prefetch0(&m->cacheline0);
> }	
> 
> > However, that would be an ABI change, so I'm ok to have this as-is
> for now.

Typo: API change, not ABI change.

> 
> do you mean api change? just asking to make sure i understand what i'm
> doing.
> 
> as i understand how this extension (marker) works removing the
> cacheline0 marker would not alter the layout of the struct. that is the
> sizeof the struct, sizeof any field nor the offset of any field changes
> would change by the marker removal.

Correctly understood, Tyler.

The struct layout is unmodified, so it's not an ABI change.
However, it's an API change, because applications cannot access the field anymore.

Although DPDK itself doesn't use the field, other applications might use rte_prefetch0(&m->cacheline0) instead of rte_mbuf_prefetch_part1(m).
After checking in-house, I can mention at least one company doing that. ;-)

We should keep the cacheline0 field and not break the API. Not for my sake, but for other applications. :-)