From patchwork Tue Jan 30 23:26:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Retzlaff X-Patchwork-Id: 136224 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2D46B43A13; Wed, 31 Jan 2024 00:26:22 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E93FE427E5; Wed, 31 Jan 2024 00:26:17 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 5C84D40266 for ; Wed, 31 Jan 2024 00:26:15 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 9A5BB20B2001; Tue, 30 Jan 2024 15:26:14 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9A5BB20B2001 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1706657174; bh=9Jv+SWKaSnZmeQkr/VWAa5pHlL0z48Xtz6pJzRgqnV0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rbfo0cB27D1hC7sZLwwjeZihFjErz7WYZfkszQgz5t7jHuGlpwIZxjdQavzEkfYu1 hcXNUElbZL2sk09dzifXhUMiP4QaM7iTAO2tPxXw96WNbslTPIzgl+tENF+vKUhM0c YToYVJYHv0HhCDAVurMVSCTq/jw3uv5/VaeK1MaU= From: Tyler Retzlaff To: dev@dpdk.org Cc: Andrew Boyer , Andrew Rybchenko , Bruce Richardson , Chenbo Xia , Konstantin Ananyev , Maxime Coquelin , Tyler Retzlaff Subject: [PATCH] mbuf: replace GCC marker extension with C11 anonymous unions Date: Tue, 30 Jan 2024 15:26:13 -0800 Message-Id: <1706657173-26166-2-git-send-email-roretzla@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1706657173-26166-1-git-send-email-roretzla@linux.microsoft.com> References: <1706657173-26166-1-git-send-email-roretzla@linux.microsoft.com> 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 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(-) diff --git a/drivers/net/ionic/ionic_lif.c b/drivers/net/ionic/ionic_lif.c index 25b490d..fd99f39 100644 --- a/drivers/net/ionic/ionic_lif.c +++ b/drivers/net/ionic/ionic_lif.c @@ -725,8 +725,8 @@ rte_compiler_barrier(); - RTE_BUILD_BUG_ON(sizeof(rxm.rearm_data[0]) != sizeof(uint64_t)); - return rxm.rearm_data[0]; + RTE_BUILD_BUG_ON(sizeof(rxm.rearm_data) != sizeof(uint64_t)); + return rxm.rearm_data; } static uint64_t @@ -743,8 +743,8 @@ rte_compiler_barrier(); - RTE_BUILD_BUG_ON(sizeof(rxm.rearm_data[0]) != sizeof(uint64_t)); - return rxm.rearm_data[0]; + RTE_BUILD_BUG_ON(sizeof(rxm.rearm_data) != sizeof(uint64_t)); + return rxm.rearm_data; } int diff --git a/drivers/net/ionic/ionic_rxtx_sg.c b/drivers/net/ionic/ionic_rxtx_sg.c index ab8e56e..a569dd1 100644 --- a/drivers/net/ionic/ionic_rxtx_sg.c +++ b/drivers/net/ionic/ionic_rxtx_sg.c @@ -285,7 +285,7 @@ info[0] = NULL; /* Set the mbuf metadata based on the cq entry */ - rxm->rearm_data[0] = rxq->rearm_data; + rxm->rearm_data = rxq->rearm_data; rxm->pkt_len = cq_desc_len; rxm->data_len = RTE_MIN(rxq->hdr_seg_size, cq_desc_len); left = cq_desc_len - rxm->data_len; @@ -298,7 +298,7 @@ info[i] = NULL; /* Set the chained mbuf metadata */ - rxm_seg->rearm_data[0] = rxq->rearm_seg_data; + rxm_seg->rearm_data = rxq->rearm_seg_data; rxm_seg->data_len = RTE_MIN(rxq->seg_size, left); left -= rxm_seg->data_len; diff --git a/drivers/net/ionic/ionic_rxtx_simple.c b/drivers/net/ionic/ionic_rxtx_simple.c index 5f81856..1978610 100644 --- a/drivers/net/ionic/ionic_rxtx_simple.c +++ b/drivers/net/ionic/ionic_rxtx_simple.c @@ -256,7 +256,7 @@ info[0] = NULL; /* Set the mbuf metadata based on the cq entry */ - rxm->rearm_data[0] = rxq->rearm_data; + rxm->rearm_data = rxq->rearm_data; rxm->pkt_len = cq_desc_len; rxm->data_len = cq_desc_len; diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c index 2677003..23918d5 100644 --- a/drivers/net/sfc/sfc_ef100_rx.c +++ b/drivers/net/sfc/sfc_ef100_rx.c @@ -553,9 +553,9 @@ struct sfc_ef100_rxq { pkt = sfc_ef100_rx_next_mbuf(rxq); __rte_mbuf_raw_sanity_check(pkt); - RTE_BUILD_BUG_ON(sizeof(pkt->rearm_data[0]) != + RTE_BUILD_BUG_ON(sizeof(pkt->rearm_data) != sizeof(rxq->rearm_data)); - pkt->rearm_data[0] = rxq->rearm_data; + pkt->rearm_data = rxq->rearm_data; /* data_off already moved past Rx prefix */ rx_prefix = (const efx_xword_t *)sfc_ef100_rx_pkt_prefix(pkt); @@ -759,8 +759,8 @@ struct sfc_ef100_rxq { /* rearm_data covers structure members filled in above */ rte_compiler_barrier(); - RTE_BUILD_BUG_ON(sizeof(m.rearm_data[0]) != sizeof(uint64_t)); - return m.rearm_data[0]; + RTE_BUILD_BUG_ON(sizeof(m.rearm_data) != sizeof(uint64_t)); + return m.rearm_data; } static sfc_dp_rx_qcreate_t sfc_ef100_rx_qcreate; diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c index 30a320d..60bc098 100644 --- a/drivers/net/sfc/sfc_ef10_rx.c +++ b/drivers/net/sfc/sfc_ef10_rx.c @@ -322,8 +322,8 @@ struct sfc_ef10_rxq { m = rxd->mbuf; - RTE_BUILD_BUG_ON(sizeof(m->rearm_data[0]) != sizeof(rxq->rearm_data)); - m->rearm_data[0] = rxq->rearm_data; + RTE_BUILD_BUG_ON(sizeof(m->rearm_data) != sizeof(rxq->rearm_data)); + m->rearm_data = rxq->rearm_data; /* Classify packet based on Rx event */ /* Mask RSS hash offload flag if RSS is not enabled */ @@ -377,9 +377,9 @@ struct sfc_ef10_rxq { rxq->completed = pending; } - RTE_BUILD_BUG_ON(sizeof(m->rearm_data[0]) != + RTE_BUILD_BUG_ON(sizeof(m->rearm_data) != sizeof(rxq->rearm_data)); - m->rearm_data[0] = rxq->rearm_data; + m->rearm_data = rxq->rearm_data; /* Event-dependent information is the same */ m->ol_flags = m0->ol_flags; @@ -633,8 +633,8 @@ struct sfc_ef10_rxq { /* rearm_data covers structure members filled in above */ rte_compiler_barrier(); - RTE_BUILD_BUG_ON(sizeof(m.rearm_data[0]) != sizeof(uint64_t)); - return m.rearm_data[0]; + RTE_BUILD_BUG_ON(sizeof(m.rearm_data) != sizeof(uint64_t)); + return m.rearm_data; } static sfc_dp_rx_qcreate_t sfc_ef10_rx_qcreate; diff --git a/drivers/net/virtio/virtio_rxtx_packed_avx.h b/drivers/net/virtio/virtio_rxtx_packed_avx.h index 584ac72..a9ce53f 100644 --- a/drivers/net/virtio/virtio_rxtx_packed_avx.h +++ b/drivers/net/virtio/virtio_rxtx_packed_avx.h @@ -36,10 +36,10 @@ /* Load four mbufs rearm data */ RTE_BUILD_BUG_ON(REFCNT_BITS_OFFSET >= 64); RTE_BUILD_BUG_ON(SEG_NUM_BITS_OFFSET >= 64); - __m256i mbufs = _mm256_set_epi64x(*tx_pkts[3]->rearm_data, - *tx_pkts[2]->rearm_data, - *tx_pkts[1]->rearm_data, - *tx_pkts[0]->rearm_data); + __m256i mbufs = _mm256_set_epi64x(tx_pkts[3]->rearm_data, + tx_pkts[2]->rearm_data, + tx_pkts[1]->rearm_data, + tx_pkts[0]->rearm_data); /* refcnt=1 and nb_segs=1 */ __m256i mbuf_ref = _mm256_set1_epi64x(DEFAULT_REARM_DATA); diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h index 5688683..d731ea0 100644 --- a/lib/mbuf/rte_mbuf_core.h +++ b/lib/mbuf/rte_mbuf_core.h @@ -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. */ + }; #if RTE_IOVA_IN_MBUF /** * Physical address of segment buffer. @@ -487,69 +488,77 @@ struct rte_mbuf { #endif /* next 8 bytes are initialised on RX descriptor rearm */ - RTE_MARKER64 rearm_data; - uint16_t data_off; - - /** - * Reference counter. Its size should at least equal to the size - * of port field (16 bits), to support zero-copy broadcast. - * It should only be accessed using the following functions: - * rte_mbuf_refcnt_update(), rte_mbuf_refcnt_read(), and - * rte_mbuf_refcnt_set(). The functionality of these functions (atomic, - * or non-atomic) is controlled by the RTE_MBUF_REFCNT_ATOMIC flag. - */ - RTE_ATOMIC(uint16_t) refcnt; + union { + uint64_t rearm_data; + struct { + uint16_t data_off; + + /** + * Reference counter. Its size should at least equal to the size + * of port field (16 bits), to support zero-copy broadcast. + * It should only be accessed using the following functions: + * rte_mbuf_refcnt_update(), rte_mbuf_refcnt_read(), and + * rte_mbuf_refcnt_set(). The functionality of these functions (atomic, + * or non-atomic) is controlled by the RTE_MBUF_REFCNT_ATOMIC flag. + */ + RTE_ATOMIC(uint16_t) refcnt; - /** - * Number of segments. Only valid for the first segment of an mbuf - * chain. - */ - uint16_t nb_segs; + /** + * Number of segments. Only valid for the first segment of an mbuf + * chain. + */ + uint16_t nb_segs; - /** Input port (16 bits to support more than 256 virtual ports). - * The event eth Tx adapter uses this field to specify the output port. - */ - uint16_t port; + /** Input port (16 bits to support more than 256 virtual ports). + * The event eth Tx adapter uses this field to specify the output port. + */ + uint16_t port; - uint64_t ol_flags; /**< Offload features. */ + uint64_t ol_flags; /**< Offload features. */ + }; + }; /* remaining bytes are set on RX when pulling packet from descriptor */ - RTE_MARKER rx_descriptor_fields1; - - /* - * The packet type, which is the combination of outer/inner L2, L3, L4 - * and tunnel types. The packet_type is about data really present in the - * mbuf. Example: if vlan stripping is enabled, a received vlan packet - * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the - * vlan is stripped from the data. - */ union { - uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */ - __extension__ + void *rx_descriptor_fields1; + + /* + * The packet type, which is the combination of outer/inner L2, L3, L4 + * and tunnel types. The packet_type is about data really present in the + * mbuf. Example: if vlan stripping is enabled, a received vlan packet + * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the + * vlan is stripped from the data. + */ struct { - uint8_t l2_type:4; /**< (Outer) L2 type. */ - uint8_t l3_type:4; /**< (Outer) L3 type. */ - uint8_t l4_type:4; /**< (Outer) L4 type. */ - uint8_t tun_type:4; /**< Tunnel type. */ union { - uint8_t inner_esp_next_proto; - /**< ESP next protocol type, valid if - * RTE_PTYPE_TUNNEL_ESP tunnel type is set - * on both Tx and Rx. - */ + uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */ __extension__ struct { - uint8_t inner_l2_type:4; - /**< Inner L2 type. */ - uint8_t inner_l3_type:4; - /**< Inner L3 type. */ + uint8_t l2_type:4; /**< (Outer) L2 type. */ + uint8_t l3_type:4; /**< (Outer) L3 type. */ + uint8_t l4_type:4; /**< (Outer) L4 type. */ + uint8_t tun_type:4; /**< Tunnel type. */ + union { + uint8_t inner_esp_next_proto; + /**< ESP next protocol type, valid if + * RTE_PTYPE_TUNNEL_ESP tunnel type is set + * on both Tx and Rx. + */ + __extension__ + struct { + uint8_t inner_l2_type:4; + /**< Inner L2 type. */ + uint8_t inner_l3_type:4; + /**< Inner L3 type. */ + }; + }; + uint8_t inner_l4_type:4; /**< Inner L4 type. */ }; }; - uint8_t inner_l4_type:4; /**< Inner L4 type. */ + uint32_t pkt_len; /**< Total pkt len: sum of all segments. */ }; }; - uint32_t pkt_len; /**< Total pkt len: sum of all segments. */ uint16_t data_len; /**< Amount of data in segment buffer. */ /** VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_VLAN is set. */ uint16_t vlan_tci; @@ -595,21 +604,23 @@ struct rte_mbuf { struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ /* second cache line - fields only used in slow path or on TX */ - RTE_MARKER cacheline1 __rte_cache_min_aligned; + union { + void *cacheline1; #if RTE_IOVA_IN_MBUF - /** - * Next segment of scattered packet. Must be NULL in the last - * segment or in case of non-segmented packet. - */ - struct rte_mbuf *next; + /** + * Next segment of scattered packet. Must be NULL in the last + * segment or in case of non-segmented packet. + */ + struct rte_mbuf *next; #else - /** - * Reserved for dynamic fields - * when the next pointer is in first cache line (i.e. RTE_IOVA_IN_MBUF is 0). - */ - uint64_t dynfield2; + /** + * Reserved for dynamic fields + * when the next pointer is in first cache line (i.e. RTE_IOVA_IN_MBUF is 0). + */ + uint64_t dynfield2; #endif + }; /* fields to support TX offloads */ union {