On Tue, Nov 5, 2024 at 12:08 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > 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?
Marking a field won't have an effect, unless packing the structure: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html When used on a struct, or struct member, the aligned attribute can only increase the alignment; in order to decrease it, the packed attribute must be specified as well. When used as part of a typedef, the aligned attribute can both increase and decrease alignment, and specifying the packed attribute generates a warning. -- David Marchand
