[PATCH 1/2] meson/mlx5: Suppress -Wunused-value diagnostic

[PATCH 1/2] meson/mlx5: Suppress -Wunused-value diagnostic

[Sebastian Brzezinka]

From: Alexey Marchuk <alexeymar@mellanox.com>

mlx5 common library checks if several symbols/definitions
are presented in system header files. If some are not
presented, they will be enabled by mlx5_glue library.
The problem appears with clang and '-Werror' - code
generated by meson is not compiled due to unused variable:

Code:

        #include <infiniband/mlx5dv.h>
        int main(void) {
            /* If it's not defined as a macro, try to use as a symbol */
            #ifndef mlx5dv_create_flow_action_packet_reformat
                mlx5dv_create_flow_action_packet_reformat;
            #endif
            return 0;
        }
Compiler stdout:

Compiler stderr:
 /hpc/local/work/alexeymar/repo/spdk/dpdk/build-tmp/meson-private/tmp5obnak86/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
                mlx5dv_create_flow_action_packet_reformat;
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

As result, almost all symbols are enabled in mlx5_glue while
they exist is system headers. As result, we get multiple
symbols redefenitions when we compile mlx5_common.
As a solution for this problem we can suppress
-Wunused-vaurable using pragma

DPDK 23.11 note:
Starting with commit below, all cflags are passed to the has_header_symbol().
(33d6694) build: use C11 standard
To make sure that the symbol is properly detected, the pedantic flags needs to
be removed.

Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 drivers/common/mlx5/linux/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
index cdee40c553..f3b8e5741b 100644
--- a/drivers/common/mlx5/linux/meson.build
+++ b/drivers/common/mlx5/linux/meson.build
@@ -209,7 +209,11 @@ if  libmtcr_ul_found
 endif
 
 foreach arg:has_sym_args
-    mlx5_config.set(arg[0], cc.has_header_symbol(arg[1], arg[2], dependencies: libs, args: cflags))
+    file_prefix = '#pragma clang diagnostic ignored "-Wunused-value"'
+    cflags += [
+            '-Wno-pedantic',
+    ]
+    mlx5_config.set(arg[0], cc.has_header_symbol(arg[1], arg[2], prefix : file_prefix, dependencies: libs, args: cflags))
 endforeach
 foreach arg:has_member_args
     file_prefix = '#include <' + arg[1] + '>'
-- 
2.34.1

[PATCH 2/2] ARM64: Cross-Compilation Support

[Sebastian Brzezinka]

From: Krishna Kanth Reddy <krish.reddy@samsung.com>

Modified the Configuration file to use the latest ARM Cross-Compiler.

Fixed the linker errors for the undefined references to the APIs
isal_deflate_init, isal_deflate, isal_inflate_init, isal_inflate,
isal_inflate_stateless, isal_deflate_stateless,
isal_deflate_set_hufftables in the case of ARM Cross-Compilation.

Signed-off-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 config/arm/arm64_armv8_linux_gcc  | 10 +++++-----
 drivers/compress/isal/meson.build |  4 ++++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/config/arm/arm64_armv8_linux_gcc b/config/arm/arm64_armv8_linux_gcc
index 529694b49d..6dfb8aa206 100644
--- a/config/arm/arm64_armv8_linux_gcc
+++ b/config/arm/arm64_armv8_linux_gcc
@@ -1,9 +1,9 @@
 [binaries]
-c = ['ccache', 'aarch64-linux-gnu-gcc']
-cpp = ['ccache', 'aarch64-linux-gnu-g++']
-ar = 'aarch64-linux-gnu-gcc-ar'
-strip = 'aarch64-linux-gnu-strip'
-pkgconfig = 'aarch64-linux-gnu-pkg-config'
+c = ['ccache', 'aarch64-none-linux-gnu-gcc']
+cpp = ['ccache', 'aarch64-none-linux-gnu-cpp']
+ar = 'aarch64-none-linux-gnu-gcc-ar'
+strip = 'aarch64-none-linux-gnu-strip'
+pkgconfig = 'aarch64-none-linux-gnu-pkg-config'
 pcap-config = ''
 
 [host_machine]
diff --git a/drivers/compress/isal/meson.build b/drivers/compress/isal/meson.build
index 4b3eaa2274..b3cc990053 100644
--- a/drivers/compress/isal/meson.build
+++ b/drivers/compress/isal/meson.build
@@ -5,6 +5,10 @@ dep = dependency('libisal', required: false, method: 'pkg-config')
 if not dep.found()
     build = false
     reason = 'missing dependency, "libisal"'
+    isal_dep = cc.find_library('libisal', required: false)
+    if isal_dep.found()
+        ext_deps += isal_dep
+    endif
 endif
 
 deps += 'bus_vdev'
-- 
2.34.1

Re: [PATCH 2/2] ARM64: Cross-Compilation Support

[Jerin Jacob]

On Fri, Apr 12, 2024 at 4:43 PM Sebastian Brzezinka
<sebastian.brzezinka@intel.com> wrote:
>
> From: Krishna Kanth Reddy <krish.reddy@samsung.com>
>
> Modified the Configuration file to use the latest ARM Cross-Compiler.
>
> Fixed the linker errors for the undefined references to the APIs
> isal_deflate_init, isal_deflate, isal_inflate_init, isal_inflate,
> isal_inflate_stateless, isal_deflate_stateless,
> isal_deflate_set_hufftables in the case of ARM Cross-Compilation.
>
> Signed-off-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>

There is comment[1] in v1. v2 is not addressed the comment nor
provided any feedback.
Also please add change log in patch(i.e what is changed from a version
to another version)

[1]
https://patches.dpdk.org/project/dpdk/patch/20240405110700.3279213-1-sebastian.brzezinka@intel.com/

> ---
>  config/arm/arm64_armv8_linux_gcc  | 10 +++++-----
>  drivers/compress/isal/meson.build |  4 ++++
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/config/arm/arm64_armv8_linux_gcc b/config/arm/arm64_armv8_linux_gcc
> index 529694b49d..6dfb8aa206 100644
> --- a/config/arm/arm64_armv8_linux_gcc
> +++ b/config/arm/arm64_armv8_linux_gcc
> @@ -1,9 +1,9 @@
>  [binaries]
> -c = ['ccache', 'aarch64-linux-gnu-gcc']
> -cpp = ['ccache', 'aarch64-linux-gnu-g++']
> -ar = 'aarch64-linux-gnu-gcc-ar'
> -strip = 'aarch64-linux-gnu-strip'
> -pkgconfig = 'aarch64-linux-gnu-pkg-config'
> +c = ['ccache', 'aarch64-none-linux-gnu-gcc']
> +cpp = ['ccache', 'aarch64-none-linux-gnu-cpp']
> +ar = 'aarch64-none-linux-gnu-gcc-ar'
> +strip = 'aarch64-none-linux-gnu-strip'
> +pkgconfig = 'aarch64-none-linux-gnu-pkg-config'
>  pcap-config = ''
>
>  [host_machine]
> diff --git a/drivers/compress/isal/meson.build b/drivers/compress/isal/meson.build
> index 4b3eaa2274..b3cc990053 100644
> --- a/drivers/compress/isal/meson.build
> +++ b/drivers/compress/isal/meson.build
> @@ -5,6 +5,10 @@ dep = dependency('libisal', required: false, method: 'pkg-config')
>  if not dep.found()
>      build = false
>      reason = 'missing dependency, "libisal"'
> +    isal_dep = cc.find_library('libisal', required: false)
> +    if isal_dep.found()
> +        ext_deps += isal_dep
> +    endif
>  endif
>
>  deps += 'bus_vdev'
> --
> 2.34.1
>

Re: [PATCH 2/2] ARM64: Cross-Compilation Support

[Sebastian Brzezinka]

I re-uploaded this patch to get rid of the banner added by the mail service, so there are no meaningful changes added to v2. 

> Is aarch64-none-linux* latest compiler?

According to https://developer.arm.com/downloads/-/gnu-a, there is no toolchain named aarch64-linux-gnu*. 

Re: [PATCH 2/2] ARM64: Cross-Compilation Support

[Jerin Jacob]

On Thu, May 23, 2024 at 5:56 PM Sebastian Brzezinka
<sebastian.brzezinka@intel.com> wrote:
>
> I re-uploaded this patch to get rid of the banner added by the mail service, so there are no meaningful changes added to v2.
>
> > Is aarch64-none-linux* latest compiler?
>
> According to https://developer.arm.com/downloads/-/gnu-a, there is no toolchain named aarch64-linux-gnu*.

All distribution OS packages use aarch64-linux-gnu* for cross compilation.

https://jensd.be/1126/linux/cross-compiling-for-arm-or-aarch64-on-debian-or-ubuntu

Re: [PATCH 1/2] meson/mlx5: Suppress -Wunused-value diagnostic

[Stephen Hemminger]

On Fri, 12 Apr 2024 13:13:16 +0200
Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:

> From: Alexey Marchuk <alexeymar@mellanox.com>
> 
> mlx5 common library checks if several symbols/definitions
> are presented in system header files. If some are not
> presented, they will be enabled by mlx5_glue library.
> The problem appears with clang and '-Werror' - code
> generated by meson is not compiled due to unused variable:
> 
> Code:
> 
>         #include <infiniband/mlx5dv.h>
>         int main(void) {
>             /* If it's not defined as a macro, try to use as a symbol */
>             #ifndef mlx5dv_create_flow_action_packet_reformat
>                 mlx5dv_create_flow_action_packet_reformat;
>             #endif
>             return 0;
>         }
> Compiler stdout:
> 
> Compiler stderr:
>  /hpc/local/work/alexeymar/repo/spdk/dpdk/build-tmp/meson-private/tmp5obnak86/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>                 mlx5dv_create_flow_action_packet_reformat;
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> As result, almost all symbols are enabled in mlx5_glue while
> they exist is system headers. As result, we get multiple
> symbols redefenitions when we compile mlx5_common.
> As a solution for this problem we can suppress
> -Wunused-vaurable using pragma
> 
> DPDK 23.11 note:
> Starting with commit below, all cflags are passed to the has_header_symbol().
> (33d6694) build: use C11 standard
> To make sure that the symbol is properly detected, the pedantic flags needs to
> be removed.
> 
> Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>  drivers/common/mlx5/linux/meson.build | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
> index cdee40c553..f3b8e5741b 100644
> --- a/drivers/common/mlx5/linux/meson.build
> +++ b/drivers/common/mlx5/linux/meson.build
> @@ -209,7 +209,11 @@ if  libmtcr_ul_found
>  endif
>  
>  foreach arg:has_sym_args
> -    mlx5_config.set(arg[0], cc.has_header_symbol(arg[1], arg[2], dependencies: libs, args: cflags))
> +    file_prefix = '#pragma clang diagnostic ignored "-Wunused-value"'
> +    cflags += [
> +            '-Wno-pedantic',
> +    ]
> +    mlx5_config.set(arg[0], cc.has_header_symbol(arg[1], arg[2], prefix : file_prefix, dependencies: libs, args: cflags))
>  endforeach
>  foreach arg:has_member_args
>      file_prefix = '#include <' + arg[1] + '>'


Sigh, never liked that mlx5 decided to use pedantic unlike other drivers.
And the build process for this driver has become more baroque and unique compared
to all the other drivers in DPDK. What value to the user is this complexity?

Re: [PATCH 1/2] meson/mlx5: Suppress -Wunused-value diagnostic

[Stephen Hemminger]

On Fri, 12 Apr 2024 13:13:16 +0200
Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:

> From: Alexey Marchuk <alexeymar@mellanox.com>
> 
> mlx5 common library checks if several symbols/definitions
> are presented in system header files. If some are not
> presented, they will be enabled by mlx5_glue library.
> The problem appears with clang and '-Werror' - code
> generated by meson is not compiled due to unused variable:
> 
> Code:
> 
>         #include <infiniband/mlx5dv.h>
>         int main(void) {
>             /* If it's not defined as a macro, try to use as a symbol */
>             #ifndef mlx5dv_create_flow_action_packet_reformat
>                 mlx5dv_create_flow_action_packet_reformat;
>             #endif
>             return 0;
>         }
> Compiler stdout:
> 
> Compiler stderr:
>  /hpc/local/work/alexeymar/repo/spdk/dpdk/build-tmp/meson-private/tmp5obnak86/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>                 mlx5dv_create_flow_action_packet_reformat;
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> As result, almost all symbols are enabled in mlx5_glue while
> they exist is system headers. As result, we get multiple
> symbols redefenitions when we compile mlx5_common.
> As a solution for this problem we can suppress
> -Wunused-vaurable using pragma
> 
> DPDK 23.11 note:
> Starting with commit below, all cflags are passed to the has_header_symbol().
> (33d6694) build: use C11 standard
> To make sure that the symbol is properly detected, the pedantic flags needs to
> be removed.
> 
> Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>  drivers/common/mlx5/linux/meson.build | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/common/mlx5/linux/meson.build b/drivers/common/mlx5/linux/meson.build
> index cdee40c553..f3b8e5741b 100644
> --- a/drivers/common/mlx5/linux/meson.build
> +++ b/drivers/common/mlx5/linux/meson.build
> @@ -209,7 +209,11 @@ if  libmtcr_ul_found
>  endif
>  
>  foreach arg:has_sym_args
> -    mlx5_config.set(arg[0], cc.has_header_symbol(arg[1], arg[2], dependencies: libs, args: cflags))
> +    file_prefix = '#pragma clang diagnostic ignored "-Wunused-value"'
> +    cflags += [
> +            '-Wno-pedantic',
> +    ]
> +    mlx5_config.set(arg[0], cc.has_header_symbol(arg[1], arg[2], prefix : file_prefix, dependencies: libs, args: cflags))
>  endforeach
>  foreach arg:has_member_args
>      file_prefix = '#include <' + arg[1] + '>'

NAK
This adds more complexity to already overly complex meson build for mlx5.