From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 5CBC2DE5 for ; Wed, 25 Jan 2017 16:07:28 +0100 (CET) Received: from nat-pool-rdu-t.redhat.com ([66.187.233.202] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1cWPAJ-0006mT-69; Wed, 25 Jan 2017 10:07:18 -0500 Date: Wed, 25 Jan 2017 10:07:10 -0500 From: Neil Horman To: Shreyansh Jain Cc: Thomas Monjalon , Hemant Agrawal , Ferruh Yigit , "dev@dpdk.org" , "bruce.richardson@intel.com" , "john.mcnamara@intel.com" , "jerin.jacob@caviumnetworks.com" Message-ID: <20170125150710.GA8488@neilslaptop.think-freely.org> References: <1484832240-2048-1-git-send-email-hemant.agrawal@nxp.com> <228ff5e7-2fa8-7731-681d-e4759bff93cb@nxp.com> <20101825.1zhD9dk20U@xps13> <20170125122324.GA4611@hmswarspite.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCHv6 16/33] drivers/pool/dpaa2: adding hw offloaded mempool 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: , X-List-Received-Date: Wed, 25 Jan 2017 15:07:28 -0000 On Wed, Jan 25, 2017 at 01:34:47PM +0000, Shreyansh Jain wrote: > > > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: Wednesday, January 25, 2017 5:53 PM > > To: Thomas Monjalon > > Cc: Hemant Agrawal ; Ferruh Yigit > > ; Shreyansh Jain ; > > dev@dpdk.org; bruce.richardson@intel.com; john.mcnamara@intel.com; > > jerin.jacob@caviumnetworks.com > > Subject: Re: [dpdk-dev] [PATCHv6 16/33] drivers/pool/dpaa2: adding hw > > offloaded mempool > > > > On Tue, Jan 24, 2017 at 06:28:59PM +0100, Thomas Monjalon wrote: > > > 2017-01-24 20:07, Hemant Agrawal: > > > > On 1/24/2017 4:19 PM, Ferruh Yigit wrote: > > > > > On 1/24/2017 9:12 AM, Shreyansh Jain wrote: > > > > >> On Monday 23 January 2017 11:04 PM, Ferruh Yigit wrote: > > > > >>> On 1/23/2017 11:59 AM, Hemant Agrawal wrote: > > > > >>>> +# library dependencies > > > > >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_eal > > > > >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_mempool > > > > >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += > > lib/librte_common_dpaa2_qbman > > > > >>> > > > > >>> This dependeny doesn not looks correct, there is no folder like that. > > > > >> > > > > >> This is something even I need to understand. From the DEPDIRS what I > > > > >> understood was that though it refers to a directory, it essentially > > > > >> links libraries in build/lib/*. > > > > >> > > > > >> Further, somehow the development is deploying drivers/bus, > > > > >> drivers/common and drivers/pool in lib/* under the name specified as > > > > >> LIB in Makefile. My understanding was that it is expected behavior and > > > > >> not special because of drivers folder. > > > > >> > > > > >> Thus, above line only links lib/librte_common_dpaa2_qbman generated by > > > > >> drivers/common/dpaa2/qbman code. > > > > >> > > > > >> In fact, I think, this might also one of the issues why a parallel > > > > >> shared build fails for DPAA2 PMD (added in Cover letter). > > > > >> The dependency graph cannot create a graph for drivers/common > > > > >> as dependency for drivers/net or drivers/bus and hence parallel build > > > > >> fails because of missing libraries which are being parallely compiled. > > > > > > > > > > DEPDIRS-y is mainly to resolve dependencies for compilation order, and > > > > > should point to the folder, > > > > > > > > > > Following line will cause "librte_eal" to be compiled before driver: > > > > > DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_eal > > > > > > > > > > So "lib/librte_common_dpaa2_qbman" does not makes more sense, since > > > > > there is no folder like that. > > > > > > > > > > > > > > > Somewhere in the history, with following commit, DEPDIRS-y gained a > > side > > > > > effect, it has been used to set dynamic linking dependencies, to fix > > > > > underlinking issue: > > > > > bf5a46fa5972 ("mk: generate internal library dependencies") > > > > > > > > > > I guess you are having that line to benefit from this side effect, but > > > > > this can be done with following more properly: > > > > > LDLIBS += lib/librte_common_dpaa2_qbman > > > > > > > > > > > > > > > To resolve the drivers/net to drivers/common dependency, following line > > > > > in this Makefile should work: > > > > > DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += drivers/common/dpaa2 > > > > > > > > > > This adds following, which will cause "drivers/common" compiled before > > > > > any "drivers/net": > > > > > LOCAL_DEPDIRS-drivers/net += drivers/common > > > > > > > > Thanks for your suggestion. This is one thing, I am not yet able to fix. > > > > > > > > Based on your suggestions: > > > > e.g. > > > > LDLIBS += -lrte_common_dpaa2_qbman > > > > DEPDIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += drivers/common/dpaa2 > > > > > > > > It does add entry in the ".depdirs" > > > > ./arm64-dpaa2-linuxapp-gcc/.depdirs:168:LOCAL_DEPDIRS-drivers/bus += > > > > drivers/common > > > > ./arm64-dpaa2-linuxapp-gcc/.depdirs:170:LOCAL_DEPDIRS-drivers += lib > > > > ./arm64-dpaa2-linuxapp-gcc/.depdirs:172:LOCAL_DEPDIRS-drivers += lib > > > > ./arm64-dpaa2-linuxapp-gcc/.depdirs:174:LOCAL_DEPDIRS-drivers/pool += > > > > drivers/common > > > > > > > > However, we keep on getting: > > > > LD librte_bus_fslmc.so.1.1 > > > > aarch64-linux-gnu-gcc: error: drivers/common/dpaa2: No such file or > > > > directory > > > > make[6]: *** [librte_bus_fslmc.so.1.1] Error 1 > > > > > > Probably because of: > > > > > > # Translate DEPDIRS-y into LDLIBS > > > # Ignore (sub)directory dependencies which do not provide an actual library > > > _IGNORE_DIRS = lib/librte_eal/% lib/librte_compat > > > _DEPDIRS = $(filter-out $(_IGNORE_DIRS),$(DEPDIRS-y)) > > > _LDDIRS = $(subst librte_ether,librte_ethdev,$(_DEPDIRS)) > > > LDLIBS += $(subst lib/lib,-l,$(_LDDIRS)) > > > > > > It shows one important thing: qbman is not a driver, it is a lib. > > > So drivers/common/dpaa2 should be handled differently. > > > > > > Solution 1: tweak mk/rte.lib.mk for directories in drivers/common/ > > > Solution 2: host your bus libs outside of DPDK > > Please do not go with suggestion two, the more libraries get hosted outside > > of > > the project, the less likely any sort of test/build/ongoing maintenence from > > the > > community can be expected. If you're going to go with solution (2), then you > > may as well host the entire PMD outside of the DPDK project, and thats more > > undesireable. > > Agree with you. Hosting a part of PMD (or PMD itself) outside of DPDK is not > a preferred way for me as well. Besides being non-user-friendly, this has > obvious disadvantage of a fragmented software which will quickly become > difficult to manage/maintain. > > But, renaming so many variables also is not an easy choice (assuming > that the suggestion from you for MAP_STATIC_SYMBOL is not in place - still > investigating on that). I'm not sure what you mean by this, MAP_STATIC_SYMBOL is an available macro in dpdk already, and you can ifndef...define...endif it to a no-op so that you can keep the usage outside of dpdk. Neil > Merging everything together has already been ruled out in initial RFC > Discussions. > > > > > Neil > > > > > >