DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
@ 2015-04-29 15:25 Keith Wiles
  2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Keith Wiles @ 2015-04-29 15:25 UTC (permalink / raw)
  To: dev

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(-)

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
-- 
2.3.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable
  2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles
@ 2015-04-29 15:25 ` Keith Wiles
  2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon
  2015-04-30  9:45 ` Olivier MATZ
  2 siblings, 0 replies; 12+ messages in thread
From: Keith Wiles @ 2015-04-29 15:25 UTC (permalink / raw)
  To: dev

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 doc/build-sdk-quick.txt                          | 1 +
 doc/guides/prog_guide/build_app.rst              | 2 +-
 doc/guides/prog_guide/dev_kit_build_system.rst   | 2 ++
 doc/guides/prog_guide/dev_kit_root_make_help.rst | 2 +-
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/doc/build-sdk-quick.txt b/doc/build-sdk-quick.txt
index 041a40e..26d5442 100644
--- a/doc/build-sdk-quick.txt
+++ b/doc/build-sdk-quick.txt
@@ -13,6 +13,7 @@ Build variables
 	EXTRA_CPPFLAGS   preprocessor options
 	EXTRA_CFLAGS     compiler options
 	EXTRA_LDFLAGS    linker options
+	EXTRA_LDLIBS     linker libary options
 	RTE_KERNELDIR    linux headers path
 	CROSS     toolchain prefix
 	V         verbose
diff --git a/doc/guides/prog_guide/build_app.rst b/doc/guides/prog_guide/build_app.rst
index d4a3261..0680dee 100644
--- a/doc/guides/prog_guide/build_app.rst
+++ b/doc/guides/prog_guide/build_app.rst
@@ -123,6 +123,6 @@ chapter for details.
 
 *   CPPFLAGS: The flags to use to provide flags to the C preprocessor (only useful when assembling .S files)
 
-*   LDLIBS: A list of libraries to link with (for example, -L /path/to/libfoo - lfoo)
+*   LDLIBS-y: A list of libraries to link with (for example, -L /path/to/libfoo - lfoo) Use EXTRA_LDLIBS to add more options.
 
 *   NO_AUTOLIBS: If set, the libraries provided by the framework will not be included in the LDLIBS variable automatically.
diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst
index cf5c96f..b8ef167 100644
--- a/doc/guides/prog_guide/dev_kit_build_system.rst
+++ b/doc/guides/prog_guide/dev_kit_build_system.rst
@@ -413,6 +413,8 @@ Variables that Can be Set/Overridden by the User in a Makefile or Command Line
 
 *   EXTRA_LDFLAGS: The content of this variable is appended after LDFLAGS when linking.
 
+*   EXTRA_LDLIBS: The content of this variable is appended after LDLIBS when linking.
+
 *   EXTRA_ASFLAGS: The content of this variable is appended after ASFLAGS when assembling.
 
 *   EXTRA_CPPFLAGS: The content of this variable is appended after CPPFLAGS when using a C preprocessor on assembly files.
diff --git a/doc/guides/prog_guide/dev_kit_root_make_help.rst b/doc/guides/prog_guide/dev_kit_root_make_help.rst
index 4f30192..fdc5fea 100644
--- a/doc/guides/prog_guide/dev_kit_root_make_help.rst
+++ b/doc/guides/prog_guide/dev_kit_root_make_help.rst
@@ -205,7 +205,7 @@ The following variables can be specified on the command line:
 
     Enable dependency debugging. This provides some useful information about why a target is built or not.
 
-*   EXTRA_CFLAGS=, EXTRA_LDFLAGS=, EXTRA_ASFLAGS=, EXTRA_CPPFLAGS=
+*   EXTRA_CFLAGS=, EXTRA_LDFLAGS=, EXTRA_LDLIBS=, EXTRA_ASFLAGS=, EXTRA_CPPFLAGS=
 
     Append specific compilation, link or asm flags.
 
-- 
2.3.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles
  2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles
@ 2015-04-29 17:17 ` Thomas Monjalon
  2015-04-29 17:49   ` Wiles, Keith
  2015-04-30  9:45 ` Olivier MATZ
  2 siblings, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2015-04-29 17:17 UTC (permalink / raw)
  To: Keith Wiles; +Cc: dev

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).

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon
@ 2015-04-29 17:49   ` Wiles, Keith
  0 siblings, 0 replies; 12+ messages in thread
From: Wiles, Keith @ 2015-04-29 17:49 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



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).

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles
  2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles
  2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon
@ 2015-04-30  9:45 ` Olivier MATZ
  2015-04-30 13:24   ` Wiles, Keith
  2 siblings, 1 reply; 12+ messages in thread
From: Olivier MATZ @ 2015-04-30  9:45 UTC (permalink / raw)
  To: Keith Wiles, dev

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30  9:45 ` Olivier MATZ
@ 2015-04-30 13:24   ` Wiles, Keith
  2015-04-30 13:38     ` Olivier MATZ
  0 siblings, 1 reply; 12+ messages in thread
From: Wiles, Keith @ 2015-04-30 13:24 UTC (permalink / raw)
  To: Olivier MATZ, dev



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
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30 13:24   ` Wiles, Keith
@ 2015-04-30 13:38     ` Olivier MATZ
  2015-04-30 14:31       ` Wiles, Keith
  0 siblings, 1 reply; 12+ messages in thread
From: Olivier MATZ @ 2015-04-30 13:38 UTC (permalink / raw)
  To: Wiles, Keith, dev

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
>>>
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30 13:38     ` Olivier MATZ
@ 2015-04-30 14:31       ` Wiles, Keith
  2015-04-30 15:56         ` Olivier MATZ
  2015-04-30 16:22         ` Bruce Richardson
  0 siblings, 2 replies; 12+ messages in thread
From: Wiles, Keith @ 2015-04-30 14:31 UTC (permalink / raw)
  To: Olivier MATZ, dev



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
>>>>
>>>
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30 14:31       ` Wiles, Keith
@ 2015-04-30 15:56         ` Olivier MATZ
  2015-04-30 16:22         ` Bruce Richardson
  1 sibling, 0 replies; 12+ messages in thread
From: Olivier MATZ @ 2015-04-30 15:56 UTC (permalink / raw)
  To: Wiles, Keith, dev

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
>>>>>
>>>>
>>>
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30 14:31       ` Wiles, Keith
  2015-04-30 15:56         ` Olivier MATZ
@ 2015-04-30 16:22         ` Bruce Richardson
  2015-04-30 16:33           ` Wiles, Keith
  1 sibling, 1 reply; 12+ messages in thread
From: Bruce Richardson @ 2015-04-30 16:22 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: dev

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

 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30 16:22         ` Bruce Richardson
@ 2015-04-30 16:33           ` Wiles, Keith
  2015-05-01  9:09             ` Bruce Richardson
  0 siblings, 1 reply; 12+ messages in thread
From: Wiles, Keith @ 2015-04-30 16:33 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev



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
>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
  2015-04-30 16:33           ` Wiles, Keith
@ 2015-05-01  9:09             ` Bruce Richardson
  0 siblings, 0 replies; 12+ messages in thread
From: Bruce Richardson @ 2015-05-01  9:09 UTC (permalink / raw)
  To: Wiles, Keith; +Cc: dev

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-05-01  9:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-29 15:25 [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Keith Wiles
2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles
2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon
2015-04-29 17:49   ` Wiles, Keith
2015-04-30  9:45 ` Olivier MATZ
2015-04-30 13:24   ` Wiles, Keith
2015-04-30 13:38     ` Olivier MATZ
2015-04-30 14:31       ` Wiles, Keith
2015-04-30 15:56         ` Olivier MATZ
2015-04-30 16:22         ` Bruce Richardson
2015-04-30 16:33           ` Wiles, Keith
2015-05-01  9:09             ` Bruce Richardson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).