[PATCH v11 00/30] fix packing of structs when building with MSVC

[David Marchand]

On Fri, Jan 10, 2025 at 11:17 PM Andre Muezerie
<andremue at linux.microsoft.com> wrote:
>
> MSVC struct packing is not compatible with GCC. Different alternatives
> were considered:
>
> 1) Have a macro __RTE_PACKED(decl) to which the struct/union is passed
>    and the macro would define the struct/union with the appropriate
>    packing attribute for the compiler in use.
>
> Advantages:
>  * Can be placed in front of a struct, or even in the middle. Good
>    for readability.
>  * Does not require a different macro to be placed at the end of the
>    structure.
>
> However, problems can arise when compiler directives are present in the
> struct, as they become arguments for __RTE_PACKED macro. This is not
> portable. Two problematic situations observed in the DPDK code:
>
>   a) #defines mentioned in the struct. In this situation we could just
>      move the #define out of the struct.
>
>   b) #if/#ifdef/#elif mentioned in the struct.
>      This is a somewhat common pattern in structs where fields change
>      based on endianness and would require code duplication to be
>      handled, which makes the code hard to read and maintain.
>
> 2) Have macros __rte_msvc_pack_begin and __rte_msvc_pack_end which
>    would be placed at the beginning and end of the struct/union
>    respectively. Concerns were raised about having macros for
>    specific compilers, or even having compiler names mentioned in
>    the macros' names.
>
> 3) Instead of providing macros exclusively for MSVC and for GCC,
>    have a macro __rte_packed_begin and __rte_packed_end which would
>    be placed at the beginning and end of the struct/union respectively.
>    With MSVC both macros end up having a purpose. With GCC and Clang
>    only __rte_packed_end has a purpose, as can be seen below.
>    This makes the solution generic and is the approach taken in this
>    patchset.
>
>    #ifdef RTE_TOOLCHAIN_MSVC
>    #define __rte_packed_begin __pragma(pack(push, 1))
>    #define __rte_packed_end __pragma(pack(pop))
>    #else
>    #define __rte_packed_begin
>    #define __rte_packed_end __attribute__((__packed__))
>    #endif
>
>    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.
>
> Macro __rte_packed is marked deprecated and the two new macros represent
> the new way to enable packing in the DPDK code.
>
> Script checkpatches.sh was enhanced to ensure that:
>  * __rte_packed_begin and __rte_packed_end show up in pairs.
>  * __rte_packed_begin is not used with enums.
>  * __rte_packed_begin is only used after struct, union,
>    __rte_cache_aligned, __rte_cache_min_aligned or __rte_aligned

Recheck-request: ci/iol-marvell-Functional


-- 
David Marchand