diff mbox series

[v8,4/4] net: provide IP-related API on any OS

Message ID 20210407222249.6729-5-dmitry.kozliuk@gmail.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers show
Series eal/windows: do not expose POSIX symbols | expand

Checks

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

Commit Message

Dmitry Kozlyuk April 7, 2021, 10:22 p.m. UTC
Users of <rte_ip.h> relied on it to provide IP-related defines,
like IPPROTO_* constants, but still had to include POSIX headers
for inet_pton() and other standard IP-related facilities.

Extend <rte_ip.h> so that it is a single header to gain access
to IP-related facilities on any OS. Use it to replace POSIX includes
in components enabled on Windows. Move missing constants from Windows
networking shim to OS shim header and include it where needed.

Remove Windows networking shim that is no longer needed.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 drivers/net/i40e/i40e_fdir.c                 |  1 +
 drivers/net/mlx5/mlx5.h                      |  1 -
 drivers/net/mlx5/mlx5_flow.c                 |  4 +--
 drivers/net/mlx5/mlx5_flow.h                 |  3 +-
 drivers/net/mlx5/mlx5_mac.c                  |  1 -
 examples/cmdline/commands.c                  |  5 ---
 examples/cmdline/parse_obj_list.c            |  2 --
 lib/librte_cmdline/cmdline.c                 |  1 -
 lib/librte_cmdline/cmdline_parse.c           |  2 --
 lib/librte_cmdline/cmdline_parse_etheraddr.c |  6 ----
 lib/librte_cmdline/cmdline_parse_ipaddr.c    |  6 ----
 lib/librte_cmdline/cmdline_parse_ipaddr.h    |  2 +-
 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 ------
 lib/librte_eal/windows/include/rte_os_shim.h |  8 +++++
 lib/librte_eal/windows/include/sys/socket.h  | 24 -------------
 lib/librte_ethdev/rte_ethdev.c               | 12 +++----
 lib/librte_ethdev/rte_ethdev_core.h          |  1 -
 lib/librte_net/rte_ip.h                      |  7 ++++
 lib/librte_net/rte_net.c                     |  1 +
 21 files changed, 24 insertions(+), 141 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

Olivier Matz April 8, 2021, 11:45 a.m. UTC | #1
Hi Dmitry,

On Thu, Apr 08, 2021 at 01:22:49AM +0300, Dmitry Kozlyuk wrote:
> Users of <rte_ip.h> relied on it to provide IP-related defines,
> like IPPROTO_* constants, but still had to include POSIX headers
> for inet_pton() and other standard IP-related facilities.
> 
> Extend <rte_ip.h> so that it is a single header to gain access
> to IP-related facilities on any OS. Use it to replace POSIX includes
> in components enabled on Windows. Move missing constants from Windows
> networking shim to OS shim header and include it where needed.
> 
> Remove Windows networking shim that is no longer needed.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  drivers/net/i40e/i40e_fdir.c                 |  1 +
>  drivers/net/mlx5/mlx5.h                      |  1 -
>  drivers/net/mlx5/mlx5_flow.c                 |  4 +--
>  drivers/net/mlx5/mlx5_flow.h                 |  3 +-
>  drivers/net/mlx5/mlx5_mac.c                  |  1 -
>  examples/cmdline/commands.c                  |  5 ---
>  examples/cmdline/parse_obj_list.c            |  2 --
>  lib/librte_cmdline/cmdline.c                 |  1 -
>  lib/librte_cmdline/cmdline_parse.c           |  2 --
>  lib/librte_cmdline/cmdline_parse_etheraddr.c |  6 ----
>  lib/librte_cmdline/cmdline_parse_ipaddr.c    |  6 ----
>  lib/librte_cmdline/cmdline_parse_ipaddr.h    |  2 +-
>  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 ------
>  lib/librte_eal/windows/include/rte_os_shim.h |  8 +++++
>  lib/librte_eal/windows/include/sys/socket.h  | 24 -------------
>  lib/librte_ethdev/rte_ethdev.c               | 12 +++----
>  lib/librte_ethdev/rte_ethdev_core.h          |  1 -
>  lib/librte_net/rte_ip.h                      |  7 ++++
>  lib/librte_net/rte_net.c                     |  1 +
>  21 files changed, 24 insertions(+), 141 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

I see it has already been discussed for posix functions like close() or
strdup(), so I won't reopen the door too long ;)

Since DPDK is a network-oriented project, it provides network defines or
structure, prefixed with rte_. This API is on some aspects more complete
than the one provided in libc (for instance, more protocol headers are
available) . So, to me, it would make sense to define RTE_IPPROTO_* and
replace usages of IPPROTO_*, and avoid inclusions of network libc
headers in DPDK code.

This can be done later, if there is a consensus.

> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index c572d003cb..e7361bf520 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_shim.h>
>  

If I understand the logic, rte_ip.h provides OS-specific IP-related
stuff (like IPPROTO_*), and rte_os_shim.h provides the POSIX definitions
that are missing after including rte_ip.h.

Would it make sense to include rte_os_shim.h from rte_ip.h, so that
including rte_ip.h is always sufficient? Or is it because we want to
avoid implicit inclusion of rte_os_shim.h?

Thanks,
Olivier
Dmitry Kozlyuk April 8, 2021, 7:51 p.m. UTC | #2
2021-04-08 13:45 (UTC+0200), Olivier Matz:
[...]
> > diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> > index c572d003cb..e7361bf520 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_shim.h>
> >    
> 
> If I understand the logic, rte_ip.h provides OS-specific IP-related
> stuff (like IPPROTO_*), and rte_os_shim.h provides the POSIX definitions
> that are missing after including rte_ip.h.
> 
> Would it make sense to include rte_os_shim.h from rte_ip.h, so that
> including rte_ip.h is always sufficient? Or is it because we want to
> avoid implicit inclusion of rte_os_shim.h?

Yes, currently rte_os_shim.h is not exposed at all.
It it ever will, this reason still applies.
Olivier Matz April 9, 2021, 1:33 p.m. UTC | #3
On Thu, Apr 08, 2021 at 10:51:34PM +0300, Dmitry Kozlyuk wrote:
> 2021-04-08 13:45 (UTC+0200), Olivier Matz:
> [...]
> > > diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> > > index c572d003cb..e7361bf520 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_shim.h>
> > >    
> > 
> > If I understand the logic, rte_ip.h provides OS-specific IP-related
> > stuff (like IPPROTO_*), and rte_os_shim.h provides the POSIX definitions
> > that are missing after including rte_ip.h.
> > 
> > Would it make sense to include rte_os_shim.h from rte_ip.h, so that
> > including rte_ip.h is always sufficient? Or is it because we want to
> > avoid implicit inclusion of rte_os_shim.h?
> 
> Yes, currently rte_os_shim.h is not exposed at all.
> It it ever will, this reason still applies.

Ok, thank you for the clarification.

Acked-by: Olivier Matz <olivier.matz@6wind.com>
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index c572d003cb..e7361bf520 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_shim.h>
 
 #include "i40e_logs.h"
 #include "base/i40e_type.h"
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 6faba4fbb1..392e89d3f5 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -10,7 +10,6 @@ 
 #include <stdbool.h>
 #include <stdint.h>
 #include <limits.h>
-#include <netinet/in.h>
 #include <sys/queue.h>
 
 #include <rte_pci.h>
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index c347f8130e..0f1a9c5ed9 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3,12 +3,11 @@ 
  * Copyright 2016 Mellanox Technologies, Ltd
  */
 
-#include <netinet/in.h>
-#include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
 #include <string.h>
 #include <stdbool.h>
+#include <sys/queue.h>
 
 #include <rte_common.h>
 #include <rte_ether.h>
@@ -8241,4 +8240,3 @@  mlx5_release_tunnel_hub(__rte_unused struct mlx5_dev_ctx_shared *sh,
 {
 }
 #endif /* HAVE_IBV_FLOW_DV_SUPPORT */
-
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 8324e188e1..d03eb0a7a7 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -5,11 +5,10 @@ 
 #ifndef RTE_PMD_MLX5_FLOW_H_
 #define RTE_PMD_MLX5_FLOW_H_
 
-#include <netinet/in.h>
-#include <sys/queue.h>
 #include <stdalign.h>
 #include <stdint.h>
 #include <string.h>
+#include <sys/queue.h>
 
 #include <rte_alarm.h>
 #include <rte_mtr.h>
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index a7946f7756..19981d26d8 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -8,7 +8,6 @@ 
 #include <string.h>
 #include <inttypes.h>
 #include <errno.h>
-#include <netinet/in.h>
 
 #include <rte_ether.h>
 #include <ethdev_driver.h>
diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c
index f43eacfbad..9ce8ef389f 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -8,12 +8,7 @@ 
 #include <stdint.h>
 #include <string.h>
 #include <stdlib.h>
-#include <stdarg.h>
 #include <errno.h>
-#include <netinet/in.h>
-#ifdef RTE_EXEC_ENV_FREEBSD
-#include <sys/socket.h>
-#endif
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
diff --git a/examples/cmdline/parse_obj_list.c b/examples/cmdline/parse_obj_list.c
index b04adbea58..959bcd1452 100644
--- a/examples/cmdline/parse_obj_list.c
+++ b/examples/cmdline/parse_obj_list.c
@@ -6,11 +6,9 @@ 
 
 #include <stdio.h>
 #include <inttypes.h>
-#include <stdarg.h>
 #include <errno.h>
 #include <ctype.h>
 #include <string.h>
-#include <netinet/in.h>
 
 #include <cmdline_parse.h>
 #include <cmdline_parse_ipaddr.h>
diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index 49770869bb..a176d15130 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -12,7 +12,6 @@ 
 #include <inttypes.h>
 #include <fcntl.h>
 #include <errno.h>
-#include <netinet/in.h>
 
 #include <rte_string_fns.h>
 
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index fe366841cd..f5cc934782 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -11,8 +11,6 @@ 
 #include <inttypes.h>
 #include <ctype.h>
 
-#include <netinet/in.h>
-
 #include <rte_string_fns.h>
 
 #include "cmdline_private.h"
diff --git a/lib/librte_cmdline/cmdline_parse_etheraddr.c b/lib/librte_cmdline/cmdline_parse_etheraddr.c
index 5cb10de321..433b828a72 100644
--- a/lib/librte_cmdline/cmdline_parse_etheraddr.c
+++ b/lib/librte_cmdline/cmdline_parse_etheraddr.c
@@ -5,13 +5,7 @@ 
  */
 
 #include <stdio.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <errno.h>
-#include <inttypes.h>
-#include <ctype.h>
 #include <string.h>
-#include <sys/types.h>
 
 #include <rte_string_fns.h>
 #include <rte_ether.h>
diff --git a/lib/librte_cmdline/cmdline_parse_ipaddr.c b/lib/librte_cmdline/cmdline_parse_ipaddr.c
index f8dbdf204c..5e278c963f 100644
--- a/lib/librte_cmdline/cmdline_parse_ipaddr.c
+++ b/lib/librte_cmdline/cmdline_parse_ipaddr.c
@@ -6,14 +6,8 @@ 
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <stdarg.h>
-#include <inttypes.h>
-#include <ctype.h>
 #include <string.h>
 #include <errno.h>
-#include <arpa/inet.h>
-#include <netinet/in.h>
-#include <sys/socket.h>
 
 #include <rte_string_fns.h>
 
diff --git a/lib/librte_cmdline/cmdline_parse_ipaddr.h b/lib/librte_cmdline/cmdline_parse_ipaddr.h
index 0ba81647bc..0118c31d44 100644
--- a/lib/librte_cmdline/cmdline_parse_ipaddr.h
+++ b/lib/librte_cmdline/cmdline_parse_ipaddr.h
@@ -8,7 +8,7 @@ 
 #define _PARSE_IPADDR_H_
 
 #include <cmdline_parse.h>
-#include <netinet/in.h>
+#include <rte_ip.h>
 
 #ifdef __cplusplus
 extern "C" {
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_shim.h b/lib/librte_eal/windows/include/rte_os_shim.h
index edd9a1082c..f40fb62d1d 100644
--- a/lib/librte_eal/windows/include/rte_os_shim.h
+++ b/lib/librte_eal/windows/include/rte_os_shim.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_SHIM_ */
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_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 3059aa55b3..441f01df53 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2,18 +2,14 @@ 
  * Copyright(c) 2010-2017 Intel Corporation
  */
 
-#include <sys/types.h>
-#include <sys/queue.h>
 #include <ctype.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <stdarg.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdbool.h>
 #include <stdint.h>
-#include <inttypes.h>
-#include <netinet/in.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/queue.h>
 
 #include <rte_byteorder.h>
 #include <rte_log.h>
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 918a34ed1f..4679d948fa 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -6,7 +6,6 @@ 
 #define _RTE_ETHDEV_CORE_H_
 
 #include <pthread.h>
-#include <sys/types.h>
 
 /**
  * @file
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index b59c4d67a3..8c189009b0 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -16,9 +16,16 @@ 
  */
 
 #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..d680accc16 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_shim.h>
 
 /* get l3 packet type from ip6 next protocol */
 static uint32_t