[dpdk-dev] [PATCH v3 0/7] MinGW-w64 support

[dpdk-dev] [PATCH v3 0/7] MinGW-w64 support

[Dmitry Kozlyuk]

This patch series add support for building DPDK using MinGW-w64.

MinGW-w64 provides GNU toolchain and independent platform SDK on
Windows. It also supports cross-compilation to Windows from POSIX
systems by providing cross tollchains and libraries [0]. It does NOT
emulate a full POSIX environment, like Cygwin or MSYS do.

There are advantages in using MinGW-w64 in addition to Clang:

1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
   implementation, GNU getopt, and Windows platform SDK.

2. Easier porting of POSIX applications using DPDK to Windows, because
   application code can use the same benefits as mentioned above.

3. Having both primary compilers enabled on Windows provides more
   diagnostics and generally prevents non-portable code.

[0]: http://mingw-w64.org

v3 Changes:

    Rebase onto the latest Windows EAL.
    Prevent format attribute conflict with upcoming v20.05 patches.
    Remove redundant CFLAGS from Meson.
    Remove links to Meson bugtracker from docs (PR merged into upstream).
    Fix Clang warnings using about GNU options.

v2 Changes:

    Add patch to use lowercase system header filenames.
    Move Meson cross-file for x86 to arch directory.
    Change wording in comments.
    Add Meson version warning in documentation.

---

Dmitry Kozlyuk (7):
  eal: introduce portable format attribute
  eal/windows: use lowercase filenames for system headers
  eal/windows: support builing with MinGW-w64
  build: MinGW-w64 support for Meson
  build: add cross-file for MinGW-w64
  doc: guide for Windows build using MinGW-w64
  build: fix linker warnings with Clang on Windows

 config/meson.build                          | 26 +++++++--
 config/x86/meson_mingw.txt                  | 14 +++++
 doc/guides/windows_gsg/build_dpdk.rst       | 63 ++++++++++++++++++---
 lib/librte_cmdline/cmdline.h                |  4 +-
 lib/librte_eal/common/include/rte_common.h  | 17 +++++-
 lib/librte_eal/common/include/rte_debug.h   |  2 +-
 lib/librte_eal/common/include/rte_devargs.h |  2 +-
 lib/librte_eal/common/include/rte_log.h     |  4 +-
 lib/librte_eal/meson.build                  |  3 +
 lib/librte_eal/windows/eal/eal.c            |  6 +-
 lib/librte_eal/windows/eal/include/getopt.h |  4 ++
 lib/librte_eal/windows/eal/include/rte_os.h |  6 +-
 lib/meson.build                             |  8 ++-
 13 files changed, 134 insertions(+), 25 deletions(-)
 create mode 100644 config/x86/meson_mingw.txt

-- 
2.25.0

[dpdk-dev] [PATCH v3 1/7] eal: introduce portable format attribute

[Dmitry Kozlyuk]

When using __attribute__((format(...)) on functions, GCC on Windows
assumes MS-specific format string by default, even if the underlying
stdio implementation is ANSI-compliant (either MS Unicersal CRT
or MinGW implementation). Wrap attribute into a macro that forces
GNU-specific format string when using GCC.

Use this new attribute for logging and panic messages in EAL
and for output strings in cmdline library.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.h                |  4 +++-
 lib/librte_eal/common/include/rte_common.h  | 17 ++++++++++++++++-
 lib/librte_eal/common/include/rte_debug.h   |  2 +-
 lib/librte_eal/common/include/rte_devargs.h |  2 +-
 lib/librte_eal/common/include/rte_log.h     |  4 ++--
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 27d2effdf..243f99d20 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -7,6 +7,8 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
+#include <rte_common.h>
+
 #include <termios.h>
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -34,7 +36,7 @@ struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
 void cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
-	__attribute__((format(printf,2,3)));
+	__rte_format_printf(2, 3);
 int cmdline_in(struct cmdline *cl, const char *buf, int size);
 int cmdline_write_char(struct rdline *rdl, char c);
 
diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index 4b5f3a31f..226c1a011 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -89,6 +89,21 @@ typedef uint16_t unaligned_uint16_t;
  */
 #define RTE_SET_USED(x) (void)(x)
 
+/**
+ * Check format string and its arguments at compile-time.
+ *
+ * GCC on Windows assumes MS-specific format string by default,
+ * even if the underlying stdio implementation is ANSI-compliant,
+ * so this must be overridden.
+ */
+#if defined(RTE_TOOLCHAIN_GCC)
+#define __rte_format_printf(format_index, first_arg) \
+	__attribute__((format(gnu_printf, format_index, first_arg)))
+#else
+#define __rte_format_printf(format_index, first_arg) \
+	__attribute__((format(printf, format_index, first_arg)))
+#endif
+
 #define RTE_PRIORITY_LOG 101
 #define RTE_PRIORITY_BUS 110
 #define RTE_PRIORITY_CLASS 120
@@ -784,7 +799,7 @@ rte_str_to_size(const char *str)
 void
 rte_exit(int exit_code, const char *format, ...)
 	__attribute__((noreturn))
-	__attribute__((format(printf, 2, 3)));
+	__rte_format_printf(2, 3);
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/common/include/rte_debug.h b/lib/librte_eal/common/include/rte_debug.h
index 748d32c80..7edd4b89c 100644
--- a/lib/librte_eal/common/include/rte_debug.h
+++ b/lib/librte_eal/common/include/rte_debug.h
@@ -73,7 +73,7 @@ void __rte_panic(const char *funcname , const char *format, ...)
 #endif
 #endif
 	__attribute__((noreturn))
-	__attribute__((format(printf, 2, 3)));
+	__rte_format_printf(2, 3);
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 882dfa0ab..898efa0d6 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -137,7 +137,7 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev);
 int
 rte_devargs_parsef(struct rte_devargs *da,
 		   const char *format, ...)
-__attribute__((format(printf, 2, 0)));
+__rte_format_printf(2, 0);
 
 /**
  * Insert an rte_devargs in the global list.
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 1bb0e6694..a0d1f4837 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -282,7 +282,7 @@ int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
 	__attribute__((cold))
 #endif
 #endif
-	__attribute__((format(printf, 3, 4)));
+	__rte_format_printf(3, 4);
 
 /**
  * Generates a log message.
@@ -311,7 +311,7 @@ int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
  *   - Negative on error.
  */
 int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
-	__attribute__((format(printf,3,0)));
+	__rte_format_printf(3, 0);
 
 /**
  * Generates a log message.
-- 
2.25.0

[dpdk-dev] [PATCH v3 2/7] eal/windows: use lowercase filenames for system headers

[Dmitry Kozlyuk]

Mixed case in Windows header names causes errors when cross-compiling
from Linux with case-sensitive filesystem using MinGW, because MinGW
distribution provides all platform SDK headers in lowercase. The change
does not affect Windows native builds on case-insensitive filesystems
(NTFS default).

Reported-by: Narcisa Ana Maria Vasile <Narcisa.Vasile@microsoft.com>
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/windows/eal/include/rte_os.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/windows/eal/include/rte_os.h b/lib/librte_eal/windows/eal/include/rte_os.h
index 9e762617b..c351f2c84 100644
--- a/lib/librte_eal/windows/eal/include/rte_os.h
+++ b/lib/librte_eal/windows/eal/include/rte_os.h
@@ -15,8 +15,8 @@
 extern "C" {
 #endif
 
-#include <Windows.h>
-#include <BaseTsd.h>
+#include <windows.h>
+#include <basetsd.h>
 #include <pthread.h>
 #include <stdio.h>
 
-- 
2.25.0

[dpdk-dev] [PATCH v3 3/7] eal/windows: support builing with MinGW-w64

[Dmitry Kozlyuk]

Disable bundled getopt implementation and GNU extensions shim when
building in GNU environment.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/meson.build                  | 3 +++
 lib/librte_eal/windows/eal/eal.c            | 6 ++++--
 lib/librte_eal/windows/eal/include/getopt.h | 4 ++++
 lib/librte_eal/windows/eal/include/rte_os.h | 2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index 4be5118ce..1730d603f 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -20,6 +20,9 @@ endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
 	cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
+if cc.has_header('getopt.h')
+	cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
+endif
 sources = common_sources + env_sources
 objs = common_objs + env_objs
 headers = common_headers + env_headers
diff --git a/lib/librte_eal/windows/eal/eal.c b/lib/librte_eal/windows/eal/eal.c
index 34852d42c..e4b50df3b 100644
--- a/lib/librte_eal/windows/eal/eal.c
+++ b/lib/librte_eal/windows/eal/eal.c
@@ -2,9 +2,11 @@
  * Copyright(c) 2019 Intel Corporation
  */
 
-#include <sys/stat.h>
-#include <io.h>
 #include <fcntl.h>
+#include <io.h>
+#include <share.h>
+#include <sys/stat.h>
+
 #include <rte_debug.h>
 #include <rte_eal.h>
 #include <eal_memcfg.h>
diff --git a/lib/librte_eal/windows/eal/include/getopt.h b/lib/librte_eal/windows/eal/include/getopt.h
index 2eebe54e3..6f57af454 100644
--- a/lib/librte_eal/windows/eal/include/getopt.h
+++ b/lib/librte_eal/windows/eal/include/getopt.h
@@ -26,7 +26,11 @@
 #ifndef NEED_USUAL_GETOPT
 
 /* Use system getopt */
+#ifdef RTE_TOOLCHAIN_GCC
+#include_next <getopt.h>
+#else
 #include <getopt.h>
+#endif
 
 #else /* NEED_USUAL_GETOPT */
 
diff --git a/lib/librte_eal/windows/eal/include/rte_os.h b/lib/librte_eal/windows/eal/include/rte_os.h
index c351f2c84..4b9117dc0 100644
--- a/lib/librte_eal/windows/eal/include/rte_os.h
+++ b/lib/librte_eal/windows/eal/include/rte_os.h
@@ -57,6 +57,7 @@ int eal_thread_create(pthread_t *thread);
  */
 void eal_create_cpu_map(void);
 
+#ifndef RTE_TOOLCHAIN_GCC
 static inline int
 asprintf(char **buffer, const char *format, ...)
 {
@@ -80,6 +81,7 @@ asprintf(char **buffer, const char *format, ...)
 	}
 	return ret;
 }
+#endif /* RTE_TOOLCHAIN_GCC */
 
 /* cpu_set macros implementation */
 #define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
-- 
2.25.0

[dpdk-dev] [PATCH v3 4/7] build: MinGW-w64 support for Meson

[Dmitry Kozlyuk]

MinGW-w64 linker does not mimic MS linker options, so the build system
must differentiate between linkers on Windows. Use GNU linker options
with GCC and MS linker options with Clang.

MinGW-w64 by default uses MSVCRT stdio, which does not comply to ANSI,
most notably its formatting and string handling functions. MinGW-w64
support for the Universal CRT (UCRT) is ongoing, but the toolchain
provides its own standard-complying implementation of stdio. The latter
is used in the patch to support formatting in DPDK.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build | 14 ++++++++++++++
 lib/meson.build    |  8 ++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 6c46767e3..61eeec0de 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -14,6 +14,10 @@ foreach env:supported_exec_envs
 	set_variable('is_' + env, exec_env == env)
 endforeach
 
+# MS linker requires special treatment.
+# FIXME: use cc.get_linker_id() with Meson >= 0.54
+is_ms_linker = is_windows and (cc.get_id() == 'clang')
+
 # set the major version, which might be used by drivers and libraries
 # depending on the configuration options
 pver = meson.project_version().split('.')
@@ -247,6 +251,16 @@ if is_freebsd
 	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
 endif
 
+if is_windows
+	# Minimum supported API is Windows 7.
+	add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
+
+	# Use MinGW-w64 stdio, because DPDK assumes ANSI-compliant formatting.
+	if cc.get_id() == 'gcc'
+		add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
+	endif
+endif
+
 if get_option('b_lto')
 	if cc.has_argument('-ffat-lto-objects')
 		add_project_arguments('-ffat-lto-objects', language: 'c')
diff --git a/lib/meson.build b/lib/meson.build
index 0af3efab2..9c3cc55d5 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -148,12 +148,16 @@ foreach l:libraries
 				command: [map_to_def_cmd, '@INPUT@', '@OUTPUT@'],
 				input: version_map,
 				output: 'rte_@0@_exports.def'.format(name))
-			lk_deps = [version_map, def_file]
-			if is_windows
+
+			if is_ms_linker
 				lk_args = ['-Wl,/def:' + def_file.full_path(),
 					'-Wl,/implib:lib\\' + implib]
 			else
 				lk_args = ['-Wl,--version-script=' + version_map]
+			endif
+
+			lk_deps = [version_map, def_file]
+			if not is_windows
 				# on unix systems check the output of the
 				# experimental syms script, using it as a
 				# dependency of the .so build
-- 
2.25.0

[dpdk-dev] [PATCH v3 5/7] build: add cross-file for MinGW-w64

[Dmitry Kozlyuk]

Add Meson configuration to cross-compile for Windows using MinGW-w64.
It may require adjustments in some cases, but at least it provides
the foundation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/x86/meson_mingw.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 config/x86/meson_mingw.txt

diff --git a/config/x86/meson_mingw.txt b/config/x86/meson_mingw.txt
new file mode 100644
index 000000000..80f04343a
--- /dev/null
+++ b/config/x86/meson_mingw.txt
@@ -0,0 +1,14 @@
+[binaries]
+c = '/usr/bin/x86_64-w64-mingw32-gcc'
+cpp = '/usr/bin/x86_64-w64-mingw32-g++'
+ld = '/usr/bin/x86_64-w64-mingw32-ld' # Meson 0.53.0
+c_ld = '/usr/bin/x86_64-w64-mingw32-ld'
+ar = '/usr/bin/x86_64-w64-mingw32-ar'
+strip = '/usr/bin/x86_64-w64-mingw32-strip'
+pkgconfig = '/usr/bin/x86_64-w64-mingw32-pkg-config'
+
+[host_machine]
+system = 'windows'
+cpu_family = 'x86_64'
+cpu = 'native'
+endian = 'little'
-- 
2.25.0

[dpdk-dev] [PATCH v3 6/7] doc: guide for Windows build using MinGW-w64

[Dmitry Kozlyuk]

Instructions for different toolchains presented as options on the
corresponging steps of the guide, so that common parts may be reused.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 doc/guides/windows_gsg/build_dpdk.rst | 63 +++++++++++++++++++++++----
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/doc/guides/windows_gsg/build_dpdk.rst b/doc/guides/windows_gsg/build_dpdk.rst
index 6711e07e2..c77384785 100644
--- a/doc/guides/windows_gsg/build_dpdk.rst
+++ b/doc/guides/windows_gsg/build_dpdk.rst
@@ -7,15 +7,22 @@ Compiling the DPDK Target from Source
 System Requirements
 -------------------
 
-The DPDK and its applications require the Clang-LLVM C compiler
-and Microsoft MSVC linker.
+Building the DPDK and its applications requires one of the following
+environments:
+
+* The Clang-LLVM C compiler and Microsoft MSVC linker.
+* The MinGW-w64 toolchain (either native or cross).
+
 The Meson Build system is used to prepare the sources for compilation
 with the Ninja backend.
 The installation of these tools is covered in this section.
 
 
+Option 1. Clang-LLVM C Compiler and Microsoft MSVC Linker
+---------------------------------------------------------
+
 Install the Compiler
---------------------
+~~~~~~~~~~~~~~~~~~~~
 
 Download and install the clang compiler from
 `LLVM website <http://releases.llvm.org/download.html>`_.
@@ -25,7 +32,7 @@ For example, Clang-LLVM direct download link::
 
 
 Install the Linker
-------------------
+~~~~~~~~~~~~~~~~~~
 
 Download and install the Build Tools for Visual Studio to link and build the
 files on windows,
@@ -34,6 +41,15 @@ When installing build tools, select the "Visual C++ build tools" option
 and ensure the Windows SDK is selected.
 
 
+Option 2. MinGW-w64 Toolchain
+-----------------------------
+
+Obtain the latest version from
+`MinGW-w64 website <http://mingw-w64.org/doku.php/download>`_.
+On Windows, install to a folder without spaces in its name, like ``C:\MinGW``.
+This path is assumed for the rest of this guide.
+
+
 Install the Build System
 ------------------------
 
@@ -43,6 +59,12 @@ A good option to choose is the MSI installer for both meson and ninja together::
 
 	http://mesonbuild.com/Getting-meson.html#installing-meson-and-ninja-with-the-msi-installer%22
 
+.. warning::
+
+    Meson 0.47.1 is recommended, Meson 0.52 is the latest version known
+    to build DPDK successfully. Meson 0.53 has fatal issues with Clang
+    that prevent both native and cross-compilation.
+
 Install the Backend
 -------------------
 
@@ -56,23 +78,41 @@ Build the code
 The build environment is setup to build the EAL and the helloworld example by
 default.
 
-Using the ninja backend
-~~~~~~~~~~~~~~~~~~~~~~~~
+Option 1. Native Build on Windows
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Specifying the compiler might be required to complete the meson command.
+When using Clang-LLVM, specifying the compiler might be required to complete
+the meson command:
 
 .. code-block:: console
 
     set CC=clang
 
+When using MinGW-w64, it is sufficient to have toolchain executables in PATH:
+
+.. code-block:: console
+
+    set PATH=C:\MinGW\mingw64\bin;%PATH%
+
 To compile the examples, the flag ``-Dexamples`` is required.
 
 .. code-block:: console
 
     cd C:\Users\me\dpdk
     meson -Dexamples=helloworld build
-    cd build
-    ninja
+    ninja -C build
+
+Option 2. Cross-Compile with MinGW-w64
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The cross-file option must be specified for Meson.
+Depending on the distribution, paths in this file may need adjustments.
+
+.. code-block:: console
+
+    meson --cross-file config/x86/meson_mingw.txt -Dexamples=helloworld build
+    ninja -C build
+
 
 Run the helloworld example
 ==========================
@@ -87,3 +127,8 @@ Navigate to the examples in the build directory and run `dpdk-helloworld.exe`.
     hello from core 3
     hello from core 0
     hello from core 2
+
+Note for MinGW-w64: applications are linked to ``libwinpthread-1.dll``
+by default. To run the example, either add toolchain executables directory
+to the PATH or copy the library to the working directory.
+Alternatively, static linking may be used (mind the LGPLv2.1 license).
-- 
2.25.0

[dpdk-dev] [PATCH v3 7/7] build: fix linker warnings with Clang on Windows

[Dmitry Kozlyuk]

Clang on Windows doesn't use pthread for now, while MinGW does. Removing
`-pthread` option with MS linker fixes the following warning:

    clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]

Option `--no-as-needed` is meaningless for PE output. Disabling it on
Windows fixes the following warning:

    LINK : warning LNK4044: unrecognized option '/-no-as-needed'; ignored

Fixes: 98edcbb5a ("eal/windows: introduce Windows support")

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 61eeec0de..69e53e41d 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -112,11 +112,15 @@ dpdk_conf.set('RTE_TOOLCHAIN_' + toolchain.to_upper(), 1)
 
 dpdk_conf.set('RTE_ARCH_64', cc.sizeof('void *') == 8)
 
-add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
+if not is_windows
+	add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
+endif
 
-# use pthreads
-add_project_link_arguments('-pthread', language: 'c')
-dpdk_extra_ldflags += '-pthread'
+# use pthreads if available for the platform
+if not is_ms_linker
+	add_project_link_arguments('-pthread', language: 'c')
+	dpdk_extra_ldflags += '-pthread'
+endif
 
 # on some OS, maths functions are in a separate library
 if cc.find_library('m', required : false).found()
-- 
2.25.0

Re: [dpdk-dev] [PATCH v3 4/7] build: MinGW-w64 support for Meson

[Thomas Monjalon]

18/02/2020 01:02, Dmitry Kozlyuk:
> --- a/config/meson.build
> +++ b/config/meson.build
> +# MS linker requires special treatment.
> +# FIXME: use cc.get_linker_id() with Meson >= 0.54
> +is_ms_linker = is_windows and (cc.get_id() == 'clang')

Please could you replace this FIXME with an actual fix in meson.build?

Re: [dpdk-dev] [PATCH v3 4/7] build: MinGW-w64 support for Meson

[Bruce Richardson]

On Tue, Feb 18, 2020 at 03:26:47PM +0100, Thomas Monjalon wrote:
> 18/02/2020 01:02, Dmitry Kozlyuk:
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > +# MS linker requires special treatment.
> > +# FIXME: use cc.get_linker_id() with Meson >= 0.54
> > +is_ms_linker = is_windows and (cc.get_id() == 'clang')
> 
> Please could you replace this FIXME with an actual fix in meson.build?
> 
Best not to at this point, as even with an explicit version check for the
new function, you can still get a warning from meson about using functions
from versions greater than baseline. In this release we have removed most,
if not all, existing warnings and I'd rather not add any back in.

Suggest replacing the fixme with a NOTE or TODO, since it's not a bug that
needs to be fixed, rather a future enhancement once we update our meson
baseline.

/Bruce

Re: [dpdk-dev] [PATCH v3 0/7] MinGW-w64 support

[William Tu]

On Mon, Feb 17, 2020 at 4:02 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> This patch series add support for building DPDK using MinGW-w64.
>
> MinGW-w64 provides GNU toolchain and independent platform SDK on
> Windows. It also supports cross-compilation to Windows from POSIX
> systems by providing cross tollchains and libraries [0]. It does NOT
> emulate a full POSIX environment, like Cygwin or MSYS do.
>
> There are advantages in using MinGW-w64 in addition to Clang:
>
> 1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
>    implementation, GNU getopt, and Windows platform SDK.
>
> 2. Easier porting of POSIX applications using DPDK to Windows, because
>    application code can use the same benefits as mentioned above.
>
> 3. Having both primary compilers enabled on Windows provides more
>    diagnostics and generally prevents non-portable code.
>
> [0]: http://mingw-w64.org
>
> v3 Changes:
>
>     Rebase onto the latest Windows EAL.
>     Prevent format attribute conflict with upcoming v20.05 patches.
>     Remove redundant CFLAGS from Meson.
>     Remove links to Meson bugtracker from docs (PR merged into upstream).
>     Fix Clang warnings using about GNU options.
>
> v2 Changes:
>
>     Add patch to use lowercase system header filenames.
>     Move Meson cross-file for x86 to arch directory.
>     Change wording in comments.
>     Add Meson version warning in documentation.
>
> ---

I have to apply below diff to make it work:

diff --git a/lib/librte_eal/windows/eal/include/pthread.h
b/lib/librte_eal/windows/eal/include/pthread.h
index 4ac24de0aa27..b9dd18e56815 100644
--- a/lib/librte_eal/windows/eal/include/pthread.h
+++ b/lib/librte_eal/windows/eal/include/pthread.h
@@ -14,7 +14,7 @@
 extern "C" {
 #endif

-#include <Windows.h>
+#include <windows.h>

 #define PTHREAD_BARRIER_SERIAL_THREAD TRUE


Otherwise, the series look good to me.
Acked-by: William Tu <u9012063@gmail.com>

Thanks
William

Re: [dpdk-dev] [PATCH v3 6/7] doc: guide for Windows build using MinGW-w64

[William Tu]

On Mon, Feb 17, 2020 at 4:03 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> Instructions for different toolchains presented as options on the
> corresponging steps of the guide, so that common parts may be reused.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  doc/guides/windows_gsg/build_dpdk.rst | 63 +++++++++++++++++++++++----
>  1 file changed, 54 insertions(+), 9 deletions(-)
>
> diff --git a/doc/guides/windows_gsg/build_dpdk.rst b/doc/guides/windows_gsg/build_dpdk.rst
> index 6711e07e2..c77384785 100644
> --- a/doc/guides/windows_gsg/build_dpdk.rst
> +++ b/doc/guides/windows_gsg/build_dpdk.rst
> @@ -7,15 +7,22 @@ Compiling the DPDK Target from Source
>  System Requirements
>  -------------------
>
> -The DPDK and its applications require the Clang-LLVM C compiler
> -and Microsoft MSVC linker.
> +Building the DPDK and its applications requires one of the following
> +environments:
> +
> +* The Clang-LLVM C compiler and Microsoft MSVC linker.
> +* The MinGW-w64 toolchain (either native or cross).
> +
>  The Meson Build system is used to prepare the sources for compilation
>  with the Ninja backend.
>  The installation of these tools is covered in this section.
>
>
> +Option 1. Clang-LLVM C Compiler and Microsoft MSVC Linker
> +---------------------------------------------------------
> +
>  Install the Compiler
> ---------------------
> +~~~~~~~~~~~~~~~~~~~~
>
>  Download and install the clang compiler from
>  `LLVM website <http://releases.llvm.org/download.html>`_.
> @@ -25,7 +32,7 @@ For example, Clang-LLVM direct download link::
>
>
>  Install the Linker
> -------------------
> +~~~~~~~~~~~~~~~~~~
>
>  Download and install the Build Tools for Visual Studio to link and build the
>  files on windows,
> @@ -34,6 +41,15 @@ When installing build tools, select the "Visual C++ build tools" option
>  and ensure the Windows SDK is selected.
>
>
> +Option 2. MinGW-w64 Toolchain
> +-----------------------------
> +
> +Obtain the latest version from
> +`MinGW-w64 website <http://mingw-w64.org/doku.php/download>`_.
> +On Windows, install to a folder without spaces in its name, like ``C:\MinGW``.
> +This path is assumed for the rest of this guide.
> +
> +
>  Install the Build System
>  ------------------------
>
> @@ -43,6 +59,12 @@ A good option to choose is the MSI installer for both meson and ninja together::
>
>         http://mesonbuild.com/Getting-meson.html#installing-meson-and-ninja-with-the-msi-installer%22
>
> +.. warning::
> +
> +    Meson 0.47.1 is recommended, Meson 0.52 is the latest version known
> +    to build DPDK successfully. Meson 0.53 has fatal issues with Clang
> +    that prevent both native and cross-compilation.
> +
>  Install the Backend
>  -------------------
>
> @@ -56,23 +78,41 @@ Build the code
>  The build environment is setup to build the EAL and the helloworld example by
>  default.
>
> -Using the ninja backend
> -~~~~~~~~~~~~~~~~~~~~~~~~
> +Option 1. Native Build on Windows
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> -Specifying the compiler might be required to complete the meson command.
> +When using Clang-LLVM, specifying the compiler might be required to complete
> +the meson command:
>
>  .. code-block:: console
>
>      set CC=clang
>
> +When using MinGW-w64, it is sufficient to have toolchain executables in PATH:
> +
> +.. code-block:: console
> +
> +    set PATH=C:\MinGW\mingw64\bin;%PATH%
> +
>  To compile the examples, the flag ``-Dexamples`` is required.
>
>  .. code-block:: console
>
>      cd C:\Users\me\dpdk
>      meson -Dexamples=helloworld build
> -    cd build
> -    ninja
> +    ninja -C build
> +
> +Option 2. Cross-Compile with MinGW-w64
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The cross-file option must be specified for Meson.
> +Depending on the distribution, paths in this file may need adjustments.
> +
> +.. code-block:: console
> +
> +    meson --cross-file config/x86/meson_mingw.txt -Dexamples=helloworld build
> +    ninja -C build
> +
>
>  Run the helloworld example
>  ==========================
> @@ -87,3 +127,8 @@ Navigate to the examples in the build directory and run `dpdk-helloworld.exe`.
>      hello from core 3
>      hello from core 0
>      hello from core 2
> +
> +Note for MinGW-w64: applications are linked to ``libwinpthread-1.dll``
> +by default. To run the example, either add toolchain executables directory
> +to the PATH or copy the library to the working directory.
> +Alternatively, static linking may be used (mind the LGPLv2.1 license).
> --

btw, just share my experience.

Using mingw-w64 4.0.4 on ubuntu 1604 does not work due to
lib/librte_eal.a(librte_eal_common_eal_common_thread.c.obj):eal_common_thread.c:(.text+0x1f):
undefined reference to `EnterSynchronizationBarrier'
https://sourceforge.net/p/mingw-w64/bugs/562/

This is fixed in mingw-w64 5.0.3 ubuntu 1804.
William

[dpdk-dev] [PATCH v4 0/7] MinGW-w64 support

[Dmitry Kozlyuk]

This patch series add support for building DPDK using MinGW-w64.

MinGW-w64 provides GNU toolchain and independent platform SDK on
Windows. It also supports cross-compilation to Windows from POSIX
systems by providing cross tollchains and libraries [0]. It does NOT
emulate a full POSIX environment, like Cygwin or MSYS do.

There are advantages in using MinGW-w64 in addition to Clang:

1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
   implementation, GNU getopt, and Windows platform SDK.

2. Easier porting of POSIX applications using DPDK to Windows, because
   application code can use the same benefits as mentioned above.

3. Having both primary compilers enabled on Windows provides more
   diagnostics and generally prevents non-portable code.

[0]: http://mingw-w64.org

v4 Changes:

    Fix Windows headers include case.
    Recommend the latest Meson in docs (build fixed with 0.53.2).

v3 Changes:

    Rebase onto the latest Windows EAL.
    Prevent format attribute conflict with upcoming v20.05 patches.
    Remove redundant CFLAGS from Meson.
    Remove links to Meson bugtracker from docs (PR merged into upstream).
    Fix Clang warnings using about GNU options.

v2 Changes:

    Add patch to use lowercase system header filenames.
    Move Meson cross-file for x86 to arch directory.
    Change wording in comments.
    Add Meson version warning in documentation.

Dmitry Kozlyuk (7):
  eal: introduce portable format attribute
  eal/windows: use lowercase filenames for system headers
  eal/windows: support builing with MinGW-w64
  build: MinGW-w64 support for Meson
  build: add cross-file for MinGW-w64
  doc: guide for Windows build using MinGW-w64
  build: fix linker warnings with Clang on Windows

 config/meson.build                           | 26 ++++++--
 config/x86/meson_mingw.txt                   | 13 ++++
 doc/guides/windows_gsg/build_dpdk.rst        | 62 +++++++++++++++++---
 lib/librte_cmdline/cmdline.h                 |  4 +-
 lib/librte_eal/common/include/rte_common.h   | 17 +++++-
 lib/librte_eal/common/include/rte_debug.h    |  2 +-
 lib/librte_eal/common/include/rte_devargs.h  |  2 +-
 lib/librte_eal/common/include/rte_log.h      |  4 +-
 lib/librte_eal/meson.build                   |  3 +
 lib/librte_eal/windows/eal/eal.c             |  6 +-
 lib/librte_eal/windows/eal/include/getopt.h  |  4 ++
 lib/librte_eal/windows/eal/include/pthread.h |  2 +-
 lib/librte_eal/windows/eal/include/rte_os.h  |  6 +-
 lib/meson.build                              |  8 ++-
 14 files changed, 133 insertions(+), 26 deletions(-)
 create mode 100644 config/x86/meson_mingw.txt

-- 
2.25.1

[dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Dmitry Kozlyuk]

When using __attribute__((format(...)) on functions, GCC on Windows
assumes MS-specific format string by default, even if the underlying
stdio implementation is ANSI-compliant (either MS Unicersal CRT
or MinGW implementation). Wrap attribute into a macro that forces
GNU-specific format string when using GCC.

Use this new attribute for logging and panic messages in EAL
and for output strings in cmdline library.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_cmdline/cmdline.h                |  4 +++-
 lib/librte_eal/common/include/rte_common.h  | 17 ++++++++++++++++-
 lib/librte_eal/common/include/rte_debug.h   |  2 +-
 lib/librte_eal/common/include/rte_devargs.h |  2 +-
 lib/librte_eal/common/include/rte_log.h     |  4 ++--
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 27d2effdf..243f99d20 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -7,6 +7,8 @@
 #ifndef _CMDLINE_H_
 #define _CMDLINE_H_
 
+#include <rte_common.h>
+
 #include <termios.h>
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -34,7 +36,7 @@ struct cmdline *cmdline_new(cmdline_parse_ctx_t *ctx, const char *prompt, int s_
 void cmdline_set_prompt(struct cmdline *cl, const char *prompt);
 void cmdline_free(struct cmdline *cl);
 void cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
-	__attribute__((format(printf,2,3)));
+	__rte_format_printf(2, 3);
 int cmdline_in(struct cmdline *cl, const char *buf, int size);
 int cmdline_write_char(struct rdline *rdl, char c);
 
diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index 4b5f3a31f..226c1a011 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -89,6 +89,21 @@ typedef uint16_t unaligned_uint16_t;
  */
 #define RTE_SET_USED(x) (void)(x)
 
+/**
+ * Check format string and its arguments at compile-time.
+ *
+ * GCC on Windows assumes MS-specific format string by default,
+ * even if the underlying stdio implementation is ANSI-compliant,
+ * so this must be overridden.
+ */
+#if defined(RTE_TOOLCHAIN_GCC)
+#define __rte_format_printf(format_index, first_arg) \
+	__attribute__((format(gnu_printf, format_index, first_arg)))
+#else
+#define __rte_format_printf(format_index, first_arg) \
+	__attribute__((format(printf, format_index, first_arg)))
+#endif
+
 #define RTE_PRIORITY_LOG 101
 #define RTE_PRIORITY_BUS 110
 #define RTE_PRIORITY_CLASS 120
@@ -784,7 +799,7 @@ rte_str_to_size(const char *str)
 void
 rte_exit(int exit_code, const char *format, ...)
 	__attribute__((noreturn))
-	__attribute__((format(printf, 2, 3)));
+	__rte_format_printf(2, 3);
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/common/include/rte_debug.h b/lib/librte_eal/common/include/rte_debug.h
index 748d32c80..7edd4b89c 100644
--- a/lib/librte_eal/common/include/rte_debug.h
+++ b/lib/librte_eal/common/include/rte_debug.h
@@ -73,7 +73,7 @@ void __rte_panic(const char *funcname , const char *format, ...)
 #endif
 #endif
 	__attribute__((noreturn))
-	__attribute__((format(printf, 2, 3)));
+	__rte_format_printf(2, 3);
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 882dfa0ab..898efa0d6 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -137,7 +137,7 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev);
 int
 rte_devargs_parsef(struct rte_devargs *da,
 		   const char *format, ...)
-__attribute__((format(printf, 2, 0)));
+__rte_format_printf(2, 0);
 
 /**
  * Insert an rte_devargs in the global list.
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 1bb0e6694..a0d1f4837 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -282,7 +282,7 @@ int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
 	__attribute__((cold))
 #endif
 #endif
-	__attribute__((format(printf, 3, 4)));
+	__rte_format_printf(3, 4);
 
 /**
  * Generates a log message.
@@ -311,7 +311,7 @@ int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
  *   - Negative on error.
  */
 int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
-	__attribute__((format(printf,3,0)));
+	__rte_format_printf(3, 0);
 
 /**
  * Generates a log message.
-- 
2.25.1

[dpdk-dev] [PATCH v4 2/7] eal/windows: use lowercase filenames for system headers

[Dmitry Kozlyuk]

Mixed case in Windows header names causes errors when cross-compiling
from Linux with case-sensitive filesystem using MinGW, because MinGW
distribution provides all platform SDK headers in lowercase. The change
does not affect Windows native builds on case-insensitive filesystems
(NTFS default).

Reported-by: Narcisa Ana Maria Vasile <Narcisa.Vasile@microsoft.com>
Reported-by: William Tu <u9012063@gmail.com>
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/windows/eal/include/pthread.h | 2 +-
 lib/librte_eal/windows/eal/include/rte_os.h  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/windows/eal/include/pthread.h b/lib/librte_eal/windows/eal/include/pthread.h
index 4ac24de0a..b9dd18e56 100644
--- a/lib/librte_eal/windows/eal/include/pthread.h
+++ b/lib/librte_eal/windows/eal/include/pthread.h
@@ -14,7 +14,7 @@
 extern "C" {
 #endif
 
-#include <Windows.h>
+#include <windows.h>
 
 #define PTHREAD_BARRIER_SERIAL_THREAD TRUE
 
diff --git a/lib/librte_eal/windows/eal/include/rte_os.h b/lib/librte_eal/windows/eal/include/rte_os.h
index c76be1216..95a19b2d3 100644
--- a/lib/librte_eal/windows/eal/include/rte_os.h
+++ b/lib/librte_eal/windows/eal/include/rte_os.h
@@ -15,8 +15,8 @@
 extern "C" {
 #endif
 
-#include <Windows.h>
-#include <BaseTsd.h>
+#include <windows.h>
+#include <basetsd.h>
 #include <pthread.h>
 #include <stdio.h>
 
-- 
2.25.1

[dpdk-dev] [PATCH v4 3/7] eal/windows: support builing with MinGW-w64

[Dmitry Kozlyuk]

Disable bundled getopt implementation and GNU extensions shim when
building in GNU environment.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/meson.build                  | 3 +++
 lib/librte_eal/windows/eal/eal.c            | 6 ++++--
 lib/librte_eal/windows/eal/include/getopt.h | 4 ++++
 lib/librte_eal/windows/eal/include/rte_os.h | 2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index 4be5118ce..1730d603f 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -20,6 +20,9 @@ endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
 	cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
+if cc.has_header('getopt.h')
+	cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG']
+endif
 sources = common_sources + env_sources
 objs = common_objs + env_objs
 headers = common_headers + env_headers
diff --git a/lib/librte_eal/windows/eal/eal.c b/lib/librte_eal/windows/eal/eal.c
index 34852d42c..e4b50df3b 100644
--- a/lib/librte_eal/windows/eal/eal.c
+++ b/lib/librte_eal/windows/eal/eal.c
@@ -2,9 +2,11 @@
  * Copyright(c) 2019 Intel Corporation
  */
 
-#include <sys/stat.h>
-#include <io.h>
 #include <fcntl.h>
+#include <io.h>
+#include <share.h>
+#include <sys/stat.h>
+
 #include <rte_debug.h>
 #include <rte_eal.h>
 #include <eal_memcfg.h>
diff --git a/lib/librte_eal/windows/eal/include/getopt.h b/lib/librte_eal/windows/eal/include/getopt.h
index 2eebe54e3..6f57af454 100644
--- a/lib/librte_eal/windows/eal/include/getopt.h
+++ b/lib/librte_eal/windows/eal/include/getopt.h
@@ -26,7 +26,11 @@
 #ifndef NEED_USUAL_GETOPT
 
 /* Use system getopt */
+#ifdef RTE_TOOLCHAIN_GCC
+#include_next <getopt.h>
+#else
 #include <getopt.h>
+#endif
 
 #else /* NEED_USUAL_GETOPT */
 
diff --git a/lib/librte_eal/windows/eal/include/rte_os.h b/lib/librte_eal/windows/eal/include/rte_os.h
index 95a19b2d3..e1e0378e6 100644
--- a/lib/librte_eal/windows/eal/include/rte_os.h
+++ b/lib/librte_eal/windows/eal/include/rte_os.h
@@ -57,6 +57,7 @@ int eal_thread_create(pthread_t *thread);
  */
 void eal_create_cpu_map(void);
 
+#ifndef RTE_TOOLCHAIN_GCC
 static inline int
 asprintf(char **buffer, const char *format, ...)
 {
@@ -83,6 +84,7 @@ asprintf(char **buffer, const char *format, ...)
 	}
 	return ret;
 }
+#endif /* RTE_TOOLCHAIN_GCC */
 
 /* cpu_set macros implementation */
 #define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
-- 
2.25.1

[dpdk-dev] [PATCH v4 4/7] build: MinGW-w64 support for Meson

[Dmitry Kozlyuk]

MinGW-w64 linker does not mimic MS linker options, so the build system
must differentiate between linkers on Windows. Use GNU linker options
with GCC and MS linker options with Clang.

MinGW-w64 by default uses MSVCRT stdio, which does not comply to ANSI,
most notably its formatting and string handling functions. MinGW-w64
support for the Universal CRT (UCRT) is ongoing, but the toolchain
provides its own standard-complying implementation of stdio. The latter
is used in the patch to support formatting in DPDK.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build | 14 ++++++++++++++
 lib/meson.build    |  8 ++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index d3d2370ce..68aeb8470 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -14,6 +14,10 @@ foreach env:supported_exec_envs
 	set_variable('is_' + env, exec_env == env)
 endforeach
 
+# MS linker requires special treatment.
+# TODO: use cc.get_linker_id() with Meson >= 0.54
+is_ms_linker = is_windows and (cc.get_id() == 'clang')
+
 # set the major version, which might be used by drivers and libraries
 # depending on the configuration options
 pver = meson.project_version().split('.')
@@ -247,6 +251,16 @@ if is_freebsd
 	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
 endif
 
+if is_windows
+	# Minimum supported API is Windows 7.
+	add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
+
+	# Use MinGW-w64 stdio, because DPDK assumes ANSI-compliant formatting.
+	if cc.get_id() == 'gcc'
+		add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
+	endif
+endif
+
 if get_option('b_lto')
 	if cc.has_argument('-ffat-lto-objects')
 		add_project_arguments('-ffat-lto-objects', language: 'c')
diff --git a/lib/meson.build b/lib/meson.build
index 0af3efab2..9c3cc55d5 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -148,12 +148,16 @@ foreach l:libraries
 				command: [map_to_def_cmd, '@INPUT@', '@OUTPUT@'],
 				input: version_map,
 				output: 'rte_@0@_exports.def'.format(name))
-			lk_deps = [version_map, def_file]
-			if is_windows
+
+			if is_ms_linker
 				lk_args = ['-Wl,/def:' + def_file.full_path(),
 					'-Wl,/implib:lib\\' + implib]
 			else
 				lk_args = ['-Wl,--version-script=' + version_map]
+			endif
+
+			lk_deps = [version_map, def_file]
+			if not is_windows
 				# on unix systems check the output of the
 				# experimental syms script, using it as a
 				# dependency of the .so build
-- 
2.25.1

[dpdk-dev] [PATCH v4 5/7] build: add cross-file for MinGW-w64

[Dmitry Kozlyuk]

Add Meson configuration to cross-compile for Windows using MinGW-w64.
It may require adjustments in some cases, but at least it provides
the foundation.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/x86/meson_mingw.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 config/x86/meson_mingw.txt

diff --git a/config/x86/meson_mingw.txt b/config/x86/meson_mingw.txt
new file mode 100644
index 000000000..fac33cfcd
--- /dev/null
+++ b/config/x86/meson_mingw.txt
@@ -0,0 +1,13 @@
+[binaries]
+c = '/usr/bin/x86_64-w64-mingw32-gcc'
+cpp = '/usr/bin/x86_64-w64-mingw32-g++'
+ld = '/usr/bin/x86_64-w64-mingw32-ld'
+ar = '/usr/bin/x86_64-w64-mingw32-ar'
+strip = '/usr/bin/x86_64-w64-mingw32-strip'
+pkgconfig = '/usr/bin/x86_64-w64-mingw32-pkg-config'
+
+[host_machine]
+system = 'windows'
+cpu_family = 'x86_64'
+cpu = 'native'
+endian = 'little'
-- 
2.25.1

[dpdk-dev] [PATCH v4 6/7] doc: guide for Windows build using MinGW-w64

[Dmitry Kozlyuk]

Instructions for different toolchains presented as options on the
corresponging steps of the guide, so that common parts may be reused.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 doc/guides/windows_gsg/build_dpdk.rst | 62 +++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/doc/guides/windows_gsg/build_dpdk.rst b/doc/guides/windows_gsg/build_dpdk.rst
index 6711e07e2..d46e84e3f 100644
--- a/doc/guides/windows_gsg/build_dpdk.rst
+++ b/doc/guides/windows_gsg/build_dpdk.rst
@@ -7,15 +7,22 @@ Compiling the DPDK Target from Source
 System Requirements
 -------------------
 
-The DPDK and its applications require the Clang-LLVM C compiler
-and Microsoft MSVC linker.
+Building the DPDK and its applications requires one of the following
+environments:
+
+* The Clang-LLVM C compiler and Microsoft MSVC linker.
+* The MinGW-w64 toolchain (either native or cross).
+
 The Meson Build system is used to prepare the sources for compilation
 with the Ninja backend.
 The installation of these tools is covered in this section.
 
 
+Option 1. Clang-LLVM C Compiler and Microsoft MSVC Linker
+---------------------------------------------------------
+
 Install the Compiler
---------------------
+~~~~~~~~~~~~~~~~~~~~
 
 Download and install the clang compiler from
 `LLVM website <http://releases.llvm.org/download.html>`_.
@@ -25,7 +32,7 @@ For example, Clang-LLVM direct download link::
 
 
 Install the Linker
-------------------
+~~~~~~~~~~~~~~~~~~
 
 Download and install the Build Tools for Visual Studio to link and build the
 files on windows,
@@ -34,6 +41,18 @@ When installing build tools, select the "Visual C++ build tools" option
 and ensure the Windows SDK is selected.
 
 
+Option 2. MinGW-w64 Toolchain
+-----------------------------
+
+Obtain the latest version from
+`MinGW-w64 website <http://mingw-w64.org/doku.php/download>`_.
+On Windows, install to a folder without spaces in its name, like ``C:\MinGW``.
+This path is assumed for the rest of this guide.
+
+Version 4.0.4 for Ubuntu 16.04 cannot be used due to a
+`MinGW-w64 bug <https://sourceforge.net/p/mingw-w64/bugs/562/>`_.
+
+
 Install the Build System
 ------------------------
 
@@ -43,6 +62,8 @@ A good option to choose is the MSI installer for both meson and ninja together::
 
 	http://mesonbuild.com/Getting-meson.html#installing-meson-and-ninja-with-the-msi-installer%22
 
+Recommended version is either Meson 0.47.1 (baseline) or the latest release.
+
 Install the Backend
 -------------------
 
@@ -56,23 +77,41 @@ Build the code
 The build environment is setup to build the EAL and the helloworld example by
 default.
 
-Using the ninja backend
-~~~~~~~~~~~~~~~~~~~~~~~~
+Option 1. Native Build on Windows
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Specifying the compiler might be required to complete the meson command.
+When using Clang-LLVM, specifying the compiler might be required to complete
+the meson command:
 
 .. code-block:: console
 
     set CC=clang
 
+When using MinGW-w64, it is sufficient to have toolchain executables in PATH:
+
+.. code-block:: console
+
+    set PATH=C:\MinGW\mingw64\bin;%PATH%
+
 To compile the examples, the flag ``-Dexamples`` is required.
 
 .. code-block:: console
 
     cd C:\Users\me\dpdk
     meson -Dexamples=helloworld build
-    cd build
-    ninja
+    ninja -C build
+
+Option 2. Cross-Compile with MinGW-w64
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The cross-file option must be specified for Meson.
+Depending on the distribution, paths in this file may need adjustments.
+
+.. code-block:: console
+
+    meson --cross-file config/x86/meson_mingw.txt -Dexamples=helloworld build
+    ninja -C build
+
 
 Run the helloworld example
 ==========================
@@ -87,3 +126,8 @@ Navigate to the examples in the build directory and run `dpdk-helloworld.exe`.
     hello from core 3
     hello from core 0
     hello from core 2
+
+Note for MinGW-w64: applications are linked to ``libwinpthread-1.dll``
+by default. To run the example, either add toolchain executables directory
+to the PATH or copy the library to the working directory.
+Alternatively, static linking may be used (mind the LGPLv2.1 license).
-- 
2.25.1

[dpdk-dev] [PATCH v4 7/7] build: fix linker warnings with Clang on Windows

[Dmitry Kozlyuk]

Clang on Windows doesn't use pthread for now, while MinGW does. Removing
`-pthread` option with MS linker fixes the following warning:

    clang: warning: argument unused during compilation: '-pthread'
        [-Wunused-command-line-argument]

Option `--no-as-needed` is meaningless for PE output. Disabling it on
Windows fixes the following warning:

    LINK : warning LNK4044: unrecognized option '/-no-as-needed'; ignored

Fixes: 98edcbb5a ("eal/windows: introduce Windows support")

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 68aeb8470..abedd76f2 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -112,11 +112,15 @@ dpdk_conf.set('RTE_TOOLCHAIN_' + toolchain.to_upper(), 1)
 
 dpdk_conf.set('RTE_ARCH_64', cc.sizeof('void *') == 8)
 
-add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
+if not is_windows
+	add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
+endif
 
-# use pthreads
-add_project_link_arguments('-pthread', language: 'c')
-dpdk_extra_ldflags += '-pthread'
+# use pthreads if available for the platform
+if not is_ms_linker
+	add_project_link_arguments('-pthread', language: 'c')
+	dpdk_extra_ldflags += '-pthread'
+endif
 
 # on some OS, maths functions are in a separate library
 if cc.find_library('m', required : false).found()
-- 
2.25.1

Re: [dpdk-dev] [PATCH v4 0/7] MinGW-w64 support

[William Tu]

On Wed, Feb 26, 2020 at 8:25 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> This patch series add support for building DPDK using MinGW-w64.
>
> MinGW-w64 provides GNU toolchain and independent platform SDK on
> Windows. It also supports cross-compilation to Windows from POSIX
> systems by providing cross tollchains and libraries [0]. It does NOT
> emulate a full POSIX environment, like Cygwin or MSYS do.
>
> There are advantages in using MinGW-w64 in addition to Clang:
>
> 1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
>    implementation, GNU getopt, and Windows platform SDK.
>
> 2. Easier porting of POSIX applications using DPDK to Windows, because
>    application code can use the same benefits as mentioned above.
>
> 3. Having both primary compilers enabled on Windows provides more
>    diagnostics and generally prevents non-portable code.
>
> [0]: http://mingw-w64.org
>
> v4 Changes:
>
>     Fix Windows headers include case.
>     Recommend the latest Meson in docs (build fixed with 0.53.2).
>
> v3 Changes:
>
>     Rebase onto the latest Windows EAL.
>     Prevent format attribute conflict with upcoming v20.05 patches.
>     Remove redundant CFLAGS from Meson.
>     Remove links to Meson bugtracker from docs (PR merged into upstream).
>     Fix Clang warnings using about GNU options.
>
> v2 Changes:
>
>     Add patch to use lowercase system header filenames.
>     Move Meson cross-file for x86 to arch directory.
>     Change wording in comments.
>     Add Meson version warning in documentation.
>
> Dmitry Kozlyuk (7):
>   eal: introduce portable format attribute
>   eal/windows: use lowercase filenames for system headers
>   eal/windows: support builing with MinGW-w64
>   build: MinGW-w64 support for Meson
>   build: add cross-file for MinGW-w64
>   doc: guide for Windows build using MinGW-w64
>   build: fix linker warnings with Clang on Windows
>
>  config/meson.build                           | 26 ++++++--
>  config/x86/meson_mingw.txt                   | 13 ++++
>  doc/guides/windows_gsg/build_dpdk.rst        | 62 +++++++++++++++++---
>  lib/librte_cmdline/cmdline.h                 |  4 +-
>  lib/librte_eal/common/include/rte_common.h   | 17 +++++-
>  lib/librte_eal/common/include/rte_debug.h    |  2 +-
>  lib/librte_eal/common/include/rte_devargs.h  |  2 +-
>  lib/librte_eal/common/include/rte_log.h      |  4 +-
>  lib/librte_eal/meson.build                   |  3 +
>  lib/librte_eal/windows/eal/eal.c             |  6 +-
>  lib/librte_eal/windows/eal/include/getopt.h  |  4 ++
>  lib/librte_eal/windows/eal/include/pthread.h |  2 +-
>  lib/librte_eal/windows/eal/include/rte_os.h  |  6 +-
>  lib/meson.build                              |  8 ++-
>  14 files changed, 133 insertions(+), 26 deletions(-)
>  create mode 100644 config/x86/meson_mingw.txt
>
I reviewed and tested the v4 series and everything works ok.

Tested-by: William Tu <u9012063@gmail.com>

Thanks

Re: [dpdk-dev] [PATCH v4 0/7] MinGW-w64 support

[Kadam, Pallavi]

On 2/26/2020 8:25 PM, Dmitry Kozlyuk wrote:
> This patch series add support for building DPDK using MinGW-w64.
> 
> MinGW-w64 provides GNU toolchain and independent platform SDK on
> Windows. It also supports cross-compilation to Windows from POSIX
> systems by providing cross tollchains and libraries [0]. It does NOT
> emulate a full POSIX environment, like Cygwin or MSYS do.
> 
> There are advantages in using MinGW-w64 in addition to Clang:
> 
> 1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
>     implementation, GNU getopt, and Windows platform SDK.
> 
> 2. Easier porting of POSIX applications using DPDK to Windows, because
>     application code can use the same benefits as mentioned above.
> 
> 3. Having both primary compilers enabled on Windows provides more
>     diagnostics and generally prevents non-portable code.
> 
> [0]: http://mingw-w64.org
> 
> v4 Changes:
> 
>      Fix Windows headers include case.
>      Recommend the latest Meson in docs (build fixed with 0.53.2).
> 
> v3 Changes:
> 
>      Rebase onto the latest Windows EAL.
>      Prevent format attribute conflict with upcoming v20.05 patches.
>      Remove redundant CFLAGS from Meson.
>      Remove links to Meson bugtracker from docs (PR merged into upstream).
>      Fix Clang warnings using about GNU options.
> 
> v2 Changes:
> 
>      Add patch to use lowercase system header filenames.
>      Move Meson cross-file for x86 to arch directory.
>      Change wording in comments.
>      Add Meson version warning in documentation.
> 
> Dmitry Kozlyuk (7):
>    eal: introduce portable format attribute
>    eal/windows: use lowercase filenames for system headers
>    eal/windows: support builing with MinGW-w64
>    build: MinGW-w64 support for Meson
>    build: add cross-file for MinGW-w64
>    doc: guide for Windows build using MinGW-w64
>    build: fix linker warnings with Clang on Windows
> 
>   config/meson.build                           | 26 ++++++--
>   config/x86/meson_mingw.txt                   | 13 ++++
>   doc/guides/windows_gsg/build_dpdk.rst        | 62 +++++++++++++++++---
>   lib/librte_cmdline/cmdline.h                 |  4 +-
>   lib/librte_eal/common/include/rte_common.h   | 17 +++++-
>   lib/librte_eal/common/include/rte_debug.h    |  2 +-
>   lib/librte_eal/common/include/rte_devargs.h  |  2 +-
>   lib/librte_eal/common/include/rte_log.h      |  4 +-
>   lib/librte_eal/meson.build                   |  3 +
>   lib/librte_eal/windows/eal/eal.c             |  6 +-
>   lib/librte_eal/windows/eal/include/getopt.h  |  4 ++
>   lib/librte_eal/windows/eal/include/pthread.h |  2 +-
>   lib/librte_eal/windows/eal/include/rte_os.h  |  6 +-
>   lib/meson.build                              |  8 ++-
>   14 files changed, 133 insertions(+), 26 deletions(-)
>   create mode 100644 config/x86/meson_mingw.txt
> 
Acked-by: Pallavi Kadam <pallavi.kadam@intel.com>
Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Thomas Monjalon]

27/02/2020 05:25, Dmitry Kozlyuk:
> When using __attribute__((format(...)) on functions, GCC on Windows
> assumes MS-specific format string by default, even if the underlying
> stdio implementation is ANSI-compliant (either MS Unicersal CRT
> or MinGW implementation). Wrap attribute into a macro that forces
> GNU-specific format string when using GCC.
> 
> Use this new attribute for logging and panic messages in EAL
> and for output strings in cmdline library.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
> +/**
> + * Check format string and its arguments at compile-time.
> + *
> + * GCC on Windows assumes MS-specific format string by default,
> + * even if the underlying stdio implementation is ANSI-compliant,
> + * so this must be overridden.
> + */
> +#if defined(RTE_TOOLCHAIN_GCC)
> +#define __rte_format_printf(format_index, first_arg) \
> +	__attribute__((format(gnu_printf, format_index, first_arg)))
> +#else
> +#define __rte_format_printf(format_index, first_arg) \
> +	__attribute__((format(printf, format_index, first_arg)))
> +#endif

It does not work when compiling pmdinfogen with clang and drivers with gcc.

I suggest this change (I can send a patch fixing the issue in other .h files):

+/*
+ * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
+ * while a host application (like pmdinfogen) may have another compiler.
+ * RTE_CC_IS_GNU is true if the file is compiled with GCC,
+ * no matter it is a target or host application.
+ */
+#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
+#define RTE_CC_IS_GNU
+#endif
+
+#ifdef RTE_CC_IS_GNU
-/** Define GCC_VERSION **/
-#ifdef RTE_TOOLCHAIN_GCC
 #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
                __GNUC_PATCHLEVEL__)
 #endif
@@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
  * even if the underlying stdio implementation is ANSI-compliant,
  * so this must be overridden.
  */
-#if defined(RTE_TOOLCHAIN_GCC)
+#ifdef RTE_CC_IS_GNU
 #define __rte_format_printf(format_index, first_arg) \
        __attribute__((format(gnu_printf, format_index, first_arg)))
 #else

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Dmitry Kozlyuk]

> I suggest this change (I can send a patch fixing the issue in other .h files):
>
> +/*
> + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> + * while a host application (like pmdinfogen) may have another compiler.
> + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> + * no matter it is a target or host application.
> + */
> +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> +#define RTE_CC_IS_GNU
> +#endif
> +
> +#ifdef RTE_CC_IS_GNU
> -/** Define GCC_VERSION **/
> -#ifdef RTE_TOOLCHAIN_GCC
>  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
>                 __GNUC_PATCHLEVEL__)
>  #endif
> @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
>   * even if the underlying stdio implementation is ANSI-compliant,
>   * so this must be overridden.
>   */
> -#if defined(RTE_TOOLCHAIN_GCC)
> +#ifdef RTE_CC_IS_GNU
>  #define __rte_format_printf(format_index, first_arg) \
>         __attribute__((format(gnu_printf, format_index, first_arg)))
>  #else

The code you propose LGTM itself. If you think it's a better solution than
the one proposed below, I see no problem going with it.

What I wonder is whether pmdinfogen should include the problematic code in the
first place. The errors come from declarations in rte_debug.h, but pmdinfogen
really can't use them, because definitions are compiled for different
machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
moving struct rte_pci_id to a separate header?

EAL defines are tied to the target toolchain and are consistent with each
other, from this point of view RTE_CC_IS_GNU looks like a workaround. Another
reason why pmdinfogen should not depend on EAL is that its code will differ
considerably for POSIX and Windows (ELF vs COFF, mmap vs Win32 API).

-- 
Dmitry Kozlyuk

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Thomas Monjalon]

14/03/2020 00:38, Dmitry Kozlyuk:
> > I suggest this change (I can send a patch fixing the issue in other .h files):
> >
> > +/*
> > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > + * while a host application (like pmdinfogen) may have another compiler.
> > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > + * no matter it is a target or host application.
> > + */
> > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > +#define RTE_CC_IS_GNU
> > +#endif
> > +
> > +#ifdef RTE_CC_IS_GNU
> > -/** Define GCC_VERSION **/
> > -#ifdef RTE_TOOLCHAIN_GCC
> >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> >                 __GNUC_PATCHLEVEL__)
> >  #endif
> > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> >   * even if the underlying stdio implementation is ANSI-compliant,
> >   * so this must be overridden.
> >   */
> > -#if defined(RTE_TOOLCHAIN_GCC)
> > +#ifdef RTE_CC_IS_GNU
> >  #define __rte_format_printf(format_index, first_arg) \
> >         __attribute__((format(gnu_printf, format_index, first_arg)))
> >  #else
> 
> The code you propose LGTM itself. If you think it's a better solution than
> the one proposed below, I see no problem going with it.
> 
> What I wonder is whether pmdinfogen should include the problematic code in the
> first place. The errors come from declarations in rte_debug.h, but pmdinfogen
> really can't use them, because definitions are compiled for different
> machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
> struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
> moving struct rte_pci_id to a separate header?

Splitting headers to avoid EAL dependency looks to be a bad precedent to me.

> EAL defines are tied to the target toolchain and are consistent with each
> other, from this point of view RTE_CC_IS_GNU looks like a workaround.

Yes there are some target-arch-dependent code in EAL.
But a host application is not supposed to use arch-dependent code.

> Another
> reason why pmdinfogen should not depend on EAL is that its code will differ
> considerably for POSIX and Windows (ELF vs COFF, mmap vs Win32 API).

I believe managing some OS differences should be helped with EAL.

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Bruce Richardson]

On Sun, Mar 15, 2020 at 09:36:11AM +0100, Thomas Monjalon wrote:
> 14/03/2020 00:38, Dmitry Kozlyuk:
> > > I suggest this change (I can send a patch fixing the issue in other .h files):
> > >
> > > +/*
> > > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > > + * while a host application (like pmdinfogen) may have another compiler.
> > > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > > + * no matter it is a target or host application.
> > > + */
> > > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > > +#define RTE_CC_IS_GNU
> > > +#endif
> > > +
> > > +#ifdef RTE_CC_IS_GNU
> > > -/** Define GCC_VERSION **/
> > > -#ifdef RTE_TOOLCHAIN_GCC
> > >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> > >                 __GNUC_PATCHLEVEL__)
> > >  #endif
> > > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> > >   * even if the underlying stdio implementation is ANSI-compliant,
> > >   * so this must be overridden.
> > >   */
> > > -#if defined(RTE_TOOLCHAIN_GCC)
> > > +#ifdef RTE_CC_IS_GNU
> > >  #define __rte_format_printf(format_index, first_arg) \
> > >         __attribute__((format(gnu_printf, format_index, first_arg)))
> > >  #else
> > 
> > The code you propose LGTM itself. If you think it's a better solution than
> > the one proposed below, I see no problem going with it.
> > 
> > What I wonder is whether pmdinfogen should include the problematic code in the
> > first place. The errors come from declarations in rte_debug.h, but pmdinfogen
> > really can't use them, because definitions are compiled for different
> > machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
> > struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
> > moving struct rte_pci_id to a separate header?
> 
> Splitting headers to avoid EAL dependency looks to be a bad precedent to me.
> 

Rather than splitting, we can still fix this by breaking the dependency by
just not having rte_debug.h included in rte_pci.h. From what I see, there
is no need for that include to be there, and DPDK pretty much compiles fine
with it removed - the only other change I had to make was add an extra
include into mlx5_common.h to compensate for it not getting pulled in via
rte_pci.h.

Generally, while we should ensure that each header includes any other
headers it depends upon, we must be careful that headers don't include
other headers that they don't directly use.

/Bruce

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Bruce Richardson]

On Mon, Mar 16, 2020 at 10:54:09AM +0000, Bruce Richardson wrote:
> On Sun, Mar 15, 2020 at 09:36:11AM +0100, Thomas Monjalon wrote:
> > 14/03/2020 00:38, Dmitry Kozlyuk:
> > > > I suggest this change (I can send a patch fixing the issue in other .h files):
> > > >
> > > > +/*
> > > > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > > > + * while a host application (like pmdinfogen) may have another compiler.
> > > > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > > > + * no matter it is a target or host application.
> > > > + */
> > > > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > > > +#define RTE_CC_IS_GNU
> > > > +#endif
> > > > +
> > > > +#ifdef RTE_CC_IS_GNU
> > > > -/** Define GCC_VERSION **/
> > > > -#ifdef RTE_TOOLCHAIN_GCC
> > > >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> > > >                 __GNUC_PATCHLEVEL__)
> > > >  #endif
> > > > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> > > >   * even if the underlying stdio implementation is ANSI-compliant,
> > > >   * so this must be overridden.
> > > >   */
> > > > -#if defined(RTE_TOOLCHAIN_GCC)
> > > > +#ifdef RTE_CC_IS_GNU
> > > >  #define __rte_format_printf(format_index, first_arg) \
> > > >         __attribute__((format(gnu_printf, format_index, first_arg)))
> > > >  #else
> > > 
> > > The code you propose LGTM itself. If you think it's a better solution than
> > > the one proposed below, I see no problem going with it.
> > > 
> > > What I wonder is whether pmdinfogen should include the problematic code in the
> > > first place. The errors come from declarations in rte_debug.h, but pmdinfogen
> > > really can't use them, because definitions are compiled for different
> > > machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
> > > struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
> > > moving struct rte_pci_id to a separate header?
> > 
> > Splitting headers to avoid EAL dependency looks to be a bad precedent to me.
> > 
> 
> Rather than splitting, we can still fix this by breaking the dependency by
> just not having rte_debug.h included in rte_pci.h. From what I see, there
> is no need for that include to be there, and DPDK pretty much compiles fine
> with it removed - the only other change I had to make was add an extra
> include into mlx5_common.h to compensate for it not getting pulled in via
> rte_pci.h.
> 

And by adding rte_interrupts.h to drivers/bus/ifpga/rte_bus_ifpga.h we can
remove the following headers from rte_pci.h also:

stdio.h
errno.h
stdint.h
rte_interrupts.h

This means that we go from 9 includes in the file (including rte_debug.h) to
just 4.

Regards,
/Bruce

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Thomas Monjalon]

16/03/2020 12:02, Bruce Richardson:
> On Mon, Mar 16, 2020 at 10:54:09AM +0000, Bruce Richardson wrote:
> > On Sun, Mar 15, 2020 at 09:36:11AM +0100, Thomas Monjalon wrote:
> > > 14/03/2020 00:38, Dmitry Kozlyuk:
> > > > > I suggest this change (I can send a patch fixing the issue in other .h files):
> > > > >
> > > > > +/*
> > > > > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > > > > + * while a host application (like pmdinfogen) may have another compiler.
> > > > > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > > > > + * no matter it is a target or host application.
> > > > > + */
> > > > > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > > > > +#define RTE_CC_IS_GNU
> > > > > +#endif
> > > > > +
> > > > > +#ifdef RTE_CC_IS_GNU
> > > > > -/** Define GCC_VERSION **/
> > > > > -#ifdef RTE_TOOLCHAIN_GCC
> > > > >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> > > > >                 __GNUC_PATCHLEVEL__)
> > > > >  #endif
> > > > > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> > > > >   * even if the underlying stdio implementation is ANSI-compliant,
> > > > >   * so this must be overridden.
> > > > >   */
> > > > > -#if defined(RTE_TOOLCHAIN_GCC)
> > > > > +#ifdef RTE_CC_IS_GNU
> > > > >  #define __rte_format_printf(format_index, first_arg) \
> > > > >         __attribute__((format(gnu_printf, format_index, first_arg)))
> > > > >  #else
> > > > 
> > > > The code you propose LGTM itself. If you think it's a better solution than
> > > > the one proposed below, I see no problem going with it.
> > > > 
> > > > What I wonder is whether pmdinfogen should include the problematic code in the
> > > > first place. The errors come from declarations in rte_debug.h, but pmdinfogen
> > > > really can't use them, because definitions are compiled for different
> > > > machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
> > > > struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
> > > > moving struct rte_pci_id to a separate header?
> > > 
> > > Splitting headers to avoid EAL dependency looks to be a bad precedent to me.
> > > 
> > 
> > Rather than splitting, we can still fix this by breaking the dependency by
> > just not having rte_debug.h included in rte_pci.h. From what I see, there
> > is no need for that include to be there, and DPDK pretty much compiles fine
> > with it removed - the only other change I had to make was add an extra
> > include into mlx5_common.h to compensate for it not getting pulled in via
> > rte_pci.h.
> > 
> 
> And by adding rte_interrupts.h to drivers/bus/ifpga/rte_bus_ifpga.h we can
> remove the following headers from rte_pci.h also:
> 
> stdio.h
> errno.h
> stdint.h
> rte_interrupts.h
> 
> This means that we go from 9 includes in the file (including rte_debug.h) to
> just 4.

Thank you Bruce

Are you sending the patch or you prefer I do it?

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Bruce Richardson]

On Mon, Mar 16, 2020 at 12:14:51PM +0100, Thomas Monjalon wrote:
> 16/03/2020 12:02, Bruce Richardson:
> > On Mon, Mar 16, 2020 at 10:54:09AM +0000, Bruce Richardson wrote:
> > > On Sun, Mar 15, 2020 at 09:36:11AM +0100, Thomas Monjalon wrote:
> > > > 14/03/2020 00:38, Dmitry Kozlyuk:
> > > > > > I suggest this change (I can send a patch fixing the issue in other .h files):
> > > > > >
> > > > > > +/*
> > > > > > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > > > > > + * while a host application (like pmdinfogen) may have another compiler.
> > > > > > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > > > > > + * no matter it is a target or host application.
> > > > > > + */
> > > > > > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > > > > > +#define RTE_CC_IS_GNU
> > > > > > +#endif
> > > > > > +
> > > > > > +#ifdef RTE_CC_IS_GNU
> > > > > > -/** Define GCC_VERSION **/
> > > > > > -#ifdef RTE_TOOLCHAIN_GCC
> > > > > >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> > > > > >                 __GNUC_PATCHLEVEL__)
> > > > > >  #endif
> > > > > > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> > > > > >   * even if the underlying stdio implementation is ANSI-compliant,
> > > > > >   * so this must be overridden.
> > > > > >   */
> > > > > > -#if defined(RTE_TOOLCHAIN_GCC)
> > > > > > +#ifdef RTE_CC_IS_GNU
> > > > > >  #define __rte_format_printf(format_index, first_arg) \
> > > > > >         __attribute__((format(gnu_printf, format_index, first_arg)))
> > > > > >  #else
> > > > > 
> > > > > The code you propose LGTM itself. If you think it's a better solution than
> > > > > the one proposed below, I see no problem going with it.
> > > > > 
> > > > > What I wonder is whether pmdinfogen should include the problematic code in the
> > > > > first place. The errors come from declarations in rte_debug.h, but pmdinfogen
> > > > > really can't use them, because definitions are compiled for different
> > > > > machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
> > > > > struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
> > > > > moving struct rte_pci_id to a separate header?
> > > > 
> > > > Splitting headers to avoid EAL dependency looks to be a bad precedent to me.
> > > > 
> > > 
> > > Rather than splitting, we can still fix this by breaking the dependency by
> > > just not having rte_debug.h included in rte_pci.h. From what I see, there
> > > is no need for that include to be there, and DPDK pretty much compiles fine
> > > with it removed - the only other change I had to make was add an extra
> > > include into mlx5_common.h to compensate for it not getting pulled in via
> > > rte_pci.h.
> > > 
> > 
> > And by adding rte_interrupts.h to drivers/bus/ifpga/rte_bus_ifpga.h we can
> > remove the following headers from rte_pci.h also:
> > 
> > stdio.h
> > errno.h
> > stdint.h
> > rte_interrupts.h
> > 
> > This means that we go from 9 includes in the file (including rte_debug.h) to
> > just 4.
> 
> Thank you Bruce
> 
> Are you sending the patch or you prefer I do it?
>

Just running through test-meson-builds.sh tests with it, and then I'll send
it on. 

/Bruce

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Bruce Richardson]

On Mon, Mar 16, 2020 at 12:14:51PM +0100, Thomas Monjalon wrote:
> 16/03/2020 12:02, Bruce Richardson:
> > On Mon, Mar 16, 2020 at 10:54:09AM +0000, Bruce Richardson wrote:
> > > On Sun, Mar 15, 2020 at 09:36:11AM +0100, Thomas Monjalon wrote:
> > > > 14/03/2020 00:38, Dmitry Kozlyuk:
> > > > > > I suggest this change (I can send a patch fixing the issue in other .h files):
> > > > > >
> > > > > > +/*
> > > > > > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > > > > > + * while a host application (like pmdinfogen) may have another compiler.
> > > > > > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > > > > > + * no matter it is a target or host application.
> > > > > > + */
> > > > > > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > > > > > +#define RTE_CC_IS_GNU
> > > > > > +#endif
> > > > > > +
> > > > > > +#ifdef RTE_CC_IS_GNU
> > > > > > -/** Define GCC_VERSION **/
> > > > > > -#ifdef RTE_TOOLCHAIN_GCC
> > > > > >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> > > > > >                 __GNUC_PATCHLEVEL__)
> > > > > >  #endif
> > > > > > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> > > > > >   * even if the underlying stdio implementation is ANSI-compliant,
> > > > > >   * so this must be overridden.
> > > > > >   */
> > > > > > -#if defined(RTE_TOOLCHAIN_GCC)
> > > > > > +#ifdef RTE_CC_IS_GNU
> > > > > >  #define __rte_format_printf(format_index, first_arg) \
> > > > > >         __attribute__((format(gnu_printf, format_index, first_arg)))
> > > > > >  #else
> > > > > 
> > > > > The code you propose LGTM itself. If you think it's a better solution than
> > > > > the one proposed below, I see no problem going with it.
> > > > > 
> > > > > What I wonder is whether pmdinfogen should include the problematic code in the
> > > > > first place. The errors come from declarations in rte_debug.h, but pmdinfogen
> > > > > really can't use them, because definitions are compiled for different
> > > > > machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for
> > > > > struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by
> > > > > moving struct rte_pci_id to a separate header?
> > > > 
> > > > Splitting headers to avoid EAL dependency looks to be a bad precedent to me.
> > > > 
> > > 
> > > Rather than splitting, we can still fix this by breaking the dependency by
> > > just not having rte_debug.h included in rte_pci.h. From what I see, there
> > > is no need for that include to be there, and DPDK pretty much compiles fine
> > > with it removed - the only other change I had to make was add an extra
> > > include into mlx5_common.h to compensate for it not getting pulled in via
> > > rte_pci.h.
> > > 
> > 
> > And by adding rte_interrupts.h to drivers/bus/ifpga/rte_bus_ifpga.h we can
> > remove the following headers from rte_pci.h also:
> > 
> > stdio.h
> > errno.h
> > stdint.h
> > rte_interrupts.h
> > 
> > This means that we go from 9 includes in the file (including rte_debug.h) to
> > just 4.
> 
> Thank you Bruce
> 
> Are you sending the patch or you prefer I do it?
> 
For completeness, patch now at: http://patches.dpdk.org/patch/66701/

Re: [dpdk-dev] [PATCH v4 1/7] eal: introduce portable format attribute

[Thomas Monjalon]

14/03/2020 00:38, Dmitry Kozlyuk:
> > I suggest this change (I can send a patch fixing the issue in other .h files):
> >
> > +/*
> > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC,
> > + * while a host application (like pmdinfogen) may have another compiler.
> > + * RTE_CC_IS_GNU is true if the file is compiled with GCC,
> > + * no matter it is a target or host application.
> > + */
> > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER
> > +#define RTE_CC_IS_GNU
> > +#endif
> > +
> > +#ifdef RTE_CC_IS_GNU
> > -/** Define GCC_VERSION **/
> > -#ifdef RTE_TOOLCHAIN_GCC
> >  #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
> >                 __GNUC_PATCHLEVEL__)
> >  #endif
> > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t;
> >   * even if the underlying stdio implementation is ANSI-compliant,
> >   * so this must be overridden.
> >   */
> > -#if defined(RTE_TOOLCHAIN_GCC)
> > +#ifdef RTE_CC_IS_GNU
> >  #define __rte_format_printf(format_index, first_arg) \
> >         __attribute__((format(gnu_printf, format_index, first_arg)))
> >  #else
> 
> The code you propose LGTM itself. If you think it's a better solution than
> the one proposed below, I see no problem going with it.
> 
> What I wonder is whether pmdinfogen should include the problematic code in the
> first place. The errors come from declarations in rte_debug.h,

No the error comes from rte_common.h included by pmdinfogen.c

Please review this patch:
	https://patches.dpdk.org/patch/66702/

Re: [dpdk-dev] [PATCH v4 0/7] MinGW-w64 support

[Thomas Monjalon]

11/03/2020 18:22, William Tu:
> On Wed, Feb 26, 2020 at 8:25 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
> >
> > This patch series add support for building DPDK using MinGW-w64.
> >
> > MinGW-w64 provides GNU toolchain and independent platform SDK on
> > Windows. It also supports cross-compilation to Windows from POSIX
> > systems by providing cross tollchains and libraries [0]. It does NOT
> > emulate a full POSIX environment, like Cygwin or MSYS do.
> >
> > There are advantages in using MinGW-w64 in addition to Clang:
> >
> > 1. Cross-compilation out-of-the-box. MinGW-w64 is provides a pthread
> >    implementation, GNU getopt, and Windows platform SDK.
> >
> > 2. Easier porting of POSIX applications using DPDK to Windows, because
> >    application code can use the same benefits as mentioned above.
> >
> > 3. Having both primary compilers enabled on Windows provides more
> >    diagnostics and generally prevents non-portable code.
> >
> > [0]: http://mingw-w64.org
> >
[..]
> > Dmitry Kozlyuk (7):
> >   eal: introduce portable format attribute
> >   eal/windows: use lowercase filenames for system headers
> >   eal/windows: support builing with MinGW-w64
> >   build: MinGW-w64 support for Meson
> >   build: add cross-file for MinGW-w64
> >   doc: guide for Windows build using MinGW-w64
> >   build: fix linker warnings with Clang on Windows
> 
> I reviewed and tested the v4 series and everything works ok.
> 
> Tested-by: William Tu <u9012063@gmail.com>

The doc patch was split and squashed in relevant commits.

Applied, thanks