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 E633945D3F; Tue, 19 Nov 2024 09:32:13 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BDB9540268; Tue, 19 Nov 2024 09:32:13 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 24CFC400D5; Tue, 19 Nov 2024 09:32:12 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id E7BF4224C0; Tue, 19 Nov 2024 09:32:10 +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 v5 01/16] eal: provide pack start macro for MSVC Date: Tue, 19 Nov 2024 09:32:07 +0100 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F8D2@smartserver.smartshare.dk> In-Reply-To: <1731990941-10001-2-git-send-email-andremue@linux.microsoft.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v5 01/16] eal: provide pack start macro for MSVC Thread-Index: Ads6PJdHfHikNafKRRCLKJQCY+b8XwAHj/sQ References: <1710968771-16435-1-git-send-email-roretzla@linux.microsoft.com> <1731990941-10001-1-git-send-email-andremue@linux.microsoft.com> <1731990941-10001-2-git-send-email-andremue@linux.microsoft.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Andre Muezerie" , , Cc: , , , , , , , , , , , , , , , , , , , , , , , , , , , 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: Andre Muezerie [mailto:andremue@linux.microsoft.com] > Sent: Tuesday, 19 November 2024 05.35 >=20 > From: Tyler Retzlaff >=20 > MSVC struct packing is not compatible with GCC. Provide a macro that > can be used to push existing pack value and sets packing to 1-byte. > The existing __rte_packed macro is then used to restore the pack value > prior to the push. >=20 > Instead of providing macros exclusively for MSVC and for GCC the > existing macro is deliberately utilized to trigger a warning if no > existing packing has been pushed allowing easy identification of > locations where the __rte_msvc_pack is missing. >=20 > Signed-off-by: Tyler Retzlaff > --- > lib/eal/include/rte_common.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) >=20 > diff --git a/lib/eal/include/rte_common.h > b/lib/eal/include/rte_common.h > index 4d299f2b36..409890863e 100644 > --- a/lib/eal/include/rte_common.h > +++ b/lib/eal/include/rte_common.h > @@ -103,8 +103,10 @@ typedef uint16_t unaligned_uint16_t; > * Force a structure to be packed > */ > #ifdef RTE_TOOLCHAIN_MSVC > -#define __rte_packed > +#define __rte_msvc_pack __pragma(pack(push, 1)) > +#define __rte_packed __pragma(pack(pop)) > #else > +#define __rte_msvc_pack > #define __rte_packed __attribute__((__packed__)) > #endif >=20 > -- > 2.47.0.vfs.0.3 Before proceeding with this, can we please discuss the alternative, = proposed here: https://inbox.dpdk.org/dev/CAJFAV8yStgiBbe+Nkt9mC30r0+ZP64_kGuRHOzqd90RD2= HXZyw@mail.gmail.com/ The definition of the packing macro in OVS, for reference: https://github.com/openvswitch/ovs/blob/main/include/openvswitch/compiler= .h#L209 The current solution requires __rte_packed to be placed at the end of a = structure, although __attribute__((packed)) is normally allowed at the = beginning (between the "struct" tag and the name of the structure), = which introduces a high risk of contributors placing it "incorrectly", = thus causing errors. I have a strong preference for an __RTE_PACKED(decl) variant. Here's a third alternative: #ifdef RTE_TOOLCHAIN_MSVC #define __rte_msvc_pack_begin __pragma(pack(push, 1)) #define __rte_msvc_pack_end __pragma(pack(pop)) #else #define __rte_msvc_pack_begin #define __rte_msvc_pack_end #endif The third alternative is also problematic, e.g. if a contributor forgets = the _end after the structure declaration, or adds another structure = declaration before the _end. -Morten