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 BF2D1A0526; Tue, 10 Nov 2020 14:19:31 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 965DE2B93; Tue, 10 Nov 2020 14:19:29 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 05B15F64 for ; Tue, 10 Nov 2020 14:19:27 +0100 (CET) IronPort-SDR: HTNvd6iqRCFsX40RoXudcI2NxKNX6qDmkV7IjsI2wxChVq98Xn8QmXAsOhwbouTT6eCAcOBAnO 6Ov8k3kpcb4g== X-IronPort-AV: E=McAfee;i="6000,8403,9800"; a="149246119" X-IronPort-AV: E=Sophos;i="5.77,466,1596524400"; d="scan'208";a="149246119" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Nov 2020 05:19:24 -0800 IronPort-SDR: E7HBySuoDzLeWW/VS1BSsr8v/zxgCJ+wwU5Ih1A6bjt0wfTIb6IOy0QvN5FID5Y2fFkhh1KBKZ dq1QvRqiPWqQ== X-IronPort-AV: E=Sophos;i="5.77,466,1596524400"; d="scan'208";a="473416989" Received: from bricha3-mobl.ger.corp.intel.com ([10.213.241.186]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 10 Nov 2020 05:19:22 -0800 Date: Tue, 10 Nov 2020 13:19:19 +0000 From: Bruce Richardson To: Thomas Monjalon Cc: dev@dpdk.org, david.marchand@redhat.com Message-ID: <20201110131919.GG1641@bricha3-MOBL.ger.corp.intel.com> References: <20201027173836.891184-1-bruce.richardson@intel.com> <2594616.PNtlxbl9Qa@thomas> <20201110113435.GF1641@bricha3-MOBL.ger.corp.intel.com> <9843499.ZJUyaPxYeA@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9843499.ZJUyaPxYeA@thomas> Subject: Re: [dpdk-dev] [PATCH] devtools/test-meson-builds: allow custom set of examples 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 Tue, Nov 10, 2020 at 02:04:21PM +0100, Thomas Monjalon wrote: > 10/11/2020 12:34, Bruce Richardson: > > On Tue, Nov 10, 2020 at 12:25:13PM +0100, Thomas Monjalon wrote: > > > 10/11/2020 11:08, Bruce Richardson: > > > > On Mon, Nov 09, 2020 at 08:26:10PM +0100, Thomas Monjalon wrote: > > > > > 09/11/2020 19:02, Bruce Richardson: > > > > > > On Mon, Nov 09, 2020 at 06:09:51PM +0100, Thomas Monjalon wrote: > > > > > > > 27/10/2020 18:38, Bruce Richardson: > > > > > > > > To test the installation process of DPDK using "ninja install" > > > > > > > > test-meson-builds.sh builds a subset of the examples using "make". > > > > > > > > To allow more flexibility for people testing, allow the set of > > > > > > > > examples chosen for this make test to be overridden using variable > > > > > > > > "DPDK_BUILD_TEST_EXAMPLES" in the environment. > > > > > > > > > > > > > > > > Since a number of example apps link against drivers directly even > > > > > > > > for shared builds, we need to ensure that LD_LIBRARY_PATH points to > > > > > > > > the main DPDK lib folder so any dependencies of those drivers can > > > > > > > > be found e.g. that the PCI/vdev bus driver .so is found. [All > > > > > > > > drivers are symlinked from drivers dir back to lib dir on install, > > > > > > > > so only one dir rather than two is needed in the path.] > > > > > > > [...] > > > > > > > > +libdir=$(dirname $(find $DESTDIR -name librte_eal.so)) +export > > > > > > > > LD_LIBRARY_PATH=$libdir:$LD_LIBRARY_PATH > > > > > > > > > > > > > > I don't get why libdir is required for some examples, and not for > > > > > > > others? The pkg-config file is not enough? > > > > > > > > > > > > > > > > > > > It's only needed for examples that link against drivers directly. > > > > > > > > > > > > I believe it's needed in those cases, because app linker flags > > > > > > (including e.g. -lrte_pmd_bond) occur before the pkg-config flags, > > > > > > which means that the linker at that point does not have the path to > > > > > > find the dependencies of the driver. [In a normal build, this wouldn't > > > > > > be necessary because the library directory would be a standard path] > > > > > > > > > > If it's just a matter of ordering, it would be a better example to fix > > > > > the ordering in the Makefile, isn't it? > > > > > > > > > > > > > I thought about that, but it seems strange to have the DPDK linker flags > > > > appear before the application specific flags. The general style is to have > > > > the apps own linker flags apply first, and then the list of dependencies, > > > > so that any app linker flags take precedence. The other option is to have > > > > two separate LDFLAG variables for the app, one for before pkg-config flags > > > > and the other afterwards, but that is an untidy solution. > > > > > > > > Therefore, I think it's better to keep the ordering in the examples as-is > > > > and just set the library path in the script. It's only a single extra > > > > assignment that is necessary because we are installing to a non-standard > > > > path using DESTDIR. > > > > > > It is OK to add LD_LIBRARY_PATH in test-meson-builds.sh because > > > DPDK is "installed" with a DESTDIR prefix. > > > > > > But this change is unrelated to test more examples, > > > it is a general fix for examples compilation. > > > The thing I'm missing, as I said above, > > > "why libdir is required for some examples, and not for others?" > > > I feel something wrong made linking work where it should not. > > > > > > > As I explained above, libdir is required for examples that link directly > > against DPDK drivers in shared builds. There are only a few examples that > > do so, which is why the majority worked fine, and why this was not needed > > before. > > Sorry I don't understand how we link other libraries without LD_LIBRARY_PATH. > Where the library path was taken from before this change? > pkg-config outputs the library path via -L flag for all the other libraries, which is why they are picked up. Right now the flags passed are (in the case of the bonding example app): LDFLAGS += -lrte_net_bond ... LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk) ... build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build $(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED) which resolves to (roughly): "-lrte_net_bond -L/path/to/dpdk/libs -lrte_ethdev ..." This means that all the regular DPDK libs are found based off the "-L" flag, but the rte_net_bond and its dependencies are not as they occur before the -L flag. The alternative, as you suggested previously, is to move the -lrte_net_bond to the end of the list, but that strikes me as less elegant as the LDFLAGS for the app need to be split into two. A third choice is to modify the makefiles to use pkg-config "--libs-only-L" flag to put in the relevant pkg-config path flag before the rte_net_bond. (However, I have yet to check that all versions of pkg-config support this flag, since we've been bitten in the past of using flags supported by only some versions on some distros!) LDFLAGS += $(shell $(PKGCONF) --libs-only-L libdpdk) -lrte_net_bond Overall, given we are looking at a test script where we install to a non-standard location, putting that location in the LD_LIBRARY_PATH seems the easiest, sane solution. It also most closely matches real-world use, where if one installs DPDK to a novel location, the path to the library folder does need to be set somewhere for the runtime loader to find the libs. /Bruce