DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Shahaf Shuler <shahafs@mellanox.com>
Cc: "Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Yongseok Koh" <yskoh@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH] mk: fix application compilation with lmnl and mlx5
Date: Tue, 24 Jul 2018 14:55:56 +0200	[thread overview]
Message-ID: <20180724125556.GS5211@6wind.com> (raw)
In-Reply-To: <DB7PR05MB44268F70B4DC8FA96C3EF5EDC3550@DB7PR05MB4426.eurprd05.prod.outlook.com>

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.

-- 
Adrien Mazarguil
6WIND

  parent reply	other threads:[~2018-07-24 12:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-24  9:29 Nelio Laranjeiro
2018-07-24  9:32 ` Adrien Mazarguil
2018-07-26 12:05   ` Thomas Monjalon
2018-07-24 11:21 ` Shahaf Shuler
2018-07-24 11:44   ` Nélio Laranjeiro
2018-07-24 11:53     ` Shahaf Shuler
2018-07-24 12:55   ` Adrien Mazarguil [this message]
2018-07-24 13:03     ` Shahaf Shuler
2018-07-24 15:13       ` Adrien Mazarguil
2018-07-25  6:39         ` Shahaf Shuler
2018-07-25  9:07           ` Adrien Mazarguil
2018-07-25 13:23             ` Shahaf Shuler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180724125556.GS5211@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=shahafs@mellanox.com \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).