> From: Bruce Richardson [mailto:bruce.richard...@intel.com] > Sent: Tuesday, 5 November 2024 12.03 > > 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?
No, MSVC aligned() can only increase alignment, not decrease it. I checked it a few minutes ago, getting the same idea. ;-)
