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 DAE0745F64; Sat, 28 Dec 2024 15:41:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CC8C840289; Sat, 28 Dec 2024 15:41:43 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id E674E40280 for ; Sat, 28 Dec 2024 15:41:41 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id CB0A7206AA; Sat, 28 Dec 2024 15:41:40 +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 v7 02/29] eal/include: add new packing macros Date: Sat, 28 Dec 2024 15:41:39 +0100 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F977@smartserver.smartshare.dk> In-Reply-To: <1734981122-4729-3-git-send-email-andremue@linux.microsoft.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v7 02/29] eal/include: add new packing macros Thread-Index: AdtVbp7pwVzaKrKLSsKe6VYS3x/DhgDxBWBQ References: <1710968771-16435-1-git-send-email-roretzla@linux.microsoft.com> <1734981122-4729-1-git-send-email-andremue@linux.microsoft.com> <1734981122-4729-3-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: Monday, 23 December 2024 20.12 >=20 > MSVC struct packing is not compatible with GCC. Add macro > __rte_packed_begin which can be used to push existing pack value > and set packing to 1-byte. Add macro __rte_packed_end to restore > the pack value prior to the push. Nice to have: Some alternative solutions were discussed on the mailing list. Please consider adding a brief summary of them with their = benefits/disadvantages to the patch description, either here or in the = cover letter (0/NN) of the patch series. >=20 > Macro __rte_packed_end is deliberately utilized to trigger a > MSVC compiler warning if no existing packing has been pushed allowing > easy identification of locations where the __rte_packed_begin is > missing. >=20 > Macro __rte_packed was marked as deprecated. >=20 > Signed-off-by: Andre Muezerie > Acked-by: Tyler Retzlaff > --- > lib/eal/include/rte_common.h | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) >=20 > diff --git a/lib/eal/include/rte_common.h > b/lib/eal/include/rte_common.h > index 4d299f2b36..e40910fa20 100644 > --- a/lib/eal/include/rte_common.h > +++ b/lib/eal/include/rte_common.h > @@ -99,13 +99,28 @@ typedef uint32_t unaligned_uint32_t; > typedef uint16_t unaligned_uint16_t; > #endif >=20 > +/** > + * @deprecated > + * @see __rte_packed_begin > + * @see __rte_packed_end > + * > + * Force a structure to be packed > + */ > +#ifdef RTE_TOOLCHAIN_MSVC > +#define __rte_packed RTE_DEPRECATED(__rte_packed) > +#else > +#define __rte_packed (RTE_DEPRECATED(__rte_packed) > __attribute__((__packed__))) > +#endif > + > /** > * Force a structure to be packed > */ > #ifdef RTE_TOOLCHAIN_MSVC > -#define __rte_packed > +#define __rte_packed_begin __pragma(pack(push, 1)) > +#define __rte_packed_end __pragma(pack(pop)) > #else > -#define __rte_packed __attribute__((__packed__)) > +#define __rte_packed_begin > +#define __rte_packed_end __attribute__((__packed__)) > #endif >=20 > /** > -- > 2.47.0.vfs.0.3 In the following patches in this series, the placement of the = __rte_packed_begin attribute is wrong for GCC. Like the __rte_aligned() attribute, the __rte_packed_begin attribute = needs to be after the "struct" keyword, not before. The GCC documentation [1] says: You may specify type attributes in an enum, struct or union type = declaration or definition by placing them immediately after the struct, = union or enum keyword. You can also place them just past the closing curly brace of the = definition, but this is less preferred because logically the type should = be fully defined at the closing brace. You can also include type attributes in a typedef declaration. [1] https://gcc.gnu.org/onlinedocs/gcc/Type-Attributes.html The documentation comment preceding each macro in rte_common.h should = include a brief description of how to use the macro. This goes for most macros in rte_common.h, not only the macros for = packing.