[PATCH v5 01/16] eal: provide pack start macro for MSVC
Morten Brørup
mb at smartsharesystems.com
Tue Nov 19 09:32:07 CET 2024
> From: Andre Muezerie [mailto:andremue at linux.microsoft.com]
> Sent: Tuesday, 19 November 2024 05.35
>
> From: Tyler Retzlaff <roretzla at linux.microsoft.com>
>
> 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 <roretzla at linux.microsoft.com>
> ---
> 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
Before proceeding with this, can we please discuss the alternative, proposed here:
https://inbox.dpdk.org/dev/CAJFAV8yStgiBbe+Nkt9mC30r0+ZP64_kGuRHOzqd90RD2HXZyw@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
More information about the dev
mailing list