Re: [PATCH v6 00/30] fix packing of structs when building with MSVC

[David Marchand <david.marchand@redhat.com>]

On Mon, Dec 23, 2024 at 12:03 PM David Marchand
<david.marchand@redhat.com> wrote:
> > v6:
> >   * replace __rte_msvc_pack with __rte_packed_begin
> >   * replace __rte_packed with __rte_packed_end
> >   * update checkpatches.sh to ensure __rte_packed_begin and
> >     __rte_packed_end are used in pairs
>
> I had mentionned this in a separate thread.
> Why not do like OVS and have a RTE_PACKED() macro?
>
> #ifdef RTE_TOOLCHAIN_MSVC
> #define RTE_PACKED(...) __pragma(pack(push, 1)) __VA_ARGS__ __pragma(pack(pop))
> #else
> #define RTE_PACKED(...) __VA_ARGS__ __attribute__((__packed__))
> #endif

Mm, in practice, this would be problematic with struct where
endianness matters (for example).

In file included from ../../../git/pub/dpdk.org/main/lib/net/rte_ip.h:9,
                 from ../../../git/pub/dpdk.org/main/lib/ethdev/rte_flow.h:25,
                 from
../../../git/pub/dpdk.org/main/lib/ethdev/rte_eth_ctrl.h:11,
                 from
../../../git/pub/dpdk.org/main/lib/ethdev/rte_ethdev.h:1472,
                 from
../../../git/pub/dpdk.org/main/lib/ethdev/ethdev_driver.h:21,
                 from
../../../git/pub/dpdk.org/main/drivers/net/failsafe/failsafe_ops.c:15:
../../../git/pub/dpdk.org/main/lib/net/rte_ip4.h:48:1: error:
embedding a directive within macro arguments is not portable [-Werror]
   48 | #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
      | ^
../../../git/pub/dpdk.org/main/lib/net/rte_ip4.h:51:1: error:
embedding a directive within macro arguments is not portable [-Werror]
   51 | #elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
      | ^
../../../git/pub/dpdk.org/main/lib/net/rte_ip4.h:54:1: error:
embedding a directive within macro arguments is not portable [-Werror]
   54 | #endif
      | ^


>
> This removes the need for updating checkpatch.
> Plus, builds on Linux will catch issues (hopefully by the author of
> the change, before submitting).
>
>
> >   * remove __rte_packed
>
> Please mark it deprecated for now (see RTE_DEPRECATED / add a
> deprecation notice) and we will remove it in 25.11.

Still, please rework this part.


-- 
David Marchand