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 56403A04DD; Thu, 26 Nov 2020 18:15:48 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EA2D1CA32; Thu, 26 Nov 2020 18:15:45 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 874E4CA2E; Thu, 26 Nov 2020 18:15:43 +0100 (CET) IronPort-SDR: G70h9Tm/lMcetteU7kDlXTw0m+wvzP4QNc2EMEY4QluiwOL0VRHOlsRjRnpiDM1vmSdoxAQs0v 0a3kDmCHTjPA== X-IronPort-AV: E=McAfee;i="6000,8403,9817"; a="171528208" X-IronPort-AV: E=Sophos;i="5.78,372,1599548400"; d="scan'208";a="171528208" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Nov 2020 09:15:38 -0800 IronPort-SDR: uFtqJifO08tUDyF1xU1Kt0iCaskt9nKY8bDzDqKKCRZBux+niRILrp42opqXyvJaHefA1456ma oj3wr0LRxVZQ== X-IronPort-AV: E=Sophos;i="5.78,372,1599548400"; d="scan'208";a="479410138" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.21.198]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 26 Nov 2020 09:15:35 -0800 Date: Thu, 26 Nov 2020 17:15:32 +0000 From: Bruce Richardson To: Thomas Monjalon Cc: Olivier Matz , stable@dpdk.org, dev@dpdk.org, Harry van Haaren , Keith Wiles , Luca Boccassi Message-ID: <20201126171532.GJ1340@bricha3-MOBL.ger.corp.intel.com> References: <20201126142042.24741-1-olivier.matz@6wind.com> <20201126163226.GZ1898@platinum> <20201126163703.GH1340@bricha3-MOBL.ger.corp.intel.com> <2510990.SFP7MGkWmT@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2510990.SFP7MGkWmT@thomas> Subject: Re: [dpdk-dev] [dpdk-stable] [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 06:07:32PM +0100, Thomas Monjalon wrote: > 26/11/2020 17:37, Bruce Richardson: > > On Thu, Nov 26, 2020 at 05:32:26PM +0100, Olivier Matz wrote: > > > 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 > > > > > --- > > > > > > > > > > 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. > > > > > > > Well, since this is something clearly broken that is of use to you, I think > > we should strive to get some fix for it in. Based on the lateness of the > > hour, I think your patch is pretty close to the least-risky option we could > > take here. Therefore, despite my previous comment about not needing to > > limit the additional linking flag to static-builds only, I would suggest - > > out of an abundance of caution - that we do so, and keep the rest of your > > patch as-is for 20.11. Thereafter we can look at other possible approaches > > and simplifications in 21.02 and backport them. > > > > So, for your patch with "if get_option('default_library') ..." checks added > > appropriately: > > > > Acked-by: Bruce Richardson > > The most critical issue is for externally built applications > statically linked using external plugins. > This patch fixes only internal apps built with meson. > > I am for keeping the issue as-is for 20.11.0, > because it is really too late to change a compilation option. > We have already been bitten by compilers not supporting an option > or being buggy in specific contexts. I prefer not taking this risk > for a fix which helps nobody really. > I'm ok with that fairly conservative stance. In general the workaround for this issue is to set the appropriate ldflags when building. For those building their own app, I would assume adding the flag can be fairly easily done, and for anyone building their own DPDK who needs it, it can be set via "-Dc_link_args" meson flag. Anyone using pre-built DPDK from packagers will already have a shared library build anyway. /Bruce