[dpdk-dev,v4,1/2] Simplify the ifdefs in rte.app.mk.

Message ID

1430321158-17805-1-git-send-email-keith.wiles@intel.com (mailing list archive)

State

Superseded, archived

Headers

From: Keith Wiles <keith.wiles@intel.com>
To: dev@dpdk.org
Date: Wed, 29 Apr 2015 10:25:57 -0500
Message-Id: <1430321158-17805-1-git-send-email-keith.wiles@intel.com>
Subject: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
Precedence: list
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Commit Message

Wiles, Keith April 29, 2015, 3:25 p.m. UTC

  Trying to 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.

Added a new variable called EXTRA_LDLIBS to be used by example apps
instead of using LDLIBS directly.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 examples/dpdk_qat/Makefile         |   4 +-
 examples/vm_power_manager/Makefile |   2 +-
 mk/rte.app.mk                      | 254 ++++++++++---------------------------
 mk/rte.hostapp.mk                  |   4 +-
 mk/rte.shared.mk                   |  12 +-
 5 files changed, 77 insertions(+), 199 deletions(-)

Comments

Thomas Monjalon April 29, 2015, 5:17 p.m. UTC | #1

2015-04-29 10:25, Keith Wiles:
> -#   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) 2015 6WIND S.A.

A correct update of copyright dates should be
- 2014
+ 2014-2015

Please do not send a new version only for this change.
4 versions of the same patch in 1 day should be enough :)
Let's wait for more feedback on the idea of your patch, especially from
the maintainer (Olivier).

Wiles, Keith April 29, 2015, 5:49 p.m. UTC | #2

On 4/29/15, 12:17 PM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:

>2015-04-29 10:25, Keith Wiles:
>> -#   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) 2015 6WIND S.A.
>
>A correct update of copyright dates should be
>- 2014
>+ 2014-2015

If I have to send a new patch for other stuff I will update the copyright,
if not does that mean I have send a patch at some point or can it be
handled.
>
>Please do not send a new version only for this change.
>4 versions of the same patch in 1 day should be enough :)

Multiple versions in one day I was trying to get the record for the most
updates in one day. :-)
>Let's wait for more feedback on the idea of your patch, especially from
>the maintainer (Olivier).

Olivier Matz April 30, 2015, 9:45 a.m. UTC | #3

Hi Keith,

Thank you for submitting a clean-up. Please see some comments below.

On 04/29/2015 05:25 PM, Keith Wiles wrote:
> Trying to 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.
>
> Added a new variable called EXTRA_LDLIBS to be used by example apps
> instead of using LDLIBS directly.

If I understand well, the goal of this patch is only a cleanup in
rte.app.mk, but at the end, it changes the makefile user "API",
which could probably be a problem for applications using the
dpdk makefile framework.

Why not just having an temporary internal variable (let's say
_LDLIBS-y) that would allow to do the clean-up without modifying
the user interface?

Also, with your patch, the approach for EXTRA_LDLIBS would be
different than CFLAGS or LDFLAGS:
- CFLAGS/LDFLAGS are in Makefiles only
- EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
   to add flags to the default ones

I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
keeping a compatibility with existing application Makefiles.

What do you think?

Regards,
Olivier



>
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>   examples/dpdk_qat/Makefile         |   4 +-
>   examples/vm_power_manager/Makefile |   2 +-
>   mk/rte.app.mk                      | 254 ++++++++++---------------------------
>   mk/rte.hostapp.mk                  |   4 +-
>   mk/rte.shared.mk                   |  12 +-
>   5 files changed, 77 insertions(+), 199 deletions(-)
>
> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile
> index f1e06a1..90ca1d3 100644
> --- a/examples/dpdk_qat/Makefile
> +++ b/examples/dpdk_qat/Makefile
> @@ -77,8 +77,8 @@ else
>   ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a
>   endif
>
> -LDLIBS += -L$(ICP_ROOT)/build
> -LDLIBS += $(ICP_LIBRARY_PATH) \
> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build
> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \
>                   -lz \
>                   -losal \
>                   -ladf_proxy \
> diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile
> index 113dbc4..8fb78d4 100644
> --- a/examples/vm_power_manager/Makefile
> +++ b/examples/vm_power_manager/Makefile
> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c
>   CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
>   CFLAGS += $(WERROR_FLAGS)
>
> -LDLIBS += -lvirt
> +EXTRA_LDLIBS += -lvirt
>
>   # workaround for a gcc bug with noreturn attribute
>   # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 62a76ae..c41de82 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) 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)
> -LDLIBS += -lrte_cfgfile
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
> -LDLIBS += -lrte_pmd_bond
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y)
> -LDLIBS += -lrte_pmd_xenvirt
> -LDLIBS += -lxenstore
> -endif
> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)          += -lrte_kvargs
> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)            += -lrte_mbuf
> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)         += -lrte_ip_frag
> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)           += -lethdev
> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC)          += -lrte_malloc
> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)         += -lrte_mempool
> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING)            += -lrte_ring
> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)             += -lrte_eal
> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)         += -lrte_cmdline
> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)         += -lrte_cfgfile
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)        += -lrte_pmd_bond
> +
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lrte_pmd_xenvirt
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lxenstore
>
>   ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>   # plugins (link only if static libraries)
>
> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y)
> -LDLIBS += -lrte_pmd_vmxnet3_uio
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y)
> -LDLIBS += -lrte_pmd_virtio
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y)
> -LDLIBS += -lrte_pmd_enic
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
> -LDLIBS += -lrte_pmd_i40e
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
> -LDLIBS += -lrte_pmd_fm10k
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
> -LDLIBS += -lrte_pmd_ixgbe
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y)
> -LDLIBS += -lrte_pmd_e1000
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
> -LDLIBS += -lrte_pmd_mlx4
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
> -LDLIBS += -lrte_pmd_ring
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> -LDLIBS += -lrte_pmd_pcap
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
> -LDLIBS += -lrte_pmd_af_packet
> -endif
> -
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y)
> -LDLIBS += -lrte_pmd_null
> -endif
> -
> -endif # plugins
> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)     += -lrte_pmd_vmxnet3_uio
> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD)      += -lrte_pmd_virtio
> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)        += -lrte_pmd_enic
> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)        += -lrte_pmd_i40e
> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)       += -lrte_pmd_fm10k
> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)       += -lrte_pmd_ixgbe
> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)       += -lrte_pmd_e1000
> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)        += -lrte_pmd_mlx4
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)        += -lrte_pmd_ring
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)        += -lrte_pmd_pcap
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)   += -lrte_pmd_af_packet
> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)        += -lrte_pmd_null
> +
> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB)
>
>   endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>
> -LDLIBS += $(EXECENV_LDLIBS)
> -
> -LDLIBS += --end-group
> -
> -LDLIBS += --no-whole-archive
> +LDLIBS-y += $(EXECENV_LDLIBS)
> +LDLIBS-y += --end-group
> +LDLIBS-y += --no-whole-archive
>
>   endif # ifeq ($(NO_AUTOLIBS),)
>
> -LDLIBS += $(CPU_LDLIBS)
> +LDLIBS-y += $(EXTRA_LDLIBS)
>
>   .PHONY: all
>   all: install
> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1)
>   override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>   O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
>   	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call linkerprefix,$(LDFLAGS)) \
> -	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
> +	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y))
>   else
>   O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
> -	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
> +	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y)
>   endif
>   O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>   O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \
>   -include .$(APP).cmd
>
>   # path where libraries are retrieved
> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS)))
> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y)))
> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>
>   # list of .a files that are linked to this application
> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS)))
> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y)))
>
>   # list of found libraries files (useful for deps). If not found, the
>   # library is silently ignored and dep won't be checked
> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
> index c44d0f8..51e8c1d 100644
> --- a/mk/rte.hostapp.mk
> +++ b/mk/rte.hostapp.mk
> @@ -58,7 +58,7 @@ build: _postbuild
>   exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>
>   O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \
> -	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS)
> +	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y)
>   O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>   O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  HOSTLD $(@)")
>   O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \
>   # list of .a files that are linked to this application
>   LDLIBS_FILES := $(wildcard \
>   	$(addprefix $(RTE_OUTPUT)/lib/, \
> -	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
> +	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))))
>
>   #
>   # Compile executable file if needed
> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
> index fc6b0b4..56f20bb 100644
> --- a/mk/rte.shared.mk
> +++ b/mk/rte.shared.mk
> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>   ifeq ($(LINK_USING_CC),1)
>   override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>   O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
> -	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
> +	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y))
>   else
>   O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
> -	-shared -o $@ $(OBJS-y) $(LDLIBS)
> +	-shared -o $@ $(OBJS-y) $(LDLIBS-y)
>   endif
>
>   O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight
> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \
>   -include .$(SHARED).cmd
>
>   # path where libraries are retrieved
> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS)))
> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y)))
> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>
>   # list of .a files that are linked to this application
> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS)))
> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y)))
>
>   # list of found libraries files (useful for deps). If not found, the
>   # library is silently ignored and dep won't be checked
>

Wiles, Keith April 30, 2015, 1:24 p.m. UTC | #4

On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:

>Hi Keith,
>
>Thank you for submitting a clean-up. Please see some comments below.
>
>On 04/29/2015 05:25 PM, Keith Wiles wrote:
>> Trying to 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.
>>
>> Added a new variable called EXTRA_LDLIBS to be used by example apps
>> instead of using LDLIBS directly.
>
>If I understand well, the goal of this patch is only a cleanup in
>rte.app.mk, but at the end, it changes the makefile user "API",
>which could probably be a problem for applications using the
>dpdk makefile framework.
>
>Why not just having an temporary internal variable (let's say
>_LDLIBS-y) that would allow to do the clean-up without modifying
>the user interface?
>
>Also, with your patch, the approach for EXTRA_LDLIBS would be
>different than CFLAGS or LDFLAGS:
>- CFLAGS/LDFLAGS are in Makefiles only
>- EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
>   to add flags to the default ones
>
>I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
>keeping a compatibility with existing application Makefiles.

The docs for DPDK 28.3.6 states they can be used for both command line and
Makefile, so I think I like the current solution unless everyone wants it
as you suggested.

http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.html


>
>What do you think?
>
>Regards,
>Olivier
>
>
>
>>
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>>   examples/dpdk_qat/Makefile         |   4 +-
>>   examples/vm_power_manager/Makefile |   2 +-
>>   mk/rte.app.mk                      | 254
>>++++++++++---------------------------
>>   mk/rte.hostapp.mk                  |   4 +-
>>   mk/rte.shared.mk                   |  12 +-
>>   5 files changed, 77 insertions(+), 199 deletions(-)
>>
>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile
>> index f1e06a1..90ca1d3 100644
>> --- a/examples/dpdk_qat/Makefile
>> +++ b/examples/dpdk_qat/Makefile
>> @@ -77,8 +77,8 @@ else
>>   ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a
>>   endif
>>
>> -LDLIBS += -L$(ICP_ROOT)/build
>> -LDLIBS += $(ICP_LIBRARY_PATH) \
>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build
>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \
>>                   -lz \
>>                   -losal \
>>                   -ladf_proxy \
>> diff --git a/examples/vm_power_manager/Makefile
>>b/examples/vm_power_manager/Makefile
>> index 113dbc4..8fb78d4 100644
>> --- a/examples/vm_power_manager/Makefile
>> +++ b/examples/vm_power_manager/Makefile
>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c
>>   CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
>>   CFLAGS += $(WERROR_FLAGS)
>>
>> -LDLIBS += -lvirt
>> +EXTRA_LDLIBS += -lvirt
>>
>>   # workaround for a gcc bug with noreturn attribute
>>   # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>> index 62a76ae..c41de82 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) 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)
>> -LDLIBS += -lrte_cfgfile
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
>> -LDLIBS += -lrte_pmd_bond
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y)
>> -LDLIBS += -lrte_pmd_xenvirt
>> -LDLIBS += -lxenstore
>> -endif
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)          += -lrte_kvargs
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)            += -lrte_mbuf
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)         += -lrte_ip_frag
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)           += -lethdev
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC)          += -lrte_malloc
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)         += -lrte_mempool
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING)            += -lrte_ring
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)             += -lrte_eal
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)         += -lrte_cmdline
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)         += -lrte_cfgfile
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)        += -lrte_pmd_bond
>> +
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lrte_pmd_xenvirt
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lxenstore
>>
>>   ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>>   # plugins (link only if static libraries)
>>
>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y)
>> -LDLIBS += -lrte_pmd_vmxnet3_uio
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y)
>> -LDLIBS += -lrte_pmd_virtio
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y)
>> -LDLIBS += -lrte_pmd_enic
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
>> -LDLIBS += -lrte_pmd_i40e
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
>> -LDLIBS += -lrte_pmd_fm10k
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
>> -LDLIBS += -lrte_pmd_ixgbe
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y)
>> -LDLIBS += -lrte_pmd_e1000
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
>> -LDLIBS += -lrte_pmd_mlx4
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
>> -LDLIBS += -lrte_pmd_ring
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>> -LDLIBS += -lrte_pmd_pcap
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
>> -LDLIBS += -lrte_pmd_af_packet
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y)
>> -LDLIBS += -lrte_pmd_null
>> -endif
>> -
>> -endif # plugins
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)     += -lrte_pmd_vmxnet3_uio
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD)      += -lrte_pmd_virtio
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)        += -lrte_pmd_enic
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)        += -lrte_pmd_i40e
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)       += -lrte_pmd_fm10k
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)       += -lrte_pmd_ixgbe
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)       += -lrte_pmd_e1000
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)        += -lrte_pmd_mlx4
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)        += -lrte_pmd_ring
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)        += -lrte_pmd_pcap
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)   += -lrte_pmd_af_packet
>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)        += -lrte_pmd_null
>> +
>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB)
>>
>>   endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>>
>> -LDLIBS += $(EXECENV_LDLIBS)
>> -
>> -LDLIBS += --end-group
>> -
>> -LDLIBS += --no-whole-archive
>> +LDLIBS-y += $(EXECENV_LDLIBS)
>> +LDLIBS-y += --end-group
>> +LDLIBS-y += --no-whole-archive
>>
>>   endif # ifeq ($(NO_AUTOLIBS),)
>>
>> -LDLIBS += $(CPU_LDLIBS)
>> +LDLIBS-y += $(EXTRA_LDLIBS)
>>
>>   .PHONY: all
>>   all: install
>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1)
>>   override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>   O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
>>   	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call
>>linkerprefix,$(LDFLAGS)) \
>> -	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
>> +	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y))
>>   else
>>   O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>> -	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
>> +	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y)
>>   endif
>>   O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>   O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \
>>   -include .$(APP).cmd
>>
>>   # path where libraries are retrieved
>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>-Wl$(comma)-L%,$(LDLIBS)))
>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>-Wl$(comma)-L%,$(LDLIBS-y)))
>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>
>>   # list of .a files that are linked to this application
>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>-Wl$(comma)-l%,$(LDLIBS)))
>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>-Wl$(comma)-l%,$(LDLIBS-y)))
>>
>>   # list of found libraries files (useful for deps). If not found, the
>>   # library is silently ignored and dep won't be checked
>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
>> index c44d0f8..51e8c1d 100644
>> --- a/mk/rte.hostapp.mk
>> +++ b/mk/rte.hostapp.mk
>> @@ -58,7 +58,7 @@ build: _postbuild
>>   exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>>
>>   O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \
>> -	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS)
>> +	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y)
>>   O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>   O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  HOSTLD $(@)")
>>   O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \
>>   # list of .a files that are linked to this application
>>   LDLIBS_FILES := $(wildcard \
>>   	$(addprefix $(RTE_OUTPUT)/lib/, \
>> -	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
>> +	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))))
>>
>>   #
>>   # Compile executable file if needed
>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
>> index fc6b0b4..56f20bb 100644
>> --- a/mk/rte.shared.mk
>> +++ b/mk/rte.shared.mk
>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst
>>%,%.cmd,$(1))))
>>   ifeq ($(LINK_USING_CC),1)
>>   override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>   O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@))
>>$(EXTRA_LDFLAGS) \
>> -	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
>> +	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y))
>>   else
>>   O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>> -	-shared -o $@ $(OBJS-y) $(LDLIBS)
>> +	-shared -o $@ $(OBJS-y) $(LDLIBS-y)
>>   endif
>>
>>   O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight
>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \
>>   -include .$(SHARED).cmd
>>
>>   # path where libraries are retrieved
>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>-Wl$(comma)-L%,$(LDLIBS)))
>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>-Wl$(comma)-L%,$(LDLIBS-y)))
>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>
>>   # list of .a files that are linked to this application
>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>-Wl$(comma)-l%,$(LDLIBS)))
>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>-Wl$(comma)-l%,$(LDLIBS-y)))
>>
>>   # list of found libraries files (useful for deps). If not found, the
>>   # library is silently ignored and dep won't be checked
>>
>

Olivier Matz April 30, 2015, 1:38 p.m. UTC | #5

Hi Keith,

On 04/30/2015 03:24 PM, Wiles, Keith wrote:
>
>
> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>
>> Hi Keith,
>>
>> Thank you for submitting a clean-up. Please see some comments below.
>>
>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
>>> Trying to 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.
>>>
>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
>>> instead of using LDLIBS directly.
>>
>> If I understand well, the goal of this patch is only a cleanup in
>> rte.app.mk, but at the end, it changes the makefile user "API",
>> which could probably be a problem for applications using the
>> dpdk makefile framework.
>>
>> Why not just having an temporary internal variable (let's say
>> _LDLIBS-y) that would allow to do the clean-up without modifying
>> the user interface?
>>
>> Also, with your patch, the approach for EXTRA_LDLIBS would be
>> different than CFLAGS or LDFLAGS:
>> - CFLAGS/LDFLAGS are in Makefiles only
>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
>>    to add flags to the default ones
>>
>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
>> keeping a compatibility with existing application Makefiles.
>
> The docs for DPDK 28.3.6 states they can be used for both command line and
> Makefile, so I think I like the current solution unless everyone wants it
> as you suggested.
>
> http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.html

 From the link you have sent:

- About CFLAGS:

"28.3.4. Variables that Can be Set/Overridden in a Makefile Only
[...]
CFLAGS: Flags to use for C compilation. The user should use += to append 
data in this variable."

nothing in 28.3.6


- About EXTRA_CFLAGS:

nothing in 28.3.4

"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile 
or Command Line
[...]
EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when 
compiling."


It seems it's the description I have made above.

Also, you don't answer to the Makefile API issue. From what I
understand, your patch would break external Makefiles using
LDLIBS just for doing an internal clean-up.


Regards,
Olivier


>
>
>>
>> What do you think?
>>
>> Regards,
>> Olivier
>>
>>
>>
>>>
>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>> ---
>>>    examples/dpdk_qat/Makefile         |   4 +-
>>>    examples/vm_power_manager/Makefile |   2 +-
>>>    mk/rte.app.mk                      | 254
>>> ++++++++++---------------------------
>>>    mk/rte.hostapp.mk                  |   4 +-
>>>    mk/rte.shared.mk                   |  12 +-
>>>    5 files changed, 77 insertions(+), 199 deletions(-)
>>>
>>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile
>>> index f1e06a1..90ca1d3 100644
>>> --- a/examples/dpdk_qat/Makefile
>>> +++ b/examples/dpdk_qat/Makefile
>>> @@ -77,8 +77,8 @@ else
>>>    ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a
>>>    endif
>>>
>>> -LDLIBS += -L$(ICP_ROOT)/build
>>> -LDLIBS += $(ICP_LIBRARY_PATH) \
>>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build
>>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \
>>>                    -lz \
>>>                    -losal \
>>>                    -ladf_proxy \
>>> diff --git a/examples/vm_power_manager/Makefile
>>> b/examples/vm_power_manager/Makefile
>>> index 113dbc4..8fb78d4 100644
>>> --- a/examples/vm_power_manager/Makefile
>>> +++ b/examples/vm_power_manager/Makefile
>>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c
>>>    CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
>>>    CFLAGS += $(WERROR_FLAGS)
>>>
>>> -LDLIBS += -lvirt
>>> +EXTRA_LDLIBS += -lvirt
>>>
>>>    # workaround for a gcc bug with noreturn attribute
>>>    # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
>>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>>> index 62a76ae..c41de82 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) 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)
>>> -LDLIBS += -lrte_cfgfile
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
>>> -LDLIBS += -lrte_pmd_bond
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y)
>>> -LDLIBS += -lrte_pmd_xenvirt
>>> -LDLIBS += -lxenstore
>>> -endif
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)          += -lrte_kvargs
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)            += -lrte_mbuf
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)         += -lrte_ip_frag
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)           += -lethdev
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC)          += -lrte_malloc
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)         += -lrte_mempool
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING)            += -lrte_ring
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)             += -lrte_eal
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)         += -lrte_cmdline
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)         += -lrte_cfgfile
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)        += -lrte_pmd_bond
>>> +
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lrte_pmd_xenvirt
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lxenstore
>>>
>>>    ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>>>    # plugins (link only if static libraries)
>>>
>>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y)
>>> -LDLIBS += -lrte_pmd_vmxnet3_uio
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y)
>>> -LDLIBS += -lrte_pmd_virtio
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y)
>>> -LDLIBS += -lrte_pmd_enic
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
>>> -LDLIBS += -lrte_pmd_i40e
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
>>> -LDLIBS += -lrte_pmd_fm10k
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
>>> -LDLIBS += -lrte_pmd_ixgbe
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y)
>>> -LDLIBS += -lrte_pmd_e1000
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
>>> -LDLIBS += -lrte_pmd_mlx4
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
>>> -LDLIBS += -lrte_pmd_ring
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>>> -LDLIBS += -lrte_pmd_pcap
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
>>> -LDLIBS += -lrte_pmd_af_packet
>>> -endif
>>> -
>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y)
>>> -LDLIBS += -lrte_pmd_null
>>> -endif
>>> -
>>> -endif # plugins
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)     += -lrte_pmd_vmxnet3_uio
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD)      += -lrte_pmd_virtio
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)        += -lrte_pmd_enic
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)        += -lrte_pmd_i40e
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)       += -lrte_pmd_fm10k
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)       += -lrte_pmd_ixgbe
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)       += -lrte_pmd_e1000
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)        += -lrte_pmd_mlx4
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)        += -lrte_pmd_ring
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)        += -lrte_pmd_pcap
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)   += -lrte_pmd_af_packet
>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)        += -lrte_pmd_null
>>> +
>>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB)
>>>
>>>    endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>>>
>>> -LDLIBS += $(EXECENV_LDLIBS)
>>> -
>>> -LDLIBS += --end-group
>>> -
>>> -LDLIBS += --no-whole-archive
>>> +LDLIBS-y += $(EXECENV_LDLIBS)
>>> +LDLIBS-y += --end-group
>>> +LDLIBS-y += --no-whole-archive
>>>
>>>    endif # ifeq ($(NO_AUTOLIBS),)
>>>
>>> -LDLIBS += $(CPU_LDLIBS)
>>> +LDLIBS-y += $(EXTRA_LDLIBS)
>>>
>>>    .PHONY: all
>>>    all: install
>>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1)
>>>    override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>>    O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
>>>    	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call
>>> linkerprefix,$(LDFLAGS)) \
>>> -	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
>>> +	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y))
>>>    else
>>>    O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>>> -	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
>>> +	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y)
>>>    endif
>>>    O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>>    O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
>>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \
>>>    -include .$(APP).cmd
>>>
>>>    # path where libraries are retrieved
>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>> -Wl$(comma)-L%,$(LDLIBS)))
>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>> -Wl$(comma)-L%,$(LDLIBS-y)))
>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>>
>>>    # list of .a files that are linked to this application
>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>> -Wl$(comma)-l%,$(LDLIBS)))
>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>> -Wl$(comma)-l%,$(LDLIBS-y)))
>>>
>>>    # list of found libraries files (useful for deps). If not found, the
>>>    # library is silently ignored and dep won't be checked
>>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
>>> index c44d0f8..51e8c1d 100644
>>> --- a/mk/rte.hostapp.mk
>>> +++ b/mk/rte.hostapp.mk
>>> @@ -58,7 +58,7 @@ build: _postbuild
>>>    exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>>>
>>>    O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \
>>> -	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS)
>>> +	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y)
>>>    O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>>    O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  HOSTLD $(@)")
>>>    O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
>>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \
>>>    # list of .a files that are linked to this application
>>>    LDLIBS_FILES := $(wildcard \
>>>    	$(addprefix $(RTE_OUTPUT)/lib/, \
>>> -	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
>>> +	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))))
>>>
>>>    #
>>>    # Compile executable file if needed
>>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
>>> index fc6b0b4..56f20bb 100644
>>> --- a/mk/rte.shared.mk
>>> +++ b/mk/rte.shared.mk
>>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst
>>> %,%.cmd,$(1))))
>>>    ifeq ($(LINK_USING_CC),1)
>>>    override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>>    O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@))
>>> $(EXTRA_LDFLAGS) \
>>> -	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
>>> +	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y))
>>>    else
>>>    O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>>> -	-shared -o $@ $(OBJS-y) $(LDLIBS)
>>> +	-shared -o $@ $(OBJS-y) $(LDLIBS-y)
>>>    endif
>>>
>>>    O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight
>>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \
>>>    -include .$(SHARED).cmd
>>>
>>>    # path where libraries are retrieved
>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>> -Wl$(comma)-L%,$(LDLIBS)))
>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>> -Wl$(comma)-L%,$(LDLIBS-y)))
>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>>
>>>    # list of .a files that are linked to this application
>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>> -Wl$(comma)-l%,$(LDLIBS)))
>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>> -Wl$(comma)-l%,$(LDLIBS-y)))
>>>
>>>    # list of found libraries files (useful for deps). If not found, the
>>>    # library is silently ignored and dep won't be checked
>>>
>>
>

Wiles, Keith April 30, 2015, 2:31 p.m. UTC | #6

On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:

>Hi Keith,
>
>On 04/30/2015 03:24 PM, Wiles, Keith wrote:
>>
>>
>> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>>
>>> Hi Keith,
>>>
>>> Thank you for submitting a clean-up. Please see some comments below.
>>>
>>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
>>>> Trying to 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.
>>>>
>>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
>>>> instead of using LDLIBS directly.
>>>
>>> If I understand well, the goal of this patch is only a cleanup in
>>> rte.app.mk, but at the end, it changes the makefile user "API",
>>> which could probably be a problem for applications using the
>>> dpdk makefile framework.
>>>
>>> Why not just having an temporary internal variable (let's say
>>> _LDLIBS-y) that would allow to do the clean-up without modifying
>>> the user interface?
>>>
>>> Also, with your patch, the approach for EXTRA_LDLIBS would be
>>> different than CFLAGS or LDFLAGS:
>>> - CFLAGS/LDFLAGS are in Makefiles only
>>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
>>>    to add flags to the default ones
>>>
>>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
>>> keeping a compatibility with existing application Makefiles.
>>
>> The docs for DPDK 28.3.6 states they can be used for both command line
>>and
>> Makefile, so I think I like the current solution unless everyone wants
>>it
>> as you suggested.
>>
>> 
>>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.htm
>>l
>
> From the link you have sent:
>
>- About CFLAGS:
>
>"28.3.4. Variables that Can be Set/Overridden in a Makefile Only
>[...]
>CFLAGS: Flags to use for C compilation. The user should use += to append
>data in this variable."
>
>nothing in 28.3.6
>
>
>- About EXTRA_CFLAGS:
>
>nothing in 28.3.4
>
>"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile
>or Command Line
>[...]
>EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when
>compiling."

The point was that EXTRA_XXX can be used for command line and Makefile as
it was pointed out in a previous email the assumption was EXTRA_XXX was
only for the command line. (Just to make sure we understood EXTRA_XXX was
not just for command line options.) This was the reason I sent the link an
to point out using EXTRA_XXX is a much cleaner method then allowing
someone to modify what I believe is an internal variable.
>
>
>It seems it's the description I have made above.
>
>Also, you don't answer to the Makefile API issue. From what I
>understand, your patch would break external Makefiles using
>LDLIBS just for doing an internal clean-up.

I believe using these variables CFLAGS, LDLIBS, Š Are really internal
variables and EXTRA_XXX should have been used instead. I would like to fix
that issue and I do not feel the impact is that big as it is a simple
search replace problem. But it looks like we want to keep the old way just
for a very minor change IMHO.

I will send out a new patch using _LDLIBS-y as the internal variable and
then set LDLIBS at the correct location towards the end of the file.

Does that work for you?

Regards,
++Keith
>
>
>Regards,
>Olivier
>
>
>>
>>
>>>
>>> What do you think?
>>>
>>> Regards,
>>> Olivier
>>>
>>>
>>>
>>>>
>>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>>> ---
>>>>    examples/dpdk_qat/Makefile         |   4 +-
>>>>    examples/vm_power_manager/Makefile |   2 +-
>>>>    mk/rte.app.mk                      | 254
>>>> ++++++++++---------------------------
>>>>    mk/rte.hostapp.mk                  |   4 +-
>>>>    mk/rte.shared.mk                   |  12 +-
>>>>    5 files changed, 77 insertions(+), 199 deletions(-)
>>>>
>>>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile
>>>> index f1e06a1..90ca1d3 100644
>>>> --- a/examples/dpdk_qat/Makefile
>>>> +++ b/examples/dpdk_qat/Makefile
>>>> @@ -77,8 +77,8 @@ else
>>>>    ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a
>>>>    endif
>>>>
>>>> -LDLIBS += -L$(ICP_ROOT)/build
>>>> -LDLIBS += $(ICP_LIBRARY_PATH) \
>>>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build
>>>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \
>>>>                    -lz \
>>>>                    -losal \
>>>>                    -ladf_proxy \
>>>> diff --git a/examples/vm_power_manager/Makefile
>>>> b/examples/vm_power_manager/Makefile
>>>> index 113dbc4..8fb78d4 100644
>>>> --- a/examples/vm_power_manager/Makefile
>>>> +++ b/examples/vm_power_manager/Makefile
>>>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c
>>>>    CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
>>>>    CFLAGS += $(WERROR_FLAGS)
>>>>
>>>> -LDLIBS += -lvirt
>>>> +EXTRA_LDLIBS += -lvirt
>>>>
>>>>    # workaround for a gcc bug with noreturn attribute
>>>>    # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
>>>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>>>> index 62a76ae..c41de82 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) 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)
>>>> -LDLIBS += -lrte_cfgfile
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
>>>> -LDLIBS += -lrte_pmd_bond
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y)
>>>> -LDLIBS += -lrte_pmd_xenvirt
>>>> -LDLIBS += -lxenstore
>>>> -endif
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)          += -lrte_kvargs
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)            += -lrte_mbuf
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)         += -lrte_ip_frag
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)           += -lethdev
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC)          += -lrte_malloc
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)         += -lrte_mempool
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING)            += -lrte_ring
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)             += -lrte_eal
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)         += -lrte_cmdline
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)         += -lrte_cfgfile
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)        += -lrte_pmd_bond
>>>> +
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lrte_pmd_xenvirt
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lxenstore
>>>>
>>>>    ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>>>>    # plugins (link only if static libraries)
>>>>
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y)
>>>> -LDLIBS += -lrte_pmd_vmxnet3_uio
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y)
>>>> -LDLIBS += -lrte_pmd_virtio
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y)
>>>> -LDLIBS += -lrte_pmd_enic
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
>>>> -LDLIBS += -lrte_pmd_i40e
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
>>>> -LDLIBS += -lrte_pmd_fm10k
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
>>>> -LDLIBS += -lrte_pmd_ixgbe
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y)
>>>> -LDLIBS += -lrte_pmd_e1000
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
>>>> -LDLIBS += -lrte_pmd_mlx4
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
>>>> -LDLIBS += -lrte_pmd_ring
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>>>> -LDLIBS += -lrte_pmd_pcap
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
>>>> -LDLIBS += -lrte_pmd_af_packet
>>>> -endif
>>>> -
>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y)
>>>> -LDLIBS += -lrte_pmd_null
>>>> -endif
>>>> -
>>>> -endif # plugins
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)     += -lrte_pmd_vmxnet3_uio
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD)      += -lrte_pmd_virtio
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)        += -lrte_pmd_enic
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)        += -lrte_pmd_i40e
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)       += -lrte_pmd_fm10k
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)       += -lrte_pmd_ixgbe
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)       += -lrte_pmd_e1000
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)        += -lrte_pmd_mlx4
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)        += -lrte_pmd_ring
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)        += -lrte_pmd_pcap
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)   += -lrte_pmd_af_packet
>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)        += -lrte_pmd_null
>>>> +
>>>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB)
>>>>
>>>>    endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>>>>
>>>> -LDLIBS += $(EXECENV_LDLIBS)
>>>> -
>>>> -LDLIBS += --end-group
>>>> -
>>>> -LDLIBS += --no-whole-archive
>>>> +LDLIBS-y += $(EXECENV_LDLIBS)
>>>> +LDLIBS-y += --end-group
>>>> +LDLIBS-y += --no-whole-archive
>>>>
>>>>    endif # ifeq ($(NO_AUTOLIBS),)
>>>>
>>>> -LDLIBS += $(CPU_LDLIBS)
>>>> +LDLIBS-y += $(EXTRA_LDLIBS)
>>>>
>>>>    .PHONY: all
>>>>    all: install
>>>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1)
>>>>    override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>>>    O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
>>>>    	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call
>>>> linkerprefix,$(LDFLAGS)) \
>>>> -	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
>>>> +	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y))
>>>>    else
>>>>    O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>>>> -	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
>>>> +	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y)
>>>>    endif
>>>>    O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>>>    O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
>>>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \
>>>>    -include .$(APP).cmd
>>>>
>>>>    # path where libraries are retrieved
>>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>> -Wl$(comma)-L%,$(LDLIBS)))
>>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>> -Wl$(comma)-L%,$(LDLIBS-y)))
>>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>>>
>>>>    # list of .a files that are linked to this application
>>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>> -Wl$(comma)-l%,$(LDLIBS)))
>>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>> -Wl$(comma)-l%,$(LDLIBS-y)))
>>>>
>>>>    # list of found libraries files (useful for deps). If not found,
>>>>the
>>>>    # library is silently ignored and dep won't be checked
>>>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
>>>> index c44d0f8..51e8c1d 100644
>>>> --- a/mk/rte.hostapp.mk
>>>> +++ b/mk/rte.hostapp.mk
>>>> @@ -58,7 +58,7 @@ build: _postbuild
>>>>    exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>>>>
>>>>    O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \
>>>> -	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS)
>>>> +	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y)
>>>>    O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>>>    O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  HOSTLD $(@)")
>>>>    O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
>>>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \
>>>>    # list of .a files that are linked to this application
>>>>    LDLIBS_FILES := $(wildcard \
>>>>    	$(addprefix $(RTE_OUTPUT)/lib/, \
>>>> -	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
>>>> +	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))))
>>>>
>>>>    #
>>>>    # Compile executable file if needed
>>>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
>>>> index fc6b0b4..56f20bb 100644
>>>> --- a/mk/rte.shared.mk
>>>> +++ b/mk/rte.shared.mk
>>>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst
>>>> %,%.cmd,$(1))))
>>>>    ifeq ($(LINK_USING_CC),1)
>>>>    override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>>>    O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@))
>>>> $(EXTRA_LDFLAGS) \
>>>> -	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
>>>> +	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y))
>>>>    else
>>>>    O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>>>> -	-shared -o $@ $(OBJS-y) $(LDLIBS)
>>>> +	-shared -o $@ $(OBJS-y) $(LDLIBS-y)
>>>>    endif
>>>>
>>>>    O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight
>>>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \
>>>>    -include .$(SHARED).cmd
>>>>
>>>>    # path where libraries are retrieved
>>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>> -Wl$(comma)-L%,$(LDLIBS)))
>>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>> -Wl$(comma)-L%,$(LDLIBS-y)))
>>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>>>
>>>>    # list of .a files that are linked to this application
>>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>> -Wl$(comma)-l%,$(LDLIBS)))
>>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>> -Wl$(comma)-l%,$(LDLIBS-y)))
>>>>
>>>>    # list of found libraries files (useful for deps). If not found,
>>>>the
>>>>    # library is silently ignored and dep won't be checked
>>>>
>>>
>>
>

Olivier Matz April 30, 2015, 3:56 p.m. UTC | #7

Hi,

On 04/30/2015 04:31 PM, Wiles, Keith wrote:
>
>
> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>
>> Hi Keith,
>>
>> On 04/30/2015 03:24 PM, Wiles, Keith wrote:
>>>
>>>
>>> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>>>
>>>> Hi Keith,
>>>>
>>>> Thank you for submitting a clean-up. Please see some comments below.
>>>>
>>>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
>>>>> Trying to 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.
>>>>>
>>>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
>>>>> instead of using LDLIBS directly.
>>>>
>>>> If I understand well, the goal of this patch is only a cleanup in
>>>> rte.app.mk, but at the end, it changes the makefile user "API",
>>>> which could probably be a problem for applications using the
>>>> dpdk makefile framework.
>>>>
>>>> Why not just having an temporary internal variable (let's say
>>>> _LDLIBS-y) that would allow to do the clean-up without modifying
>>>> the user interface?
>>>>
>>>> Also, with your patch, the approach for EXTRA_LDLIBS would be
>>>> different than CFLAGS or LDFLAGS:
>>>> - CFLAGS/LDFLAGS are in Makefiles only
>>>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
>>>>     to add flags to the default ones
>>>>
>>>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
>>>> keeping a compatibility with existing application Makefiles.
>>>
>>> The docs for DPDK 28.3.6 states they can be used for both command line
>>> and
>>> Makefile, so I think I like the current solution unless everyone wants
>>> it
>>> as you suggested.
>>>
>>>
>>> http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.htm
>>> l
>>
>>  From the link you have sent:
>>
>> - About CFLAGS:
>>
>> "28.3.4. Variables that Can be Set/Overridden in a Makefile Only
>> [...]
>> CFLAGS: Flags to use for C compilation. The user should use += to append
>> data in this variable."
>>
>> nothing in 28.3.6
>>
>>
>> - About EXTRA_CFLAGS:
>>
>> nothing in 28.3.4
>>
>> "28.3.6. Variables that Can be Set/Overridden by the User in a Makefile
>> or Command Line
>> [...]
>> EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when
>> compiling."
>
> The point was that EXTRA_XXX can be used for command line and Makefile as
> it was pointed out in a previous email the assumption was EXTRA_XXX was
> only for the command line. (Just to make sure we understood EXTRA_XXX was
> not just for command line options.) This was the reason I sent the link an
> to point out using EXTRA_XXX is a much cleaner method then allowing
> someone to modify what I believe is an internal variable.

Although it is allowed, using EXTRA_xxx in a Makefile does not look to
be a good idea in my opinion.

For instance, EXTRA_CFLAGS can be used to append something to the
CFLAGS variable. If we also use EXTRA_CFLAGS in the Makefile,
specifying it on the command line would override its value.

So, CFLAGS and LDLIBS are not internal variables. The DPDK build
framework is designed to make use of these variables provided by
the application Makefile.


>> It seems it's the description I have made above.
>>
>> Also, you don't answer to the Makefile API issue. From what I
>> understand, your patch would break external Makefiles using
>> LDLIBS just for doing an internal clean-up.
>
> I believe using these variables CFLAGS, LDLIBS, Š Are really internal
> variables and EXTRA_XXX should have been used instead. I would like to fix
> that issue and I do not feel the impact is that big as it is a simple
> search replace problem. But it looks like we want to keep the old way just
> for a very minor change IMHO.
>
> I will send out a new patch using _LDLIBS-y as the internal variable and
> then set LDLIBS at the correct location towards the end of the file.
>
> Does that work for you?

Yes, I think it's better. Not breaking the API for external Makefile
is important fir users.

Regards,
Olivier



>
> Regards,
> ++Keith
>>
>>
>> Regards,
>> Olivier
>>
>>
>>>
>>>
>>>>
>>>> What do you think?
>>>>
>>>> Regards,
>>>> Olivier
>>>>
>>>>
>>>>
>>>>>
>>>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>>>> ---
>>>>>     examples/dpdk_qat/Makefile         |   4 +-
>>>>>     examples/vm_power_manager/Makefile |   2 +-
>>>>>     mk/rte.app.mk                      | 254
>>>>> ++++++++++---------------------------
>>>>>     mk/rte.hostapp.mk                  |   4 +-
>>>>>     mk/rte.shared.mk                   |  12 +-
>>>>>     5 files changed, 77 insertions(+), 199 deletions(-)
>>>>>
>>>>> diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile
>>>>> index f1e06a1..90ca1d3 100644
>>>>> --- a/examples/dpdk_qat/Makefile
>>>>> +++ b/examples/dpdk_qat/Makefile
>>>>> @@ -77,8 +77,8 @@ else
>>>>>     ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a
>>>>>     endif
>>>>>
>>>>> -LDLIBS += -L$(ICP_ROOT)/build
>>>>> -LDLIBS += $(ICP_LIBRARY_PATH) \
>>>>> +EXTRA_LDLIBS += -L$(ICP_ROOT)/build
>>>>> +EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \
>>>>>                     -lz \
>>>>>                     -losal \
>>>>>                     -ladf_proxy \
>>>>> diff --git a/examples/vm_power_manager/Makefile
>>>>> b/examples/vm_power_manager/Makefile
>>>>> index 113dbc4..8fb78d4 100644
>>>>> --- a/examples/vm_power_manager/Makefile
>>>>> +++ b/examples/vm_power_manager/Makefile
>>>>> @@ -48,7 +48,7 @@ SRCS-y += channel_monitor.c
>>>>>     CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
>>>>>     CFLAGS += $(WERROR_FLAGS)
>>>>>
>>>>> -LDLIBS += -lvirt
>>>>> +EXTRA_LDLIBS += -lvirt
>>>>>
>>>>>     # workaround for a gcc bug with noreturn attribute
>>>>>     # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
>>>>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>>>>> index 62a76ae..c41de82 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) 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)
>>>>> -LDLIBS += -lrte_cfgfile
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
>>>>> -LDLIBS += -lrte_pmd_bond
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y)
>>>>> -LDLIBS += -lrte_pmd_xenvirt
>>>>> -LDLIBS += -lxenstore
>>>>> -endif
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)          += -lrte_kvargs
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)            += -lrte_mbuf
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)         += -lrte_ip_frag
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)           += -lethdev
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC)          += -lrte_malloc
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)         += -lrte_mempool
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_RING)            += -lrte_ring
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)             += -lrte_eal
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)         += -lrte_cmdline
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)         += -lrte_cfgfile
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)        += -lrte_pmd_bond
>>>>> +
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lrte_pmd_xenvirt
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lxenstore
>>>>>
>>>>>     ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>>>>>     # plugins (link only if static libraries)
>>>>>
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_vmxnet3_uio
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_virtio
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_enic
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_i40e
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_fm10k
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_ixgbe
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_e1000
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
>>>>> -LDLIBS += -lrte_pmd_mlx4
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
>>>>> -LDLIBS += -lrte_pmd_ring
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>>>>> -LDLIBS += -lrte_pmd_pcap
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
>>>>> -LDLIBS += -lrte_pmd_af_packet
>>>>> -endif
>>>>> -
>>>>> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y)
>>>>> -LDLIBS += -lrte_pmd_null
>>>>> -endif
>>>>> -
>>>>> -endif # plugins
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)     += -lrte_pmd_vmxnet3_uio
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD)      += -lrte_pmd_virtio
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)        += -lrte_pmd_enic
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)        += -lrte_pmd_i40e
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)       += -lrte_pmd_fm10k
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)       += -lrte_pmd_ixgbe
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)       += -lrte_pmd_e1000
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)        += -lrte_pmd_mlx4
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)        += -lrte_pmd_ring
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)        += -lrte_pmd_pcap
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)   += -lrte_pmd_af_packet
>>>>> +LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)        += -lrte_pmd_null
>>>>> +
>>>>> +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB)
>>>>>
>>>>>     endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>>>>>
>>>>> -LDLIBS += $(EXECENV_LDLIBS)
>>>>> -
>>>>> -LDLIBS += --end-group
>>>>> -
>>>>> -LDLIBS += --no-whole-archive
>>>>> +LDLIBS-y += $(EXECENV_LDLIBS)
>>>>> +LDLIBS-y += --end-group
>>>>> +LDLIBS-y += --no-whole-archive
>>>>>
>>>>>     endif # ifeq ($(NO_AUTOLIBS),)
>>>>>
>>>>> -LDLIBS += $(CPU_LDLIBS)
>>>>> +LDLIBS-y += $(EXTRA_LDLIBS)
>>>>>
>>>>>     .PHONY: all
>>>>>     all: install
>>>>> @@ -286,10 +164,10 @@ ifeq ($(LINK_USING_CC),1)
>>>>>     override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>>>>     O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
>>>>>     	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call
>>>>> linkerprefix,$(LDFLAGS)) \
>>>>> -	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
>>>>> +	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y))
>>>>>     else
>>>>>     O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>>>>> -	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
>>>>> +	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y)
>>>>>     endif
>>>>>     O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>>>>     O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
>>>>> @@ -302,12 +180,12 @@ O_TO_EXE_DO = @set -e; \
>>>>>     -include .$(APP).cmd
>>>>>
>>>>>     # path where libraries are retrieved
>>>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>>> -Wl$(comma)-L%,$(LDLIBS)))
>>>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>>>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>>> -Wl$(comma)-L%,$(LDLIBS-y)))
>>>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>>>>
>>>>>     # list of .a files that are linked to this application
>>>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>>>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>>> -Wl$(comma)-l%,$(LDLIBS)))
>>>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>>>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>>> -Wl$(comma)-l%,$(LDLIBS-y)))
>>>>>
>>>>>     # list of found libraries files (useful for deps). If not found,
>>>>> the
>>>>>     # library is silently ignored and dep won't be checked
>>>>> diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
>>>>> index c44d0f8..51e8c1d 100644
>>>>> --- a/mk/rte.hostapp.mk
>>>>> +++ b/mk/rte.hostapp.mk
>>>>> @@ -58,7 +58,7 @@ build: _postbuild
>>>>>     exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>>>>>
>>>>>     O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \
>>>>> -	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS)
>>>>> +	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y)
>>>>>     O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
>>>>>     O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  HOSTLD $(@)")
>>>>>     O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
>>>>> @@ -72,7 +72,7 @@ O_TO_EXE_DO = @set -e; \
>>>>>     # list of .a files that are linked to this application
>>>>>     LDLIBS_FILES := $(wildcard \
>>>>>     	$(addprefix $(RTE_OUTPUT)/lib/, \
>>>>> -	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
>>>>> +	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))))
>>>>>
>>>>>     #
>>>>>     # Compile executable file if needed
>>>>> diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
>>>>> index fc6b0b4..56f20bb 100644
>>>>> --- a/mk/rte.shared.mk
>>>>> +++ b/mk/rte.shared.mk
>>>>> @@ -60,10 +60,10 @@ exe2cmd = $(strip $(call dotfile,$(patsubst
>>>>> %,%.cmd,$(1))))
>>>>>     ifeq ($(LINK_USING_CC),1)
>>>>>     override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
>>>>>     O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@))
>>>>> $(EXTRA_LDFLAGS) \
>>>>> -	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
>>>>> +	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y))
>>>>>     else
>>>>>     O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
>>>>> -	-shared -o $@ $(OBJS-y) $(LDLIBS)
>>>>> +	-shared -o $@ $(OBJS-y) $(LDLIBS-y)
>>>>>     endif
>>>>>
>>>>>     O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight
>>>>> @@ -77,12 +77,12 @@ O_TO_SO_DO = @set -e; \
>>>>>     -include .$(SHARED).cmd
>>>>>
>>>>>     # path where libraries are retrieved
>>>>> -LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>>> -Wl$(comma)-L%,$(LDLIBS)))
>>>>> -LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
>>>>> +LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter
>>>>> -Wl$(comma)-L%,$(LDLIBS-y)))
>>>>> +LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
>>>>>
>>>>>     # list of .a files that are linked to this application
>>>>> -LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
>>>>> -LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>>> -Wl$(comma)-l%,$(LDLIBS)))
>>>>> +LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
>>>>> +LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter
>>>>> -Wl$(comma)-l%,$(LDLIBS-y)))
>>>>>
>>>>>     # list of found libraries files (useful for deps). If not found,
>>>>> the
>>>>>     # library is silently ignored and dep won't be checked
>>>>>
>>>>
>>>
>>
>

Bruce Richardson April 30, 2015, 4:22 p.m. UTC | #8

On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote:
> 
> 
> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
> 
> >Hi Keith,
> >
> >On 04/30/2015 03:24 PM, Wiles, Keith wrote:
> >>
> >>
> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
> >>
> >>> Hi Keith,
> >>>
> >>> Thank you for submitting a clean-up. Please see some comments below.
> >>>
> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
> >>>> Trying to 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.
> >>>>
> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
> >>>> instead of using LDLIBS directly.
> >>>
> >>> If I understand well, the goal of this patch is only a cleanup in
> >>> rte.app.mk, but at the end, it changes the makefile user "API",
> >>> which could probably be a problem for applications using the
> >>> dpdk makefile framework.
> >>>
> >>> Why not just having an temporary internal variable (let's say
> >>> _LDLIBS-y) that would allow to do the clean-up without modifying
> >>> the user interface?
> >>>
> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be
> >>> different than CFLAGS or LDFLAGS:
> >>> - CFLAGS/LDFLAGS are in Makefiles only
> >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
> >>>    to add flags to the default ones
> >>>
> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
> >>> keeping a compatibility with existing application Makefiles.
> >>
> >> The docs for DPDK 28.3.6 states they can be used for both command line
> >>and
> >> Makefile, so I think I like the current solution unless everyone wants
> >>it
> >> as you suggested.
> >>
> >> 
> >>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.htm
> >>l
> >
> > From the link you have sent:
> >
> >- About CFLAGS:
> >
> >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only
> >[...]
> >CFLAGS: Flags to use for C compilation. The user should use += to append
> >data in this variable."
> >
> >nothing in 28.3.6
> >
> >
> >- About EXTRA_CFLAGS:
> >
> >nothing in 28.3.4
> >
> >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile
> >or Command Line
> >[...]
> >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS when
> >compiling."
> 
> The point was that EXTRA_XXX can be used for command line and Makefile as
> it was pointed out in a previous email the assumption was EXTRA_XXX was
> only for the command line. (Just to make sure we understood EXTRA_XXX was
> not just for command line options.) This was the reason I sent the link an
> to point out using EXTRA_XXX is a much cleaner method then allowing
> someone to modify what I believe is an internal variable.

Just beware that setting EXTRA_* flags on the commandline can override their
values in the makefiles, and cause unexpected compilation problems. Therefore,
it tends to be best to avoid using the EXTRA_* variables for variables essential
to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the if the
useroverrides those with something else things should still work, but putting
"-I/path/to/include" would not be.

/Bruce

Wiles, Keith April 30, 2015, 4:33 p.m. UTC | #9

On 4/30/15, 11:22 AM, "Richardson, Bruce" <bruce.richardson@intel.com>
wrote:

>On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote:
>> 
>> 
>> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>> 
>> >Hi Keith,
>> >
>> >On 04/30/2015 03:24 PM, Wiles, Keith wrote:
>> >>
>> >>
>> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>> >>
>> >>> Hi Keith,
>> >>>
>> >>> Thank you for submitting a clean-up. Please see some comments below.
>> >>>
>> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
>> >>>> Trying to 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.
>> >>>>
>> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
>> >>>> instead of using LDLIBS directly.
>> >>>
>> >>> If I understand well, the goal of this patch is only a cleanup in
>> >>> rte.app.mk, but at the end, it changes the makefile user "API",
>> >>> which could probably be a problem for applications using the
>> >>> dpdk makefile framework.
>> >>>
>> >>> Why not just having an temporary internal variable (let's say
>> >>> _LDLIBS-y) that would allow to do the clean-up without modifying
>> >>> the user interface?
>> >>>
>> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be
>> >>> different than CFLAGS or LDFLAGS:
>> >>> - CFLAGS/LDFLAGS are in Makefiles only
>> >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
>> >>>    to add flags to the default ones
>> >>>
>> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
>> >>> keeping a compatibility with existing application Makefiles.
>> >>
>> >> The docs for DPDK 28.3.6 states they can be used for both command
>>line
>> >>and
>> >> Makefile, so I think I like the current solution unless everyone
>>wants
>> >>it
>> >> as you suggested.
>> >>
>> >> 
>> 
>>>>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.h
>>>>tm
>> >>l
>> >
>> > From the link you have sent:
>> >
>> >- About CFLAGS:
>> >
>> >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only
>> >[...]
>> >CFLAGS: Flags to use for C compilation. The user should use += to
>>append
>> >data in this variable."
>> >
>> >nothing in 28.3.6
>> >
>> >
>> >- About EXTRA_CFLAGS:
>> >
>> >nothing in 28.3.4
>> >
>> >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile
>> >or Command Line
>> >[...]
>> >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS
>>when
>> >compiling."
>> 
>> The point was that EXTRA_XXX can be used for command line and Makefile
>>as
>> it was pointed out in a previous email the assumption was EXTRA_XXX was
>> only for the command line. (Just to make sure we understood EXTRA_XXX
>>was
>> not just for command line options.) This was the reason I sent the link
>>an
>> to point out using EXTRA_XXX is a much cleaner method then allowing
>> someone to modify what I believe is an internal variable.
>
>Just beware that setting EXTRA_* flags on the commandline can override
>their
>values in the makefiles, and cause unexpected compilation problems.
>Therefore,
>it tends to be best to avoid using the EXTRA_* variables for variables
>essential
>to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the
>if the
>useroverrides those with something else things should still work, but
>putting
>"-I/path/to/include" would not be.

On the command line and makefile you should be using += and not just = or
you run into this problem.
>
>/Bruce
>
>

Bruce Richardson May 1, 2015, 9:09 a.m. UTC | #10

On Thu, Apr 30, 2015 at 05:33:36PM +0100, Wiles, Keith wrote:
> 
> 
> On 4/30/15, 11:22 AM, "Richardson, Bruce" <bruce.richardson@intel.com>
> wrote:
> 
> >On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote:
> >> 
> >> 
> >> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
> >> 
> >> >Hi Keith,
> >> >
> >> >On 04/30/2015 03:24 PM, Wiles, Keith wrote:
> >> >>
> >> >>
> >> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
> >> >>
> >> >>> Hi Keith,
> >> >>>
> >> >>> Thank you for submitting a clean-up. Please see some comments below.
> >> >>>
> >> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
> >> >>>> Trying to 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.
> >> >>>>
> >> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
> >> >>>> instead of using LDLIBS directly.
> >> >>>
> >> >>> If I understand well, the goal of this patch is only a cleanup in
> >> >>> rte.app.mk, but at the end, it changes the makefile user "API",
> >> >>> which could probably be a problem for applications using the
> >> >>> dpdk makefile framework.
> >> >>>
> >> >>> Why not just having an temporary internal variable (let's say
> >> >>> _LDLIBS-y) that would allow to do the clean-up without modifying
> >> >>> the user interface?
> >> >>>
> >> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be
> >> >>> different than CFLAGS or LDFLAGS:
> >> >>> - CFLAGS/LDFLAGS are in Makefiles only
> >> >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
> >> >>>    to add flags to the default ones
> >> >>>
> >> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
> >> >>> keeping a compatibility with existing application Makefiles.
> >> >>
> >> >> The docs for DPDK 28.3.6 states they can be used for both command
> >>line
> >> >>and
> >> >> Makefile, so I think I like the current solution unless everyone
> >>wants
> >> >>it
> >> >> as you suggested.
> >> >>
> >> >> 
> >> 
> >>>>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.h
> >>>>tm
> >> >>l
> >> >
> >> > From the link you have sent:
> >> >
> >> >- About CFLAGS:
> >> >
> >> >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only
> >> >[...]
> >> >CFLAGS: Flags to use for C compilation. The user should use += to
> >>append
> >> >data in this variable."
> >> >
> >> >nothing in 28.3.6
> >> >
> >> >
> >> >- About EXTRA_CFLAGS:
> >> >
> >> >nothing in 28.3.4
> >> >
> >> >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile
> >> >or Command Line
> >> >[...]
> >> >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS
> >>when
> >> >compiling."
> >> 
> >> The point was that EXTRA_XXX can be used for command line and Makefile
> >>as
> >> it was pointed out in a previous email the assumption was EXTRA_XXX was
> >> only for the command line. (Just to make sure we understood EXTRA_XXX
> >>was
> >> not just for command line options.) This was the reason I sent the link
> >>an
> >> to point out using EXTRA_XXX is a much cleaner method then allowing
> >> someone to modify what I believe is an internal variable.
> >
> >Just beware that setting EXTRA_* flags on the commandline can override
> >their
> >values in the makefiles, and cause unexpected compilation problems.
> >Therefore,
> >it tends to be best to avoid using the EXTRA_* variables for variables
> >essential
> >to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the
> >if the
> >useroverrides those with something else things should still work, but
> >putting
> >"-I/path/to/include" would not be.
> 
> On the command line and makefile you should be using += and not just = or
> you run into this problem.

Using "+=" on the commandline is not normal. It's also rather tricky to do
at times, if a value is already defined, as bash shell does not add in whitespace
to the existing variable appropriately. For example, I have EXTRA_CFLAGS set to
'-g -Wfatal-errors' in my .bashrc so I always get debug builds that stop on first
error. Building the dpdk.org as below works fine:

 EXTRA_CFLAGS=-g gmake
 == Build lib
 == Build lib/librte_compat
 ...

However, using += causes very strange behaviour:

$ EXTRA_CFLAGS+=-g gmake
== Build lib
== Build lib/librte_compat
== Build lib/librte_eal
== Build lib/librte_net
== Build lib/librte_eal/common
== Build lib/librte_eal/bsdapp
== Build lib/librte_eal/bsdapp/eal
== Build lib/librte_eal/bsdapp/contigmem
== Build lib/librte_eal/bsdapp/nic_uio
Warning: Object directory not changed from original /usr/home/bruce/dpdk.org/x86_64-native-bsdapp-clang/build/lib/librte_eal/bsdapp/nic_uio
Warning: Object directory not changed from original /usr/home/bruce/dpdk.org/x86_64-native-bsdapp-clang/build/lib/librte_eal/bsdapp/contigmem
  CC eal.o
  CC eal_memory.o
  CC eal_hugepage_info.o
  CC eal_thread.o
error: unknown warning option '-Wfatal-errors=g' [-Werror,-Wunknown-warning-option]

So, in short, you must assume that EXTRA_CFLAGS will be specified using "=" on
the commandline, and that things build appropriately when the user does so.
[Given you use "+=" internally in the makefiles, I assume this is the case, but
flagging this as something worth double-checking]

/Bruce

Patch

diff --git a/examples/dpdk_qat/Makefile b/examples/dpdk_qat/Makefile
index f1e06a1..90ca1d3 100644
--- a/examples/dpdk_qat/Makefile
+++ b/examples/dpdk_qat/Makefile
@@ -77,8 +77,8 @@  else
 ICP_LIBRARY_PATH = $(ICP_ROOT)/build/libicp_qa_al.a
 endif
 
-LDLIBS += -L$(ICP_ROOT)/build
-LDLIBS += $(ICP_LIBRARY_PATH) \
+EXTRA_LDLIBS += -L$(ICP_ROOT)/build
+EXTRA_LDLIBS += $(ICP_LIBRARY_PATH) \
                 -lz \
                 -losal \
                 -ladf_proxy \
diff --git a/examples/vm_power_manager/Makefile b/examples/vm_power_manager/Makefile
index 113dbc4..8fb78d4 100644
--- a/examples/vm_power_manager/Makefile
+++ b/examples/vm_power_manager/Makefile
@@ -48,7 +48,7 @@  SRCS-y += channel_monitor.c
 CFLAGS += -O3 -I$(RTE_SDK)/lib/librte_power/
 CFLAGS += $(WERROR_FLAGS)
 
-LDLIBS += -lvirt
+EXTRA_LDLIBS += -lvirt
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 62a76ae..c41de82 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) 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)
-LDLIBS += -lrte_cfgfile
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_BOND),y)
-LDLIBS += -lrte_pmd_bond
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_XENVIRT),y)
-LDLIBS += -lrte_pmd_xenvirt
-LDLIBS += -lxenstore
-endif
+LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)          += -lrte_kvargs
+LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)            += -lrte_mbuf
+LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)         += -lrte_ip_frag
+LDLIBS-$(CONFIG_RTE_LIBRTE_ETHER)           += -lethdev
+LDLIBS-$(CONFIG_RTE_LIBRTE_MALLOC)          += -lrte_malloc
+LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)         += -lrte_mempool
+LDLIBS-$(CONFIG_RTE_LIBRTE_RING)            += -lrte_ring
+LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)             += -lrte_eal
+LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)         += -lrte_cmdline
+LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)         += -lrte_cfgfile
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)        += -lrte_pmd_bond
+
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lrte_pmd_xenvirt
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)     += -lxenstore
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 # plugins (link only if static libraries)
 
-ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y)
-LDLIBS += -lrte_pmd_vmxnet3_uio
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y)
-LDLIBS += -lrte_pmd_virtio
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y)
-LDLIBS += -lrte_pmd_enic
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
-LDLIBS += -lrte_pmd_i40e
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
-LDLIBS += -lrte_pmd_fm10k
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
-LDLIBS += -lrte_pmd_ixgbe
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_E1000_PMD),y)
-LDLIBS += -lrte_pmd_e1000
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_MLX4_PMD),y)
-LDLIBS += -lrte_pmd_mlx4
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
-LDLIBS += -lrte_pmd_ring
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
-LDLIBS += -lrte_pmd_pcap
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
-LDLIBS += -lrte_pmd_af_packet
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_NULL),y)
-LDLIBS += -lrte_pmd_null
-endif
-
-endif # plugins
+LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)     += -lrte_pmd_vmxnet3_uio
+LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD)      += -lrte_pmd_virtio
+LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)        += -lrte_pmd_enic
+LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)        += -lrte_pmd_i40e
+LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)       += -lrte_pmd_fm10k
+LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD)       += -lrte_pmd_ixgbe
+LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)       += -lrte_pmd_e1000
+LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)        += -lrte_pmd_mlx4
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING)        += -lrte_pmd_ring
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)        += -lrte_pmd_pcap
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)   += -lrte_pmd_af_packet
+LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL)        += -lrte_pmd_null
+
+endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB)
 
 endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
 
-LDLIBS += $(EXECENV_LDLIBS)
-
-LDLIBS += --end-group
-
-LDLIBS += --no-whole-archive
+LDLIBS-y += $(EXECENV_LDLIBS)
+LDLIBS-y += --end-group
+LDLIBS-y += --no-whole-archive
 
 endif # ifeq ($(NO_AUTOLIBS),)
 
-LDLIBS += $(CPU_LDLIBS)
+LDLIBS-y += $(EXTRA_LDLIBS)
 
 .PHONY: all
 all: install
@@ -286,10 +164,10 @@  ifeq ($(LINK_USING_CC),1)
 override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
 O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
 	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call linkerprefix,$(LDFLAGS)) \
-	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
+	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS-y))
 else
 O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
+	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS-y)
 endif
 O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
 O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
@@ -302,12 +180,12 @@  O_TO_EXE_DO = @set -e; \
 -include .$(APP).cmd
 
 # path where libraries are retrieved
-LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS)))
-LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
+LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y)))
+LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
 
 # list of .a files that are linked to this application
-LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
-LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS)))
+LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
+LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y)))
 
 # list of found libraries files (useful for deps). If not found, the
 # library is silently ignored and dep won't be checked
diff --git a/mk/rte.hostapp.mk b/mk/rte.hostapp.mk
index c44d0f8..51e8c1d 100644
--- a/mk/rte.hostapp.mk
+++ b/mk/rte.hostapp.mk
@@ -58,7 +58,7 @@  build: _postbuild
 exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
 
 O_TO_EXE = $(HOSTCC) $(HOST_LDFLAGS) $(LDFLAGS_$(@)) \
-	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS)
+	$(EXTRA_HOST_LDFLAGS) -o $@ $(OBJS-y) $(LDLIBS-y)
 O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
 O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  HOSTLD $(@)")
 O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
@@ -72,7 +72,7 @@  O_TO_EXE_DO = @set -e; \
 # list of .a files that are linked to this application
 LDLIBS_FILES := $(wildcard \
 	$(addprefix $(RTE_OUTPUT)/lib/, \
-	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))))
+	$(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))))
 
 #
 # Compile executable file if needed
diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
index fc6b0b4..56f20bb 100644
--- a/mk/rte.shared.mk
+++ b/mk/rte.shared.mk
@@ -60,10 +60,10 @@  exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
 ifeq ($(LINK_USING_CC),1)
 override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
 O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
+	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS-y))
 else
 O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-	-shared -o $@ $(OBJS-y) $(LDLIBS)
+	-shared -o $@ $(OBJS-y) $(LDLIBS-y)
 endif
 
 O_TO_SO_STR = $(subst ','\'',$(O_TO_SO)) #'# fix syntax highlight
@@ -77,12 +77,12 @@  O_TO_SO_DO = @set -e; \
 -include .$(SHARED).cmd
 
 # path where libraries are retrieved
-LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS)))
-LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
+LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS-y)))
+LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS-y)))
 
 # list of .a files that are linked to this application
-LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
-LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS)))
+LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS-y)))
+LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS-y)))
 
 # list of found libraries files (useful for deps). If not found, the
 # library is silently ignored and dep won't be checked