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

[Shreyansh Jain]

Hello Neil,

> -----Original Message-----
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Wednesday, January 25, 2017 8:37 PM
> To: Shreyansh Jain <shreyansh.jain at nxp.com>
> Cc: Thomas Monjalon <thomas.monjalon at 6wind.com>; Hemant Agrawal
> <hemant.agrawal at nxp.com>; Ferruh Yigit <ferruh.yigit at intel.com>;
> dev at dpdk.org; bruce.richardson at intel.com; john.mcnamara at intel.com;
> jerin.jacob at caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCHv6 16/33] drivers/pool/dpaa2: adding hw
> offloaded mempool
> 
> On Wed, Jan 25, 2017 at 01:34:47PM +0000, Shreyansh Jain wrote:
> >
> >
> > > -----Original Message-----
> > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > Sent: Wednesday, January 25, 2017 5:53 PM
> > > To: Thomas Monjalon <thomas.monjalon at 6wind.com>
> > > Cc: Hemant Agrawal <hemant.agrawal at nxp.com>; Ferruh Yigit
> > > <ferruh.yigit at intel.com>; Shreyansh Jain <shreyansh.jain at nxp.com>;
> > > dev at dpdk.org; bruce.richardson at intel.com; john.mcnamara at intel.com;
> > > jerin.jacob at 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.
 
I only meant that as of writing the last mail I haven't had yet investigated/
tried that way. That is all.
I am not doubting this method (at least not until I try and reach some
roadblock).

> 
> Neil
> 
> > Merging everything together has already been ruled out in initial RFC
> > Discussions.
> >
> > >
> > > Neil
> > >
> > > >
> >