From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 5E25DA04DD;
Thu, 26 Nov 2020 17:32:33 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 3D019CA00;
Thu, 26 Nov 2020 17:32:32 +0100 (CET)
Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com
[209.85.221.67]) by dpdk.org (Postfix) with ESMTP id 5E17FC9A8
for <dev@dpdk.org>; Thu, 26 Nov 2020 17:32:29 +0100 (CET)
Received: by mail-wr1-f67.google.com with SMTP id z7so2772981wrn.3
for <dev@dpdk.org>; Thu, 26 Nov 2020 08:32:29 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google;
h=date:from:to:cc:subject:message-id:references:mime-version
:content-disposition:in-reply-to:user-agent;
bh=5f/TWPZrVMIu0EXKRm6KvNALn1AD1ipwZFYn8YzHw5g=;
b=kC1BTRxfisIax/NlXKoSNxepiZfmi0hRCu1rPtrXN3brMYoPKHSBXKrOFPTvRae8ov
F/R792ani5h8q05hz36E0KnQrMjqwyU+wxG+EspvpwhSvEJI7tYuL3gUfPmTnEE3TyyE
vJS0sa1RuZe4B1KjbuGnudKO9nZPdhCmvhI4LSdjqGlln9GvL5/L8HDbXmOls9m/TyK8
/a8nsD0tXscGOy+SshPm+v2QpTaa+eMP2kpUlK5fS6HN3TApslX8FLKihWdgIwFdl/4K
R2K4HLRYaJBqDPU9OP72n/6a1l3/6JoDkao4pEPeXyOLp5/cenxdgvBk2h23P19p1Wn9
1kwQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:date:from:to:cc:subject:message-id:references
:mime-version:content-disposition:in-reply-to:user-agent;
bh=5f/TWPZrVMIu0EXKRm6KvNALn1AD1ipwZFYn8YzHw5g=;
b=NioPOiuZK8y138Hcb+l+mzmGosRVcUKIkWczCzsxYMSwQXx1KlH/Lx1QZeWN8Eu1P+
hTGMRqcjLho1acSoVUMrvx3BCo/AMT51TQtEKdhtVF634gETDEluaB531D+HFWS5UGW7
681jy3BNjgaIdeZ3Mfb7KLq0vjkx7spGw/0jCSRB1KHv1+Sd3LSuGJZTOBaT7Kagd+5c
YpuJV1ACcdH0NGbmUGA8ao9P8SFHD4WyA2HmuL7vkJ+X3bvpm86lmrL6CqCiieZ5HA8P
e8Mis01F/5E21UEUflybE5f0HYx5bC3gYEap4YG27V5FR65RhRnDOjNYTIIr7jWKVAq3
I9YQ==
X-Gm-Message-State: AOAM530IE/bekJB7xVk3j3flVCN29r+8m72ZdXoatjTMN5v1rT7qAG7Q
q1QeeYwhoAMdrkQe6+vgz24sKnCSPoZMwg==
X-Google-Smtp-Source: ABdhPJyf16QlCM3zRFfJ3d5A2Rizf+rbNPPV+rPQLFHfpuYxexf1O6Y99+fA86cQTjZo3eZlriZAIQ==
X-Received: by 2002:adf:e541:: with SMTP id z1mr4994123wrm.389.1606408348124;
Thu, 26 Nov 2020 08:32:28 -0800 (PST)
Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr.
[2a01:cb0c:5:a600:3456:36f7:e65e:d1a0])
by smtp.gmail.com with ESMTPSA id e27sm11309150wrc.9.2020.11.26.08.32.27
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Thu, 26 Nov 2020 08:32:27 -0800 (PST)
Date: Thu, 26 Nov 2020 17:32:26 +0100
From: Olivier Matz <olivier.matz@6wind.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org, Harry van Haaren <harry.van.haaren@intel.com>,
Keith Wiles <keith.wiles@intel.com>,
Luca Boccassi <luca.boccassi@gmail.com>, stable@dpdk.org
Message-ID: <20201126163226.GZ1898@platinum>
References: <20201126142042.24741-1-olivier.matz@6wind.com>
<20201126152846.GD1340@bricha3-MOBL.ger.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201126152846.GD1340@bricha3-MOBL.ger.corp.intel.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Subject: Re: [dpdk-dev] [PATCH] app: fix plugin load on static builds
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Hi Bruce,
Thanks for the feedback.
On Thu, Nov 26, 2020 at 03:28:46PM +0000, Bruce Richardson wrote:
> On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> > When dpdk is compiled as static libraries, it is not possible
> > to load a plugin from an application. We get the following error:
> >
> > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> >
> > This happens because the dpdk symbols are not exported. Add them to the
> > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > previously present when compiled with Makefiles, it was introduced in
> > commit f9a08f650211 ("eal: add support for shared object drivers")
> >
> > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >
> > Hi,
> >
> > Maybe the patch can be improved: I suppose that --export-dynamic
> > should only be added in case we are linking in static mode. Any
> > help about how to do that is welcome.
>
> get_option('default_library') == 'static'
>
> However, if the flag is harmless in shared linking mode, I don't think we
> need bother with this.
ok
> >
> > There is probably a better place to define the default ldflags for
> > all applications (to factorize between app and example).
> >
> > Also, should this flag be advertised in pkg-config?
> >
> Perhaps. However, I'm not sure how common it would be for people to static
> link their own apps with DPDK and then want to plug in extra drivers after
> the fact? Are there any possible negative impacts to making that change?
I don't know if it is common, but this is something we do :)
> If we weren't right before the release deadline I'd definitely suggest
> adding it to the pkg-config files. At this late stage in release, I'm more
> cautious.
Yes, it is too late for 20.11. Maybe even for this patch without
updating pkg-config.
>
> > Thanks,
> > Olivier
> >
> >
> > app/meson.build | 3 +++
> > examples/meson.build | 4 +++-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/meson.build b/app/meson.build
> > index eb74f215a3..92479c7729 100644
> > --- a/app/meson.build
> > +++ b/app/meson.build
> > @@ -25,6 +25,7 @@ apps = [
> > lib_execinfo = cc.find_library('execinfo', required: false)
> >
> > default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> > +default_ldflags = ['-Wl,--export-dynamic']
> >
> > foreach app:apps
> > build = true
> > @@ -32,6 +33,7 @@ foreach app:apps
> > sources = []
> > includes = []
> > cflags = default_cflags
> > + ldflags = default_ldflags
> > objs = [] # other object files to link against, used e.g. for
> > # instruction-set optimized versions of code
> >
> > @@ -58,6 +60,7 @@ foreach app:apps
> > executable('dpdk-' + name,
> > sources,
> > c_args: cflags,
> > + link_args: ldflags,
> > link_whole: link_libs,
> > dependencies: dep_objs,
> > install_rpath: join_paths(get_option('prefix'),
> > diff --git a/examples/meson.build b/examples/meson.build
> > index 46ec80919e..def4540e8f 100644
> > --- a/examples/meson.build
> > +++ b/examples/meson.build
> > @@ -63,6 +63,7 @@ default_cflags = machine_args
> > if cc.has_argument('-Wno-format-truncation')
> > default_cflags += '-Wno-format-truncation'
> > endif
> > +default_ldflags = ['-Wl,--export-dynamic'] + dpdk_extra_ldflags
> >
> > foreach example: examples
> > name = example.split('/')[-1]
> > @@ -70,6 +71,7 @@ foreach example: examples
> > sources = []
> > allow_experimental_apis = false
> > cflags = default_cflags
> > + ldflags = default_ldflags
> >
> > ext_deps = [execinfo]
> > includes = [include_directories(example)]
> > @@ -91,7 +93,7 @@ foreach example: examples
> > executable('dpdk-' + name, sources,
> > include_directories: includes,
> > link_whole: link_whole_libs,
> > - link_args: dpdk_extra_ldflags,
> > + link_args: ldflags,
> > c_args: cflags,
> > dependencies: dep_objs)
> > elif not allow_skips
> > --
> > 2.25.1
> >
>
> Patch looks reasonable to me. In terms of other approaches, we have:
>
> 1. Add "add_project_link_arguments('-Wl,--export-dynamic', language: 'c')"
> to "config/meson.build". This has the advantage of being a lot shorter, but
> it would apply to all .so's too, so not sure if there is any impact there.
I will check if there is any impact on .so with this approach.
> 2. If there is no reason why any particular app would want to not provide
> this flag, you can skip defining a new ldflags variable and just add the
> flag into the existing link_args line for each app and example. However,
> your approach here, though longer, is more in keeping with the style of
> what is already there.