On Fri, Jan 10, 2025 at 11:17 PM Andre Muezerie <andre...@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
