[dpdk-dev] [PATCH 00/10] Add an option to use LTO for DPDK build

[dpdk-dev] [PATCH 00/10] Add an option to use LTO for DPDK build

[Andrzej Ostruszka]

This patch series adds an option to make use of link time optimization
(if compiler has support for it).  It is split as follows:
- 1st patch (build) is the enablement
- remaining patches are fixes for the warnings produced by the compiler
  and they are split by directory/subsystem so their maintainers can
  easily find and verify the changes - please note that there are two
  groups:
  * errors (or possible errors) - with title "fix possible use ..."
  * false positives - warnings that _I_ think are not valid and the
    changes are made only to silence the compiler.


Andrzej Ostruszka (10):
  build: add an option to enable LTO build
  eventdev: fix possible use of uninitialized var
  app/eventdev: fix maybe-uninitialized warnings for LTO build
  event/octeontx2: fix maybe-uninitialized warnings for LTO build
  app/test: fix maybe-uninitialized warnings for LTO build
  net/dpaa2: fix possible use of uninitialized vars
  net/e1000: fix maybe-uninitialized warnings for LTO build
  net/i40e: fix maybe-uninitialized warnings for LTO build
  net/ifc: fix maybe-uninitialized warnings for LTO build
  net/qede: fix maybe-uninitialized warnings for LTO build

 .travis.yml                                   |  7 ++++
 app/test-eventdev/test_perf_common.c          |  2 +-
 app/test-eventdev/test_pipeline_common.c      |  4 +--
 app/test/test_hash_readwrite.c                |  2 +-
 app/test/test_link_bonding_mode4.c            |  6 ++--
 app/test/test_memzone.c                       |  3 +-
 config/common_base                            |  5 +++
 config/meson.build                            |  9 +++++
 doc/guides/prog_guide/lto.rst                 | 36 +++++++++++++++++++
 doc/guides/rel_notes/release_19_11.rst        |  8 +++++
 drivers/event/octeontx2/otx2_tim_worker.h     |  2 +-
 drivers/net/dpaa2/base/dpaa2_hw_dpni.c        |  1 +
 drivers/net/dpaa2/mc/dpkg.c                   |  2 +-
 drivers/net/dpaa2/mc/dpni.c                   |  9 +++--
 drivers/net/e1000/base/e1000_82543.c          |  2 +-
 drivers/net/e1000/base/e1000_ich8lan.c        |  2 +-
 drivers/net/e1000/base/e1000_phy.c            |  2 +-
 drivers/net/i40e/i40e_ethdev.c                |  2 +-
 drivers/net/ifc/ifcvf_vdpa.c                  | 14 +++++---
 drivers/net/qede/base/ecore_mcp.c             | 13 +++----
 lib/librte_eventdev/rte_event_timer_adapter.c |  8 ++---
 meson_options.txt                             |  2 ++
 mk/toolchain/clang/rte.toolchain-compat.mk    |  4 +++
 mk/toolchain/clang/rte.vars.mk                |  8 +++++
 mk/toolchain/gcc/rte.toolchain-compat.mk      |  4 +++
 mk/toolchain/gcc/rte.vars.mk                  | 12 +++++++
 mk/toolchain/icc/rte.vars.mk                  |  8 +++++
 27 files changed, 146 insertions(+), 31 deletions(-)
 create mode 100644 doc/guides/prog_guide/lto.rst

-- 
2.17.1

[dpdk-dev] [PATCH 01/10] build: add an option to enable LTO build

[Andrzej Ostruszka]

This patch adds an option to enable link time optimization.  In addition
to LTO option itself (-flto) fat-lto-objects are being used.  This is
because during the build pmdinfogen scans the generated ELF objects to
find this_pmd_name* symbol in symbol table.  Without fat-lto-objects gcc
produces ELF only with extra symbols for internal use during linking and
clang does not produce ELF at all (only LLVM IR bitcode).

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 .travis.yml                                |  7 +++++
 config/common_base                         |  5 +++
 config/meson.build                         |  9 ++++++
 doc/guides/prog_guide/lto.rst              | 36 ++++++++++++++++++++++
 doc/guides/rel_notes/release_19_11.rst     |  8 +++++
 meson_options.txt                          |  2 ++
 mk/toolchain/clang/rte.toolchain-compat.mk |  4 +++
 mk/toolchain/clang/rte.vars.mk             |  8 +++++
 mk/toolchain/gcc/rte.toolchain-compat.mk   |  4 +++
 mk/toolchain/gcc/rte.vars.mk               | 12 ++++++++
 mk/toolchain/icc/rte.vars.mk               |  8 +++++
 11 files changed, 103 insertions(+)
 create mode 100644 doc/guides/prog_guide/lto.rst

diff --git a/.travis.yml b/.travis.yml
index 781f9f666..a9506b13a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -31,6 +31,7 @@ env:
   - DEF_LIB="static" OPTS="-Denable_kmods=false"
   - DEF_LIB="shared" OPTS="-Denable_kmods=false"
   - DEF_LIB="shared" RUN_TESTS=1 BUILD_DOCS=1
+  - DEF_LIB="static" OPTS="-Denable_lto=true"
 
 matrix:
   include:
@@ -100,6 +101,12 @@ matrix:
       apt:
         packages:
           - *extra_packages
+  - env: DEF_LIB="static" OPTS="-Denable_lto=true" EXTRA_PACKAGES=1
+    compiler: gcc
+    addons:
+      apt:
+        packages:
+          - *extra_packages
 
 
 script: ./.ci/${TRAVIS_OS_NAME}-build.sh
diff --git a/config/common_base b/config/common_base
index 8ef75c203..73a55fdec 100644
--- a/config/common_base
+++ b/config/common_base
@@ -49,6 +49,11 @@ CONFIG_RTE_FORCE_INTRINSICS=n
 #
 CONFIG_RTE_ARCH_STRICT_ALIGN=n
 
+#
+# Enable link time optimization
+#
+CONFIG_RTE_ENABLE_LTO=n
+
 #
 # Compile to share library
 #
diff --git a/config/meson.build b/config/meson.build
index 2bafea530..b224d0cb8 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -196,3 +196,12 @@ add_project_arguments('-D_GNU_SOURCE', language: 'c')
 if is_freebsd
 	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
 endif
+
+if get_option('enable_lto')
+	if cc.has_argument('-flto -ffat-lto-objects')
+		add_project_arguments('-flto -ffat-lto-objects', language: 'c')
+		add_project_link_arguments('-flto', language: 'c')
+	else
+		message('compiler does not support LTO')
+	endif
+endif
diff --git a/doc/guides/prog_guide/lto.rst b/doc/guides/prog_guide/lto.rst
new file mode 100644
index 000000000..effa0fcc3
--- /dev/null
+++ b/doc/guides/prog_guide/lto.rst
@@ -0,0 +1,36 @@
+Link Time Optimization
+======================
+
+The DPDK framework supports compilation with link time optimization
+turned on.  This depends obviously on the capabilities of the compiler
+to do "whole program" optimization at link time and is available only
+for compilers that support that feature (gcc, clang and icc).  To be
+more specific compiler have to support creation of ELF objects
+containing both normal code and internal representation
+(fat-lto-objects).  This is required since during build some code is
+generated by parsing produced ELF objects (pmdinfogen).
+
+The amount of performance gain that one can get from LTO depends on the
+compiler and the code that is being compiled.  However LTO is also
+useful for additional code analysis done by the compiler.  In particular
+due to interprocedural analysis compiler can produce additional warnings
+about variables that might be used uninitialized.  Some of these
+warnings might be "false positives" though and you might need to
+explicitly initialize variable in order to silence the compiler.
+
+Link time optimization can be enabled for whole DPDK framework by
+setting:
+
+.. code-block:: console
+    CONFIG_ENABLE_LTO=y
+
+in config file for the case of make based build and by:
+
+.. code-block:: console
+    meson build -Denable_lto=true
+    ninja -C build
+
+for the case of meson based build.
+
+Please note that turning LTO on causes considerable extension of
+compilation time.
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index 27cfbd9e3..639b7ed53 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -56,6 +56,14 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+**Added build support for Link Time Optimization.**
+
+ LTO is an optimization technique used by the compiler to perform whole
+ program analysis and optimization at link time.  In order to do that
+ compilers store their internal representation of the source code that
+ the linker uses at the final stage of compilation process.
+
+ See :doc:`../prog_guide/lto` for more information:
 
 Removed Items
 -------------
diff --git a/meson_options.txt b/meson_options.txt
index 448f3e63d..a315e0f82 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -6,6 +6,8 @@ option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
 	description: 'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
 option('enable_docs', type: 'boolean', value: false,
 	description: 'build documentation')
+option('enable_lto', type: 'boolean', value: false,
+	description: 'Enable link time optimization')
 option('enable_kmods', type: 'boolean', value: true,
 	description: 'build kernel modules')
 option('examples', type: 'string', value: '',
diff --git a/mk/toolchain/clang/rte.toolchain-compat.mk b/mk/toolchain/clang/rte.toolchain-compat.mk
index e6189b498..78f96c648 100644
--- a/mk/toolchain/clang/rte.toolchain-compat.mk
+++ b/mk/toolchain/clang/rte.toolchain-compat.mk
@@ -20,3 +20,7 @@ CLANG_MINOR_VERSION := $(shell echo $(CLANG_VERSION) | cut -f2 -d.)
 ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -lt 35 && echo 1), 1)
 	CC_SUPPORTS_Z := false
 endif
+
+ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -lt 60 && echo 1), 1)
+	CONFIG_RTE_ENABLE_LTO=n
+endif
diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk
index 3c49dc568..3b1fa05f9 100644
--- a/mk/toolchain/clang/rte.vars.mk
+++ b/mk/toolchain/clang/rte.vars.mk
@@ -48,6 +48,14 @@ endif
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+ifeq ($(CONFIG_RTE_ENABLE_LTO),y)
+# 'fat-lto' is used since pmdinfogen needs to have 'this_pmd_nameX'
+# exported in symbol table and without this option only internal
+# representation is present.
+TOOLCHAIN_CFLAGS += -flto -ffat-lto-objects
+TOOLCHAIN_LDFLAGS += -flto
+endif
+
 # workaround clang bug with warning "missing field initializer" for "= {0}"
 WERROR_FLAGS += -Wno-missing-field-initializers
 
diff --git a/mk/toolchain/gcc/rte.toolchain-compat.mk b/mk/toolchain/gcc/rte.toolchain-compat.mk
index ea40a11c0..ad4fad83c 100644
--- a/mk/toolchain/gcc/rte.toolchain-compat.mk
+++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
@@ -88,6 +88,10 @@ else
 		MACHINE_CFLAGS := $(filter-out -march% -mtune% -msse%,$(MACHINE_CFLAGS))
 	endif
 
+	ifeq ($(shell test $(GCC_VERSION) -lt 45 && echo 1), 1)
+		CONFIG_RTE_ENABLE_LTO=n
+	endif
+
 	# Disable thunderx PMD for gcc < 4.7
 	ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
 		CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD=d
diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index b852fcfd7..9fc704193 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -62,6 +62,18 @@ endif
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+ifeq ($(CONFIG_RTE_ENABLE_LTO),y)
+# 'fat-lto' is used since pmdinfogen needs to have 'this_pmd_nameX'
+# exported in symbol table and without this option only internal
+# representation is present.
+TOOLCHAIN_CFLAGS += -flto -ffat-lto-objects
+TOOLCHAIN_LDFLAGS += -flto
+# workaround for GCC bug 81440
+ifeq ($(shell test $(GCC_VERSION) -lt 80 && echo 1), 1)
+WERROR_FLAGS += -Wno-lto-type-mismatch
+endif
+endif
+
 # workaround GCC bug with warning "missing initializer" for "= {0}"
 ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
 WERROR_FLAGS += -Wno-missing-field-initializers
diff --git a/mk/toolchain/icc/rte.vars.mk b/mk/toolchain/icc/rte.vars.mk
index aa1422bf1..8aa87aa1e 100644
--- a/mk/toolchain/icc/rte.vars.mk
+++ b/mk/toolchain/icc/rte.vars.mk
@@ -54,5 +54,13 @@ endif
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+ifeq ($(CONFIG_RTE_ENABLE_LTO),y)
+# 'fat-lto' is used since pmdinfogen needs to have 'this_pmd_nameX'
+# exported in symbol table and without this option only internal
+# representation is present.
+TOOLCHAIN_CFLAGS += -flto -ffat-lto-objects
+TOOLCHAIN_LDFLAGS += -flto
+endif
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
-- 
2.17.1

Re: [dpdk-dev] [PATCH 01/10] build: add an option to enable LTO build

[Bruce Richardson]

On Thu, Sep 05, 2019 at 11:32:30AM +0200, Andrzej Ostruszka wrote:
> This patch adds an option to enable link time optimization.  In addition
> to LTO option itself (-flto) fat-lto-objects are being used.  This is
> because during the build pmdinfogen scans the generated ELF objects to
> find this_pmd_name* symbol in symbol table.  Without fat-lto-objects gcc
> produces ELF only with extra symbols for internal use during linking and
> clang does not produce ELF at all (only LLVM IR bitcode).
> 
> Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
> ---
<snip>
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -6,6 +6,8 @@ option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
>  	description: 'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
>  option('enable_docs', type: 'boolean', value: false,
>  	description: 'build documentation')
> +option('enable_lto', type: 'boolean', value: false,
> +	description: 'Enable link time optimization')
>  option('enable_kmods', type: 'boolean', value: true,
>  	description: 'build kernel modules')
>  option('examples', type: 'string', value: '',

Should not need a new option here. There is already a built-in option
"b_lto" which we can reuse.

/Bruce

Re: [dpdk-dev] [PATCH 01/10] build: add an option to enable LTO build

[Andrzej Ostruszka]

On 9/5/19 11:36 AM, Bruce Richardson wrote:
> Should not need a new option here. There is already a built-in option
> "b_lto" which we can reuse.

Thank you Bruce, I don't know much about meson so I missed that.
Will try to figure out how to use that.

Regards
Andrzej

PS. Apologies to all maintainers for possibly getting duplicates - I've
made a mistake in DPDK mailing list e-mail and have resent the patch
series to dev@dpdk.org separately.  Please reply to these so that
everything is visible to the community.

Re: [dpdk-dev] [PATCH 01/10] build: add an option to enable LTO build

[Bruce Richardson]

On Thu, Sep 05, 2019 at 11:43:30AM +0200, Andrzej Ostruszka wrote:
> On 9/5/19 11:36 AM, Bruce Richardson wrote:
> > Should not need a new option here. There is already a built-in option
> > "b_lto" which we can reuse.
> 
> Thank you Bruce, I don't know much about meson so I missed that.
> Will try to figure out how to use that.
> 

No problem.
The options are documented here: https://mesonbuild.com/Builtin-options.html
and you can query them just the same as with the options added for the
project. Therefore, the only thing that really should change is that in the
meson.build file you check the built-in option and add the fat-lto-objects
flag. 

Incidentally, I think if the fat-lto-objects flag is not supported you may
want to error out if lto is enabled, as meson will add the lto flag itself
if the option is set.

Regards,
/Bruce

[dpdk-dev] [PATCH 02/10] eventdev: fix possible use of uninitialized var

[Andrzej Ostruszka]

Fix the logic for the case of event queue allowing all schedule types.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 lib/librte_eventdev/rte_event_timer_adapter.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c b/lib/librte_eventdev/rte_event_timer_adapter.c
index 5ce399eca..161e21a68 100644
--- a/lib/librte_eventdev/rte_event_timer_adapter.c
+++ b/lib/librte_eventdev/rte_event_timer_adapter.c
@@ -706,11 +706,11 @@ check_destination_event_queue(struct rte_event_timer *evtim,
 				       RTE_EVENT_QUEUE_ATTR_SCHEDULE_TYPE,
 				       &sched_type);
 
-	if ((ret < 0 && ret != -EOVERFLOW) ||
-	    evtim->ev.sched_type != sched_type)
-		return -1;
+	if ((ret == 0 && evtim->ev.sched_type == sched_type) ||
+	    ret == -EOVERFLOW)
+		return 0;
 
-	return 0;
+	return -1;
 }
 
 static int
-- 
2.17.1

[dpdk-dev] [PATCH 03/10] app/eventdev: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 app/test-eventdev/test_perf_common.c     | 2 +-
 app/test-eventdev/test_pipeline_common.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/test-eventdev/test_perf_common.c b/app/test-eventdev/test_perf_common.c
index aa925a7ef..a974685cb 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -439,7 +439,7 @@ perf_event_timer_adapter_setup(struct test_perf *t)
 
 		if (!(adapter_info.caps &
 				RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)) {
-			uint32_t service_id;
+			uint32_t service_id = -1U;
 
 			rte_event_timer_adapter_service_id_get(wl,
 					&service_id);
diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-eventdev/test_pipeline_common.c
index 16c49b860..813d0cf44 100644
--- a/app/test-eventdev/test_pipeline_common.c
+++ b/app/test-eventdev/test_pipeline_common.c
@@ -306,7 +306,7 @@ pipeline_event_rx_adapter_setup(struct evt_options *opt, uint8_t stride,
 		}
 
 		if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT)) {
-			uint32_t service_id;
+			uint32_t service_id = -1U;
 
 			rte_event_eth_rx_adapter_service_id_get(prod,
 					&service_id);
@@ -358,7 +358,7 @@ pipeline_event_tx_adapter_setup(struct evt_options *opt,
 		}
 
 		if (!(cap & RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT)) {
-			uint32_t service_id;
+			uint32_t service_id = -1U;
 
 			rte_event_eth_tx_adapter_service_id_get(consm,
 					&service_id);
-- 
2.17.1

[dpdk-dev] [PATCH 04/10] event/octeontx2: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/event/octeontx2/otx2_tim_worker.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/event/octeontx2/otx2_tim_worker.h b/drivers/event/octeontx2/otx2_tim_worker.h
index b193e2cab..50db6543c 100644
--- a/drivers/event/octeontx2/otx2_tim_worker.h
+++ b/drivers/event/octeontx2/otx2_tim_worker.h
@@ -337,7 +337,7 @@ tim_add_entry_brst(struct otx2_tim_ring * const tim_ring,
 		   const struct otx2_tim_ent *ents,
 		   const uint16_t nb_timers, const uint8_t flags)
 {
-	struct otx2_tim_ent *chunk;
+	struct otx2_tim_ent *chunk = NULL;
 	struct otx2_tim_bkt *bkt;
 	uint16_t chunk_remainder;
 	uint16_t index = 0;
-- 
2.17.1

[dpdk-dev] [PATCH 05/10] app/test: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 app/test/test_hash_readwrite.c     | 2 +-
 app/test/test_link_bonding_mode4.c | 6 ++++--
 app/test/test_memzone.c            | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 4376b099b..615767fb6 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -298,7 +298,7 @@ test_rw_reader(void *arg)
 
 	begin = rte_rdtsc_precise();
 	for (i = 0; i < read_cnt; i++) {
-		void *data;
+		void *data = arg;
 		rte_hash_lookup_data(tbl_rw_test_param.h,
 				tbl_rw_test_param.keys + i,
 				&data);
diff --git a/app/test/test_link_bonding_mode4.c b/app/test/test_link_bonding_mode4.c
index bbb4e9cce..8ca376dda 100644
--- a/app/test/test_link_bonding_mode4.c
+++ b/app/test/test_link_bonding_mode4.c
@@ -224,7 +224,7 @@ configure_ethdev(uint16_t port_id, uint8_t start)
 static int
 add_slave(struct slave_conf *slave, uint8_t start)
 {
-	struct rte_ether_addr addr, addr_check;
+	struct rte_ether_addr addr, addr_check = { { 0 } };
 
 	/* Some sanity check */
 	RTE_VERIFY(test_params.slave_ports <= slave &&
@@ -578,7 +578,9 @@ bond_get_update_timeout_ms(void)
 {
 	struct rte_eth_bond_8023ad_conf conf;
 
-	rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf);
+	if (rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf) < 0)
+		return 0;
+
 	return conf.update_timeout_ms;
 }
 
diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
index 7501b63c5..c284dcb44 100644
--- a/app/test/test_memzone.c
+++ b/app/test/test_memzone.c
@@ -476,7 +476,8 @@ find_max_block_free_size(unsigned int align, unsigned int socket_id)
 	struct rte_malloc_socket_stats stats;
 	size_t len, overhead;
 
-	rte_malloc_get_socket_stats(socket_id, &stats);
+	if (rte_malloc_get_socket_stats(socket_id, &stats) < 0)
+		return 0;
 
 	len = stats.greatest_free_size;
 	overhead = MALLOC_ELEM_OVERHEAD;
-- 
2.17.1

Re: [dpdk-dev] [PATCH 05/10] app/test: fix maybe-uninitialized warnings for LTO build

[Chas Williams]

On 9/5/19 5:32 AM, Andrzej Ostruszka wrote:
> During LTO build compiler reports some 'false positive' warnings about
> variables being possibly used uninitialized.  This patch silences these
> warnings.
> 
> Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
> ---
>   app/test/test_hash_readwrite.c     | 2 +-
>   app/test/test_link_bonding_mode4.c | 6 ++++--
>   app/test/test_memzone.c            | 3 ++-
>   3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
> index 4376b099b..615767fb6 100644
> --- a/app/test/test_hash_readwrite.c
> +++ b/app/test/test_hash_readwrite.c
> @@ -298,7 +298,7 @@ test_rw_reader(void *arg)
>   
>   	begin = rte_rdtsc_precise();
>   	for (i = 0; i < read_cnt; i++) {
> -		void *data;
> +		void *data = arg;
>   		rte_hash_lookup_data(tbl_rw_test_param.h,
>   				tbl_rw_test_param.keys + i,
>   				&data);
> diff --git a/app/test/test_link_bonding_mode4.c b/app/test/test_link_bonding_mode4.c
> index bbb4e9cce..8ca376dda 100644
> --- a/app/test/test_link_bonding_mode4.c
> +++ b/app/test/test_link_bonding_mode4.c
> @@ -224,7 +224,7 @@ configure_ethdev(uint16_t port_id, uint8_t start)
>   static int
>   add_slave(struct slave_conf *slave, uint8_t start)
>   {
> -	struct rte_ether_addr addr, addr_check;
> +	struct rte_ether_addr addr, addr_check = { { 0 } };
>   
>   	/* Some sanity check */
>   	RTE_VERIFY(test_params.slave_ports <= slave &&
> @@ -578,7 +578,9 @@ bond_get_update_timeout_ms(void)
>   {
>   	struct rte_eth_bond_8023ad_conf conf;
>   
> -	rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf);
> +	if (rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf) < 0)
> +		return 0;
> +

This would return a delay of 0ms, which doesn't feel like it captures 
the intent of conf_get failing. Perhaps a TEST_ASSERT_FAILURE() instead?

>   	return conf.update_timeout_ms;
>   }
>   
> diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
> index 7501b63c5..c284dcb44 100644
> --- a/app/test/test_memzone.c
> +++ b/app/test/test_memzone.c
> @@ -476,7 +476,8 @@ find_max_block_free_size(unsigned int align, unsigned int socket_id)
>   	struct rte_malloc_socket_stats stats;
>   	size_t len, overhead;
>   
> -	rte_malloc_get_socket_stats(socket_id, &stats);
> +	if (rte_malloc_get_socket_stats(socket_id, &stats) < 0)
> +		return 0;
>   
>   	len = stats.greatest_free_size;
>   	overhead = MALLOC_ELEM_OVERHEAD;
> 

Re: [dpdk-dev] [PATCH 05/10] app/test: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

On 9/5/19 3:25 PM, Chas Williams wrote:
[...]
>> @@ -578,7 +578,9 @@ bond_get_update_timeout_ms(void)
>>   {
>>   	struct rte_eth_bond_8023ad_conf conf;
>>   
>> -	rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf);
>> +	if (rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf) < 0)
>> +		return 0;
>> +
> 
> This would return a delay of 0ms, which doesn't feel like it captures 
> the intent of conf_get failing. Perhaps a TEST_ASSERT_FAILURE() instead?

Chas, I've sent today reworked version.  I've taken this remark into
account however TEST_ASSERT_FAILURE() is not really appropriate here
since this macro is not really an assert.  It returns -1, which does not
play well with the signature of this function (return is unsigned) and
how it is used.  So I've just inlined the logs.

Regards
Andrzej

[dpdk-dev] [PATCH 06/10] net/dpaa2: fix possible use of uninitialized vars

[Andrzej Ostruszka]

This patch fixes 'maybe-uninitialized' warnings reported by compiler
when using LTO.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/net/dpaa2/base/dpaa2_hw_dpni.c | 1 +
 drivers/net/dpaa2/mc/dpkg.c            | 2 +-
 drivers/net/dpaa2/mc/dpni.c            | 9 ++++++---
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa2/base/dpaa2_hw_dpni.c b/drivers/net/dpaa2/base/dpaa2_hw_dpni.c
index 56e2e56a3..2f6534a31 100644
--- a/drivers/net/dpaa2/base/dpaa2_hw_dpni.c
+++ b/drivers/net/dpaa2/base/dpaa2_hw_dpni.c
@@ -51,6 +51,7 @@ rte_pmd_dpaa2_set_custom_hash(uint16_t port_id,
 	kg_cfg.extracts[0].type = DPKG_EXTRACT_FROM_DATA;
 	kg_cfg.extracts[0].extract.from_data.offset = offset;
 	kg_cfg.extracts[0].extract.from_data.size = size;
+	kg_cfg.extracts[0].num_of_byte_masks = 0;
 	kg_cfg.num_extracts = 1;
 
 	ret = dpkg_prepare_key_cfg(&kg_cfg, p_params);
diff --git a/drivers/net/dpaa2/mc/dpkg.c b/drivers/net/dpaa2/mc/dpkg.c
index 80f94f40e..7aa63ea12 100644
--- a/drivers/net/dpaa2/mc/dpkg.c
+++ b/drivers/net/dpaa2/mc/dpkg.c
@@ -63,7 +63,7 @@ dpkg_prepare_key_cfg(const struct dpkg_profile_cfg *cfg, uint8_t *key_cfg_buf)
 		dpkg_set_field(extr->extract_type, EXTRACT_TYPE,
 			       cfg->extracts[i].type);
 
-		for (j = 0; j < DPKG_NUM_OF_MASKS; j++) {
+		for (j = 0; j < extr->num_of_byte_masks; j++) {
 			extr->masks[j].mask = cfg->extracts[i].masks[j].mask;
 			extr->masks[j].offset =
 				cfg->extracts[i].masks[j].offset;
diff --git a/drivers/net/dpaa2/mc/dpni.c b/drivers/net/dpaa2/mc/dpni.c
index 362cd476f..b74a1a317 100644
--- a/drivers/net/dpaa2/mc/dpni.c
+++ b/drivers/net/dpaa2/mc/dpni.c
@@ -1803,10 +1803,13 @@ int dpni_set_congestion_notification(struct fsl_mc_io *mc_io,
 	cmd_params->qtype = qtype;
 	cmd_params->tc = tc_id;
 	cmd_params->congestion_point = cfg->cg_point;
-	cmd_params->cgid = (uint8_t)cfg->cgid;
-	cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
+	if (cfg->cg_point == DPNI_CP_CONGESTION_GROUP)
+		cmd_params->cgid = (uint8_t)cfg->cgid;
+	if (cfg->dest_cfg.dest_type != DPNI_DEST_NONE) {
+		cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
+		cmd_params->dest_priority = cfg->dest_cfg.priority;
+	}
 	cmd_params->notification_mode = cpu_to_le16(cfg->notification_mode);
-	cmd_params->dest_priority = cfg->dest_cfg.priority;
 	cmd_params->message_iova = cpu_to_le64(cfg->message_iova);
 	cmd_params->message_ctx = cpu_to_le64(cfg->message_ctx);
 	cmd_params->threshold_entry = cpu_to_le32(cfg->threshold_entry);
-- 
2.17.1

[dpdk-dev] [PATCH 07/10] net/e1000: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/net/e1000/base/e1000_82543.c   | 2 +-
 drivers/net/e1000/base/e1000_ich8lan.c | 2 +-
 drivers/net/e1000/base/e1000_phy.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_82543.c b/drivers/net/e1000/base/e1000_82543.c
index 810899eb6..dfde2a8b9 100644
--- a/drivers/net/e1000/base/e1000_82543.c
+++ b/drivers/net/e1000/base/e1000_82543.c
@@ -1027,7 +1027,7 @@ STATIC s32 e1000_setup_copper_link_82543(struct e1000_hw *hw)
 {
 	u32 ctrl;
 	s32 ret_val;
-	bool link;
+	bool link = true;
 
 	DEBUGFUNC("e1000_setup_copper_link_82543");
 
diff --git a/drivers/net/e1000/base/e1000_ich8lan.c b/drivers/net/e1000/base/e1000_ich8lan.c
index accc6ea01..5241cf698 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -5533,7 +5533,7 @@ void e1000_igp3_phy_powerdown_workaround_ich8lan(struct e1000_hw *hw)
 void e1000_gig_downshift_workaround_ich8lan(struct e1000_hw *hw)
 {
 	s32 ret_val;
-	u16 reg_data;
+	u16 reg_data = 0;
 
 	DEBUGFUNC("e1000_gig_downshift_workaround_ich8lan");
 
diff --git a/drivers/net/e1000/base/e1000_phy.c b/drivers/net/e1000/base/e1000_phy.c
index 7d08f836f..956c06747 100644
--- a/drivers/net/e1000/base/e1000_phy.c
+++ b/drivers/net/e1000/base/e1000_phy.c
@@ -1664,7 +1664,7 @@ s32 e1000_copper_link_autoneg(struct e1000_hw *hw)
 s32 e1000_setup_copper_link_generic(struct e1000_hw *hw)
 {
 	s32 ret_val;
-	bool link;
+	bool link = true;
 
 	DEBUGFUNC("e1000_setup_copper_link_generic");
 
-- 
2.17.1

[dpdk-dev] [PATCH 08/10] net/i40e: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/net/i40e/i40e_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 4e40b7ab5..525a6b69b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -8376,7 +8376,7 @@ static int
 i40e_add_vxlan_port(struct i40e_pf *pf, uint16_t port, int udp_type)
 {
 	int  idx, ret;
-	uint8_t filter_idx;
+	uint8_t filter_idx = 0;
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 
 	idx = i40e_get_vxlan_port_idx(pf, port);
-- 
2.17.1

[dpdk-dev] [PATCH 09/10] net/ifc: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/net/ifc/ifcvf_vdpa.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ifc/ifcvf_vdpa.c b/drivers/net/ifc/ifcvf_vdpa.c
index 8de9ef199..4cdff8712 100644
--- a/drivers/net/ifc/ifcvf_vdpa.c
+++ b/drivers/net/ifc/ifcvf_vdpa.c
@@ -304,8 +304,8 @@ vdpa_ifcvf_stop(struct ifcvf_internal *internal)
 	struct ifcvf_hw *hw = &internal->hw;
 	uint32_t i;
 	int vid;
-	uint64_t features;
-	uint64_t log_base, log_size;
+	uint64_t features = 0;
+	uint64_t log_base = 0, log_size = 0;
 	uint64_t len;
 
 	vid = internal->vid;
@@ -348,6 +348,8 @@ vdpa_enable_vfio_intr(struct ifcvf_internal *internal, bool m_rx)
 	struct rte_vhost_vring vring;
 	int fd;
 
+	vring.callfd = -1;
+
 	nr_vring = rte_vhost_get_vring_num(internal->vid);
 
 	irq_set = (struct vfio_irq_set *)irq_set_buf;
@@ -442,6 +444,7 @@ notify_relay(void *arg)
 	}
 	internal->epfd = epfd;
 
+	vring.kickfd = -1;
 	for (qid = 0; qid < q_num; qid++) {
 		ev.events = EPOLLIN | EPOLLPRI;
 		rte_vhost_get_vhost_vring(internal->vid, qid, &vring);
@@ -577,7 +580,7 @@ m_ifcvf_start(struct ifcvf_internal *internal)
 	struct ifcvf_hw *hw = &internal->hw;
 	uint32_t i, nr_vring;
 	int vid, ret;
-	struct rte_vhost_vring vq;
+	struct rte_vhost_vring vq = { 0 };
 	void *vring_buf;
 	uint64_t m_vring_iova = IFCVF_MEDIATED_VRING;
 	uint64_t size;
@@ -721,6 +724,7 @@ vring_relay(void *arg)
 	}
 	internal->epfd = epfd;
 
+	vring.kickfd = -1;
 	for (qid = 0; qid < q_num; qid++) {
 		ev.events = EPOLLIN | EPOLLPRI;
 		rte_vhost_get_vhost_vring(vid, qid, &vring);
@@ -930,11 +934,11 @@ ifcvf_dev_close(int vid)
 static int
 ifcvf_set_features(int vid)
 {
-	uint64_t features;
+	uint64_t features = 0;
 	int did;
 	struct internal_list *list;
 	struct ifcvf_internal *internal;
-	uint64_t log_base, log_size;
+	uint64_t log_base = 0, log_size = 0;
 
 	did = rte_vhost_get_vdpa_device_id(vid);
 	list = find_internal_resource_by_did(did);
-- 
2.17.1

[dpdk-dev] [PATCH 10/10] net/qede: fix maybe-uninitialized warnings for LTO build

[Andrzej Ostruszka]

During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/net/qede/base/ecore_mcp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/qede/base/ecore_mcp.c b/drivers/net/qede/base/ecore_mcp.c
index 6c6560688..666c0fe12 100644
--- a/drivers/net/qede/base/ecore_mcp.c
+++ b/drivers/net/qede/base/ecore_mcp.c
@@ -2278,7 +2278,7 @@ enum _ecore_status_t ecore_mcp_trans_speed_mask(struct ecore_hwfn *p_hwfn,
 						struct ecore_ptt *p_ptt,
 						u32 *p_speed_mask)
 {
-	u32 transceiver_type, transceiver_state;
+	u32 transceiver_type = ETH_TRANSCEIVER_TYPE_NONE, transceiver_state;
 
 	ecore_mcp_get_transceiver_data(p_hwfn, p_ptt, &transceiver_state,
 				       &transceiver_type);
@@ -3163,7 +3163,8 @@ enum _ecore_status_t ecore_mcp_nvm_put_file_begin(struct ecore_dev *p_dev,
 enum _ecore_status_t ecore_mcp_nvm_write(struct ecore_dev *p_dev, u32 cmd,
 					 u32 addr, u8 *p_buf, u32 len)
 {
-	u32 buf_idx, buf_size, nvm_cmd, nvm_offset, resp, param;
+	u32 buf_idx, buf_size, nvm_cmd, nvm_offset;
+	u32 resp = FW_MSG_CODE_ERROR, param;
 	struct ecore_hwfn *p_hwfn = ECORE_LEADING_HWFN(p_dev);
 	enum _ecore_status_t rc = ECORE_INVAL;
 	struct ecore_ptt *p_ptt;
@@ -3370,7 +3371,7 @@ enum _ecore_status_t ecore_mcp_gpio_read(struct ecore_hwfn *p_hwfn,
 					 u16 gpio, u32 *gpio_val)
 {
 	enum _ecore_status_t rc = ECORE_SUCCESS;
-	u32 drv_mb_param = 0, rsp;
+	u32 drv_mb_param = 0, rsp = 0;
 
 	drv_mb_param = (gpio << DRV_MB_PARAM_GPIO_NUMBER_OFFSET);
 
@@ -3391,7 +3392,7 @@ enum _ecore_status_t ecore_mcp_gpio_write(struct ecore_hwfn *p_hwfn,
 					  u16 gpio, u16 gpio_val)
 {
 	enum _ecore_status_t rc = ECORE_SUCCESS;
-	u32 drv_mb_param = 0, param, rsp;
+	u32 drv_mb_param = 0, param, rsp = 0;
 
 	drv_mb_param = (gpio << DRV_MB_PARAM_GPIO_NUMBER_OFFSET) |
 		(gpio_val << DRV_MB_PARAM_GPIO_VALUE_OFFSET);
@@ -3481,7 +3482,7 @@ enum _ecore_status_t ecore_mcp_bist_clock_test(struct ecore_hwfn *p_hwfn,
 enum _ecore_status_t ecore_mcp_bist_nvm_test_get_num_images(
 	struct ecore_hwfn *p_hwfn, struct ecore_ptt *p_ptt, u32 *num_images)
 {
-	u32 drv_mb_param = 0, rsp;
+	u32 drv_mb_param = 0, rsp = 0;
 	enum _ecore_status_t rc = ECORE_SUCCESS;
 
 	drv_mb_param = (DRV_MB_PARAM_BIST_NVM_TEST_NUM_IMAGES <<
@@ -3865,7 +3866,7 @@ enum _ecore_status_t
 __ecore_mcp_resc_lock(struct ecore_hwfn *p_hwfn, struct ecore_ptt *p_ptt,
 		      struct ecore_resc_lock_params *p_params)
 {
-	u32 param = 0, mcp_resp, mcp_param;
+	u32 param = 0, mcp_resp = 0, mcp_param = 0;
 	u8 opcode;
 	enum _ecore_status_t rc;
 
-- 
2.17.1