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

[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:
>>>> Adding NXP DPAA2 architecture specific mempool support
>>>> Each mempool instance is represented by a DPBP object
>>>> from the FSL-MC bus.
>>>>
>>>> This patch also registers a dpaa2 type MEMPOOL OPS
>>>>
>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
>>>> ---
>>> <...>
>>>
>>>> diff --git a/drivers/common/Makefile b/drivers/common/Makefile
>>>> index b52931c..0bb75b5 100644
>>>> --- a/drivers/common/Makefile
>>>> +++ b/drivers/common/Makefile
>>>> @@ -35,7 +35,11 @@ ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_PMD),y)
>>>>  CONFIG_RTE_LIBRTE_DPAA2_COMMON = $(CONFIG_RTE_LIBRTE_DPAA2_PMD)
>>>>  endif
>>>>
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_FSLMC_BUS),y)
>>>> +ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_POOL),y)
>>>> +CONFIG_RTE_LIBRTE_DPAA2_COMMON = $(CONFIG_RTE_LIBRTE_DPAA2_POOL)
>>>> +endif
>>>> +
>>>> +ifneq ($(CONFIG_RTE_LIBRTE_FSLMC_BUS),y)
>>>
>>> I guess this is a typo, but this prevents DPAA2_COMMON to be compiled !!
>>
>> It should be 'ifeq' rather than 'ifneq'.
>
>> And it will prevent COMMON
>> compilation only if CONFIG_RTE_LIBRTE_FSLMC_BUS=n which is not the case
>> right now.
>
> It was the case for me for x86 config, but you are right it is not the
> default case for arm.
>
>>
>> We will fix it.
>>
>>>
>>>>  CONFIG_RTE_LIBRTE_DPAA2_COMMON = $(CONFIG_RTE_LIBRTE_FSLMC_BUS)
>>>>  endif
>>>>
>>>
>>> <...>
>>>> +# 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

>>
>>>
>>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_bus_fslmc
>>>> +
>>>> +include $(RTE_SDK)/mk/rte.lib.mk
>>>
>>> <...>
>>>
>>
>
>