On Tue, Nov 05, 2024 at 11:49:52AM +0100, David Marchand wrote: > On Tue, Nov 5, 2024 at 11:20 AM Morten Brørup <m...@smartsharesystems.com> > wrote: > > > But there is still the question of packed structures with MSVC. > > > Tyler proposal seemed to rely on the current __rte_packed conventional > > > position. > > > https://patchwork.dpdk.org/project/dpdk/patch/1713225913-20792-2-git- > > > send-email-roret...@linux.microsoft.com/ > > > Note that I am not a fan of this push/pop stuff. > > > > > > Maybe Andre will find a better solution. > > > > If we cannot come up with a clean solution that looks like an attribute > > (like GCC), we should accept MSVC's style with push/pop and learn to live > > with it. > > Well, there is probably not many solutions. > OVS does the same as what you suggest. > > > > > > Perhaps something like: > > > > #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 > > > > This would also move the "packed" information to the top of the struct, > > making the code easier to read - i.e. easier to notice that the structure > > is packed when not hidden away at the end of the structure. > > __RTE_PACKED(struct __rte_aligned(2) rte_ipv4_hdr { > ... > }); > > Agreed, looks better. > Not convinced it looks better myself. Rather than packing the structure, can we instead put aligned(2) on the 32-bit fields inside it?
/Bruce