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 3205443ABA; Tue, 13 Feb 2024 07:45:51 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1F02F40691; Tue, 13 Feb 2024 07:45:46 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 44AE5402AD for ; Tue, 13 Feb 2024 07:45:43 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 7465B207ECB4; Mon, 12 Feb 2024 22:45:42 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 7465B207ECB4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1707806742; bh=fdnAKnFetdr0NPQ6w7Xybqeod6Ws1cpFCeI2YnuQ0JE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dlxAAxdbyIOYRDAxvmt+l/MY12ks12J/d2brIJOMUJTIh7gOqFC9HP8xLnhzzd/5u UN3FBrrNtxK/c3JwyAT0+43vPikC8V3DdJzHy7xz0AE7i1Vn4m8F4aOXGVqGOikA2E oo5hTGr2S4Fe6hIH2+7tHLVwdh7zcT1jQRPGxy+E= From: Tyler Retzlaff To: dev@dpdk.org Cc: Andrew Boyer , Andrew Rybchenko , Bruce Richardson , Chenbo Xia , Konstantin Ananyev , Maxime Coquelin , mb@smartsharesystems.com, Tyler Retzlaff Subject: [PATCH v2] mbuf: replace GCC marker extension with C11 anonymous unions Date: Mon, 12 Feb 2024 22:45:41 -0800 Message-Id: <1707806741-29694-2-git-send-email-roretzla@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1707806741-29694-1-git-send-email-roretzla@linux.microsoft.com> References: <1706657173-26166-2-git-send-email-roretzla@linux.microsoft.com> <1707806741-29694-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, net/ixgbe 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/ixgbe/ixgbe_rxtx_vec_sse.c | 8 +- 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 | 276 ++++++++++++++++------------ 8 files changed, 179 insertions(+), 147 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/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c index f60808d..bc0525b 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c @@ -98,10 +98,10 @@ desc_to_olflags_v_ipsec(__m128i descs[4], struct rte_mbuf **rx_pkts) { __m128i sterr, rearm, tmp_e, tmp_p; - uint32_t *rearm0 = (uint32_t *)rx_pkts[0]->rearm_data + 2; - uint32_t *rearm1 = (uint32_t *)rx_pkts[1]->rearm_data + 2; - uint32_t *rearm2 = (uint32_t *)rx_pkts[2]->rearm_data + 2; - uint32_t *rearm3 = (uint32_t *)rx_pkts[3]->rearm_data + 2; + uint32_t *rearm0 = (uint32_t *)&rx_pkts[0]->rearm_data + 2; + uint32_t *rearm1 = (uint32_t *)&rx_pkts[1]->rearm_data + 2; + uint32_t *rearm2 = (uint32_t *)&rx_pkts[2]->rearm_data + 2; + uint32_t *rearm3 = (uint32_t *)&rx_pkts[3]->rearm_data + 2; const __m128i ipsec_sterr_msk = _mm_set1_epi32(IXGBE_RXDADV_IPSEC_STATUS_SECP | IXGBE_RXDADV_IPSEC_ERROR_AUTH_FAILED); 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..3867c19 100644 --- a/lib/mbuf/rte_mbuf_core.h +++ b/lib/mbuf/rte_mbuf_core.h @@ -464,152 +464,179 @@ enum { * The generic rte_mbuf, containing a packet mbuf. */ struct rte_mbuf { - RTE_MARKER cacheline0; - - void *buf_addr; /**< Virtual address of segment buffer. */ + union { + struct { + union { + void *cacheline0; + void *buf_addr; /**< Virtual address of segment buffer. */ + }; #if RTE_IOVA_IN_MBUF - /** - * Physical address of segment buffer. - * This field is undefined if the build is configured to use only - * virtual address as IOVA (i.e. RTE_IOVA_IN_MBUF is 0). - * Force alignment to 8-bytes, so as to ensure we have the exact - * same mbuf cacheline0 layout for 32-bit and 64-bit. This makes - * working on vector drivers easier. - */ - rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t)); + /** + * Physical address of segment buffer. + * This field is undefined if the build is configured to use only + * virtual address as IOVA (i.e. RTE_IOVA_IN_MBUF is 0). + * Force alignment to 8-bytes, so as to ensure we have the exact + * same mbuf cacheline0 layout for 32-bit and 64-bit. This makes + * working on vector drivers easier. + */ + rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t)); #else - /** - * Next segment of scattered packet. - * This field is valid when physical address field is undefined. - * Otherwise next pointer in the second cache line will be used. - */ - struct rte_mbuf *next; + /** + * Next segment of scattered packet. + * This field is valid when physical address field is undefined. + * Otherwise next pointer in the second cache line will be used. + */ + struct rte_mbuf *next; #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; - - /** - * 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; - - uint64_t ol_flags; /**< Offload features. */ + /* next 8 bytes are initialised on RX descriptor rearm */ + 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; + + /** 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; + }; + }; - /* remaining bytes are set on RX when pulling packet from descriptor */ - RTE_MARKER rx_descriptor_fields1; + uint64_t ol_flags; /**< Offload features. */ - /* - * 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__ - 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. */ + /* remaining bytes are set on RX when pulling packet from descriptor */ 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. + 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. */ - __extension__ struct { - uint8_t inner_l2_type:4; - /**< Inner L2 type. */ - uint8_t inner_l3_type:4; - /**< Inner L3 type. */ + union { + /** < L2/L3/L4 and tunnel information. */ + uint32_t packet_type; + __extension__ + struct { + /**< (Outer) L2 type. */ + uint8_t l2_type:4; + /**< (Outer) L3 type. */ + uint8_t l3_type:4; + /**< (Outer) L4 type. */ + uint8_t l4_type:4; + /**< Tunnel type. */ + uint8_t tun_type:4; + 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. */ + }; + }; + /**< Inner L4 type. */ + uint8_t inner_l4_type:4; + }; + }; + /**< Total pkt len: sum of all segments. */ + uint32_t pkt_len; }; }; - uint8_t inner_l4_type:4; /**< Inner L4 type. */ - }; - }; - 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; + 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; - union { - union { - uint32_t rss; /**< RSS hash result if RSS enabled */ - struct { + union { union { + uint32_t rss; /**< RSS hash result if RSS enabled */ struct { - uint16_t hash; - uint16_t id; - }; - uint32_t lo; - /**< Second 4 flexible bytes */ - }; - uint32_t hi; - /**< First 4 flexible bytes or FD ID, dependent - * on RTE_MBUF_F_RX_FDIR_* flag in ol_flags. - */ - } fdir; /**< Filter identifier if FDIR enabled */ - struct rte_mbuf_sched sched; - /**< Hierarchical scheduler : 8 bytes */ - struct { - uint32_t reserved1; - uint16_t reserved2; - uint16_t txq; - /**< The event eth Tx adapter uses this field - * to store Tx queue id. - * @see rte_event_eth_tx_adapter_txq_set() - */ - } txadapter; /**< Eventdev ethdev Tx adapter */ - uint32_t usr; - /**< User defined tags. See rte_distributor_process() */ - } hash; /**< hash information */ - }; + union { + struct { + uint16_t hash; + uint16_t id; + }; + uint32_t lo; + /**< Second 4 flexible bytes */ + }; + uint32_t hi; + /**< First 4 flexible bytes or FD ID, dependent + * on RTE_MBUF_F_RX_FDIR_* flag in ol_flags. + */ + } fdir; /**< Filter identifier if FDIR enabled */ + struct rte_mbuf_sched sched; + /**< Hierarchical scheduler : 8 bytes */ + struct { + uint32_t reserved1; + uint16_t reserved2; + uint16_t txq; + /**< The event eth Tx adapter uses this field + * to store Tx queue id. + * @see rte_event_eth_tx_adapter_txq_set() + */ + } txadapter; /**< Eventdev ethdev Tx adapter */ + uint32_t usr; + /**< User defined tags. See rte_distributor_process() */ + } hash; /**< hash information */ + }; - /** Outer VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_QINQ is set. */ - uint16_t vlan_tci_outer; + /** Outer VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_QINQ is set. */ + uint16_t vlan_tci_outer; - uint16_t buf_len; /**< Length of segment buffer. */ + uint16_t buf_len; /**< Length of segment buffer. */ - struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ + struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ + + }; + uint8_t pad_cacheline0[RTE_CACHE_LINE_MIN_SIZE]; + }; /* cacheline0 */ /* 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 { @@ -664,6 +691,11 @@ struct rte_mbuf { uint32_t dynfield1[9]; /**< Reserved for dynamic fields. */ } __rte_cache_aligned; +static_assert(offsetof(struct rte_mbuf, cacheline1) == RTE_CACHE_LINE_MIN_SIZE, + "offsetof cacheline1"); +static_assert(sizeof(struct rte_mbuf) == RTE_CACHE_LINE_MIN_SIZE * 2, + "sizeof struct rte_mbuf"); + /** * Function typedef of callback to free externally attached buffer. */ -- 1.8.3.1