[dpdk-dev,2/2] mk: clean up static link with DPAA libraries

  The bus and mempool dependencies should be declared after the PMD
libraries needing them.

Moreover there is no need to disable the PMDs at the Makefile level,
in case the dependencies are not met.
Such dependencies should be handled at configuration time.

The other side effect of this clean-up is to take into account
the mempool option CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 mk/rte.app.mk | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)
  

Hi Thomas,
On 2/23/2018 4:23 AM, Thomas Monjalon wrote:
> The bus and mempool dependencies should be declared after the PMD
> libraries needing them.
> 
> Moreover there is no need to disable the PMDs at the Makefile level,
> in case the dependencies are not met.
> Such dependencies should be handled at configuration time.
> 
> The other side effect of this clean-up is to take into account
> the mempool option CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>   mk/rte.app.mk | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)

This patch looks good, however I think we (NXP) need to also set some 
dependency check in makefile to avoid dpaaX PMD compilations, if bus is 
not available. I am working on a patch for the same.

Regards,
Hemant

> 
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 3eb41d176..b71a71663 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -127,11 +127,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lrte_pmd_bnx2x -lz
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_BNXT_PMD)       += -lrte_pmd_bnxt
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD)      += -lrte_pmd_cxgbe
> -ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_BUS)       += -lrte_bus_dpaa
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_MEMPOOL)   += -lrte_mempool_dpaa
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_PMD)       += -lrte_pmd_dpaa
> -endif
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_pmd_dpaa2
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)      += -lrte_pmd_e1000
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_ENA_PMD)        += -lrte_pmd_ena
> @@ -204,16 +200,8 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)    += -lrte_pmd_armv8
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)    += -L$(ARMV8_CRYPTO_LIB_PATH) -larmv8_crypto
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_MRVL_CRYPTO) += -L$(LIBMUSDK_PATH)/lib -lrte_pmd_mrvl_crypto -lmusdk
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER) += -lrte_pmd_crypto_scheduler
> -ifeq ($(CONFIG_RTE_LIBRTE_FSLMC_BUS),y)
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC)   += -lrte_pmd_dpaa2_sec
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC)   += -lrte_mempool_dpaa2
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC)   += -lrte_bus_fslmc
> -endif # CONFIG_RTE_LIBRTE_FSLMC_BUS
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC)   += -lrte_bus_dpaa
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC)   += -lrte_pmd_dpaa_sec
> -endif # CONFIG_RTE_LIBRTE_DPAA_BUS
>   
>   endif # CONFIG_RTE_LIBRTE_CRYPTODEV
>   
> @@ -232,10 +220,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_RAWDEV),y)
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_RAWDEV) += -lrte_pmd_skeleton_rawdev
>   endif # CONFIG_RTE_LIBRTE_RAWDEV
>   
> -ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_PMD),y)
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_bus_fslmc
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_mempool_dpaa2
> -endif # CONFIG_RTE_LIBRTE_DPAA2_PMD
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_BUS)       += -lrte_bus_dpaa
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_MEMPOOL)   += -lrte_mempool_dpaa
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS)      += -lrte_bus_fslmc
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL)  += -lrte_mempool_dpaa2
>   
>   endif # !CONFIG_RTE_BUILD_SHARED_LIBS
>   
>
  

23/02/2018 07:25, Hemant Agrawal:
> Hi Thomas,
> On 2/23/2018 4:23 AM, Thomas Monjalon wrote:
> > The bus and mempool dependencies should be declared after the PMD
> > libraries needing them.
> > 
> > Moreover there is no need to disable the PMDs at the Makefile level,
> > in case the dependencies are not met.
> > Such dependencies should be handled at configuration time.
> > 
> > The other side effect of this clean-up is to take into account
> > the mempool option CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >   mk/rte.app.mk | 20 ++++----------------
> >   1 file changed, 4 insertions(+), 16 deletions(-)
> 
> This patch looks good, however I think we (NXP) need to also set some 
> dependency check in makefile to avoid dpaaX PMD compilations, if bus is 
> not available. I am working on a patch for the same.

Why do you think such check is necessary?
If the PMD is enable but not the bus, it is a configuration error.
A good configuration system would resolve it automatically,
but currently we do the configuration manually, so the user must fix
its configuration file.
  

On 2/23/2018 2:55 PM, Thomas Monjalon wrote:
> 23/02/2018 07:25, Hemant Agrawal:
>> Hi Thomas,
>> On 2/23/2018 4:23 AM, Thomas Monjalon wrote:
>>> The bus and mempool dependencies should be declared after the PMD
>>> libraries needing them.
>>>
>>> Moreover there is no need to disable the PMDs at the Makefile level,
>>> in case the dependencies are not met.
>>> Such dependencies should be handled at configuration time.
>>>
>>> The other side effect of this clean-up is to take into account
>>> the mempool option CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL.
>>>
>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>> ---
>>>    mk/rte.app.mk | 20 ++++----------------
>>>    1 file changed, 4 insertions(+), 16 deletions(-)
>>
>> This patch looks good, however I think we (NXP) need to also set some
>> dependency check in makefile to avoid dpaaX PMD compilations, if bus is
>> not available. I am working on a patch for the same.
> 
> Why do you think such check is necessary?
> If the PMD is enable but not the bus, it is a configuration error.
> A good configuration system would resolve it automatically,
> but currently we do the configuration manually, so the user must fix
> its configuration file.
> 
I agree with your comment. That is the ideal approach.

However, just to make it convenient and to support cases like VFIO 
disable, I have added few checks in my v2.