[3/6] net/mlx: fix library search in meson build

Message ID 20190412232451.30197-4-yskoh@mellanox.com (mailing list archive)
State Superseded, archived
Headers
Series build: fix build for arm64 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Yongseok Koh April 12, 2019, 11:24 p.m. UTC
  If MLNX_OFED is installed, there's no .pc file installed for libraries and
dependency() can't find libraries by pkg-config. By adding fallback of
using cc.find_library(), libraries are properly located.

Fixes: e30b4e566f47 ("build: improve dependency handling")
Cc: bluca@debian.org
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx4/meson.build | 19 +++++++++++--------
 drivers/net/mlx5/meson.build | 19 +++++++++++--------
 2 files changed, 22 insertions(+), 16 deletions(-)
  

Comments

Bruce Richardson April 15, 2019, 9:19 a.m. UTC | #1
On Fri, Apr 12, 2019 at 04:24:48PM -0700, Yongseok Koh wrote:
> If MLNX_OFED is installed, there's no .pc file installed for libraries and
> dependency() can't find libraries by pkg-config. By adding fallback of
> using cc.find_library(), libraries are properly located.
> 
> Fixes: e30b4e566f47 ("build: improve dependency handling")
> Cc: bluca@debian.org
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx4/meson.build | 19 +++++++++++--------
>  drivers/net/mlx5/meson.build | 19 +++++++++++--------
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
> index de020701d1..9082f69f25 100644
> --- a/drivers/net/mlx4/meson.build
> +++ b/drivers/net/mlx4/meson.build
> @@ -13,21 +13,24 @@ if pmd_dlopen
>  		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>  	]
>  endif
> -libs = [
> -	dependency('libmnl', required:false),
> -	dependency('libmlx4', required:false),
> -	dependency('libibverbs', required:false),
> -]
> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> +lib_deps = []

Minor suggestion - you can reduce the size of the diff in this patch by
defining the first array as "libnames" and keeping the actual dependency
objects as "libs".

/Bruce
  
Luca Boccassi April 15, 2019, 10:12 a.m. UTC | #2
On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
> If MLNX_OFED is installed, there's no .pc file installed for
> libraries and
> dependency() can't find libraries by pkg-config. By adding fallback
> of
> using cc.find_library(), libraries are properly located.
> 
> Fixes: e30b4e566f47 ("build: improve dependency handling")
> Cc: 
> bluca@debian.org
> 
> Cc: 
> stable@dpdk.org
> 
> 
> Signed-off-by: Yongseok Koh <
> yskoh@mellanox.com
> >
> ---
>  drivers/net/mlx4/meson.build | 19 +++++++++++--------
>  drivers/net/mlx5/meson.build | 19 +++++++++++--------
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/mlx4/meson.build
> b/drivers/net/mlx4/meson.build
> index de020701d1..9082f69f25 100644
> --- a/drivers/net/mlx4/meson.build
> +++ b/drivers/net/mlx4/meson.build
> @@ -13,21 +13,24 @@ if pmd_dlopen
>  		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>  	]
>  endif
> -libs = [
> -	dependency('libmnl', required:false),
> -	dependency('libmlx4', required:false),
> -	dependency('libibverbs', required:false),
> -]
> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> +lib_deps = []
>  build = true
>  foreach lib:libs
> -	if not lib.found()
> +	lib_dep = dependency(lib, required:false)
> +	if not lib_dep.found()
> +		lib_dep = cc.find_library(lib, required:false)

Doesn't this end up trying to link the test program to -llibmnl and
thus failing?

> +	endif
> +	if lib_dep.found()
> +		lib_deps += [ lib_dep ]
> +	else
>  		build = false
>  	endif
>  endforeach
>  # Compile PMD
>  if build
>  	allow_experimental_apis = true
> -	ext_deps += libs
> +	ext_deps += lib_deps
>  	sources = files(
>  		'mlx4.c',
>  		'mlx4_ethdev.c',
> @@ -103,7 +106,7 @@ if pmd_dlopen and build
>  		dlopen_sources,
>  		include_directories: global_inc,
>  		c_args: cflags,
> -		dependencies: libs,
> +		dependencies: libs_deps,
>  		link_args: [
>  		'-Wl,-export-dynamic',
>  		'-Wl,-h,@0@'.format(LIB_GLUE),
  
Yongseok Koh April 15, 2019, 7:48 p.m. UTC | #3
Hi,



Thanks,
Yongseok

> On Apr 15, 2019, at 3:12 AM, Luca Boccassi <bluca@debian.org> wrote:
> 
> On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
>> If MLNX_OFED is installed, there's no .pc file installed for
>> libraries and
>> dependency() can't find libraries by pkg-config. By adding fallback
>> of
>> using cc.find_library(), libraries are properly located.
>> 
>> Fixes: e30b4e566f47 ("build: improve dependency handling")
>> Cc: 
>> bluca@debian.org
>> 
>> Cc: 
>> stable@dpdk.org
>> 
>> 
>> Signed-off-by: Yongseok Koh <
>> yskoh@mellanox.com
>>> 
>> ---
>> drivers/net/mlx4/meson.build | 19 +++++++++++--------
>> drivers/net/mlx5/meson.build | 19 +++++++++++--------
>> 2 files changed, 22 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/net/mlx4/meson.build
>> b/drivers/net/mlx4/meson.build
>> index de020701d1..9082f69f25 100644
>> --- a/drivers/net/mlx4/meson.build
>> +++ b/drivers/net/mlx4/meson.build
>> @@ -13,21 +13,24 @@ if pmd_dlopen
>> 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>> 	]
>> endif
>> -libs = [
>> -	dependency('libmnl', required:false),
>> -	dependency('libmlx4', required:false),
>> -	dependency('libibverbs', required:false),
>> -]
>> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
>> +lib_deps = []
>> build = true
>> foreach lib:libs
>> -	if not lib.found()
>> +	lib_dep = dependency(lib, required:false)
>> +	if not lib_dep.found()
>> +		lib_dep = cc.find_library(lib, required:false)
> 
> Doesn't this end up trying to link the test program to -llibmnl and
> thus failing?

I also worried about that. But it works fine.
Looks meson is smart enough. :-)

>> +	endif
>> +	if lib_dep.found()
>> +		lib_deps += [ lib_dep ]
>> +	else
>> 		build = false
>> 	endif
>> endforeach
>> # Compile PMD
>> if build
>> 	allow_experimental_apis = true
>> -	ext_deps += libs
>> +	ext_deps += lib_deps
>> 	sources = files(
>> 		'mlx4.c',
>> 		'mlx4_ethdev.c',
>> @@ -103,7 +106,7 @@ if pmd_dlopen and build
>> 		dlopen_sources,
>> 		include_directories: global_inc,
>> 		c_args: cflags,
>> -		dependencies: libs,
>> +		dependencies: libs_deps,
>> 		link_args: [
>> 		'-Wl,-export-dynamic',
>> 		'-Wl,-h,@0@'.format(LIB_GLUE),
> 
> -- 
> Kind regards,
> Luca Boccassi
  
Yongseok Koh April 15, 2019, 7:48 p.m. UTC | #4
> On Apr 15, 2019, at 2:19 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Fri, Apr 12, 2019 at 04:24:48PM -0700, Yongseok Koh wrote:
>> If MLNX_OFED is installed, there's no .pc file installed for libraries and
>> dependency() can't find libraries by pkg-config. By adding fallback of
>> using cc.find_library(), libraries are properly located.
>> 
>> Fixes: e30b4e566f47 ("build: improve dependency handling")
>> Cc: bluca@debian.org
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/net/mlx4/meson.build | 19 +++++++++++--------
>> drivers/net/mlx5/meson.build | 19 +++++++++++--------
>> 2 files changed, 22 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
>> index de020701d1..9082f69f25 100644
>> --- a/drivers/net/mlx4/meson.build
>> +++ b/drivers/net/mlx4/meson.build
>> @@ -13,21 +13,24 @@ if pmd_dlopen
>> 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>> 	]
>> endif
>> -libs = [
>> -	dependency('libmnl', required:false),
>> -	dependency('libmlx4', required:false),
>> -	dependency('libibverbs', required:false),
>> -]
>> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
>> +lib_deps = []
> 
> Minor suggestion - you can reduce the size of the diff in this patch by
> defining the first array as "libnames" and keeping the actual dependency
> objects as "libs".

Sounds good to me.
Will take the suggestion in my v2.
  
Luca Boccassi April 18, 2019, 9:25 a.m. UTC | #5
On Mon, 2019-04-15 at 19:48 +0000, Yongseok Koh wrote:
> Hi,
> 
> 
> 
> Thanks,
> Yongseok
> 
> > On Apr 15, 2019, at 3:12 AM, Luca Boccassi <
> > bluca@debian.org
> > > wrote:
> > 
> > On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
> > > If MLNX_OFED is installed, there's no .pc file installed for
> > > libraries and
> > > dependency() can't find libraries by pkg-config. By adding
> > > fallback
> > > of
> > > using cc.find_library(), libraries are properly located.
> > > 
> > > Fixes: e30b4e566f47 ("build: improve dependency handling")
> > > Cc: 
> > > bluca@debian.org
> > > 
> > > 
> > > Cc: 
> > > stable@dpdk.org
> > > 
> > > 
> > > 
> > > Signed-off-by: Yongseok Koh <
> > > yskoh@mellanox.com
> > > 
> > > 
> > > ---
> > > drivers/net/mlx4/meson.build | 19 +++++++++++--------
> > > drivers/net/mlx5/meson.build | 19 +++++++++++--------
> > > 2 files changed, 22 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/net/mlx4/meson.build
> > > b/drivers/net/mlx4/meson.build
> > > index de020701d1..9082f69f25 100644
> > > --- a/drivers/net/mlx4/meson.build
> > > +++ b/drivers/net/mlx4/meson.build
> > > @@ -13,21 +13,24 @@ if pmd_dlopen
> > > 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
> > > 	]
> > > endif
> > > -libs = [
> > > -	dependency('libmnl', required:false),
> > > -	dependency('libmlx4', required:false),
> > > -	dependency('libibverbs', required:false),
> > > -]
> > > +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> > > +lib_deps = []
> > > build = true
> > > foreach lib:libs
> > > -	if not lib.found()
> > > +	lib_dep = dependency(lib, required:false)
> > > +	if not lib_dep.found()
> > > +		lib_dep = cc.find_library(lib, required:false)
> > 
> > Doesn't this end up trying to link the test program to -llibmnl and
> > thus failing?
> 
> I also worried about that. But it works fine.
> Looks meson is smart enough. :-)

Sorry, not to be skeptical, but at least with the meson version I was
using when doing something similar I'm sure this didn't work -
find_library just takes the parameter, adds "-l" in front of it and
uses it to compile.

In the meson configure log, do you see:

Dependency libmlx4 found: NO
Library libmlx4 found: YES

?
  
Bruce Richardson April 18, 2019, 10:14 a.m. UTC | #6
On Thu, Apr 18, 2019 at 10:25:19AM +0100, Luca Boccassi wrote:
> On Mon, 2019-04-15 at 19:48 +0000, Yongseok Koh wrote:
> > Hi,
> > 
> > 
> > 
> > Thanks,
> > Yongseok
> > 
> > > On Apr 15, 2019, at 3:12 AM, Luca Boccassi <
> > > bluca@debian.org
> > > > wrote:
> > > 
> > > On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
> > > > If MLNX_OFED is installed, there's no .pc file installed for
> > > > libraries and
> > > > dependency() can't find libraries by pkg-config. By adding
> > > > fallback
> > > > of
> > > > using cc.find_library(), libraries are properly located.
> > > > 
> > > > Fixes: e30b4e566f47 ("build: improve dependency handling")
> > > > Cc: 
> > > > bluca@debian.org
> > > > 
> > > > 
> > > > Cc: 
> > > > stable@dpdk.org
> > > > 
> > > > 
> > > > 
> > > > Signed-off-by: Yongseok Koh <
> > > > yskoh@mellanox.com
> > > > 
> > > > 
> > > > ---
> > > > drivers/net/mlx4/meson.build | 19 +++++++++++--------
> > > > drivers/net/mlx5/meson.build | 19 +++++++++++--------
> > > > 2 files changed, 22 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/mlx4/meson.build
> > > > b/drivers/net/mlx4/meson.build
> > > > index de020701d1..9082f69f25 100644
> > > > --- a/drivers/net/mlx4/meson.build
> > > > +++ b/drivers/net/mlx4/meson.build
> > > > @@ -13,21 +13,24 @@ if pmd_dlopen
> > > > 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
> > > > 	]
> > > > endif
> > > > -libs = [
> > > > -	dependency('libmnl', required:false),
> > > > -	dependency('libmlx4', required:false),
> > > > -	dependency('libibverbs', required:false),
> > > > -]
> > > > +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> > > > +lib_deps = []
> > > > build = true
> > > > foreach lib:libs
> > > > -	if not lib.found()
> > > > +	lib_dep = dependency(lib, required:false)
> > > > +	if not lib_dep.found()
> > > > +		lib_dep = cc.find_library(lib, required:false)
> > > 
> > > Doesn't this end up trying to link the test program to -llibmnl and
> > > thus failing?
> > 
> > I also worried about that. But it works fine.
> > Looks meson is smart enough. :-)
> 
> Sorry, not to be skeptical, but at least with the meson version I was
> using when doing something similar I'm sure this didn't work -
> find_library just takes the parameter, adds "-l" in front of it and
> uses it to compile.
> 
> In the meson configure log, do you see:
> 
> Dependency libmlx4 found: NO
> Library libmlx4 found: YES
> 

My understanding was the same as yours, Luca, and I'm pretty sure older
versions used to have that restriction. I think we should - out of an
abundance of caution - remove the "lib" prefix from all library/dependency
requests.

/Bruce
  
Yongseok Koh April 18, 2019, 11:25 a.m. UTC | #7
> On Apr 18, 2019, at 3:14 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Thu, Apr 18, 2019 at 10:25:19AM +0100, Luca Boccassi wrote:
>> On Mon, 2019-04-15 at 19:48 +0000, Yongseok Koh wrote:
>>> Hi,
>>> 
>>> 
>>> 
>>> Thanks,
>>> Yongseok
>>> 
>>>> On Apr 15, 2019, at 3:12 AM, Luca Boccassi <
>>>> bluca@debian.org
>>>>> wrote:
>>>> 
>>>> On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
>>>>> If MLNX_OFED is installed, there's no .pc file installed for
>>>>> libraries and
>>>>> dependency() can't find libraries by pkg-config. By adding
>>>>> fallback
>>>>> of
>>>>> using cc.find_library(), libraries are properly located.
>>>>> 
>>>>> Fixes: e30b4e566f47 ("build: improve dependency handling")
>>>>> Cc: 
>>>>> bluca@debian.org
>>>>> 
>>>>> 
>>>>> Cc: 
>>>>> stable@dpdk.org
>>>>> 
>>>>> 
>>>>> 
>>>>> Signed-off-by: Yongseok Koh <
>>>>> yskoh@mellanox.com
>>>>> 
>>>>> 
>>>>> ---
>>>>> drivers/net/mlx4/meson.build | 19 +++++++++++--------
>>>>> drivers/net/mlx5/meson.build | 19 +++++++++++--------
>>>>> 2 files changed, 22 insertions(+), 16 deletions(-)
>>>>> 
>>>>> diff --git a/drivers/net/mlx4/meson.build
>>>>> b/drivers/net/mlx4/meson.build
>>>>> index de020701d1..9082f69f25 100644
>>>>> --- a/drivers/net/mlx4/meson.build
>>>>> +++ b/drivers/net/mlx4/meson.build
>>>>> @@ -13,21 +13,24 @@ if pmd_dlopen
>>>>> 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>>>>> 	]
>>>>> endif
>>>>> -libs = [
>>>>> -	dependency('libmnl', required:false),
>>>>> -	dependency('libmlx4', required:false),
>>>>> -	dependency('libibverbs', required:false),
>>>>> -]
>>>>> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
>>>>> +lib_deps = []
>>>>> build = true
>>>>> foreach lib:libs
>>>>> -	if not lib.found()
>>>>> +	lib_dep = dependency(lib, required:false)
>>>>> +	if not lib_dep.found()
>>>>> +		lib_dep = cc.find_library(lib, required:false)
>>>> 
>>>> Doesn't this end up trying to link the test program to -llibmnl and
>>>> thus failing?
>>> 
>>> I also worried about that. But it works fine.
>>> Looks meson is smart enough. :-)
>> 
>> Sorry, not to be skeptical, but at least with the meson version I was
>> using when doing something similar I'm sure this didn't work -
>> find_library just takes the parameter, adds "-l" in front of it and
>> uses it to compile.
>> 
>> In the meson configure log, do you see:
>> 
>> Dependency libmlx4 found: NO
>> Library libmlx4 found: YES

Thanks for the note, Luca.

It worked regardless. Compilation's done successfully and the final testpmd
binary was good to run. Don't know how it worked but will change it anyway. Not
a hard task. Here's the diff.

$ git diff
diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index 9d04dd930d..2540489bb7 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -13,11 +13,11 @@ if pmd_dlopen
                '-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
        ]
 endif
-libnames = [ 'libmnl', 'libmlx4', 'libibverbs' ]
+libnames = [ 'mnl', 'mlx4', 'ibverbs' ]
 libs = []
 build = true
 foreach libname:libnames
-       lib = dependency(libname, required:false)
+       lib = dependency('lib' + libname, required:false)
        if not lib.found()
                lib = cc.find_library(libname, required:false)
        endif
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index ee8399af27..1a65c3a989 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -13,11 +13,11 @@ if pmd_dlopen
                '-DMLX5_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
        ]
 endif
-libnames = [ 'libmnl', 'libmlx5', 'libibverbs' ]
+libnames = [ 'mnl', 'mlx5', 'ibverbs' ]
 libs = []
 build = true
 foreach libname:libnames
-       lib = dependency(libname, required:false)
+       lib = dependency('lib' + libname, required:false)
        if not lib.found()
                lib = cc.find_library(libname, required:false)
        endif

Then, it will give us:

Dependency libmnl found: NO
Library mnl found: YES
Dependency libmlx5 found: NO
Library mlx5 found: YES
Dependency libibverbs found: NO
Library ibverbs found: YES


Thanks,
Yongseok
  

Patch

diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index de020701d1..9082f69f25 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -13,21 +13,24 @@  if pmd_dlopen
 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx4', required:false),
-	dependency('libibverbs', required:false),
-]
+libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
+lib_deps = []
 build = true
 foreach lib:libs
-	if not lib.found()
+	lib_dep = dependency(lib, required:false)
+	if not lib_dep.found()
+		lib_dep = cc.find_library(lib, required:false)
+	endif
+	if lib_dep.found()
+		lib_deps += [ lib_dep ]
+	else
 		build = false
 	endif
 endforeach
 # Compile PMD
 if build
 	allow_experimental_apis = true
-	ext_deps += libs
+	ext_deps += lib_deps
 	sources = files(
 		'mlx4.c',
 		'mlx4_ethdev.c',
@@ -103,7 +106,7 @@  if pmd_dlopen and build
 		dlopen_sources,
 		include_directories: global_inc,
 		c_args: cflags,
-		dependencies: libs,
+		dependencies: libs_deps,
 		link_args: [
 		'-Wl,-export-dynamic',
 		'-Wl,-h,@0@'.format(LIB_GLUE),
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index a4c684e1b5..42701c51de 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -13,20 +13,23 @@  if pmd_dlopen
 		'-DMLX5_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx5', required:false),
-	dependency('libibverbs', required:false),
-]
+libs = [ 'libmnl', 'libmlx5', 'libibverbs' ]
+lib_deps = []
 build = true
 foreach lib:libs
-	if not lib.found()
+	lib_dep = dependency(lib, required:false)
+	if not lib_dep.found()
+		lib_dep = cc.find_library(lib, required:false)
+	endif
+	if lib_dep.found()
+		lib_deps += [ lib_dep ]
+	else
 		build = false
 	endif
 endforeach
 if build
 	allow_experimental_apis = true
-	ext_deps += libs
+	ext_deps += lib_deps
 	sources = files(
 		'mlx5.c',
 		'mlx5_ethdev.c',
@@ -299,7 +302,7 @@  if pmd_dlopen and build
 		dlopen_sources,
 		include_directories: global_inc,
 		c_args: cflags,
-		dependencies: libs,
+		dependencies: lib_deps,
 		link_args: [
 		'-Wl,-export-dynamic',
 		'-Wl,-h,@0@'.format(LIB_GLUE),