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 8B83B45D42; Tue, 19 Nov 2024 12:01:01 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7BC01427A5; Tue, 19 Nov 2024 12:01:01 +0100 (CET) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id EBC0440041; Tue, 19 Nov 2024 12:00:59 +0100 (CET) Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4Xt1h30YDhz6K9DL; Tue, 19 Nov 2024 18:58:43 +0800 (CST) Received: from frapeml500006.china.huawei.com (unknown [7.182.85.219]) by mail.maildlp.com (Postfix) with ESMTPS id 07EF3140F49; Tue, 19 Nov 2024 19:00:57 +0800 (CST) Received: from frapeml500007.china.huawei.com (7.182.85.172) by frapeml500006.china.huawei.com (7.182.85.219) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Tue, 19 Nov 2024 12:00:56 +0100 Received: from frapeml500007.china.huawei.com ([7.182.85.172]) by frapeml500007.china.huawei.com ([7.182.85.172]) with mapi id 15.01.2507.039; Tue, 19 Nov 2024 12:00:56 +0100 From: Konstantin Ananyev To: =?iso-8859-1?Q?Morten_Br=F8rup?= , "Andre Muezerie" , "roretzla@linux.microsoft.com" , "techboard@dpdk.org" CC: "Yuying.Zhang@intel.com" , "aman.deep.singh@intel.com" , "anatoly.burakov@intel.com" , "bruce.richardson@intel.com" , "byron.marohn@intel.com" , "conor.walsh@intel.com" , "cristian.dumitrescu@intel.com" , "david.hunt@intel.com" , "dev@dpdk.org" , "dsosnowski@nvidia.com" , "gakhil@marvell.com" , "jerinj@marvell.com" , "jingjing.wu@intel.com" , "kirill.rybalchenko@intel.com" , "konstantin.v.ananyev@yandex.ru" , "matan@nvidia.com" , "orika@nvidia.com" , "radu.nicolau@intel.com" , "ruifeng.wang@arm.com" , "sameh.gobriel@intel.com" , "sivaprasad.tummala@amd.com" , "skori@marvell.com" , "stephen@networkplumber.org" , "suanmingm@nvidia.com" , "vattunuru@marvell.com" , "viacheslavo@nvidia.com" , "vladimir.medvedkin@intel.com" , "yipeng1.wang@intel.com" Subject: RE: [PATCH v5 01/16] eal: provide pack start macro for MSVC Thread-Topic: [PATCH v5 01/16] eal: provide pack start macro for MSVC Thread-Index: AQHbOjyrJhJ3IxC6eke7ncBGaklbs7K+NcyAgAA6B5A= Date: Tue, 19 Nov 2024 11:00:56 +0000 Message-ID: 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> <98CBD80474FA8B44BF855DF32C47DC35E9F8D2@smartserver.smartshare.dk> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F8D2@smartserver.smartshare.dk> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.195.32.132] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 > -----Original Message----- > From: Morten Br=F8rup > Sent: Tuesday, November 19, 2024 8:32 AM > To: Andre Muezerie ; roretzla@linux.microso= ft.com; techboard@dpdk.org > Cc: Yuying.Zhang@intel.com; aman.deep.singh@intel.com; anatoly.burakov@in= tel.com; bruce.richardson@intel.com; > byron.marohn@intel.com; conor.walsh@intel.com; cristian.dumitrescu@intel.= com; david.hunt@intel.com; dev@dpdk.org; > dsosnowski@nvidia.com; gakhil@marvell.com; jerinj@marvell.com; jingjing.w= u@intel.com; kirill.rybalchenko@intel.com; > konstantin.v.ananyev@yandex.ru; matan@nvidia.com; orika@nvidia.com; radu.= nicolau@intel.com; ruifeng.wang@arm.com; > sameh.gobriel@intel.com; sivaprasad.tummala@amd.com; skori@marvell.com; s= tephen@networkplumber.org; > suanmingm@nvidia.com; vattunuru@marvell.com; viacheslavo@nvidia.com; vlad= imir.medvedkin@intel.com; > yipeng1.wang@intel.com > Subject: RE: [PATCH v5 01/16] eal: provide pack start macro for MSVC >=20 > > From: Andre Muezerie [mailto:andremue@linux.microsoft.com] > > Sent: Tuesday, 19 November 2024 05.35 > > > > From: Tyler Retzlaff > > > > 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. > > > > 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. > > > > Signed-off-by: Tyler Retzlaff > > --- > > lib/eal/include/rte_common.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > 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 > > > > -- > > 2.47.0.vfs.0.3 >=20 > Before proceeding with this, can we please discuss the alternative, propo= sed here: > https://inbox.dpdk.org/dev/CAJFAV8yStgiBbe+Nkt9mC30r0+ZP64_kGuRHOzqd90RD2= HXZyw@mail.gmail.com/ >=20 > The definition of the packing macro in OVS, for reference: > https://github.com/openvswitch/ovs/blob/main/include/openvswitch/compiler= .h#L209 Yes, that one looks much nicer to me too. Could I also ask to avoid creeping _msvc_ into DPDK namespace. >=20 > The current solution requires __rte_packed to be placed at the end of a s= tructure, although __attribute__((packed)) is normally > allowed at the beginning (between the "struct" tag and the name of the st= ructure), which introduces a high risk of contributors placing > it "incorrectly", thus causing errors. >=20 > I have a strong preference for an __RTE_PACKED(decl) variant. >=20 > 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 >=20 > 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. >=20 > -Morten