From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3F1A8A04DD; Thu, 26 Nov 2020 16:29:04 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0D5C1C9CC; Thu, 26 Nov 2020 16:29:01 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 5141AC9BC; Thu, 26 Nov 2020 16:28:58 +0100 (CET) IronPort-SDR: FhZanVZiOM2gYtqYEa7Z5gUeeKvSxmaV//Ko8cbjs0Yt+OGkV1Skv87nHb9ad7d3KMJqNx3FXN AQkmn60cOv2g== X-IronPort-AV: E=McAfee;i="6000,8403,9817"; a="257005819" X-IronPort-AV: E=Sophos;i="5.78,372,1599548400"; d="scan'208";a="257005819" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Nov 2020 07:28:52 -0800 IronPort-SDR: AB0bTSayYMTo80742UwX1Ir2QGqGZ76qDb/IazV8aatHxuIAGo/m6hEFtScIUgnJpzsyNZHwF+ nsi5RI8AVd4A== X-IronPort-AV: E=Sophos;i="5.78,372,1599548400"; d="scan'208";a="547750221" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.21.198]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 26 Nov 2020 07:28:50 -0800 Date: Thu, 26 Nov 2020 15:28:46 +0000 From: Bruce Richardson To: Olivier Matz Cc: dev@dpdk.org, Harry van Haaren , Keith Wiles , Luca Boccassi , stable@dpdk.org Message-ID: <20201126152846.GD1340@bricha3-MOBL.ger.corp.intel.com> References: <20201126142042.24741-1-olivier.matz@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201126142042.24741-1-olivier.matz@6wind.com> 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > > 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. > > 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? 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. > 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. 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. /Bruce