[v6,5/5] net: replace Windows networking shim

Message ID 20210320130525.16452-6-dmitry.kozliuk@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series eal/windows: do not expose POSIX symbols |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Dmitry Kozlyuk March 20, 2021, 1:05 p.m. UTC
  Remove networking shim from Windows EAL.

Replace it with system headers with two workarounds:

1. Windows Sockets headers contain `#define s_addr S_un.S_addr`, which
   conflicts with `s_addr` field of `struct rte_ether_hdr`. Undefining
   this macro in <rte_ether.h> had been breaking some usages of DPDK
   and Windows headers in one file.

   Renaming is planned:
   https://mails.dpdk.org/archives/dev/2021-March/201444.html

   Temporarily disable `s_addr` macro around `struct rte_ether_hdr`
   definition to avoid conflict. Place source MAC address in both
   `s_addr` and `S_un.S_addr` fields, so that access works either
   directly or through the macro.

2. Provide some IPPROTO_* constants and IPVERSION, missing on Windows.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Ranjit Menon <ranjit.menon@intel.com>
---
 drivers/net/i40e/i40e_fdir.c                  |  1 +
 lib/librte_eal/windows/include/arpa/inet.h    | 30 ---------------
 lib/librte_eal/windows/include/netinet/in.h   | 38 -------------------
 lib/librte_eal/windows/include/netinet/ip.h   | 10 -----
 .../windows/include/rte_os_internal.h         |  8 ++++
 lib/librte_eal/windows/include/sys/socket.h   | 24 ------------
 lib/librte_net/rte_ether.h                    | 26 ++++++++++---
 lib/librte_net/rte_ip.h                       |  4 ++
 lib/librte_net/rte_net.c                      |  1 +
 9 files changed, 34 insertions(+), 108 deletions(-)
 delete mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 delete mode 100644 lib/librte_eal/windows/include/netinet/in.h
 delete mode 100644 lib/librte_eal/windows/include/netinet/ip.h
 delete mode 100644 lib/librte_eal/windows/include/sys/socket.h
  

Comments

Thomas Monjalon March 26, 2021, 9:28 a.m. UTC | #1
20/03/2021 14:05, Dmitry Kozlyuk:
> Remove networking shim from Windows EAL.
> 
> Replace it with system headers with two workarounds:
> 
> 1. Windows Sockets headers contain `#define s_addr S_un.S_addr`, which
>    conflicts with `s_addr` field of `struct rte_ether_hdr`. Undefining
>    this macro in <rte_ether.h> had been breaking some usages of DPDK
>    and Windows headers in one file.

I don't understand this last sentence.

> 
>    Renaming is planned:
>    https://mails.dpdk.org/archives/dev/2021-March/201444.html
> 
>    Temporarily disable `s_addr` macro around `struct rte_ether_hdr`
>    definition to avoid conflict. Place source MAC address in both
>    `s_addr` and `S_un.S_addr` fields, so that access works either
>    directly or through the macro.

It could be a patch in itself.

> 
> 2. Provide some IPPROTO_* constants and IPVERSION, missing on Windows.

I think it belongs to previous patch about extending the IP-related API.

> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -22,6 +22,7 @@
>  #include <rte_sctp.h>
>  #include <rte_hash_crc.h>
>  #include <rte_bitmap.h>
> +#include <rte_os_internal.h>

Why is it needed?

[...]
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -17,11 +17,15 @@
>  
>  #include <stdint.h>
>  
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +#include <ws2tcpip.h>
> +#else
>  #include <sys/socket.h>
>  #include <sys/types.h>
>  #include <arpa/inet.h>
>  #include <netinet/in.h>
>  #include <netinet/ip.h>
> +#endif

Should be in previous patch about extending IP API to any OS.

> --- a/lib/librte_net/rte_net.c
> +++ b/lib/librte_net/rte_net.c
> @@ -15,6 +15,7 @@
>  #include <rte_gre.h>
>  #include <rte_mpls.h>
>  #include <rte_net.h>
> +#include <rte_os_internal.h>

Why is it needed?
  
Dmitry Kozlyuk April 1, 2021, 11:03 p.m. UTC | #2
2021-03-26 10:28 (UTC+0100), Thomas Monjalon:
> 20/03/2021 14:05, Dmitry Kozlyuk:
[...]
> > --- a/drivers/net/i40e/i40e_fdir.c
> > +++ b/drivers/net/i40e/i40e_fdir.c
> > @@ -22,6 +22,7 @@
> >  #include <rte_sctp.h>
> >  #include <rte_hash_crc.h>
> >  #include <rte_bitmap.h>
> > +#include <rte_os_internal.h>  
> 
> Why is it needed?

I deliberately put all shim definitions, like missing IPPROTO_* constants that
are used in this file, in <rte_os_internal.h> so that they are never exposed
to DPDK users. Other inclusions of this file have the same reason.
  

Patch

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index c572d003cb..b8960ff5a3 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -22,6 +22,7 @@ 
 #include <rte_sctp.h>
 #include <rte_hash_crc.h>
 #include <rte_bitmap.h>
+#include <rte_os_internal.h>
 
 #include "i40e_logs.h"
 #include "base/i40e_type.h"
diff --git a/lib/librte_eal/windows/include/arpa/inet.h b/lib/librte_eal/windows/include/arpa/inet.h
deleted file mode 100644
index 96b6984383..0000000000
--- a/lib/librte_eal/windows/include/arpa/inet.h
+++ /dev/null
@@ -1,30 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2020 Dmitry Kozlyuk
- */
-
-#ifndef _ARPA_INET_H_
-#define _ARPA_INET_H_
-
-/**
- * @file
- *
- * Compatibility header
- *
- * Although symbols declared here are present on Windows,
- * including <winsock2.h> would expose too much macros breaking common code.
- */
-
-#include <netinet/in.h>
-#include <sys/socket.h>
-
-/* defined in ws2_32.dll */
-__attribute__((stdcall))
-int
-inet_pton(int af, const char *src, void *dst);
-
-/* defined in ws2_32.dll */
-__attribute__((stdcall))
-const char *
-inet_ntop(int af, const void *src, char *dst, socklen_t size);
-
-#endif /* _ARPA_INET_H_ */
diff --git a/lib/librte_eal/windows/include/netinet/in.h b/lib/librte_eal/windows/include/netinet/in.h
deleted file mode 100644
index 6455b9ba51..0000000000
--- a/lib/librte_eal/windows/include/netinet/in.h
+++ /dev/null
@@ -1,38 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2020 Mellanox Technologies, Ltd
- */
-
-#ifndef _IN_H_
-#define _IN_H_
-
-#include <stdint.h>
-#include <sys/socket.h>
-
-#define IPPROTO_IP         0
-#define IPPROTO_HOPOPTS    0
-#define IPPROTO_ICMP       1
-#define IPPROTO_IPIP       4
-#define IPPROTO_TCP        6
-#define IPPROTO_UDP       17
-#define IPPROTO_IPV6      41
-#define IPPROTO_ROUTING   43
-#define IPPROTO_FRAGMENT  44
-#define IPPROTO_GRE       47
-#define IPPROTO_ESP       50
-#define IPPROTO_AH        51
-#define IPPROTO_ICMPV6    58
-#define IPPROTO_NONE      59
-#define IPPROTO_DSTOPTS   60
-#define IPPROTO_SCTP     132
-
-#define INET6_ADDRSTRLEN 46
-
-struct in_addr {
-	uint32_t s_addr;
-};
-
-struct in6_addr {
-	uint8_t s6_addr[16];
-};
-
-#endif
diff --git a/lib/librte_eal/windows/include/netinet/ip.h b/lib/librte_eal/windows/include/netinet/ip.h
deleted file mode 100644
index 2126498797..0000000000
--- a/lib/librte_eal/windows/include/netinet/ip.h
+++ /dev/null
@@ -1,10 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2020 Mellanox Technologies, Ltd
- */
-
-#ifndef _IP_H_
-#define _IP_H_
-
-#define IPVERSION 4
-
-#endif
diff --git a/lib/librte_eal/windows/include/rte_os_internal.h b/lib/librte_eal/windows/include/rte_os_internal.h
index 84e39eb2ae..89ac5c9cbd 100644
--- a/lib/librte_eal/windows/include/rte_os_internal.h
+++ b/lib/librte_eal/windows/include/rte_os_internal.h
@@ -25,4 +25,12 @@ 
 #define close(fd) _close(fd)
 #define unlink(path) _unlink(path)
 
+#define IPVERSION	4
+
+#define IPPROTO_IPIP	4
+#define IPPROTO_GRE	47
+#ifdef RTE_TOOLCHAIN_GCC
+#define IPPROTO_SCTP	132
+#endif
+
 #endif /* _RTE_OS_INTERNAL_H_ */
diff --git a/lib/librte_eal/windows/include/sys/socket.h b/lib/librte_eal/windows/include/sys/socket.h
deleted file mode 100644
index 9536cf8e62..0000000000
--- a/lib/librte_eal/windows/include/sys/socket.h
+++ /dev/null
@@ -1,24 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2020 Dmitry Kozlyuk
- */
-
-#ifndef _SYS_SOCKET_H_
-#define _SYS_SOCKET_H_
-
-/**
- * @file
- *
- * Compatibility header
- *
- * Although symbols declared here are present on Windows,
- * including <winsock2.h> would expose too much macros breaking common code.
- */
-
-#include <stddef.h>
-
-#define AF_INET  2
-#define AF_INET6 23
-
-typedef size_t socklen_t;
-
-#endif /* _SYS_SOCKET_H_ */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index 060b63fc9b..a303c24a8c 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,16 +253,34 @@  __rte_experimental
 int
 rte_ether_unformat_addr(const char *str, struct rte_ether_addr *eth_addr);
 
+/* Windows Sockets headers contain `#define s_addr S_un.S_addr`.
+ * Temporarily disable this macro to avoid conflict at definition.
+ * Place source MAC address in both `s_addr` and `S_un.S_addr` fields,
+ * so that access works either directly or through the macro.
+ */
+#pragma push_macro("s_addr")
+#ifdef s_addr
+#undef s_addr
+#endif
+
 /**
  * Ethernet header: Contains the destination address, source address
  * and frame type.
  */
 struct rte_ether_hdr {
 	struct rte_ether_addr d_addr; /**< Destination address. */
-	struct rte_ether_addr s_addr; /**< Source address. */
-	uint16_t ether_type;      /**< Frame type. */
+	RTE_STD_C11
+	union {
+		struct rte_ether_addr s_addr; /**< Source address. */
+		struct {
+			struct rte_ether_addr S_addr;
+		} S_un; /**< Do not use directly; use s_addr instead.*/
+	};
+	uint16_t ether_type; /**< Frame type. */
 } __rte_aligned(2);
 
+#pragma pop_macro("s_addr")
+
 /**
  * Ethernet VLAN Header.
  * Contains the 16-bit VLAN Tag Control Identifier and the Ethernet type
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 2cf7b0bd28..3c24bc4ca4 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -17,11 +17,15 @@ 
 
 #include <stdint.h>
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <ws2tcpip.h>
+#else
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <arpa/inet.h>
 #include <netinet/in.h>
 #include <netinet/ip.h>
+#endif
 
 #include <rte_byteorder.h>
 #include <rte_mbuf.h>
diff --git a/lib/librte_net/rte_net.c b/lib/librte_net/rte_net.c
index bfe5003976..b615f8f839 100644
--- a/lib/librte_net/rte_net.c
+++ b/lib/librte_net/rte_net.c
@@ -15,6 +15,7 @@ 
 #include <rte_gre.h>
 #include <rte_mpls.h>
 #include <rte_net.h>
+#include <rte_os_internal.h>
 
 /* get l3 packet type from ip6 next protocol */
 static uint32_t