[PATCHv2] include: fix sys/queue.h.
Checks
Commit Message
Currently there are a couple of header files include 'sys/queue.h',
which is a POSIX functionality. When compiling DPDK with OVS on
Windows, we encountered issues such as, found the missing header.
In file included from ../lib/dpdk.c:27:
C:\temp\dpdk\include\rte_log.h:24:10: fatal error: 'sys/queue.h' file
not found
The patch fixes it by 1) removing the #include <sys/queue.h> from
a couple of headers, replace it with #include <rte_os.h>, and
2) include sys/queue.h in the rte_os.h. As a result, for Linux/BSD,
it is using the sys/queue.h from its POSIX library, and for Windows,
DPDK will use the bundled sys/queue.h.
1) fixes the case that DPDK library shouldn't export POSIX functionality
into the environment (symbols, macros, headers etc), which cause definitions
clashing with the application.
2) fixes the case that DPDK should depend only on the C library and not
require POSIX functionality from the underlying system.
There are still a couple of headers using sys/queue.h, ex:
rte_bus_pci.h. Since it's not been used in Windows yet, we can
fix them later.
Suggested-by: Nick Connolly <nick.connolly@mayadata.io>
Suggested-by: Dmitry Kozliuk <Dmitry.Kozliuk@gmail.com>
Signed-off-by: William Tu <u9012063@gmail.com>
---
v1->v2:
- follow the suggestion by Nick and Dmitry
- http://mails.dpdk.org/archives/dev/2021-August/216304.html
---
lib/eal/freebsd/include/rte_os.h | 1 +
lib/eal/include/rte_bus.h | 2 +-
lib/eal/include/rte_class.h | 2 --
lib/eal/include/rte_dev.h | 2 +-
lib/eal/include/rte_devargs.h | 1 -
lib/eal/include/rte_log.h | 1 -
lib/eal/include/rte_service.h | 1 -
lib/eal/include/rte_tailq.h | 2 +-
lib/eal/linux/include/rte_os.h | 1 +
lib/eal/windows/include/meson.build | 4 ++++
lib/eal/windows/include/rte_os.h | 1 +
lib/eal/windows/include/sys/meson.build | 6 ++++++
lib/mempool/rte_mempool.h | 1 -
13 files changed, 16 insertions(+), 9 deletions(-)
create mode 100644 lib/eal/windows/include/sys/meson.build
Comments
Hi William,
2021-08-11 20:46 (UTC+0000), William Tu:
> Currently there are a couple of header files include 'sys/queue.h',
> which is a POSIX functionality. When compiling DPDK with OVS on
> Windows, we encountered issues such as, found the missing header.
> In file included from ../lib/dpdk.c:27:
> C:\temp\dpdk\include\rte_log.h:24:10: fatal error: 'sys/queue.h' file
> not found
>
> The patch fixes it by 1) removing the #include <sys/queue.h> from
> a couple of headers, replace it with #include <rte_os.h>, and
> 2) include sys/queue.h in the rte_os.h. As a result, for Linux/BSD,
> it is using the sys/queue.h from its POSIX library, and for Windows,
> DPDK will use the bundled sys/queue.h.
>
> 1) fixes the case that DPDK library shouldn't export POSIX functionality
> into the environment (symbols, macros, headers etc), which cause definitions
> clashing with the application.
> 2) fixes the case that DPDK should depend only on the C library and not
> require POSIX functionality from the underlying system.
Sorry, this is not exactly what was suggested here:
http://inbox.dpdk.org/dev/20210811013325.34c36220@sovereign/
The point was not to install <sys/queue.h>. Its full content is not needed
for DPDK interface, only for implementation. Public headers only need a
handful of macros for list/tailq heads and links. Those macros should be
provided by DPDK, with RTE_ prefix. For Linux and FreeBSD it will just be:
#include <sys/queue.h>
#define RTE_TAILQ_ENTRY(type) TAILQ_ENTRY(type)
/* ... */
For Windows you would have to copy definitions from <sys/queue.h>
to some public header, <rte_os.h> seems OK. All public headers that need
<sys/queue.h> macros must have them replaced with RTE_ version.
Implementation remains unchanged. No new files need to be installed,
because when DPDK is built on Windows, it uses the bundled <sys/queue.h>,
and when it is installed, only RTE_ macros you created are visible.
Macro documentation should clearly state that they are compatible with system
<sys/queue.h> for Linux and FreeBSD, and for Windows they are compatible with
the version used during build.
+Bruce, David, Ray, Tyler to confirm/object the idea.
> There are still a couple of headers using sys/queue.h, ex:
> rte_bus_pci.h. Since it's not been used in Windows yet, we can
> fix them later.
Another item I think of is a checkpatch rule to prohibit TAILQ_xxx, etc
macro in public headers after replacing them all.
> Suggested-by: Nick Connolly <nick.connolly@mayadata.io>
> Suggested-by: Dmitry Kozliuk <Dmitry.Kozliuk@gmail.com>
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
> v1->v2:
> - follow the suggestion by Nick and Dmitry
> - http://mails.dpdk.org/archives/dev/2021-August/216304.html
Please send new versions as replies to the previous ones:
git send-email --in-reply-to MSGID ...
https://doc.dpdk.org/guides/contributing/patches.html
On Wed, Aug 11, 2021 at 8:50 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> Hi William,
>
> 2021-08-11 20:46 (UTC+0000), William Tu:
> > Currently there are a couple of header files include 'sys/queue.h',
> > which is a POSIX functionality. When compiling DPDK with OVS on
> > Windows, we encountered issues such as, found the missing header.
> > In file included from ../lib/dpdk.c:27:
> > C:\temp\dpdk\include\rte_log.h:24:10: fatal error: 'sys/queue.h' file
> > not found
> >
> > The patch fixes it by 1) removing the #include <sys/queue.h> from
> > a couple of headers, replace it with #include <rte_os.h>, and
> > 2) include sys/queue.h in the rte_os.h. As a result, for Linux/BSD,
> > it is using the sys/queue.h from its POSIX library, and for Windows,
> > DPDK will use the bundled sys/queue.h.
> >
> > 1) fixes the case that DPDK library shouldn't export POSIX functionality
> > into the environment (symbols, macros, headers etc), which cause definitions
> > clashing with the application.
> > 2) fixes the case that DPDK should depend only on the C library and not
> > require POSIX functionality from the underlying system.
>
> Sorry, this is not exactly what was suggested here:
>
> http://inbox.dpdk.org/dev/20210811013325.34c36220@sovereign/
>
> The point was not to install <sys/queue.h>. Its full content is not needed
> for DPDK interface, only for implementation. Public headers only need a
> handful of macros for list/tailq heads and links. Those macros should be
> provided by DPDK, with RTE_ prefix. For Linux and FreeBSD it will just be:
>
> #include <sys/queue.h>
> #define RTE_TAILQ_ENTRY(type) TAILQ_ENTRY(type)
> /* ... */
>
> For Windows you would have to copy definitions from <sys/queue.h>
> to some public header, <rte_os.h> seems OK. All public headers that need
> <sys/queue.h> macros must have them replaced with RTE_ version.
> Implementation remains unchanged. No new files need to be installed,
> because when DPDK is built on Windows, it uses the bundled <sys/queue.h>,
> and when it is installed, only RTE_ macros you created are visible.
>
> Macro documentation should clearly state that they are compatible with system
> <sys/queue.h> for Linux and FreeBSD, and for Windows they are compatible with
> the version used during build.
>
> +Bruce, David, Ray, Tyler to confirm/object the idea.
>
> > There are still a couple of headers using sys/queue.h, ex:
> > rte_bus_pci.h. Since it's not been used in Windows yet, we can
> > fix them later.
>
> Another item I think of is a checkpatch rule to prohibit TAILQ_xxx, etc
> macro in public headers after replacing them all.
>
> > Suggested-by: Nick Connolly <nick.connolly@mayadata.io>
> > Suggested-by: Dmitry Kozliuk <Dmitry.Kozliuk@gmail.com>
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
> > v1->v2:
> > - follow the suggestion by Nick and Dmitry
> > - http://mails.dpdk.org/archives/dev/2021-August/216304.html
>
> Please send new versions as replies to the previous ones:
>
> git send-email --in-reply-to MSGID ...
>
> https://doc.dpdk.org/guides/contributing/patches.html
Hi Dmitry,
Thanks! I see your point now. I will work on the next patch.
William
@@ -11,6 +11,7 @@
*/
#include <pthread_np.h>
+#include <sys/queue.h>
typedef cpuset_t rte_cpuset_t;
#define RTE_HAS_CPUSET
@@ -19,8 +19,8 @@ extern "C" {
#endif
#include <stdio.h>
-#include <sys/queue.h>
+#include <rte_os.h>
#include <rte_log.h>
#include <rte_dev.h>
@@ -22,8 +22,6 @@
extern "C" {
#endif
-#include <sys/queue.h>
-
#include <rte_dev.h>
/** Double linked list of classes */
@@ -18,8 +18,8 @@ extern "C" {
#endif
#include <stdio.h>
-#include <sys/queue.h>
+#include <rte_os.h>
#include <rte_config.h>
#include <rte_compat.h>
#include <rte_log.h>
@@ -21,7 +21,6 @@ extern "C" {
#endif
#include <stdio.h>
-#include <sys/queue.h>
#include <rte_compat.h>
#include <rte_bus.h>
@@ -21,7 +21,6 @@ extern "C" {
#include <stdio.h>
#include <stdarg.h>
#include <stdbool.h>
-#include <sys/queue.h>
#include <rte_common.h>
#include <rte_config.h>
@@ -29,7 +29,6 @@ extern "C" {
#include<stdio.h>
#include <stdint.h>
-#include <sys/queue.h>
#include <rte_config.h>
#include <rte_lcore.h>
@@ -15,8 +15,8 @@
extern "C" {
#endif
-#include <sys/queue.h>
#include <stdio.h>
+#include <rte_os.h>
#include <rte_debug.h>
/** dummy structure type used by the rte_tailq APIs */
@@ -11,6 +11,7 @@
*/
#include <sched.h>
+#include <sys/queue.h>
#ifdef CPU_SETSIZE /* may require _GNU_SOURCE */
typedef cpu_set_t rte_cpuset_t;
@@ -8,3 +8,7 @@ headers += files(
'rte_virt2phys.h',
'rte_windows.h',
)
+
+sys_headers = []
+subdir('sys')
+install_headers(sys_headers, subdir: 'sys')
@@ -13,6 +13,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/queue.h>
#ifdef __cplusplus
extern "C" {
new file mode 100644
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2021 VMware, Inc
+
+sys_headers += files(
+ 'queue.h',
+)
@@ -38,7 +38,6 @@
#include <stdint.h>
#include <errno.h>
#include <inttypes.h>
-#include <sys/queue.h>
#include <rte_config.h>
#include <rte_spinlock.h>