From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3C44BA0093; Wed, 20 May 2020 00:15:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7F0FD1D17E; Wed, 20 May 2020 00:15:24 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 218641D174 for ; Wed, 20 May 2020 00:15:22 +0200 (CEST) IronPort-SDR: 5D7Bwevd4iwXt620hibUq014K7lbVbeXwqud6VWsevrzZAZwF6Bp5beVAE1E6M7Kjs1cCxUJfF iGa1oQ/1nhhQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 May 2020 15:15:21 -0700 IronPort-SDR: OfXQco/CHpFStEE62AptvhIATfWvIbIco6+lMUX6HmRdnLZms8qAiAkl23SOtMaRO3FhDIVYky Yv5AHYAgk5Ag== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,411,1583222400"; d="scan'208,217";a="300248810" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by orsmga008.jf.intel.com with ESMTP; 19 May 2020 15:15:21 -0700 Received: from [10.166.30.253] (10.166.30.253) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 19 May 2020 15:15:21 -0700 To: Thomas Monjalon , Dmitry Kozlyuk , CC: , , , , , , , , , , , , References: <20200519184111.4504-1-talshn@mellanox.com> <3106438.CAdn2TfLgq@thomas> <20200519225753.56c663ac@sovereign> <1935350.G7KYNPxu0G@thomas> From: Ranjit Menon Message-ID: Date: Tue, 19 May 2020 15:15:19 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <1935350.G7KYNPxu0G@thomas> Content-Language: en-US X-Originating-IP: [10.166.30.253] Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] mbuf: align rte_mbuf for Windows X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 5/19/2020 1:18 PM, Thomas Monjalon wrote: > 19/05/2020 21:57, Dmitry Kozlyuk: >> On Tue, 19 May 2020 20:49:50 +0200 >> Thomas Monjalon wrote: >> >>> +Cc more maintainers >>> >>> 19/05/2020 20:41, talshn@mellanox.com: >>>> From: Tal Shnaiderman >>>> >>>> Using uint32_t type bit-fields in Windows will pads the >>>> 'L2/L3/L4 and tunnel information' union with additional bits. >>>> >>>> This padding causes rte_mbuf size misalignment and the total size >>>> increases to 3 cache-lines. >>>> >>>> Changed packet_type bit-fields types from uint32_t to uint8_t >>>> to allow unified 2 cache-line structure size. >>>> >>>> Added the __extension__ attribute over the modified struct to avoid >>>> the warning: >>>> >>>> type of bit-field ... is a GCC extension [-pedantic] >>>> >>>> Signed-off-by: Tal Shnaiderman >>>> --- >>>> lib/librte_mbuf/rte_mbuf_core.h | 11 ++++++----- >>>> 1 file changed, 6 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/lib/librte_mbuf/rte_mbuf_core.h >>>> b/lib/librte_mbuf/rte_mbuf_core.h index b9a59c879..82441555e 100644 >>>> --- a/lib/librte_mbuf/rte_mbuf_core.h >>>> +++ b/lib/librte_mbuf/rte_mbuf_core.h >>>> @@ -521,11 +521,12 @@ struct rte_mbuf { >>>> RTE_STD_C11 >>>> union { >>>> uint32_t packet_type; /**< L2/L3/L4 and tunnel >>>> information. */ >>>> + __extension__ >>>> struct { >>>> - uint32_t l2_type:4; /**< (Outer) L2 type. >>>> */ >>>> - uint32_t l3_type:4; /**< (Outer) L3 type. >>>> */ >>>> - uint32_t l4_type:4; /**< (Outer) L4 type. >>>> */ >>>> - uint32_t tun_type:4; /**< Tunnel 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. */ >>>> RTE_STD_C11 >>>> union { >>>> uint8_t inner_esp_next_proto; >>>> @@ -541,7 +542,7 @@ struct rte_mbuf { >>>> /**< Inner L3 type. */ >>>> }; >>>> }; >>>> - uint32_t inner_l4_type:4; /**< Inner L4 >>>> type. */ >>>> + uint8_t inner_l4_type:4; /**< Inner L4 >>>> type. */ }; >>>> }; >>> >>> >> Such a clean and simple solution to what seemed to require compiler >> workaround or fix! All offsets are equal on Windows and Linux for the >> following toolchains, x86_64: >> >> * cross-compilation with MinGW-w64 6.0.0 GCC 9.3.0 >> * Windows native MinGW-w64 6.0.0 GCC 8.1.0 and Clang 9.0.1 >> >> Tested-by: Dmitry Kozlyuk > Would be interesting to see an offset comparison in little and big endian. > > BTW, this is the exact fix we used for the alignment issue in the Windows draft repo. It completely skipped my mind, when we were discussing this during the community call. See this code section in lib/librte_mbuf/rte_mbuf.h in the draft repo: uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */ struct { #ifndef _WIN64 uint32_t l2_type:4; /**< (Outer) L2 type. */ uint32_t l3_type:4; /**< (Outer) L3 type. */ uint32_t l4_type:4; /**< (Outer) L4 type. */ uint32_t tun_type:4; /**< Tunnel type. */ #else 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. */ #endif             RTE_STD_C11 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. */                 };             }; #ifndef _WIN64 uint32_t inner_l4_type:4; /**< Inner L4 type. */ #else uint8_t inner_l4_type:4; /**< Inner L4 type. */ #endif         }; We didn't have the bandwidth to test this on other OS, so we used #ifdef, but we know this allowed us to be as performant as Linux (using L3Fwd). Therefore: Acked-by: Ranjit Menon