2021-05-20 17:04 (UTC+0100), Ferruh Yigit: > On 5/20/2021 4:50 PM, Dmitry Kozlyuk wrote: > > 2021-05-20 16:27 (UTC+0100), Ferruh Yigit: > >> On 5/20/2021 4:06 PM, Dmitry Kozlyuk wrote: > >>> 2021-05-20 15:24 (UTC+0100), Ferruh Yigit: > >>>> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote: > >>> [...] > >>>>> > >>>>> It is not mandatory to rename `d_addr`, this is for consistency only. > >>>>> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`. > >>>>> > >>>>> Workaround is to define `struct rte_ether_hdr` in such a away that > >>>>> it can be used with or without `s_addr` macro (as defined on Windows) > >>>>> This can be done for Windows only or for all platforms to save space. > >>>>> > >>>>> #pragma push_macro("s_addr") > >>>>> #ifdef s_addr > >>>>> #undef s_addr > >>>>> #endif > >>>>> > >>>>> struct rte_ether_hdr { > >>>>> struct rte_ether_addr d_addr; /**< Destination address. */ > >>>>> RTE_STD_C11 > >>>>> union { > >>>>> struct rte_ether_addr s_addr; /**< Source address. */ > >>>>> struct { > >>>>> struct rte_ether_addr S_un; > >>>>> /**< MUST NOT be used directly, only via s_addr */ > >>>>> } S_addr; > >>>>> /*< MUST NOT be used directly, only via s_addr */ > >>>>> }; > >>>>> uint16_t ether_type; /**< Frame type. */ > >>>>> } __rte_aligned(2); > >>>>> > >>>>> #pragma pop_macro("s_addr") > >>>>> > >>>> > >>>> What is the problem with the workaround, why we can't live with it? > >>>> > >>>> It requires an order in include files, right? > >>> > >>> There's no problem except a tricky structure definition with fields that > >>> violate DPDK coding rules. It works with any include order. > >>> > >>> Will fix typos in v3, thanks. > >>> > >> > >> For following case, won't compiler take 's_addr' as macro? > >> > >> #include <rte_ether.h> > >> #include <winsock2.h> > >> struct rte_ether_hdr eh; > >> /* eh.s_addr.addr_bytes[0] = 0; > >> > > > > Yes, it will. The macro will expand to `S_addr.S_un` and compile > > successfully. > > will 'eh.S_addr.S_un.addr_bytes[0] = 0;' compile successfully?
Yes, only it's `S_un.S_addr`, sorry for the typo in my explanation. Both code snippets from commit message compile successfully. > > > In theory, Microsoft can change the definition of `s_addr`, and while I > > doubt > > they will, it's a valid concern and a reason to remove workaround in 21.11. > > >