mk: fix application compilation with lmnl and mlx5

  When Mellanox MLX5 PMD is compiled with
CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on libmln
is missing.

Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
Cc: adrien.mazarguil@6wind.com

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 mk/rte.app.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Tue, Jul 24, 2018 at 11:29:09AM +0200, Nelio Laranjeiro wrote:
> When Mellanox MLX5 PMD is compiled with
> CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on libmln
> is missing.
> 
> Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> Cc: adrien.mazarguil@6wind.com
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Except for libmln => libmnl typo,

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Thanks.
  

Tuesday, July 24, 2018 12:29 PM, Nelio Laranjeiro:
> Subject: [PATCH] mk: fix application compilation with lmnl and mlx5
> 
> When Mellanox MLX5 PMD is compiled with
> CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on
> libmln is missing.
> 
> Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> Cc: adrien.mazarguil@6wind.com
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  mk/rte.app.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk index f4d28c2da..ff39d37aa
> 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -149,7 +149,7 @@ else
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -
> libverbs -lmlx4
>  endif
>  ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl -
> lmnl

This issue raise some more basic question. 
The DLOPEN mode was introduced to run in systems which don't have verbs/mlx5 libs installed, because those were the only dependencies for the PMD back then.
Now we have the libmnl, which is external dependency just like rdma-core, and following your fix, hard linked also in case of DLOPEN option.
It means the whole DPDK binary/lib will be depended on libmnl and this is not what we want with DLOPEN.

Can we consider different options:
1. always statically link libmnl 
2. dlopen libmnl just like we do for verbs/mlx5


>  else
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -
> libverbs -lmlx5 -lmnl
>  endif
> --
> 2.18.0
  

On Tue, Jul 24, 2018 at 11:21:52AM +0000, Shahaf Shuler wrote:
> Tuesday, July 24, 2018 12:29 PM, Nelio Laranjeiro:
> > Subject: [PATCH] mk: fix application compilation with lmnl and mlx5
> > 
> > When Mellanox MLX5 PMD is compiled with
> > CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on
> > libmln is missing.
> > 
> > Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> > Cc: adrien.mazarguil@6wind.com
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > ---
> >  mk/rte.app.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index f4d28c2da..ff39d37aa
> > 100644
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -149,7 +149,7 @@ else
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -
> > libverbs -lmlx4
> >  endif
> >  ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> > -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> > +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl -
> > lmnl
> 
> This issue raise some more basic question. 
> The DLOPEN mode was introduced to run in systems which don't have
> verbs/mlx5 libs installed, because those were the only dependencies
> for the PMD back then.
> Now we have the libmnl, which is external dependency just like
> rdma-core, and following your fix, hard linked also in case of DLOPEN
> option.
> It means the whole DPDK binary/lib will be depended on libmnl and this
> is not what we want with DLOPEN.
> 
> Can we consider different options:
> 1. always statically link libmnl

This will force users to re-compile their application to have the missing
features disabled because some calls are not available, see the list of
HAVE_RDMA_NLDEV_* elements in the MLX5 makefile due to the issues this
Netlink stuff brings.

> 2. dlopen libmnl just like we do for verbs/mlx5

You want another glue library.  It won't be for this release in this
case, I don't have time to write such glue.

> >  else
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -
> > libverbs -lmlx5 -lmnl
> >  endif
> > --
> > 2.18.0
> 

[1] https://mails.dpdk.org/archives/dev/2018-March/092876.html
  

Tuesday, July 24, 2018 2:45 PM, Nélio Laranjeiro:
> Subject: Re: [PATCH] mk: fix application compilation with lmnl and mlx5
> 
> On Tue, Jul 24, 2018 at 11:21:52AM +0000, Shahaf Shuler wrote:
> > Tuesday, July 24, 2018 12:29 PM, Nelio Laranjeiro:
> > > Subject: [PATCH] mk: fix application compilation with lmnl and mlx5
> > >
> > > When Mellanox MLX5 PMD is compiled with
> > > CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency
> on
> > > libmln is missing.
> > >
> > > Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> > > Cc: adrien.mazarguil@6wind.com
> > >
> > > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > ---
> > >  mk/rte.app.mk | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index
> > > f4d28c2da..ff39d37aa
> > > 100644
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -149,7 +149,7 @@ else
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -
> > > libverbs -lmlx4
> > >  endif
> > >  ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> > > -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> -
> > > lmnl
> >
> > This issue raise some more basic question.
> > The DLOPEN mode was introduced to run in systems which don't have
> > verbs/mlx5 libs installed, because those were the only dependencies
> > for the PMD back then.
> > Now we have the libmnl, which is external dependency just like
> > rdma-core, and following your fix, hard linked also in case of DLOPEN
> > option.
> > It means the whole DPDK binary/lib will be depended on libmnl and this
> > is not what we want with DLOPEN.
> >
> > Can we consider different options:
> > 1. always statically link libmnl
> 
> This will force users to re-compile their application to have the missing
> features disabled because some calls are not available, see the list of
> HAVE_RDMA_NLDEV_* elements in the MLX5 makefile due to the issues this
> Netlink stuff brings.

I don't understand. 

First the HAVE_RDMA_NLDEV_* actually re-define the needed enum in case some of the user space headers are missing.
It is not to disable to support on some features in libmnl.

In the doc patch[1] is state the minimal libmnl version:
"Minimal version for libmnl is **1.0.3**."

This one contains all the needed support for switch rules?
Do we expect that w/ more switch rules we will need more feature from this lib? (not sure, it is only to open the sockets).  

[1] https://patches.dpdk.org/patch/43266/

> 
> > 2. dlopen libmnl just like we do for verbs/mlx5
> 
> You want another glue library.  It won't be for this release in this case, I don't
> have time to write such glue.

Yes I agree the first approach is better (static linkage). 

> 
> > >  else
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -
> > > libverbs -lmlx5 -lmnl
> > >  endif
> > > --
> > > 2.18.0
> >
> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma
> ils.dpdk.org%2Farchives%2Fdev%2F2018-
> March%2F092876.html&amp;data=02%7C01%7Cshahafs%40mellanox.com%7
> C206bd64b84a341f50c6e08d5f15ae0d5%7Ca652971c7d2e4d9ba6a4d149256f4
> 61b%7C0%7C0%7C636680294972217071&amp;sdata=GIfPD0xe8QnHaRh%2Bv
> BTy1p3r%2FWj3j2GlmcVPFCbSLMw%3D&amp;reserved=0
> 
> --
> Nélio Laranjeiro
> 6WIND
  

On Tue, Jul 24, 2018 at 11:21:52AM +0000, Shahaf Shuler wrote:
> Tuesday, July 24, 2018 12:29 PM, Nelio Laranjeiro:
> > Subject: [PATCH] mk: fix application compilation with lmnl and mlx5
> > 
> > When Mellanox MLX5 PMD is compiled with
> > CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on
> > libmln is missing.
> > 
> > Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> > Cc: adrien.mazarguil@6wind.com
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > ---
> >  mk/rte.app.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index f4d28c2da..ff39d37aa
> > 100644
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -149,7 +149,7 @@ else
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -
> > libverbs -lmlx4
> >  endif
> >  ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> > -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> > +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl -
> > lmnl
> 
> This issue raise some more basic question. 
> The DLOPEN mode was introduced to run in systems which don't have verbs/mlx5 libs installed, because those were the only dependencies for the PMD back then.
> Now we have the libmnl, which is external dependency just like rdma-core, and following your fix, hard linked also in case of DLOPEN option.
> It means the whole DPDK binary/lib will be depended on libmnl and this is not what we want with DLOPEN.
> 
> Can we consider different options:
> 1. always statically link libmnl 
> 2. dlopen libmnl just like we do for verbs/mlx5

Regarding 2, unlike rdma-core/MLNX_OFED, libmnl should be available pretty
much everywhere iproute2 can be found. The minimal version supported (1.0.3)
was released in 2012.

Using the glue approach for such a small library seems overkill; should we
choose this path, we must also consider to get rid of it entirely since
doing so would require more glue code than what mlx5 needs from this
library.

So with the current approach, either the application or the PMD inherits a
dependency to libmnl, depending on whether CONFIG_RTE_BUILD_SHARED_LIB is
respectively disabled or enabled.

If disabled, applications that want static linkage can specify -static as
part of their compilation flags to let the compiler automatically look for
libmnl.a as needed. To put this in perspective, this also applies to all
other dependencies it will collect while compiling DPDK (libz, libdl,
libpcap, libnuma to name a few).

In my opinion, the purpose of *_DLOPEN_DEPS is to deal with large,
nonstandard libraries where versioning issues are commonplace. This doesn't
apply to libmnl, which shouldn't be a maintenance nightmare to package
maintainers. I suggest to leave things as is.
  

Tuesday, July 24, 2018 3:56 PM, Adrien Mazarguil:
> Subject: Re: [PATCH] mk: fix application compilation with lmnl and mlx5
> 
> On Tue, Jul 24, 2018 at 11:21:52AM +0000, Shahaf Shuler wrote:
> > Tuesday, July 24, 2018 12:29 PM, Nelio Laranjeiro:
> > > Subject: [PATCH] mk: fix application compilation with lmnl and mlx5
> > >
> > > When Mellanox MLX5 PMD is compiled with
> > > CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency
> on
> > > libmln is missing.
> > >
> > > Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> > > Cc: adrien.mazarguil@6wind.com
> > >
> > > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > ---
> > >  mk/rte.app.mk | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index
> > > f4d28c2da..ff39d37aa
> > > 100644
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -149,7 +149,7 @@ else
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -
> > > libverbs -lmlx4
> > >  endif
> > >  ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> > > -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> -
> > > lmnl
> >
> > This issue raise some more basic question.
> > The DLOPEN mode was introduced to run in systems which don't have
> verbs/mlx5 libs installed, because those were the only dependencies for the
> PMD back then.
> > Now we have the libmnl, which is external dependency just like rdma-core,
> and following your fix, hard linked also in case of DLOPEN option.
> > It means the whole DPDK binary/lib will be depended on libmnl and this is
> not what we want with DLOPEN.
> >
> > Can we consider different options:
> > 1. always statically link libmnl
> > 2. dlopen libmnl just like we do for verbs/mlx5
> 
> Regarding 2, unlike rdma-core/MLNX_OFED, libmnl should be available pretty
> much everywhere iproute2 can be found. The minimal version supported
> (1.0.3) was released in 2012.
> 
> Using the glue approach for such a small library seems overkill; should we
> choose this path, we must also consider to get rid of it entirely since doing so
> would require more glue code than what mlx5 needs from this library.
> 
> So with the current approach, either the application or the PMD inherits a
> dependency to libmnl, depending on whether
> CONFIG_RTE_BUILD_SHARED_LIB is respectively disabled or enabled.
> 
> If disabled, applications that want static linkage can specify -static as part of
> their compilation flags to let the compiler automatically look for libmnl.a as
> needed. 

Can you confirm static compilation w/ libmnl is working w/o any issues? 

To put this in perspective, this also applies to all other dependencies
> it will collect while compiling DPDK (libz, libdl, libpcap, libnuma to name a
> few).
> 
> In my opinion, the purpose of *_DLOPEN_DEPS is to deal with large,
> nonstandard libraries where versioning issues are commonplace. This doesn't
> apply to libmnl, which shouldn't be a maintenance nightmare to package
> maintainers. I suggest to leave things as is.
> 
> --
> Adrien Mazarguil
> 6WIND
  

On Tue, Jul 24, 2018 at 01:03:28PM +0000, Shahaf Shuler wrote:
> Tuesday, July 24, 2018 3:56 PM, Adrien Mazarguil:
> > Subject: Re: [PATCH] mk: fix application compilation with lmnl and mlx5
> > 
> > On Tue, Jul 24, 2018 at 11:21:52AM +0000, Shahaf Shuler wrote:
> > > Tuesday, July 24, 2018 12:29 PM, Nelio Laranjeiro:
> > > > Subject: [PATCH] mk: fix application compilation with lmnl and mlx5
> > > >
> > > > When Mellanox MLX5 PMD is compiled with
> > > > CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency
> > on
> > > > libmln is missing.
> > > >
> > > > Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> > > > Cc: adrien.mazarguil@6wind.com
> > > >
> > > > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > ---
> > > >  mk/rte.app.mk | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index
> > > > f4d28c2da..ff39d37aa
> > > > 100644
> > > > --- a/mk/rte.app.mk
> > > > +++ b/mk/rte.app.mk
> > > > @@ -149,7 +149,7 @@ else
> > > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -lrte_pmd_mlx4 -
> > > > libverbs -lmlx4
> > > >  endif
> > > >  ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> > > > -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> > > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)       += -lrte_pmd_mlx5 -ldl
> > -
> > > > lmnl
> > >
> > > This issue raise some more basic question.
> > > The DLOPEN mode was introduced to run in systems which don't have
> > verbs/mlx5 libs installed, because those were the only dependencies for the
> > PMD back then.
> > > Now we have the libmnl, which is external dependency just like rdma-core,
> > and following your fix, hard linked also in case of DLOPEN option.
> > > It means the whole DPDK binary/lib will be depended on libmnl and this is
> > not what we want with DLOPEN.
> > >
> > > Can we consider different options:
> > > 1. always statically link libmnl
> > > 2. dlopen libmnl just like we do for verbs/mlx5
> > 
> > Regarding 2, unlike rdma-core/MLNX_OFED, libmnl should be available pretty
> > much everywhere iproute2 can be found. The minimal version supported
> > (1.0.3) was released in 2012.
> > 
> > Using the glue approach for such a small library seems overkill; should we
> > choose this path, we must also consider to get rid of it entirely since doing so
> > would require more glue code than what mlx5 needs from this library.
> > 
> > So with the current approach, either the application or the PMD inherits a
> > dependency to libmnl, depending on whether
> > CONFIG_RTE_BUILD_SHARED_LIB is respectively disabled or enabled.
> > 
> > If disabled, applications that want static linkage can specify -static as part of
> > their compilation flags to let the compiler automatically look for libmnl.a as
> > needed. 
> 
> Can you confirm static compilation w/ libmnl is working w/o any issues? 

Challenge accepted. Using -static is not something DPDK applications usually
do and needs a few tweaks to compile successfully though (unless you meant
something else by "static compilation"?)

First you need to force rdma-core to generate static versions of
libmlx4/libmlx5 and libiberbs as it's not the default:

 $ cmake -DENABLE_STATIC=1 .

Next, patch DPDK to remove references to -lgcc_s, -fPIC and -export-dynamic:

 $ sed -i 's/[[:space:]]*-lgcc_s//' $(git grep -l -- -lgcc_s)
 $ sed -i 's/[[:space:]]*-fPIC//' $(git grep -l -- -fPIC)
 $ sed -i 's/[[:space:]]*-export-dynamic//' $(git grep -l -- -export-dynamic)

Then append rdma-core dependencies to libmnl users (mlx5) in order to avoid
undefined references to libnl/libm stuff normally pulled by libibverbs:

 $ sed -i 's/-lmnl/-lmnl -lnl-route-3 -lnl-3 -lm/' $(git grep -l -- -lmnl)

Configure DPDK with mlx5 enabled:

 $ make config T=x86_64-native-linuxapp-gcc
 $ sed -i 's/^\(CONFIG_RTE_LIBRTE_MLX5_PMD\)=.*/\1=y/' build/.config

Finally add -static to $CC (the easiest method I could find) while compiling
DPDK:

 $ make CC='gcc -static'

You can ignore warnings about statically linking "getaddrinfo" and
friends. What matters is we now have a testpmd binary with no dependencies:

 $ readelf -d build/app/testpmd | grep NEEDED
 $

 $ ldd build/app/testpmd
         not a dynamic executable
 $

Now start testpmd with a mlx5 adapter and a couple of representors for fun:

 # ./build/app/testpmd -w 06:00.0,representor=[0-1] -n 4 -c 0x80 -- -i --rxq=1 --txq=1
 EAL: Detected 32 lcore(s)
 EAL: Detected 2 NUMA nodes
 EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
 EAL: No free hugepages reported in hugepages-1048576kB
 EAL: Probing VFIO support...
 EAL: PCI device 0000:06:00.0 on NUMA socket 0
 EAL:   probe driver: 15b3:1017 net_mlx5
 [...]
 Configuring Port 0 (socket 0)
 Port 0: 24:8A:07:8D:AE:F6
 Configuring Port 1 (socket 0)
 Port 1: A6:41:D9:89:DB:12
 Configuring Port 2 (socket 0)
 Port 2: FE:A8:15:80:DE:C0
 Checking link statuses...
 Done
 testpmd>

Phew!

> To put this in perspective, this also applies to all other dependencies
> > it will collect while compiling DPDK (libz, libdl, libpcap, libnuma to name a
> > few).
> > 
> > In my opinion, the purpose of *_DLOPEN_DEPS is to deal with large,
> > nonstandard libraries where versioning issues are commonplace. This doesn't
> > apply to libmnl, which shouldn't be a maintenance nightmare to package
> > maintainers. I suggest to leave things as is.
  

Tuesday, July 24, 2018 6:14 PM, Adrien Mazarguil:
> Subject: Re: [PATCH] mk: fix application compilation with lmnl and mlx5
> On Tue, Jul 24, 2018 at 01:03:28PM +0000, Shahaf Shuler wrote:
> > Tuesday, July 24, 2018 3:56 PM, Adrien Mazarguil:
> > > Subject: Re: [PATCH] mk: fix application compilation with lmnl and
> > > mlx5

[...]

> > > > Can we consider different options:
> > > > 1. always statically link libmnl
> > > > 2. dlopen libmnl just like we do for verbs/mlx5
> > >
> > > Regarding 2, unlike rdma-core/MLNX_OFED, libmnl should be available
> > > pretty much everywhere iproute2 can be found. The minimal version
> > > supported
> > > (1.0.3) was released in 2012.
> > >
> > > Using the glue approach for such a small library seems overkill;
> > > should we choose this path, we must also consider to get rid of it
> > > entirely since doing so would require more glue code than what mlx5
> needs from this library.
> > >
> > > So with the current approach, either the application or the PMD
> > > inherits a dependency to libmnl, depending on whether
> > > CONFIG_RTE_BUILD_SHARED_LIB is respectively disabled or enabled.
> > >
> > > If disabled, applications that want static linkage can specify
> > > -static as part of their compilation flags to let the compiler
> > > automatically look for libmnl.a as needed.
> >
> > Can you confirm static compilation w/ libmnl is working w/o any issues?
> 
> Challenge accepted.

😊

 Using -static is not something DPDK applications usually
> do and needs a few tweaks to compile successfully though (unless you
> meant something else by "static compilation"?)

Yes this is what I meant. However I was under the impression we can statically link to libmnl while keeping the rdma-core w/ dynamic linkage. 

> 
> First you need to force rdma-core to generate static versions of
> libmlx4/libmlx5 and libiberbs as it's not the default:
> 
>  $ cmake -DENABLE_STATIC=1 .
> 
> Next, patch DPDK to remove references to -lgcc_s, -fPIC and -export-
> dynamic:
> 
>  $ sed -i 's/[[:space:]]*-lgcc_s//' $(git grep -l -- -lgcc_s)  $ sed -i 's/[[:space:]]*-
> fPIC//' $(git grep -l -- -fPIC)  $ sed -i 's/[[:space:]]*-export-dynamic//' $(git
> grep -l -- -export-dynamic)

It seems the fPIC and export-dynamic are defined only when compiling the DPDK as shared library. 
The gcc_s is from eal. Do you think it is correct to put it only when compiling as shared library? It seems this approach was taken in the rte.vars.mk files. 

> 
> Then append rdma-core dependencies to libmnl users (mlx5) in order to
> avoid undefined references to libnl/libm stuff normally pulled by libibverbs:
> 
>  $ sed -i 's/-lmnl/-lmnl -lnl-route-3 -lnl-3 -lm/' $(git grep -l -- -lmnl)
> 
> Configure DPDK with mlx5 enabled:
> 
>  $ make config T=x86_64-native-linuxapp-gcc  $ sed -i
> 's/^\(CONFIG_RTE_LIBRTE_MLX5_PMD\)=.*/\1=y/' build/.config
> 
> Finally add -static to $CC (the easiest method I could find) while compiling
> DPDK:
> 
>  $ make CC='gcc -static'
> 
> You can ignore warnings about statically linking "getaddrinfo" and friends.
> What matters is we now have a testpmd binary with no dependencies:
> 
>  $ readelf -d build/app/testpmd | grep NEEDED  $
> 
>  $ ldd build/app/testpmd
>          not a dynamic executable
>  $
> 
> Now start testpmd with a mlx5 adapter and a couple of representors for fun:
> 
>  # ./build/app/testpmd -w 06:00.0,representor=[0-1] -n 4 -c 0x80 -- -i --rxq=1
> --txq=1
>  EAL: Detected 32 lcore(s)
>  EAL: Detected 2 NUMA nodes
>  EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
>  EAL: No free hugepages reported in hugepages-1048576kB
>  EAL: Probing VFIO support...
>  EAL: PCI device 0000:06:00.0 on NUMA socket 0
>  EAL:   probe driver: 15b3:1017 net_mlx5
>  [...]
>  Configuring Port 0 (socket 0)
>  Port 0: 24:8A:07:8D:AE:F6
>  Configuring Port 1 (socket 0)
>  Port 1: A6:41:D9:89:DB:12
>  Configuring Port 2 (socket 0)
>  Port 2: FE:A8:15:80:DE:C0
>  Checking link statuses...
>  Done
>  testpmd>
> 
> Phew!

Nicely done. 

So if indeed it is possible w/ some fixes on the DPDK to fully support static linkage of both rdma-core and libmnl, maybe we should consider such compilation flag for mlx drivers. This can be very good alternative to the current DLOPEN approach. 

> 
> > To put this in perspective, this also applies to all other
> > dependencies
> > > it will collect while compiling DPDK (libz, libdl, libpcap, libnuma
> > > to name a few).
> > >
> > > In my opinion, the purpose of *_DLOPEN_DEPS is to deal with large,
> > > nonstandard libraries where versioning issues are commonplace. This
> > > doesn't apply to libmnl, which shouldn't be a maintenance nightmare
> > > to package maintainers. I suggest to leave things as is.
> 
> --
> Adrien Mazarguil
> 6WIND
  

On Wed, Jul 25, 2018 at 06:39:32AM +0000, Shahaf Shuler wrote:
> Tuesday, July 24, 2018 6:14 PM, Adrien Mazarguil:
> > Subject: Re: [PATCH] mk: fix application compilation with lmnl and mlx5
> > On Tue, Jul 24, 2018 at 01:03:28PM +0000, Shahaf Shuler wrote:
> > > Tuesday, July 24, 2018 3:56 PM, Adrien Mazarguil:
> > > > Subject: Re: [PATCH] mk: fix application compilation with lmnl and
> > > > mlx5
> 
> [...]
> 
> > > > > Can we consider different options:
> > > > > 1. always statically link libmnl
> > > > > 2. dlopen libmnl just like we do for verbs/mlx5
> > > >
> > > > Regarding 2, unlike rdma-core/MLNX_OFED, libmnl should be available
> > > > pretty much everywhere iproute2 can be found. The minimal version
> > > > supported
> > > > (1.0.3) was released in 2012.
> > > >
> > > > Using the glue approach for such a small library seems overkill;
> > > > should we choose this path, we must also consider to get rid of it
> > > > entirely since doing so would require more glue code than what mlx5
> > needs from this library.
> > > >
> > > > So with the current approach, either the application or the PMD
> > > > inherits a dependency to libmnl, depending on whether
> > > > CONFIG_RTE_BUILD_SHARED_LIB is respectively disabled or enabled.
> > > >
> > > > If disabled, applications that want static linkage can specify
> > > > -static as part of their compilation flags to let the compiler
> > > > automatically look for libmnl.a as needed.
> > >
> > > Can you confirm static compilation w/ libmnl is working w/o any issues?
> > 
> > Challenge accepted.
> 
> 😊
> 
>  Using -static is not something DPDK applications usually
> > do and needs a few tweaks to compile successfully though (unless you
> > meant something else by "static compilation"?)
> 
> Yes this is what I meant. However I was under the impression we can statically link to libmnl while keeping the rdma-core w/ dynamic linkage. 

Hmm, a much simpler approach if you don't actually want to make the whole
binary static but only get rid of the libmnl.so run-time dependency is to
replace "-lmnl" with e.g. "-l:libmnl.a". Doing so makes it part of the
resulting binaries like any other DPDK libraries (librte_whatever.a).

This can also be achieved through a linker script or by simply removing the
.so version from the library search path.

However while users are free to compile DPDK however they want, we shouldn't
hard-code it this way since libmnl will eventually have multiple users in
DPDK, this will lead to inefficient symbol duplication and possible
link-time issues.

> > First you need to force rdma-core to generate static versions of
> > libmlx4/libmlx5 and libiberbs as it's not the default:
> > 
> >  $ cmake -DENABLE_STATIC=1 .
> > 
> > Next, patch DPDK to remove references to -lgcc_s, -fPIC and -export-
> > dynamic:
> > 
> >  $ sed -i 's/[[:space:]]*-lgcc_s//' $(git grep -l -- -lgcc_s)  $ sed -i 's/[[:space:]]*-
> > fPIC//' $(git grep -l -- -fPIC)  $ sed -i 's/[[:space:]]*-export-dynamic//' $(git
> > grep -l -- -export-dynamic)
> 
> It seems the fPIC and export-dynamic are defined only when compiling the DPDK as shared library. 
> The gcc_s is from eal. Do you think it is correct to put it only when compiling as shared library? It seems this approach was taken in the rte.vars.mk files. 

Removing these flags is necessary even with CONFIG_RTE_BUILD_SHARED_LIB
disabled in order to use -static. They appear to be present regardless.

Besides -fPIC, they usually do not need to be specified when generating
shared libraries. I'm almost 100% sure we could get rid of -lgcc_s without
any impact. A bit less sure about -export-dynamic, however we now use map
files to export public symbols. Someone should try.

-fPIC can be kept when generating .a libraries by the way. It's a bit less
efficient when the resulting file is linked with a program but shouldn't
hurt much. It remains mandatory if the resulting library is to be made part
of a shared library. We should keep it just in case.

> > Then append rdma-core dependencies to libmnl users (mlx5) in order to
> > avoid undefined references to libnl/libm stuff normally pulled by libibverbs:
> > 
> >  $ sed -i 's/-lmnl/-lmnl -lnl-route-3 -lnl-3 -lm/' $(git grep -l -- -lmnl)
> > 
> > Configure DPDK with mlx5 enabled:
> > 
> >  $ make config T=x86_64-native-linuxapp-gcc  $ sed -i
> > 's/^\(CONFIG_RTE_LIBRTE_MLX5_PMD\)=.*/\1=y/' build/.config
> > 
> > Finally add -static to $CC (the easiest method I could find) while compiling
> > DPDK:
> > 
> >  $ make CC='gcc -static'
> > 
> > You can ignore warnings about statically linking "getaddrinfo" and friends.
> > What matters is we now have a testpmd binary with no dependencies:
> > 
> >  $ readelf -d build/app/testpmd | grep NEEDED  $
> > 
> >  $ ldd build/app/testpmd
> >          not a dynamic executable
> >  $
> > 
> > Now start testpmd with a mlx5 adapter and a couple of representors for fun:
> > 
> >  # ./build/app/testpmd -w 06:00.0,representor=[0-1] -n 4 -c 0x80 -- -i --rxq=1
> > --txq=1
> >  EAL: Detected 32 lcore(s)
> >  EAL: Detected 2 NUMA nodes
> >  EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> >  EAL: No free hugepages reported in hugepages-1048576kB
> >  EAL: Probing VFIO support...
> >  EAL: PCI device 0000:06:00.0 on NUMA socket 0
> >  EAL:   probe driver: 15b3:1017 net_mlx5
> >  [...]
> >  Configuring Port 0 (socket 0)
> >  Port 0: 24:8A:07:8D:AE:F6
> >  Configuring Port 1 (socket 0)
> >  Port 1: A6:41:D9:89:DB:12
> >  Configuring Port 2 (socket 0)
> >  Port 2: FE:A8:15:80:DE:C0
> >  Checking link statuses...
> >  Done
> >  testpmd>
> > 
> > Phew!
> 
> Nicely done. 
> 
> So if indeed it is possible w/ some fixes on the DPDK to fully support static linkage of both rdma-core and libmnl, maybe we should consider such compilation flag for mlx drivers. This can be very good alternative to the current DLOPEN approach. 

Although this is one way to generate statically linked DPDK applications,
currently DPDK obviously doesn't support static linkage. I wouldn't
recommend it.

The approach of linking .a files instead of .so and still generate a dynamic
program also has drawbacks: duplication and link time issues with multiple
users. For instance, this may cause mlx4 and mlx5 to conflict on libibverbs
and libmnl symbols, leaving one unable to include both PMDs into their
program or prevent it from linking with these libraries for its own needs.
I don't think it's worth the trouble.

> > > To put this in perspective, this also applies to all other
> > > dependencies
> > > > it will collect while compiling DPDK (libz, libdl, libpcap, libnuma
> > > > to name a few).
> > > >
> > > > In my opinion, the purpose of *_DLOPEN_DEPS is to deal with large,
> > > > nonstandard libraries where versioning issues are commonplace. This
> > > > doesn't apply to libmnl, which shouldn't be a maintenance nightmare
> > > > to package maintainers. I suggest to leave things as is.
  

Wednesday, July 25, 2018 12:07 PM, Adrien Mazarguil:
> Subject: Re: [PATCH] mk: fix application compilation with lmnl and mlx5
> 
> The approach of linking .a files instead of .so and still generate a dynamic
> program also has drawbacks: duplication and link time issues with multiple
> users. For instance, this may cause mlx4 and mlx5 to conflict on libibverbs and
> libmnl symbols, leaving one unable to include both PMDs into their program
> or prevent it from linking with these libraries for its own needs.
> I don't think it's worth the trouble.
> 

OK,

Acked-by: Shahaf Shuler <shahafs@mellanox.com>
  

24/07/2018 11:32, Adrien Mazarguil:
> On Tue, Jul 24, 2018 at 11:29:09AM +0200, Nelio Laranjeiro wrote:
> > When Mellanox MLX5 PMD is compiled with
> > CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on libmln
> > is missing.
> > 
> > Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> > Cc: adrien.mazarguil@6wind.com
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Except for libmln => libmnl typo,
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Merged in above patch, thanks.