Re: [dpdk-dev] [PATCH v2 4/6] net/pcap: add libpcap wrappers

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2021-03-03 16:47, Ferruh Yigit:
> On 3/3/2021 4:32 PM, Dmitry Kozlyuk wrote:
[...]
> > If we can't help including <ws2tcpip.h>/<netinet/ip.h> from public headers,
> > might as well use `struct in_addr`, just replace `#include <netinet/ip.h>`
> > with `#include <rte_ip.h>` everywhere.
> > 
> > The only remaining issue will be `s_addr` macro on Windows that conflicts with
> > `struct rte_ether_addr` field `s_addr`. I don't know a better solution than
> > renaming `s_addr` to `src_addr` in DPDK (OTOH, it will be consistent with
> > `struct rte_ipvX_hdr` field naming).
> > 
> > Ferruh, what do you think?
> >   
> 
> No problem on the chosen name, that will work fine, but the 'struct 
> rte_eth_addr' is public struct, we can't rename it without breaking the user 
> applications.
> 
> Still it is possible to rename public struct, but it is a long process, need to 
> send a deprecation notice and wait for next LTS, 21.11.
> 
> Ethernet header already has following, doesn't it help:
> #undef s_addr /* Defined in winsock2.h included in windows.h. */

It helps until you do the following:

	#include <winsock2.h>	/* #define s_addr S_un.S_addr */
	#include <rte_ether.h>	/* #undef s_addr */

	struct in_addr a;
	a.s_addr = 0; /* ERROR, `s_addr` has been defined to do that */

Or the following:

	#include <rte_ether.h>
	#include <winsock2.h>

	struct rte_ether_hdr eh;
	eh.s_addr.addr_bytes[0] = 0;	/* ERROR: `addr_s` is a macro */

If you swap include order in each case, it compiles, i.e . code is fragile.
Shims redefine `struct in_addr` and we're trying to get rid of them anyway.


Here's a solution that always works, however ugly it looks. It defines
`struct rte_ether_hdr` for Windows such that `s_addr` macro works for it if
defined. We can keep it until 21.11, then remove it and rename fields. In
return, we can remove networking shims and workarounds immediately.

`S_un.S_addr` part of `struct in_addr` is documented, and thus is unlikely to
ever change:

	https://docs.microsoft.com/en-us/windows/win32/api/winsock2/ns-winsock2-in_addr


diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 060b63fc9..67d340bb2 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -23,10 +23,6 @@ extern "C" {
 #include <rte_mbuf.h>
 #include <rte_byteorder.h>
 
-#ifdef RTE_EXEC_ENV_WINDOWS /* Workaround conflict with rte_ether_hdr. */
-#undef s_addr /* Defined in winsock2.h included in windows.h. */
-#endif
-
 #define RTE_ETHER_ADDR_LEN  6 /**< Length of Ethernet address. */
 #define RTE_ETHER_TYPE_LEN  2 /**< Length of Ethernet type field. */
 #define RTE_ETHER_CRC_LEN   4 /**< Length of Ethernet CRC. */
@@ -257,6 +253,8 @@ __rte_experimental
 int
 rte_ether_unformat_addr(const char *str, struct rte_ether_addr *eth_addr);
 
+#if !defined RTE_EXEC_ENV_WINDOWS || defined __DOXYGEN__
+
 /**
  * Ethernet header: Contains the destination address, source address
  * and frame type.
@@ -267,6 +265,28 @@ struct rte_ether_hdr {
 	uint16_t ether_type;      /**< Frame type. */
 } __rte_aligned(2);
 
+#else
+
+#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;
+		} S_addr;
+	};
+	uint16_t ether_type; /**< Frame type. */
+} __rte_aligned(2);
+
+#pragma pop_macro("s_addr")
+#endif
+
 /**
  * Ethernet VLAN Header.
  * Contains the 16-bit VLAN Tag Control Identifier and the Ethernet type