[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Olivier MATZ

On 05/13/2015 04:04 PM, Wiles, Keith wrote:
>
>
> On 5/13/15, 8:56 AM, "Olivier MATZ"  wrote:
>
>> Hi Keith,
>>
>> On 05/13/2015 03:17 PM, Wiles, Keith wrote:
>
> endif # ifeq ($(NO_AUTOLIBS),)
>
> -LDLIBS += $(CPU_LDLIBS)
> +LDLIBS += $(_LDLIBS-y) $(EXTRA_LDLIBS)
>

 As discussed in the previous mail, all things that are about
 EXTRA_LDLIBS should be moved in the second patch. Therefore,
 the title of the second patch should not be "update doc...", but
 something like "mk: introduce EXTRA_LDLIBS...".

 By the way, I missed that before, but it seems that your
 patch removes CPU_LDLIBS, I don't think it's correct.
>>>
>>> I found no reference to CPU_LDLIBS in the docs or code other then then
>>> one
>>> line. We now have EXTRA_LDLIBS for the command line, right?
>>
>> Yes, but your patch says "simplify the ifdef". Removing
>> a variable (even if it is not used) in this patch is not
>> a good idea.
>>
>> Now, the CPU_CFLAGS, CPU_LDFLAGS, CPU_LDLIBS can be defined internally
>> by the rte.vars.mk in mk/arch/ or mk/machine/ directories.
>
> No docs for CPU_LDLIBS or reference to that variable, which means it does
> not exist, right?
> If it was used or documented then I would agree. Having magic variables is
> not a good idea. I will add the CPU_LDLIBS in to the line, but someone
> will have to document that variable.

First, this variable is internal to DPDK framework. It is not
documented, because the goal of the documentation is not to
document all internal variables. In one word, it's not a magic
variable at all, it is simply a variable.

Now, the heart of the matter is that your patch silently remove
this line. This is not acceptable and that's why I'm commenting
on it. The goal of patch splitting and proper title is to separate
features, and win time when searching in the git log for the cause
of a problem.

This is the same problem with EXTRA_LDLIBS. How can you justify
having 2 patches:
- "simplify the ifdef" that also adds the EXTRA_LDLIBS
  then
- "update the documentation for EXTRA_LDLIBS"

Regards,
Olivier



>>
>> Regards,
>> Olivier
>>
>



[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Olivier MATZ
Hi Keith,

On 05/13/2015 03:17 PM, Wiles, Keith wrote:
>>>
>>>endif # ifeq ($(NO_AUTOLIBS),)
>>>
>>> -LDLIBS += $(CPU_LDLIBS)
>>> +LDLIBS += $(_LDLIBS-y) $(EXTRA_LDLIBS)
>>>
>>
>> As discussed in the previous mail, all things that are about
>> EXTRA_LDLIBS should be moved in the second patch. Therefore,
>> the title of the second patch should not be "update doc...", but
>> something like "mk: introduce EXTRA_LDLIBS...".
>>
>> By the way, I missed that before, but it seems that your
>> patch removes CPU_LDLIBS, I don't think it's correct.
>
> I found no reference to CPU_LDLIBS in the docs or code other then then one
> line. We now have EXTRA_LDLIBS for the command line, right?

Yes, but your patch says "simplify the ifdef". Removing
a variable (even if it is not used) in this patch is not
a good idea.

Now, the CPU_CFLAGS, CPU_LDFLAGS, CPU_LDLIBS can be defined internally
by the rte.vars.mk in mk/arch/ or mk/machine/ directories.

Regards,
Olivier



[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Wiles, Keith


On 5/13/15, 9:28 AM, "Olivier MATZ"  wrote:

>
>On 05/13/2015 04:04 PM, Wiles, Keith wrote:
>>
>>
>> On 5/13/15, 8:56 AM, "Olivier MATZ"  wrote:
>>
>>> Hi Keith,
>>>
>>> On 05/13/2015 03:17 PM, Wiles, Keith wrote:
>>
>> endif # ifeq ($(NO_AUTOLIBS),)
>>
>> -LDLIBS += $(CPU_LDLIBS)
>> +LDLIBS += $(_LDLIBS-y) $(EXTRA_LDLIBS)
>>
>
> As discussed in the previous mail, all things that are about
> EXTRA_LDLIBS should be moved in the second patch. Therefore,
> the title of the second patch should not be "update doc...", but
> something like "mk: introduce EXTRA_LDLIBS...".
>
> By the way, I missed that before, but it seems that your
> patch removes CPU_LDLIBS, I don't think it's correct.

 I found no reference to CPU_LDLIBS in the docs or code other then then
 one
 line. We now have EXTRA_LDLIBS for the command line, right?
>>>
>>> Yes, but your patch says "simplify the ifdef". Removing
>>> a variable (even if it is not used) in this patch is not
>>> a good idea.
>>>
>>> Now, the CPU_CFLAGS, CPU_LDFLAGS, CPU_LDLIBS can be defined internally
>>> by the rte.vars.mk in mk/arch/ or mk/machine/ directories.
>>
>> No docs for CPU_LDLIBS or reference to that variable, which means it
>>does
>> not exist, right?
>> If it was used or documented then I would agree. Having magic variables
>>is
>> not a good idea. I will add the CPU_LDLIBS in to the line, but someone
>> will have to document that variable.
>
>First, this variable is internal to DPDK framework. It is not

Not referenced in the code any place except the one line and not
referenced in the docs, which means no one used that variable. By
definition this is some magic variable that someone could use only because
he knew about that variable.

As I stated before, I will add that variable back, but it must be
documented, correct?

Look at the code a number of CPU_ flags are commented about in the
code.

machine/power8/rte.vars.mk:39:#   - can define CPU_CFLAGS variable
(overridden by cmdline value) that
machine/power8/rte.vars.mk:41:#   - can define CPU_LDFLAGS variable
(overridden by cmdline value) that
machine/power8/rte.vars.mk:43:#   - can define CPU_ASFLAGS variable
(overridden by cmdline value) that
machine/power8/rte.vars.mk:53:# CPU_CFLAGS =
machine/power8/rte.vars.mk:54:# CPU_LDFLAGS =
machine/power8/rte.vars.mk:55:# CPU_ASFLAGS =
machine/wsm/rte.vars.mk:40:#   - can define CPU_CFLAGS variable (overriden
by cmdline value) that
machine/wsm/rte.vars.mk:42:#   - can define CPU_LDFLAGS variable
(overriden by cmdline value) that
machine/wsm/rte.vars.mk:44:#   - can define CPU_ASFLAGS variable
(overriden by cmdline value) that
machine/wsm/rte.vars.mk:54:# CPU_CFLAGS =
machine/wsm/rte.vars.mk:55:# CPU_LDFLAGS =
machine/wsm/rte.vars.mk:56:# CPU_ASFLAGS =
machine/native/rte.vars.mk:40:#   - can define CPU_CFLAGS variable
(overriden by cmdline value) that
machine/native/rte.vars.mk:42:#   - can define CPU_LDFLAGS variable
(overriden by cmdline value) that
machine/native/rte.vars.mk:44:#   - can define CPU_ASFLAGS variable
(overriden by cmdline value) that
machine/native/rte.vars.mk:54:# CPU_CFLAGS =
machine/native/rte.vars.mk:55:# CPU_LDFLAGS =
machine/native/rte.vars.mk:56:# CPU_ASFLAGS =
machine/native/rte.vars.mk:66:  CPU_SSE42_SUPPORT = $(shell grep SSE4\.2
/var/run/dmesg.boot 2>/dev/null)
machine/native/rte.vars.mk:67:  ifneq ($(CPU_SSE42_SUPPORT),)
machine/atm/rte.vars.mk:40:#   - can define CPU_CFLAGS variable (overriden
by cmdline value) that
machine/atm/rte.vars.mk:42:#   - can define CPU_LDFLAGS variable
(overriden by cmdline value) that
machine/atm/rte.vars.mk:44:#   - can define CPU_ASFLAGS variable
(overriden by cmdline value) that
machine/atm/rte.vars.mk:54:# CPU_CFLAGS =
machine/atm/rte.vars.mk:55:# CPU_LDFLAGS =
machine/atm/rte.vars.mk:56:# CPU_ASFLAGS =
machine/hsw/rte.vars.mk:40:#   - can define CPU_CFLAGS variable (overriden
by cmdline value) that
machine/hsw/rte.vars.mk:42:#   - can define CPU_LDFLAGS variable
(overriden by cmdline value) that
machine/hsw/rte.vars.mk:44:#   - can define CPU_ASFLAGS variable
(overriden by cmdline value) that
machine/hsw/rte.vars.mk:54:# CPU_CFLAGS =
machine/hsw/rte.vars.mk:55:# CPU_LDFLAGS =
machine/hsw/rte.vars.mk:56:# CPU_ASFLAGS =
machine/ivb/rte.vars.mk:40:#   - can define CPU_CFLAGS variable (overriden
by cmdline value) that
machine/ivb/rte.vars.mk:42:#   - can define CPU_LDFLAGS variable
(overriden by cmdline value) that
machine/ivb/rte.vars.mk:44:#   - can define CPU_ASFLAGS variable
(overriden by cmdline value) that
machine/ivb/rte.vars.mk:54:# CPU_CFLAGS =
machine/ivb/rte.vars.mk:55:# CPU_LDFLAGS =
machine/ivb/rte.vars.mk:56:# CPU_ASFLAGS =
machine/snb/rte.vars.mk:40:#   - can define CPU_CFLAGS variable (overriden
by cmdline value) that
machine/snb/rte.vars.mk:42:#   - can define CPU_LDFLAGS variable
(overriden by cmdline value) that
machine/snb/rte.vars.mk:44:#   - can define CPU_ASFLAGS variable
(overriden by cmdline value) 

[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Wiles, Keith


On 5/13/15, 9:28 AM, "Olivier MATZ"  wrote:

>
>On 05/13/2015 04:04 PM, Wiles, Keith wrote:
>>
>>
>> On 5/13/15, 8:56 AM, "Olivier MATZ"  wrote:
>>
>>> Hi Keith,
>>>
>>> On 05/13/2015 03:17 PM, Wiles, Keith wrote:
>>
>> endif # ifeq ($(NO_AUTOLIBS),)
>>
>> -LDLIBS += $(CPU_LDLIBS)
>> +LDLIBS += $(_LDLIBS-y) $(EXTRA_LDLIBS)
>>
>
> As discussed in the previous mail, all things that are about
> EXTRA_LDLIBS should be moved in the second patch. Therefore,
> the title of the second patch should not be "update doc...", but
> something like "mk: introduce EXTRA_LDLIBS...".
>
> By the way, I missed that before, but it seems that your
> patch removes CPU_LDLIBS, I don't think it's correct.

 I found no reference to CPU_LDLIBS in the docs or code other then then
 one
 line. We now have EXTRA_LDLIBS for the command line, right?
>>>
>>> Yes, but your patch says "simplify the ifdef". Removing
>>> a variable (even if it is not used) in this patch is not
>>> a good idea.
>>>
>>> Now, the CPU_CFLAGS, CPU_LDFLAGS, CPU_LDLIBS can be defined internally
>>> by the rte.vars.mk in mk/arch/ or mk/machine/ directories.
>>
>> No docs for CPU_LDLIBS or reference to that variable, which means it
>>does
>> not exist, right?
>> If it was used or documented then I would agree. Having magic variables
>>is
>> not a good idea. I will add the CPU_LDLIBS in to the line, but someone
>> will have to document that variable.
>
>First, this variable is internal to DPDK framework. It is not

Not referenced in the code any place except the one line and not
referenced in the docs, which means no one used that variable. By
definition this is some magic variable that someone could use only because
he knew about that variable.

As I stated before, I will add that variable back, but it must be
documented, correct?

>documented, because the goal of the documentation is not to
>document all internal variables. In one word, it's not a magic
>variable at all, it is simply a variable.
>
>Now, the heart of the matter is that your patch silently remove
>this line. This is not acceptable and that's why I'm commenting
>on it. The goal of patch splitting and proper title is to separate
>features, and win time when searching in the git log for the cause
>of a problem.
>
>This is the same problem with EXTRA_LDLIBS. How can you justify
>having 2 patches:
>- "simplify the ifdef" that also adds the EXTRA_LDLIBS
>  then
>- "update the documentation for EXTRA_LDLIBS"
>
>Regards,
>Olivier
>
>
>
>>>
>>> Regards,
>>> Olivier
>>>
>>
>



[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Wiles, Keith


On 5/13/15, 8:56 AM, "Olivier MATZ"  wrote:

>Hi Keith,
>
>On 05/13/2015 03:17 PM, Wiles, Keith wrote:

endif # ifeq ($(NO_AUTOLIBS),)

 -LDLIBS += $(CPU_LDLIBS)
 +LDLIBS += $(_LDLIBS-y) $(EXTRA_LDLIBS)

>>>
>>> As discussed in the previous mail, all things that are about
>>> EXTRA_LDLIBS should be moved in the second patch. Therefore,
>>> the title of the second patch should not be "update doc...", but
>>> something like "mk: introduce EXTRA_LDLIBS...".
>>>
>>> By the way, I missed that before, but it seems that your
>>> patch removes CPU_LDLIBS, I don't think it's correct.
>>
>> I found no reference to CPU_LDLIBS in the docs or code other then then
>>one
>> line. We now have EXTRA_LDLIBS for the command line, right?
>
>Yes, but your patch says "simplify the ifdef". Removing
>a variable (even if it is not used) in this patch is not
>a good idea.
>
>Now, the CPU_CFLAGS, CPU_LDFLAGS, CPU_LDLIBS can be defined internally
>by the rte.vars.mk in mk/arch/ or mk/machine/ directories.

No docs for CPU_LDLIBS or reference to that variable, which means it does
not exist, right?
If it was used or documented then I would agree. Having magic variables is
not a good idea. I will add the CPU_LDLIBS in to the line, but someone
will have to document that variable.
>
>Regards,
>Olivier
>



[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Wiles, Keith


On 5/13/15, 2:41 AM, "Olivier MATZ"  wrote:

>Hi,
>
>On 05/12/2015 09:11 PM, Keith Wiles wrote:
>> Simplify the ifdefs in rte.app.mk to make the code more
>> readable and maintainable by moving LDLIBS variable to
>> use the same style as LDLIBS-y being used in the rest
>> of the code. The new internal variable _LDLIBS should
>> not be used outside of the rte.app.mk file.
>>
>> Signed-off-by: Keith Wiles 
>> ---
>>   mk/rte.app.mk | 242
>>+++---
>>   1 file changed, 60 insertions(+), 182 deletions(-)
>>
>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>> index 62a76ae..b8030d2 100644
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> @@ -1,7 +1,7 @@
>>   #   BSD LICENSE
>>   #
>> -#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>> -#   Copyright(c) 2014 6WIND S.A.
>> +#   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
>> +#   Copyright(c) 2014-2015 6WIND S.A.
>>   #   All rights reserved.
>>   #
>>   #   Redistribution and use in source and binary forms, with or without
>> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT)
>>   endif
>>
>>   # default path for libs
>> -LDLIBS += -L$(RTE_SDK_BIN)/lib
>> +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib
>>
>>   #
>>   # Include libraries depending on config if NO_AUTOLIBS is not set
>> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib
>>   #
>>   ifeq ($(NO_AUTOLIBS),)
>>
>> -LDLIBS += --whole-archive
>> +_LDLIBS-y += --whole-archive
>>
>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
>> -LDLIBS += -l$(RTE_LIBNAME)
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME)
>>
>>   ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
>> -LDLIBS += -lrte_distributor
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y)
>> -LDLIBS += -lrte_reorder
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y)
>>   ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
>> -LDLIBS += -lrte_kni
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem
>>   endif
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y)
>> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
>> -LDLIBS += -lrte_ivshmem
>> -endif
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE)   += -lrte_pipeline
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE)  += -lrte_table
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT)   += -lrte_port
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)  += -lrte_timer
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH)   += -lrte_hash
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS)   += -lrte_jobstats
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)  += -lrte_power
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_METER)  += -lrte_meter
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y)
>> -LDLIBS += -lrte_pipeline
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrte_sched
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lm
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrt
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y)
>> -LDLIBS += -lrte_table
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y)
>> -LDLIBS += -lrte_port
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y)
>> -LDLIBS += -lrte_timer
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y)
>> -LDLIBS += -lrte_hash
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y)
>> -LDLIBS += -lrte_jobstats
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y)
>> -LDLIBS += -lrte_lpm
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y)
>> -LDLIBS += -lrte_power
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
>> -LDLIBS += -lrte_acl
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_METER),y)
>> -LDLIBS += -lrte_meter
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y)
>> -LDLIBS += -lrte_sched
>> -LDLIBS += -lm
>> -LDLIBS += -lrt
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
>> -LDLIBS += -lrte_vhost
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lrte_vhost
>>
>>   endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>> -LDLIBS += -lpcap
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)   += -lpcap
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn)
>> -LDLIBS += -lfuse
>> +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lfuse
>>   endif
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
>> -LDLIBS += -libverbs
>> -endif
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)   += -libverbs
>>
>> -LDLIBS += --start-group
>> +_LDLIBS-y += --start-group
>>
>>   ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
>> -LDLIBS += -lrte_kvargs
>> -endif
>> -
>> -ifeq 

[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-13 Thread Olivier MATZ
Hi,

On 05/12/2015 09:11 PM, Keith Wiles wrote:
> Simplify the ifdefs in rte.app.mk to make the code more
> readable and maintainable by moving LDLIBS variable to
> use the same style as LDLIBS-y being used in the rest
> of the code. The new internal variable _LDLIBS should
> not be used outside of the rte.app.mk file.
>
> Signed-off-by: Keith Wiles 
> ---
>   mk/rte.app.mk | 242 
> +++---
>   1 file changed, 60 insertions(+), 182 deletions(-)
>
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 62a76ae..b8030d2 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -1,7 +1,7 @@
>   #   BSD LICENSE
>   #
> -#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> -#   Copyright(c) 2014 6WIND S.A.
> +#   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> +#   Copyright(c) 2014-2015 6WIND S.A.
>   #   All rights reserved.
>   #
>   #   Redistribution and use in source and binary forms, with or without
> @@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT)
>   endif
>
>   # default path for libs
> -LDLIBS += -L$(RTE_SDK_BIN)/lib
> +_LDLIBS-y += -L$(RTE_SDK_BIN)/lib
>
>   #
>   # Include libraries depending on config if NO_AUTOLIBS is not set
> @@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib
>   #
>   ifeq ($(NO_AUTOLIBS),)
>
> -LDLIBS += --whole-archive
> +_LDLIBS-y += --whole-archive
>
> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
> -LDLIBS += -l$(RTE_LIBNAME)
> -endif
> +_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME)
>
>   ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>
> -ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
> -LDLIBS += -lrte_distributor
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y)
> -LDLIBS += -lrte_reorder
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder
>
> -ifeq ($(CONFIG_RTE_LIBRTE_KNI),y)
>   ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
> -LDLIBS += -lrte_kni
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem
>   endif
>
> -ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y)
> -ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
> -LDLIBS += -lrte_ivshmem
> -endif
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE)   += -lrte_pipeline
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE)  += -lrte_table
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT)   += -lrte_port
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)  += -lrte_timer
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH)   += -lrte_hash
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS)   += -lrte_jobstats
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)  += -lrte_power
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_METER)  += -lrte_meter
>
> -ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y)
> -LDLIBS += -lrte_pipeline
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrte_sched
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lm
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrt
>
> -ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y)
> -LDLIBS += -lrte_table
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PORT),y)
> -LDLIBS += -lrte_port
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y)
> -LDLIBS += -lrte_timer
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_HASH),y)
> -LDLIBS += -lrte_hash
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y)
> -LDLIBS += -lrte_jobstats
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_LPM),y)
> -LDLIBS += -lrte_lpm
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_POWER),y)
> -LDLIBS += -lrte_power
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
> -LDLIBS += -lrte_acl
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_METER),y)
> -LDLIBS += -lrte_meter
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y)
> -LDLIBS += -lrte_sched
> -LDLIBS += -lm
> -LDLIBS += -lrt
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
> -LDLIBS += -lrte_vhost
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lrte_vhost
>
>   endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> -LDLIBS += -lpcap
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)   += -lpcap
>
> -ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn)
> -LDLIBS += -lfuse
> +ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lfuse
>   endif
>
> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
> -LDLIBS += -libverbs
> -endif
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)   += -libverbs
>
> -LDLIBS += --start-group
> +_LDLIBS-y += --start-group
>
>   ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>
> -ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> -LDLIBS += -lrte_kvargs
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y)
> -LDLIBS += -lrte_mbuf
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
> -LDLIBS += -lrte_ip_frag
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y)
> -LDLIBS += -lethdev
> 

[dpdk-dev] [PATCH v8 1/2] mk:Simplify the ifdefs in rte.app.mk

2015-05-12 Thread Keith Wiles
Simplify the ifdefs in rte.app.mk to make the code more
readable and maintainable by moving LDLIBS variable to
use the same style as LDLIBS-y being used in the rest
of the code. The new internal variable _LDLIBS should
not be used outside of the rte.app.mk file.

Signed-off-by: Keith Wiles 
---
 mk/rte.app.mk | 242 +++---
 1 file changed, 60 insertions(+), 182 deletions(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 62a76ae..b8030d2 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -1,7 +1,7 @@
 #   BSD LICENSE
 #
-#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
-#   Copyright(c) 2014 6WIND S.A.
+#   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+#   Copyright(c) 2014-2015 6WIND S.A.
 #   All rights reserved.
 #
 #   Redistribution and use in source and binary forms, with or without
@@ -51,7 +51,7 @@ LDSCRIPT = $(RTE_LDSCRIPT)
 endif

 # default path for libs
-LDLIBS += -L$(RTE_SDK_BIN)/lib
+_LDLIBS-y += -L$(RTE_SDK_BIN)/lib

 #
 # Include libraries depending on config if NO_AUTOLIBS is not set
@@ -59,215 +59,93 @@ LDLIBS += -L$(RTE_SDK_BIN)/lib
 #
 ifeq ($(NO_AUTOLIBS),)

-LDLIBS += --whole-archive
+_LDLIBS-y += --whole-archive

-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
-LDLIBS += -l$(RTE_LIBNAME)
-endif
+_LDLIBS-$(CONFIG_RTE_BUILD_COMBINE_LIBS)+= -l$(RTE_LIBNAME)

 ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)

-ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
-LDLIBS += -lrte_distributor
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_REORDER),y)
-LDLIBS += -lrte_reorder
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor
+_LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder

-ifeq ($(CONFIG_RTE_LIBRTE_KNI),y)
 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
-LDLIBS += -lrte_kni
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_KNI)+= -lrte_kni
+_LDLIBS-$(CONFIG_RTE_LIBRTE_IVSHMEM)+= -lrte_ivshmem
 endif

-ifeq ($(CONFIG_RTE_LIBRTE_IVSHMEM),y)
-ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
-LDLIBS += -lrte_ivshmem
-endif
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE)   += -lrte_pipeline
+_LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE)  += -lrte_table
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PORT)   += -lrte_port
+_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)  += -lrte_timer
+_LDLIBS-$(CONFIG_RTE_LIBRTE_HASH)   += -lrte_hash
+_LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS)   += -lrte_jobstats
+_LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm
+_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)  += -lrte_power
+_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl
+_LDLIBS-$(CONFIG_RTE_LIBRTE_METER)  += -lrte_meter

-ifeq ($(CONFIG_RTE_LIBRTE_PIPELINE),y)
-LDLIBS += -lrte_pipeline
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrte_sched
+_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lm
+_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrt

-ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y)
-LDLIBS += -lrte_table
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PORT),y)
-LDLIBS += -lrte_port
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_TIMER),y)
-LDLIBS += -lrte_timer
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_HASH),y)
-LDLIBS += -lrte_hash
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_JOBSTATS),y)
-LDLIBS += -lrte_jobstats
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_LPM),y)
-LDLIBS += -lrte_lpm
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_POWER),y)
-LDLIBS += -lrte_power
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
-LDLIBS += -lrte_acl
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_METER),y)
-LDLIBS += -lrte_meter
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y)
-LDLIBS += -lrte_sched
-LDLIBS += -lm
-LDLIBS += -lrt
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
-LDLIBS += -lrte_vhost
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lrte_vhost

 endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS

-ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
-LDLIBS += -lpcap
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)   += -lpcap

-ifeq ($(CONFIG_RTE_LIBRTE_VHOST)$(CONFIG_RTE_LIBRTE_VHOST_USER),yn)
-LDLIBS += -lfuse
+ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lfuse
 endif

-ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
-LDLIBS += -libverbs
-endif
+_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)   += -libverbs

-LDLIBS += --start-group
+_LDLIBS-y += --start-group

 ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)

-ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
-LDLIBS += -lrte_kvargs
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y)
-LDLIBS += -lrte_mbuf
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-LDLIBS += -lrte_ip_frag
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_ETHER),y)
-LDLIBS += -lethdev
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_MALLOC),y)
-LDLIBS += -lrte_malloc
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_MEMPOOL),y)
-LDLIBS += -lrte_mempool
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_RING),y)
-LDLIBS += -lrte_ring
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_EAL),y)
-LDLIBS += -lrte_eal
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_CMDLINE),y)
-LDLIBS += -lrte_cmdline
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y)