Re: [PATCHv6 16/33] drivers/pool/dpaa2: adding hw offloaded mempool
From: Jerin Jacob <hidden>
Date: 2017-01-25 13:48:12
On Wed, Jan 25, 2017 at 01:34:47PM +0000, Shreyansh Jain wrote:
quoted
-----Original Message----- From: Neil Horman [mailto:nhorman@tuxdriver.com] Sent: Wednesday, January 25, 2017 5:53 PM To: Thomas Monjalon <redacted> Cc: Hemant Agrawal <redacted>; Ferruh Yigit [off-list ref]; Shreyansh Jain [off-list ref]; 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:quoted
2017-01-24 20:07, Hemant Agrawal:quoted
On 1/24/2017 4:19 PM, Ferruh Yigit wrote:quoted
On 1/24/2017 9:12 AM, Shreyansh Jain wrote:quoted
On Monday 23 January 2017 11:04 PM, Ferruh Yigit wrote:quoted
On 1/23/2017 11:59 AM, Hemant Agrawal wrote:quoted
+# 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_qbmanquoted
quoted
quoted
quoted
quoted
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 asidequoted
quoted
quoted
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/commonThanks 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 1Probably 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 DPDKPlease 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.
+1 If NXP drivers are only the consumers for the common code. Then I think, there is no harm in exposing them as non rte_ namespace.
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). Merging everything together has already been ruled out in initial RFC Discussions.quoted
Neilquoted