[dpdk-dev] net/mlx4: fix undefined behavior of RSS conversion

Message ID 20180522112533.2785-1-adrien.mazarguil@6wind.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Adrien Mazarguil May 22, 2018, 11:26 a.m. UTC
  As reported by ICC, an array initializer that uses values found in the
array being initialized, although semantically correct (GCC and clang do
not complain and generate correct code), results in undefined behavior
since initialization order is itself undefined.

This patch restores the static keyword and initializes array entries with
constant expressions as a safety measure.

Fixes: f76ccd763422 ("net/mlx4: refactor RSS conversion functions")

Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

--

Shahaf, Ferruh, please remove the "Note the loss of the [...]" paragraph of
the original commit if this patch gets squashed in it, as it is no longer
relevant in that case.
---
 drivers/net/mlx4/mlx4_flow.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)
  

Comments

Ferruh Yigit May 22, 2018, 1:01 p.m. UTC | #1
On 5/22/2018 12:26 PM, Adrien Mazarguil wrote:
> As reported by ICC, an array initializer that uses values found in the
> array being initialized, although semantically correct (GCC and clang do
> not complain and generate correct code), results in undefined behavior
> since initialization order is itself undefined.
> 
> This patch restores the static keyword and initializes array entries with
> constant expressions as a safety measure.
> 
> Fixes: f76ccd763422 ("net/mlx4: refactor RSS conversion functions")
> 
> Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> --
> 
> Shahaf, Ferruh, please remove the "Note the loss of the [...]" paragraph of
> the original commit if this patch gets squashed in it, as it is no longer
> relevant in that case.

Squashed into relevant commit in next-net, thanks.

Can you please confirm updated commit log?
  
Adrien Mazarguil May 22, 2018, 1:21 p.m. UTC | #2
On Tue, May 22, 2018 at 02:01:38PM +0100, Ferruh Yigit wrote:
> On 5/22/2018 12:26 PM, Adrien Mazarguil wrote:
> > As reported by ICC, an array initializer that uses values found in the
> > array being initialized, although semantically correct (GCC and clang do
> > not complain and generate correct code), results in undefined behavior
> > since initialization order is itself undefined.
> > 
> > This patch restores the static keyword and initializes array entries with
> > constant expressions as a safety measure.
> > 
> > Fixes: f76ccd763422 ("net/mlx4: refactor RSS conversion functions")
> > 
> > Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > 
> > --
> > 
> > Shahaf, Ferruh, please remove the "Note the loss of the [...]" paragraph of
> > the original commit if this patch gets squashed in it, as it is no longer
> > relevant in that case.
> 
> Squashed into relevant commit in next-net, thanks.
> 
> Can you please confirm updated commit log?

Looks good, thanks!
  

Patch

diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index ddb7108dc..b40e7e5c3 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -103,6 +103,12 @@  mlx4_conv_rss_types(struct priv *priv, uint64_t types, int verbs_to_dpdk)
 		TCP, UDP,
 		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_TCP_1, IPV6_UDP, IPV6_UDP_1,
 	};
+	enum {
+		VERBS_IPV4 = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
+		VERBS_IPV6 = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6,
+		VERBS_TCP = IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP,
+		VERBS_UDP = IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_DST_PORT_UDP,
+	};
 	static const uint64_t dpdk[] = {
 		[INNER] = 0,
 		[IPV4] = ETH_RSS_IPV4,
@@ -121,23 +127,23 @@  mlx4_conv_rss_types(struct priv *priv, uint64_t types, int verbs_to_dpdk)
 		[IPV6_UDP] = ETH_RSS_NONFRAG_IPV6_UDP,
 		[IPV6_UDP_1] = ETH_RSS_IPV6_UDP_EX,
 	};
-	const uint64_t verbs[RTE_DIM(dpdk)] = {
+	static const uint64_t verbs[RTE_DIM(dpdk)] = {
 		[INNER] = IBV_RX_HASH_INNER,
-		[IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
-		[IPV4_1] = verbs[IPV4],
-		[IPV4_2] = verbs[IPV4],
-		[IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6,
-		[IPV6_1] = verbs[IPV6],
-		[IPV6_2] = verbs[IPV6],
-		[IPV6_3] = verbs[IPV6],
-		[TCP] = IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP,
-		[UDP] = IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_DST_PORT_UDP,
-		[IPV4_TCP] = verbs[IPV4] | verbs[TCP],
-		[IPV4_UDP] = verbs[IPV4] | verbs[UDP],
-		[IPV6_TCP] = verbs[IPV6] | verbs[TCP],
-		[IPV6_TCP_1] = verbs[IPV6_TCP],
-		[IPV6_UDP] = verbs[IPV6] | verbs[UDP],
-		[IPV6_UDP_1] = verbs[IPV6_UDP],
+		[IPV4] = VERBS_IPV4,
+		[IPV4_1] = VERBS_IPV4,
+		[IPV4_2] = VERBS_IPV4,
+		[IPV6] = VERBS_IPV6,
+		[IPV6_1] = VERBS_IPV6,
+		[IPV6_2] = VERBS_IPV6,
+		[IPV6_3] = VERBS_IPV6,
+		[TCP] = VERBS_TCP,
+		[UDP] = VERBS_UDP,
+		[IPV4_TCP] = VERBS_IPV4 | VERBS_TCP,
+		[IPV4_UDP] = VERBS_IPV4 | VERBS_UDP,
+		[IPV6_TCP] = VERBS_IPV6 | VERBS_TCP,
+		[IPV6_TCP_1] = VERBS_IPV6 | VERBS_TCP,
+		[IPV6_UDP] = VERBS_IPV6 | VERBS_UDP,
+		[IPV6_UDP_1] = VERBS_IPV6 | VERBS_UDP,
 	};
 	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
 	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;