[dpdk-dev] [PATCH 1/5] kni: fix build with gcc 7.1

[dpdk-dev] [PATCH 1/5] kni: fix build with gcc 7.1

[Ferruh Yigit]

build error:
.../dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_main.c:
  In function ‘igb_kni_probe’:
.../dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_main.c:2483:30:
  error: ‘%d’ directive output may be truncated writing between 1 and 5
  bytes into a region of size between 0 and 11
  [-Werror=format-truncation=]
        "%d.%d, 0x%08x, %d.%d.%d",
                              ^~
.../dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_main.c:2483:8:
  note: directive argument in the range [0, 65535]
        "%d.%d, 0x%08x, %d.%d.%d",
        ^~~~~~~~~~~~~~~~~~~~~~~~~
.../dpdk/build/build/lib/librte_eal/linuxapp/kni/igb_main.c:2481:4:
  note: ‘snprintf’ output between 23 and 43 bytes into a destination of
  size 32
    snprintf(adapter->fw_version,
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        sizeof(adapter->fw_version),
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        "%d.%d, 0x%08x, %d.%d.%d",
        ~~~~~~~~~~~~~~~~~~~~~~~~~~
        fw.eep_major, fw.eep_minor, fw.etrack_id,
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        fw.or_major, fw.or_build, fw.or_patch);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixed by increasing buffer size to 43 as suggested in compiler log.

Fixes: b9ee370557f1 ("kni: update kernel driver ethtool baseline")
Cc: stable@dpdk.org

Reported-by: Nirmoy Das <ndas@suse.de>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Reviewed-by: Markos Chandras <mchandras@suse.de>
---
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h
index d077b49e9..8667f29ca 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h
@@ -607,7 +607,7 @@ struct igb_adapter {
 	int int_mode;
 	u32 rss_queues;
 	u32 vmdq_pools;
-	char fw_version[32];
+	char fw_version[43];
 	u32 wvbr;
 	struct igb_mac_addr *mac_table;
 #ifdef CONFIG_IGB_VMDQ_NETDEV
-- 
2.13.0

[dpdk-dev] [PATCH 2/5] net/i40e: fix memset size

[Ferruh Yigit]

This causes build error with gcc 7.1.1 :

...dpdk/drivers/net/i40e/i40e_flow.c:2357:2:
error: ‘memset’ used with length equal to number of elements without
       multiplication by element size [-Werror=memset-elt-size]
  memset(off_arr, 0, I40E_MAX_FLXPLD_FIED);
  ^~~~~~

...dpdk/drivers/net/i40e/i40e_flow.c:2358:2:
error: ‘memset’ used with length equal to number of elements without
       multiplication by element size [-Werror=memset-elt-size]
  memset(len_arr, 0, I40E_MAX_FLXPLD_FIED);
  ^~~~~~

Fixed by providing correct size to memset.

Fixes: 6ced3dd72f5f ("net/i40e: support flexible payload parsing for FDIR")

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
This code introduced for this release, so fix can be squashed into
relevant patch.

Cc: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index c7589ce86..08c0f8e68 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -2354,8 +2354,8 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
 	uint16_t ether_type;
 	int ret;
 
-	memset(off_arr, 0, I40E_MAX_FLXPLD_FIED);
-	memset(len_arr, 0, I40E_MAX_FLXPLD_FIED);
+	memset(off_arr, 0, sizeof(off_arr));
+	memset(len_arr, 0, sizeof(len_arr));
 	memset(flex_mask, 0, I40E_FDIR_MAX_FLEX_LEN);
 	outer_tpid = i40e_get_outer_vlan(dev);
 	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
-- 
2.13.0

[dpdk-dev] [PATCH 3/5] net/enic: fix build with gcc 7.1

[Ferruh Yigit]

build error:

.../dpdk/drivers/net/enic/base/vnic_dev.c:
  In function ‘vnic_dev_get_mac_addr’:
.../dpdk/drivers/net/enic/base/vnic_dev.c:470:12:
  error: ‘a0’ is used uninitialized in this function
  [-Werror=uninitialized]
  args[0] = *a0;
            ^~~
...dpdk/drivers/net/enic/base/vnic_dev.c:
  In function ‘vnic_dev_classifier’:
...dpdk/drivers/net/enic/base/vnic_dev.c:471:12:
  error: ‘a1’ may be used uninitialized in this function
  [-Werror=maybe-uninitialized]
  args[1] = *a1;
            ^~~
Fixed by providing initial values.

Fixes: 9913fbb91df0 ("enic/base: common code")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/enic/base/vnic_dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index 7b3aed31a..49b36555b 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -752,7 +752,7 @@ int vnic_dev_soft_reset_done(struct vnic_dev *vdev, int *done)
 
 int vnic_dev_get_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)
 {
-	u64 a0, a1 = 0;
+	u64 a0 = 0, a1 = 0;
 	int wait = 1000;
 	int err, i;
 
@@ -1129,7 +1129,7 @@ int vnic_dev_set_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)
 int vnic_dev_classifier(struct vnic_dev *vdev, u8 cmd, u16 *entry,
 	struct filter_v2 *data, struct filter_action_v2 *action_v2)
 {
-	u64 a0, a1;
+	u64 a0 = 0, a1 = 0;
 	int wait = 1000;
 	dma_addr_t tlv_pa;
 	int ret = -EINVAL;
-- 
2.13.0

[dpdk-dev] [PATCH 4/5] net/mlx5: fix build with gcc 7.1

[Ferruh Yigit]

build error:
.../dpdk/drivers/net/mlx5/mlx5_fdir.c:
  In function ‘fdir_filter_to_flow_desc’:
.../dpdk/drivers/net/mlx5/mlx5_fdir.c:146:18:
 error: this statement may fall through [-Werror=implicit-fallthrough=]
   desc->dst_port = fdir_filter->input.flow.udp4_flow.dst_port;
   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.../dpdk/drivers/net/mlx5/mlx5_fdir.c:147:2: note: here
  case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
  ^~~~

Fixed by adding fallthrough comment to the code.

Fixes: 76f5c99e6840 ("mlx5: support flow director")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/mlx5/mlx5_fdir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/mlx5/mlx5_fdir.c b/drivers/net/mlx5/mlx5_fdir.c
index 1cff41bab..c8d47489f 100644
--- a/drivers/net/mlx5/mlx5_fdir.c
+++ b/drivers/net/mlx5/mlx5_fdir.c
@@ -144,6 +144,7 @@ fdir_filter_to_flow_desc(const struct rte_eth_fdir_filter *fdir_filter,
 	case RTE_ETH_FLOW_NONFRAG_IPV4_TCP:
 		desc->src_port = fdir_filter->input.flow.udp4_flow.src_port;
 		desc->dst_port = fdir_filter->input.flow.udp4_flow.dst_port;
+		/* fallthrough */
 	case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
 		desc->src_ip[0] = fdir_filter->input.flow.ip4_flow.src_ip;
 		desc->dst_ip[0] = fdir_filter->input.flow.ip4_flow.dst_ip;
-- 
2.13.0

[dpdk-dev] [PATCH 5/5] hash: fix icc build

[Ferruh Yigit]

build error with icc version 17.0.4 (gcc version 7.0.0 compatibility):

In file included from .../dpdk/lib/librte_hash/rte_fbk_hash.h(59),
                 from .../dpdk/lib/librte_hash/rte_fbk_hash.c(54):
.../dpdk/x86_64-native-linuxapp-icc/include/rte_hash_crc.h(480):
 error #1292: unknown attribute "fallthrough"
                __attribute__ ((fallthrough));
                                ^

In file included from .../dpdk/lib/librte_hash/rte_fbk_hash.h(59),
                 from .../dpdk/lib/librte_hash/rte_fbk_hash.c(54):
.../dpdk/x86_64-native-linuxapp-icc/include/rte_hash_crc.h(486):
 error #1292: unknown attribute "fallthrough"
                __attribute__ ((fallthrough));
                                ^
This code patch hit when gcc > 7 installed and ICC doesn't recognize
fallthrough attribute.

Fixed by disabling code when compiled with ICC.

Fixes: 3dfb9facb055 ("lib: add switch fall-through comments")

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
This can be merged with the patch fixes gcc build error.

Cc: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_hash/rte_hash_crc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
index 0f485b854..b8a0cbefb 100644
--- a/lib/librte_hash/rte_hash_crc.h
+++ b/lib/librte_hash/rte_hash_crc.h
@@ -476,13 +476,13 @@ rte_hash_crc_set_alg(uint8_t alg)
 	case CRC32_SSE42_x64:
 		if (! rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T))
 			alg = CRC32_SSE42;
-#if __GNUC__ >= 7
+#if __GNUC__ >= 7 && !defined(RTE_TOOLCHAIN_ICC)
 		__attribute__ ((fallthrough));
 #endif
 	case CRC32_SSE42:
 		if (! rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_2))
 			alg = CRC32_SW;
-#if __GNUC__ >= 7
+#if __GNUC__ >= 7 && !defined(RTE_TOOLCHAIN_ICC)
 		__attribute__ ((fallthrough));
 #endif
 #endif
-- 
2.13.0

Re: [dpdk-dev] [PATCH 2/5] net/i40e: fix memset size

[Xing, Beilei]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, June 14, 2017 12:42 AM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org;
> Xing, Beilei <beilei.xing@intel.com>
> Subject: [PATCH 2/5] net/i40e: fix memset size
> 
> This causes build error with gcc 7.1.1 :
> 
> ...dpdk/drivers/net/i40e/i40e_flow.c:2357:2:
> error: ‘memset’ used with length equal to number of elements without
>        multiplication by element size [-Werror=memset-elt-size]
>   memset(off_arr, 0, I40E_MAX_FLXPLD_FIED);
>   ^~~~~~
> 
> ...dpdk/drivers/net/i40e/i40e_flow.c:2358:2:
> error: ‘memset’ used with length equal to number of elements without
>        multiplication by element size [-Werror=memset-elt-size]
>   memset(len_arr, 0, I40E_MAX_FLXPLD_FIED);
>   ^~~~~~
> 
> Fixed by providing correct size to memset.
> 
> Fixes: 6ced3dd72f5f ("net/i40e: support flexible payload parsing for FDIR")
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> This code introduced for this release, so fix can be squashed into relevant
> patch.
> 
> Cc: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> c7589ce86..08c0f8e68 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -2354,8 +2354,8 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev
> *dev,
>  	uint16_t ether_type;
>  	int ret;
> 
> -	memset(off_arr, 0, I40E_MAX_FLXPLD_FIED);
> -	memset(len_arr, 0, I40E_MAX_FLXPLD_FIED);
> +	memset(off_arr, 0, sizeof(off_arr));
> +	memset(len_arr, 0, sizeof(len_arr));
>  	memset(flex_mask, 0, I40E_FDIR_MAX_FLEX_LEN);
>  	outer_tpid = i40e_get_outer_vlan(dev);
>  	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> --
> 2.13.0

Acked-by: Beilei Xing <beilei.xing@intel.com> , thanks.

Re: [dpdk-dev] [PATCH 4/5] net/mlx5: fix build with gcc 7.1

[Adrien Mazarguil]

On Tue, Jun 13, 2017 at 05:42:11PM +0100, Ferruh Yigit wrote:
> build error:
> .../dpdk/drivers/net/mlx5/mlx5_fdir.c:
>   In function ‘fdir_filter_to_flow_desc’:
> .../dpdk/drivers/net/mlx5/mlx5_fdir.c:146:18:
>  error: this statement may fall through [-Werror=implicit-fallthrough=]
>    desc->dst_port = fdir_filter->input.flow.udp4_flow.dst_port;
>    ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> .../dpdk/drivers/net/mlx5/mlx5_fdir.c:147:2: note: here
>   case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
>   ^~~~
> 
> Fixed by adding fallthrough comment to the code.
> 
> Fixes: 76f5c99e6840 ("mlx5: support flow director")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  drivers/net/mlx5/mlx5_fdir.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_fdir.c b/drivers/net/mlx5/mlx5_fdir.c
> index 1cff41bab..c8d47489f 100644
> --- a/drivers/net/mlx5/mlx5_fdir.c
> +++ b/drivers/net/mlx5/mlx5_fdir.c
> @@ -144,6 +144,7 @@ fdir_filter_to_flow_desc(const struct rte_eth_fdir_filter *fdir_filter,
>  	case RTE_ETH_FLOW_NONFRAG_IPV4_TCP:
>  		desc->src_port = fdir_filter->input.flow.udp4_flow.src_port;
>  		desc->dst_port = fdir_filter->input.flow.udp4_flow.dst_port;
> +		/* fallthrough */
>  	case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
>  		desc->src_ip[0] = fdir_filter->input.flow.ip4_flow.src_ip;
>  		desc->dst_ip[0] = fdir_filter->input.flow.ip4_flow.dst_ip;

That's a fine workaround, therefore:

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

On a personal note though, GCC, seriously? While I understand missing breaks
inside switch statements is a common mistake, having the compiler rely on a
comment that basically says "yes, it's done on purpose" just doesn't feel
right; compilers should discard comments and not attempt to interpret them,
otherwise they have to deal with locale-related issues. It's like enforcing
checkpatch.pl rules in GCC by default. Anyway, back to work.

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH 1/5] kni: fix build with gcc 7.1

[Thomas Monjalon]

Series applied, thanks