Re: [dpdk-dev] [PATCHv6 16/33] drivers/pool/dpaa2: adding hw offloaded mempool

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/25/2017 3:29 PM, Ferruh Yigit wrote:
> On 1/24/2017 5:28 PM, 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
>>
> 
> For solution 1, following [1] seems working, "drivers/%" preferred
> against "drivers/common/%" because "drivers/bus/%" has same issue. And
> as far as I can see dpaa2 is the only driver dependency in lib folder.
> 
> [1]
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index 33a5f5a..ac4df9a 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -79,7 +79,7 @@ endif
> 
>  # Translate DEPDIRS-y into LDLIBS
>  # Ignore (sub)directory dependencies which do not provide an actual library
> -_IGNORE_DIRS = lib/librte_eal/% lib/librte_compat
> +_IGNORE_DIRS = lib/librte_eal/% lib/librte_compat drivers/%
>  _DEPDIRS = $(filter-out $(_IGNORE_DIRS),$(DEPDIRS-y))
>  _LDDIRS = $(subst librte_ether,librte_ethdev,$(_DEPDIRS))
>  LDLIBS += $(subst lib/lib,-l,$(_LDDIRS))
> 

Hi Hemant,

Overall, shared compilation with -jN is working fine for me with below
patch, putting here as reference:


diff --git a/drivers/bus/fslmc/Makefile b/drivers/bus/fslmc/Makefile
index 263c4fd..405fbaa 100644
--- a/drivers/bus/fslmc/Makefile
+++ b/drivers/bus/fslmc/Makefile
@@ -74,6 +74,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += fslmc_bus.c

 # library dependencies
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += lib/librte_eal
-DEPDIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += lib/librte_common_dpaa2_qbman
+DEPDIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += drivers/common/dpaa2
+
+LDLIBS += -lrte_common_dpaa2_qbman

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/dpaa2/Makefile b/drivers/net/dpaa2/Makefile
index d52fa39..5b1613c 100644
--- a/drivers/net/dpaa2/Makefile
+++ b/drivers/net/dpaa2/Makefile
@@ -65,8 +65,12 @@ SRCS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += dpaa2_ethdev.c
 # library dependencies
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_mempool
lib/librte_mbuf
-DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_common_dpaa2_qbman
-DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_bus_fslmc
-DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += lib/librte_pool_dpaa2
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += drivers/common/dpaa2
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += drivers/bus/fslmc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += drivers/pool/dpaa2
+
+LDLIBS += -lrte_common_dpaa2_qbman
+LDLIBS += -lrte_bus_fslmc
+LDLIBS += -lrte_pool_dpaa2

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/pool/dpaa2/Makefile b/drivers/pool/dpaa2/Makefile
index 69e1bb4..6b27cd5 100644
--- a/drivers/pool/dpaa2/Makefile
+++ b/drivers/pool/dpaa2/Makefile
@@ -65,7 +65,11 @@ SRCS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) +=
dpaa2_hw_mempool.c
 # 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
-DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_bus_fslmc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += drivers/common/dpaa2
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += drivers/bus/fslmc
+
+
+LDLIBS += -lrte_common_dpaa2_qbman
+LDLIBS += -lrte_bus_fslmc

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index 33a5f5a..ac4df9a 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -79,7 +79,7 @@ endif

 # Translate DEPDIRS-y into LDLIBS
 # Ignore (sub)directory dependencies which do not provide an actual library
-_IGNORE_DIRS = lib/librte_eal/% lib/librte_compat
+_IGNORE_DIRS = lib/librte_eal/% lib/librte_compat drivers/%
 _DEPDIRS = $(filter-out $(_IGNORE_DIRS),$(DEPDIRS-y))
 _LDDIRS = $(subst librte_ether,librte_ethdev,$(_DEPDIRS))
 LDLIBS += $(subst lib/lib,-l,$(_LDDIRS))