On 3/3/2021 6:19 PM, Dmitry Kozlyuk wrote:
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.
Got it, thanks for clarification.
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.
Agree it looks ugly :), but I am OK with above plan.
You need to send a deprecation notice for this. We can discuss there if 'd_addr'
also should be renamed for consistency, or leave with the workaround instead of
rename ...
`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
I guess this needs to be as following, in case this header included before the
windows header:
#ifdef s_addr
#pragma push_macro("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
