From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C34D843A26; Wed, 31 Jan 2024 23:55:39 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9F518402C5; Wed, 31 Jan 2024 23:55:39 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 175034026C for ; Wed, 31 Jan 2024 23:55:38 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id AF39F2079B; Wed, 31 Jan 2024 23:55:37 +0100 (CET) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] mbuf: replace GCC marker extension with C11 anonymous unions X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 31 Jan 2024 23:55:35 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F1D8@smartserver.smartshare.dk> In-Reply-To: <20240131204558.GC11576@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] mbuf: replace GCC marker extension with C11 anonymous unions Thread-Index: AdpUhoHsCXLlO2VQTJiKLQlcXNKbVwAEHKFg References: <1706657173-26166-1-git-send-email-roretzla@linux.microsoft.com> <1706657173-26166-2-git-send-email-roretzla@linux.microsoft.com> <20240131204558.GC11576@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , "Bruce Richardson" Cc: , "Andrew Boyer" , "Andrew Rybchenko" , "Chenbo Xia" , "Konstantin Ananyev" , "Maxime Coquelin" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > Sent: Wednesday, 31 January 2024 21.46 >=20 > 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 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 > > > --- > > > 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(-) > > > > > > > @@ -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. >=20 > it's actually got a use in one location. >=20 > rte_mbuf.h: >=20 > static inline void > rte_mbuf_prefetch_part1(struct rte_mbuf *m) > {=09 > rte_prefetch0(&m->cacheline0); > }=09 >=20 > > However, that would be an ABI change, so I'm ok to have this as-is > for now. Typo: API change, not ABI change. >=20 > do you mean api change? just asking to make sure i understand what i'm > doing. >=20 > 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. :-)