[dpdk-dev] [PATCH 1/2] net/mlx4: fix inadequate default in RSS converter

[dpdk-dev] [PATCH 1/2] net/mlx4: fix inadequate default in RSS converter

[Adrien Mazarguil]

Below commit documents 0 as a value standing for a default set of RSS hash
types, however the mlx4 PMD doesn't interpret it correctly and still uses
its own internal special value for that (-1).

Also, its function prototype was not updated.

Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
Fixes: 1d173da83ef2 ("net/mlx4: fix default RSS hash fields")

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

--

This is a rework of "net/mlx4: fix useless default in RSS converter" [1].

Not a candidate for stable anymore since DPDK 18.02 does not include the
flow API RSS rework.

[1] http://dpdk.org/ml/archives/dev/2018-May/100285.html
---
 drivers/net/mlx4/mlx4_flow.c | 6 +++---
 drivers/net/mlx4/mlx4_flow.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index 202779f7d..ebc9eeb8b 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -114,7 +114,7 @@ struct mlx4_drop {
  * Convert DPDK RSS hash types to their Verbs equivalent.
  *
  * This function returns the supported (default) set when @p types has
- * special value (uint64_t)-1.
+ * special value 0.
  *
  * @param priv
  *   Pointer to private structure.
@@ -160,7 +160,7 @@ mlx4_conv_rss_types(struct priv *priv, uint64_t types)
 	uint64_t conv = 0;
 	unsigned int i;
 
-	if (types == (uint64_t)-1)
+	if (!types)
 		return priv->hw_rss_sup;
 	for (i = 0; i != RTE_DIM(in); ++i)
 		if (types & in[i]) {
@@ -1384,7 +1384,7 @@ mlx4_flow_internal(struct priv *priv, struct rte_flow_error *error)
 	struct rte_flow_action_rss action_rss = {
 		.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
 		.level = 0,
-		.types = -1,
+		.types = 0,
 		.key_len = MLX4_RSS_HASH_KEY_SIZE,
 		.queue_num = queues,
 		.key = mlx4_rss_hash_key_default,
diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
index d1f1611eb..2e82903bd 100644
--- a/drivers/net/mlx4/mlx4_flow.h
+++ b/drivers/net/mlx4/mlx4_flow.h
@@ -48,7 +48,7 @@ struct rte_flow {
 
 /* mlx4_flow.c */
 
-uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t rss_hf);
+uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types);
 uint64_t mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
 int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);
 void mlx4_flow_clean(struct priv *priv);
-- 
2.11.0

[dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions

[Adrien Mazarguil]

Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
functions"), this PMD includes two similar-looking functions that convert
RSS hash fields between Verbs and DPDK formats.

This patch refactors them as a single two-way function and gets rid of
redundant helper macros.

Note the loss of the "static" keyword on the out[] (now verbs[]) array
introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations in
function") is what prevents the reliance on macro definitions for static
initializers at the expense of a few extra instructions. An acceptable
trade-off given this function is not involved in data plane operations.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/net/mlx4/mlx4_ethdev.c |   3 +-
 drivers/net/mlx4/mlx4_flow.c   | 147 ++++++++++--------------------------
 drivers/net/mlx4/mlx4_flow.h   |   4 +-
 3 files changed, 44 insertions(+), 110 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index 0a9c2e2f5..30deb3ef0 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 			ETH_LINK_SPEED_20G |
 			ETH_LINK_SPEED_40G |
 			ETH_LINK_SPEED_56G;
-	info->flow_type_rss_offloads =
-		mlx4_ibv_to_rss_types(priv->hw_rss_sup);
+	info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);
 }
 
 /**
diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index ebc9eeb8b..90d206b55 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -42,41 +42,6 @@
 #include "mlx4_rxtx.h"
 #include "mlx4_utils.h"
 
-/** IBV supported RSS hash functions combinations */
-#define MLX4_IBV_IPV4_HF ( \
-	IBV_RX_HASH_SRC_IPV4 | \
-	IBV_RX_HASH_DST_IPV4)
-#define MLX4_IBV_IPV6_HF ( \
-	IBV_RX_HASH_SRC_IPV6 | \
-	IBV_RX_HASH_DST_IPV6)
-#define MLX4_IBV_TCP_HF ( \
-	IBV_RX_HASH_SRC_PORT_TCP | \
-	IBV_RX_HASH_DST_PORT_TCP)
-#define MLX4_IBV_UDP_HF ( \
-	IBV_RX_HASH_SRC_PORT_UDP | \
-	IBV_RX_HASH_DST_PORT_UDP)
-
-/** Supported RSS hash functions combinations */
-#define MLX4_RSS_IPV4_HF ( \
-	ETH_RSS_IPV4 | \
-	ETH_RSS_FRAG_IPV4 | \
-	ETH_RSS_NONFRAG_IPV4_OTHER)
-#define MLX4_RSS_IPV6_HF ( \
-	ETH_RSS_IPV6 | \
-	ETH_RSS_FRAG_IPV6 | \
-	ETH_RSS_NONFRAG_IPV6_OTHER | \
-	ETH_RSS_IPV6_EX)
-#define MLX4_RSS_IPV4_TCP_HF ( \
-	ETH_RSS_NONFRAG_IPV4_TCP)
-#define MLX4_RSS_IPV6_TCP_HF ( \
-	ETH_RSS_NONFRAG_IPV6_TCP | \
-	ETH_RSS_IPV6_TCP_EX)
-#define MLX4_RSS_IPV4_UDP_HF ( \
-	ETH_RSS_NONFRAG_IPV4_UDP)
-#define MLX4_RSS_IPV6_UDP_HF ( \
-	ETH_RSS_NONFRAG_IPV6_UDP | \
-	ETH_RSS_IPV6_UDP_EX)
-
 /** Static initializer for a list of subsequent item types. */
 #define NEXT_ITEM(...) \
 	(const enum rte_flow_item_type []){ \
@@ -111,7 +76,7 @@ struct mlx4_drop {
 };
 
 /**
- * Convert DPDK RSS hash types to their Verbs equivalent.
+ * Convert supported RSS hash field types between DPDK and Verbs formats.
  *
  * This function returns the supported (default) set when @p types has
  * special value 0.
@@ -119,106 +84,76 @@ struct mlx4_drop {
  * @param priv
  *   Pointer to private structure.
  * @param types
- *   Hash types in DPDK format (see struct rte_eth_rss_conf).
+ *   Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
+ *   rte_eth_rss_conf) or Verbs format.
+ * @param verbs_to_dpdk
+ *   A zero value converts @p types from DPDK to Verbs, a nonzero value
+ *   performs the reverse operation.
  *
  * @return
- *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
- *   otherwise and rte_errno is set.
+ *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
+ *   rte_errno is set.
  */
 uint64_t
-mlx4_conv_rss_types(struct priv *priv, uint64_t types)
+mlx4_conv_rss_types(struct priv *priv, uint64_t types, int verbs_to_dpdk)
 {
-	enum { IPV4, IPV6, TCP, UDP, };
-	static const uint64_t in[] = {
+	enum {
+		INNER, IPV4, IPV6, TCP, UDP,
+		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
+	};
+	static const uint64_t dpdk[] = {
+		[INNER] = 0,
 		[IPV4] = (ETH_RSS_IPV4 |
 			  ETH_RSS_FRAG_IPV4 |
-			  ETH_RSS_NONFRAG_IPV4_TCP |
-			  ETH_RSS_NONFRAG_IPV4_UDP |
 			  ETH_RSS_NONFRAG_IPV4_OTHER),
 		[IPV6] = (ETH_RSS_IPV6 |
 			  ETH_RSS_FRAG_IPV6 |
-			  ETH_RSS_NONFRAG_IPV6_TCP |
-			  ETH_RSS_NONFRAG_IPV6_UDP |
 			  ETH_RSS_NONFRAG_IPV6_OTHER |
-			  ETH_RSS_IPV6_EX |
-			  ETH_RSS_IPV6_TCP_EX |
-			  ETH_RSS_IPV6_UDP_EX),
-		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
-			 ETH_RSS_NONFRAG_IPV6_TCP |
-			 ETH_RSS_IPV6_TCP_EX),
-		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
-			 ETH_RSS_NONFRAG_IPV6_UDP |
-			 ETH_RSS_IPV6_UDP_EX),
+			  ETH_RSS_IPV6_EX),
+		[TCP] = 0,
+		[UDP] = 0,
+		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
+		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
+		[IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
+			      ETH_RSS_IPV6_TCP_EX),
+		[IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
+			      ETH_RSS_IPV6_UDP_EX),
 	};
-	static const uint64_t out[RTE_DIM(in)] = {
+	const uint64_t verbs[RTE_DIM(dpdk)] = {
+		[INNER] = IBV_RX_HASH_INNER,
 		[IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
 		[IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_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_UDP] = verbs[IPV6] | verbs[UDP],
 	};
+	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
+	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
 	uint64_t seen = 0;
 	uint64_t conv = 0;
 	unsigned int i;
 
-	if (!types)
-		return priv->hw_rss_sup;
-	for (i = 0; i != RTE_DIM(in); ++i)
+	if (!types) {
+		if (!verbs_to_dpdk)
+			return priv->hw_rss_sup;
+		types = priv->hw_rss_sup;
+	}
+	for (i = 0; i != RTE_DIM(dpdk); ++i)
 		if (types & in[i]) {
 			seen |= types & in[i];
 			conv |= out[i];
 		}
-	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
+	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
+	    !(types & ~seen))
 		return conv;
 	rte_errno = ENOTSUP;
 	return (uint64_t)-1;
 }
 
 /**
- * Convert verbs RSS types to their DPDK equivalents.
- *
- * This function returns a group of RSS DPDK types given their equivalent group
- * of verbs types.
- * For example both source IPv4 and destination IPv4 verbs types are converted
- * into their equivalent RSS group types. If each of these verbs types existed
- * exclusively - no conversion would take place.
- *
- * @param types
- *   RSS hash types in verbs format.
- *
- * @return
- *   DPDK RSS hash fields supported by mlx4.
- */
-uint64_t
-mlx4_ibv_to_rss_types(uint64_t types)
-{
-	enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
-
-	static const uint64_t in[] = {
-		[IPV4] = MLX4_IBV_IPV4_HF,
-		[IPV6] = MLX4_IBV_IPV6_HF,
-		[IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
-		[IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
-		[IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
-		[IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
-	};
-	static const uint64_t out[RTE_DIM(in)] = {
-		[IPV4] = MLX4_RSS_IPV4_HF,
-		[IPV6] = MLX4_RSS_IPV6_HF,
-		[IPV4_TCP] = MLX4_RSS_IPV4_HF | MLX4_RSS_IPV4_TCP_HF,
-		[IPV6_TCP] = MLX4_RSS_IPV6_HF | MLX4_RSS_IPV6_TCP_HF,
-		[IPV4_UDP] = MLX4_RSS_IPV4_HF | MLX4_RSS_IPV4_UDP_HF,
-		[IPV6_UDP] = MLX4_RSS_IPV6_HF | MLX4_RSS_IPV6_UDP_HF,
-	};
-	uint64_t conv = 0;
-	unsigned int i;
-
-	for (i = 0; i != RTE_DIM(in); ++i)
-		if ((types & in[i]) == in[i])
-			conv |= out[i];
-	return conv;
-}
-
-/**
  * Merge Ethernet pattern item into flow rule handle.
  *
  * Additional mlx4-specific constraints on supported fields:
@@ -890,7 +825,7 @@ mlx4_flow_prepare(struct priv *priv,
 				goto exit_action_not_supported;
 			}
 			rte_errno = 0;
-			fields = mlx4_conv_rss_types(priv, rss->types);
+			fields = mlx4_conv_rss_types(priv, rss->types, 0);
 			if (fields == (uint64_t)-1 && rte_errno) {
 				msg = "unsupported RSS hash type requested";
 				goto exit_action_not_supported;
diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
index 2e82903bd..2917ebe95 100644
--- a/drivers/net/mlx4/mlx4_flow.h
+++ b/drivers/net/mlx4/mlx4_flow.h
@@ -48,8 +48,8 @@ struct rte_flow {
 
 /* mlx4_flow.c */
 
-uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types);
-uint64_t mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
+uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
+			     int verbs_to_dpdk);
 int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);
 void mlx4_flow_clean(struct priv *priv);
 int mlx4_filter_ctrl(struct rte_eth_dev *dev,
-- 
2.11.0

Re: [dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions

[Ophir Munk]

Hi Adrien,
I like the idea of having one conversion function from dpdk to verbs and vice versa.
I have some comments inline regarding the implementation.

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, May 15, 2018 6:51 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com>
> Subject: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> 
> Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> functions"), this PMD includes two similar-looking functions that convert RSS
> hash fields between Verbs and DPDK formats.
> 
> This patch refactors them as a single two-way function and gets rid of
> redundant helper macros.
> 
> Note the loss of the "static" keyword on the out[] (now verbs[]) array
> introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations
> in
> function") is what prevents the reliance on macro definitions for static
> initializers at the expense of a few extra instructions. An acceptable trade-off
> given this function is not involved in data plane operations.
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Ophir Munk <ophirmu@mellanox.com>
> ---
>  drivers/net/mlx4/mlx4_ethdev.c |   3 +-
>  drivers/net/mlx4/mlx4_flow.c   | 147 ++++++++++--------------------------
>  drivers/net/mlx4/mlx4_flow.h   |   4 +-
>  3 files changed, 44 insertions(+), 110 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4_ethdev.c
> b/drivers/net/mlx4/mlx4_ethdev.c index 0a9c2e2f5..30deb3ef0 100644
> --- a/drivers/net/mlx4/mlx4_ethdev.c
> +++ b/drivers/net/mlx4/mlx4_ethdev.c
> @@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *info)
>  			ETH_LINK_SPEED_20G |
>  			ETH_LINK_SPEED_40G |
>  			ETH_LINK_SPEED_56G;
> -	info->flow_type_rss_offloads =
> -		mlx4_ibv_to_rss_types(priv->hw_rss_sup);
> +	info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);

Calling with parameters (priv, 0, 1) parameters in not as intuitive as calling with (priv, DEFAULT_RSS, DPDK_TO_VERBS)
Would you consider using definitions for the 0, 1 parameters to increase readability?

>  }
> 
>  /**
> diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> index ebc9eeb8b..90d206b55 100644
> --- a/drivers/net/mlx4/mlx4_flow.c
> +++ b/drivers/net/mlx4/mlx4_flow.c
> @@ -42,41 +42,6 @@
>  #include "mlx4_rxtx.h"
>  #include "mlx4_utils.h"
> 
> -/** IBV supported RSS hash functions combinations */ -#define
> MLX4_IBV_IPV4_HF ( \
> -	IBV_RX_HASH_SRC_IPV4 | \
> -	IBV_RX_HASH_DST_IPV4)
> -#define MLX4_IBV_IPV6_HF ( \
> -	IBV_RX_HASH_SRC_IPV6 | \
> -	IBV_RX_HASH_DST_IPV6)
> -#define MLX4_IBV_TCP_HF ( \
> -	IBV_RX_HASH_SRC_PORT_TCP | \
> -	IBV_RX_HASH_DST_PORT_TCP)
> -#define MLX4_IBV_UDP_HF ( \
> -	IBV_RX_HASH_SRC_PORT_UDP | \
> -	IBV_RX_HASH_DST_PORT_UDP)
> -
> -/** Supported RSS hash functions combinations */ -#define
> MLX4_RSS_IPV4_HF ( \
> -	ETH_RSS_IPV4 | \
> -	ETH_RSS_FRAG_IPV4 | \
> -	ETH_RSS_NONFRAG_IPV4_OTHER)
> -#define MLX4_RSS_IPV6_HF ( \
> -	ETH_RSS_IPV6 | \
> -	ETH_RSS_FRAG_IPV6 | \
> -	ETH_RSS_NONFRAG_IPV6_OTHER | \
> -	ETH_RSS_IPV6_EX)
> -#define MLX4_RSS_IPV4_TCP_HF ( \
> -	ETH_RSS_NONFRAG_IPV4_TCP)
> -#define MLX4_RSS_IPV6_TCP_HF ( \
> -	ETH_RSS_NONFRAG_IPV6_TCP | \
> -	ETH_RSS_IPV6_TCP_EX)
> -#define MLX4_RSS_IPV4_UDP_HF ( \
> -	ETH_RSS_NONFRAG_IPV4_UDP)
> -#define MLX4_RSS_IPV6_UDP_HF ( \
> -	ETH_RSS_NONFRAG_IPV6_UDP | \
> -	ETH_RSS_IPV6_UDP_EX)
> -
>  /** Static initializer for a list of subsequent item types. */  #define
> NEXT_ITEM(...) \
>  	(const enum rte_flow_item_type []){ \
> @@ -111,7 +76,7 @@ struct mlx4_drop {
>  };
> 
>  /**
> - * Convert DPDK RSS hash types to their Verbs equivalent.
> + * Convert supported RSS hash field types between DPDK and Verbs
> formats.
>   *
>   * This function returns the supported (default) set when @p types has
>   * special value 0.
> @@ -119,106 +84,76 @@ struct mlx4_drop {
>   * @param priv
>   *   Pointer to private structure.
>   * @param types
> - *   Hash types in DPDK format (see struct rte_eth_rss_conf).
> + *   Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
> + *   rte_eth_rss_conf) or Verbs format.
> + * @param verbs_to_dpdk
> + *   A zero value converts @p types from DPDK to Verbs, a nonzero value
> + *   performs the reverse operation.
>   *
>   * @return
> - *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> - *   otherwise and rte_errno is set.
> + *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> + *   rte_errno is set.
>   */
>  uint64_t
> -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> +verbs_to_dpdk)
>  {
> -	enum { IPV4, IPV6, TCP, UDP, };
> -	static const uint64_t in[] = {
> +	enum {
> +		INNER, IPV4, IPV6, TCP, UDP,
> +		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> +	};
> +	static const uint64_t dpdk[] = {
> +		[INNER] = 0,
>  		[IPV4] = (ETH_RSS_IPV4 |
>  			  ETH_RSS_FRAG_IPV4 |
> -			  ETH_RSS_NONFRAG_IPV4_TCP |
> -			  ETH_RSS_NONFRAG_IPV4_UDP |
>  			  ETH_RSS_NONFRAG_IPV4_OTHER),
>  		[IPV6] = (ETH_RSS_IPV6 |
>  			  ETH_RSS_FRAG_IPV6 |
> -			  ETH_RSS_NONFRAG_IPV6_TCP |
> -			  ETH_RSS_NONFRAG_IPV6_UDP |
>  			  ETH_RSS_NONFRAG_IPV6_OTHER |
> -			  ETH_RSS_IPV6_EX |
> -			  ETH_RSS_IPV6_TCP_EX |
> -			  ETH_RSS_IPV6_UDP_EX),
> -		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> -			 ETH_RSS_NONFRAG_IPV6_TCP |
> -			 ETH_RSS_IPV6_TCP_EX),
> -		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> -			 ETH_RSS_NONFRAG_IPV6_UDP |
> -			 ETH_RSS_IPV6_UDP_EX),
> +			  ETH_RSS_IPV6_EX),
> +		[TCP] = 0,
> +		[UDP] = 0,
> +		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> +		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> +		[IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> +			      ETH_RSS_IPV6_TCP_EX),
> +		[IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> +			      ETH_RSS_IPV6_UDP_EX),
>  	};
> -	static const uint64_t out[RTE_DIM(in)] = {
> +	const uint64_t verbs[RTE_DIM(dpdk)] = {
> +		[INNER] = IBV_RX_HASH_INNER,
>  		[IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
>  		[IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_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_UDP] = verbs[IPV6] | verbs[UDP],
>  	};
> +	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> +	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
>  	uint64_t seen = 0;
>  	uint64_t conv = 0;
>  	unsigned int i;
> 
> -	if (!types)
> -		return priv->hw_rss_sup;
> -	for (i = 0; i != RTE_DIM(in); ++i)
> +	if (!types) {
> +		if (!verbs_to_dpdk)
> +			return priv->hw_rss_sup;
> +		types = priv->hw_rss_sup;
> +	}
> +	for (i = 0; i != RTE_DIM(dpdk); ++i)
>  		if (types & in[i]) {
>  			seen |= types & in[i];
>  			conv |= out[i];
>  		}
> -	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> +	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> +	    !(types & ~seen))
>  		return conv;
>  	rte_errno = ENOTSUP;
>  	return (uint64_t)-1;
>  }
> 

1. I wonder if [INNER] case is correct. Assuming we are given IBV_RX_HASH_INNER I think we should return with error ENOTSUP.
However in the implementation above we return 0 as a valid DPDK RSS value.
Returning a 0 value is also confusing since it is used in dpdk as "Best effort" RSS HF.

2. Assuming we are given = IBV_RX_HASH_SRC_IPV4 alone (without IBV_RX_HASH_DST_IPV4) I think we should return with error ENOSUP since mlx4 does not support a hash based on SRC IPV4 alone. However in the implementation above we return a valid answer of all the flags from [IPV4], [IPV4_TCP] and [IPV4_UDP]. Can you please explain this logic?
The same question repeats for all other "_DST_" only or "_SRC_" only alone constants.

3. Given "IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP" alone we are returning a DPDK reply which includes TCP flags + all IPV4 and IPV6 flags.  Can you please explain this logic?

As mentioned I like the idea of one function to transform either direction between verbs and dpdk but please explain the current implementation following the questions above.

>  /**
> - * Convert verbs RSS types to their DPDK equivalents.
> - *
> - * This function returns a group of RSS DPDK types given their equivalent
> group
> - * of verbs types.
> - * For example both source IPv4 and destination IPv4 verbs types are
> converted
> - * into their equivalent RSS group types. If each of these verbs types existed
> - * exclusively - no conversion would take place.
> - *
> - * @param types
> - *   RSS hash types in verbs format.
> - *
> - * @return
> - *   DPDK RSS hash fields supported by mlx4.
> - */
> -uint64_t
> -mlx4_ibv_to_rss_types(uint64_t types)
> -{
> -	enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
> -
> -	static const uint64_t in[] = {
> -		[IPV4] = MLX4_IBV_IPV4_HF,
> -		[IPV6] = MLX4_IBV_IPV6_HF,
> -		[IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
> -		[IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
> -		[IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
> -		[IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
> -	};
> -	static const uint64_t out[RTE_DIM(in)] = {
> -		[IPV4] = MLX4_RSS_IPV4_HF,
> -		[IPV6] = MLX4_RSS_IPV6_HF,
> -		[IPV4_TCP] = MLX4_RSS_IPV4_HF |
> MLX4_RSS_IPV4_TCP_HF,
> -		[IPV6_TCP] = MLX4_RSS_IPV6_HF |
> MLX4_RSS_IPV6_TCP_HF,
> -		[IPV4_UDP] = MLX4_RSS_IPV4_HF |
> MLX4_RSS_IPV4_UDP_HF,
> -		[IPV6_UDP] = MLX4_RSS_IPV6_HF |
> MLX4_RSS_IPV6_UDP_HF,
> -	};
> -	uint64_t conv = 0;
> -	unsigned int i;
> -
> -	for (i = 0; i != RTE_DIM(in); ++i)
> -		if ((types & in[i]) == in[i])
> -			conv |= out[i];
> -	return conv;
> -}
> -
> -/**
>   * Merge Ethernet pattern item into flow rule handle.
>   *
>   * Additional mlx4-specific constraints on supported fields:
> @@ -890,7 +825,7 @@ mlx4_flow_prepare(struct priv *priv,
>  				goto exit_action_not_supported;
>  			}
>  			rte_errno = 0;
> -			fields = mlx4_conv_rss_types(priv, rss->types);
> +			fields = mlx4_conv_rss_types(priv, rss->types, 0);
>  			if (fields == (uint64_t)-1 && rte_errno) {
>  				msg = "unsupported RSS hash type
> requested";
>  				goto exit_action_not_supported;
> diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
> index 2e82903bd..2917ebe95 100644
> --- a/drivers/net/mlx4/mlx4_flow.h
> +++ b/drivers/net/mlx4/mlx4_flow.h
> @@ -48,8 +48,8 @@ struct rte_flow {
> 
>  /* mlx4_flow.c */
> 
> -uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types); -uint64_t
> mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
> +uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
> +			     int verbs_to_dpdk);
>  int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);  void
> mlx4_flow_clean(struct priv *priv);  int mlx4_filter_ctrl(struct rte_eth_dev
> *dev,
> --
> 2.11.0

Re: [dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions

[Adrien Mazarguil]

Hi Ophir,

On Thu, May 17, 2018 at 05:46:45PM +0000, Ophir Munk wrote:
> Hi Adrien,
> I like the idea of having one conversion function from dpdk to verbs and vice versa.
> I have some comments inline regarding the implementation.
> 
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Tuesday, May 15, 2018 6:51 PM
> > To: Shahaf Shuler <shahafs@mellanox.com>
> > Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com>
> > Subject: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> > 
> > Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> > functions"), this PMD includes two similar-looking functions that convert RSS
> > hash fields between Verbs and DPDK formats.
> > 
> > This patch refactors them as a single two-way function and gets rid of
> > redundant helper macros.
> > 
> > Note the loss of the "static" keyword on the out[] (now verbs[]) array
> > introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations
> > in
> > function") is what prevents the reliance on macro definitions for static
> > initializers at the expense of a few extra instructions. An acceptable trade-off
> > given this function is not involved in data plane operations.
> > 
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Cc: Ophir Munk <ophirmu@mellanox.com>
> > ---
> >  drivers/net/mlx4/mlx4_ethdev.c |   3 +-
> >  drivers/net/mlx4/mlx4_flow.c   | 147 ++++++++++--------------------------
> >  drivers/net/mlx4/mlx4_flow.h   |   4 +-
> >  3 files changed, 44 insertions(+), 110 deletions(-)
> > 
> > diff --git a/drivers/net/mlx4/mlx4_ethdev.c
> > b/drivers/net/mlx4/mlx4_ethdev.c index 0a9c2e2f5..30deb3ef0 100644
> > --- a/drivers/net/mlx4/mlx4_ethdev.c
> > +++ b/drivers/net/mlx4/mlx4_ethdev.c
> > @@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct
> > rte_eth_dev_info *info)
> >  			ETH_LINK_SPEED_20G |
> >  			ETH_LINK_SPEED_40G |
> >  			ETH_LINK_SPEED_56G;
> > -	info->flow_type_rss_offloads =
> > -		mlx4_ibv_to_rss_types(priv->hw_rss_sup);
> > +	info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);
> 
> Calling with parameters (priv, 0, 1) parameters in not as intuitive as calling with (priv, DEFAULT_RSS, DPDK_TO_VERBS)
> Would you consider using definitions for the 0, 1 parameters to increase readability?

Doing so would require either new macro or enum definitions, which I would
like to avoid for such a minor internal function.

Keep in mind the 0 value standing for "defaults" RSS types is both publicly
documented [1] and documented again in mlx4_conv_rss_types().

As for DPDK_TO_VERBS, this is already the name of the parameter (int
dpdk_to_verbs), currently a boolean value. I would have defined a type if
there were more than two possible states.

[1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#action-rss

> 
> >  }
> > 
> >  /**
> > diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> > index ebc9eeb8b..90d206b55 100644
> > --- a/drivers/net/mlx4/mlx4_flow.c
> > +++ b/drivers/net/mlx4/mlx4_flow.c
> > @@ -42,41 +42,6 @@
> >  #include "mlx4_rxtx.h"
> >  #include "mlx4_utils.h"
> > 
> > -/** IBV supported RSS hash functions combinations */ -#define
> > MLX4_IBV_IPV4_HF ( \
> > -	IBV_RX_HASH_SRC_IPV4 | \
> > -	IBV_RX_HASH_DST_IPV4)
> > -#define MLX4_IBV_IPV6_HF ( \
> > -	IBV_RX_HASH_SRC_IPV6 | \
> > -	IBV_RX_HASH_DST_IPV6)
> > -#define MLX4_IBV_TCP_HF ( \
> > -	IBV_RX_HASH_SRC_PORT_TCP | \
> > -	IBV_RX_HASH_DST_PORT_TCP)
> > -#define MLX4_IBV_UDP_HF ( \
> > -	IBV_RX_HASH_SRC_PORT_UDP | \
> > -	IBV_RX_HASH_DST_PORT_UDP)
> > -
> > -/** Supported RSS hash functions combinations */ -#define
> > MLX4_RSS_IPV4_HF ( \
> > -	ETH_RSS_IPV4 | \
> > -	ETH_RSS_FRAG_IPV4 | \
> > -	ETH_RSS_NONFRAG_IPV4_OTHER)
> > -#define MLX4_RSS_IPV6_HF ( \
> > -	ETH_RSS_IPV6 | \
> > -	ETH_RSS_FRAG_IPV6 | \
> > -	ETH_RSS_NONFRAG_IPV6_OTHER | \
> > -	ETH_RSS_IPV6_EX)
> > -#define MLX4_RSS_IPV4_TCP_HF ( \
> > -	ETH_RSS_NONFRAG_IPV4_TCP)
> > -#define MLX4_RSS_IPV6_TCP_HF ( \
> > -	ETH_RSS_NONFRAG_IPV6_TCP | \
> > -	ETH_RSS_IPV6_TCP_EX)
> > -#define MLX4_RSS_IPV4_UDP_HF ( \
> > -	ETH_RSS_NONFRAG_IPV4_UDP)
> > -#define MLX4_RSS_IPV6_UDP_HF ( \
> > -	ETH_RSS_NONFRAG_IPV6_UDP | \
> > -	ETH_RSS_IPV6_UDP_EX)
> > -
> >  /** Static initializer for a list of subsequent item types. */  #define
> > NEXT_ITEM(...) \
> >  	(const enum rte_flow_item_type []){ \
> > @@ -111,7 +76,7 @@ struct mlx4_drop {
> >  };
> > 
> >  /**
> > - * Convert DPDK RSS hash types to their Verbs equivalent.
> > + * Convert supported RSS hash field types between DPDK and Verbs
> > formats.
> >   *
> >   * This function returns the supported (default) set when @p types has
> >   * special value 0.
> > @@ -119,106 +84,76 @@ struct mlx4_drop {
> >   * @param priv
> >   *   Pointer to private structure.
> >   * @param types
> > - *   Hash types in DPDK format (see struct rte_eth_rss_conf).
> > + *   Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
> > + *   rte_eth_rss_conf) or Verbs format.
> > + * @param verbs_to_dpdk
> > + *   A zero value converts @p types from DPDK to Verbs, a nonzero value
> > + *   performs the reverse operation.
> >   *
> >   * @return
> > - *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > - *   otherwise and rte_errno is set.
> > + *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > + *   rte_errno is set.
> >   */
> >  uint64_t
> > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > +verbs_to_dpdk)
> >  {
> > -	enum { IPV4, IPV6, TCP, UDP, };
> > -	static const uint64_t in[] = {
> > +	enum {
> > +		INNER, IPV4, IPV6, TCP, UDP,
> > +		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > +	};
> > +	static const uint64_t dpdk[] = {
> > +		[INNER] = 0,
> >  		[IPV4] = (ETH_RSS_IPV4 |
> >  			  ETH_RSS_FRAG_IPV4 |
> > -			  ETH_RSS_NONFRAG_IPV4_TCP |
> > -			  ETH_RSS_NONFRAG_IPV4_UDP |
> >  			  ETH_RSS_NONFRAG_IPV4_OTHER),
> >  		[IPV6] = (ETH_RSS_IPV6 |
> >  			  ETH_RSS_FRAG_IPV6 |
> > -			  ETH_RSS_NONFRAG_IPV6_TCP |
> > -			  ETH_RSS_NONFRAG_IPV6_UDP |
> >  			  ETH_RSS_NONFRAG_IPV6_OTHER |
> > -			  ETH_RSS_IPV6_EX |
> > -			  ETH_RSS_IPV6_TCP_EX |
> > -			  ETH_RSS_IPV6_UDP_EX),
> > -		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > -			 ETH_RSS_NONFRAG_IPV6_TCP |
> > -			 ETH_RSS_IPV6_TCP_EX),
> > -		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > -			 ETH_RSS_NONFRAG_IPV6_UDP |
> > -			 ETH_RSS_IPV6_UDP_EX),
> > +			  ETH_RSS_IPV6_EX),
> > +		[TCP] = 0,
> > +		[UDP] = 0,
> > +		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > +		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > +		[IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > +			      ETH_RSS_IPV6_TCP_EX),
> > +		[IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > +			      ETH_RSS_IPV6_UDP_EX),
> >  	};
> > -	static const uint64_t out[RTE_DIM(in)] = {
> > +	const uint64_t verbs[RTE_DIM(dpdk)] = {
> > +		[INNER] = IBV_RX_HASH_INNER,
> >  		[IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
> >  		[IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_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_UDP] = verbs[IPV6] | verbs[UDP],
> >  	};
> > +	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > +	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> >  	uint64_t seen = 0;
> >  	uint64_t conv = 0;
> >  	unsigned int i;
> > 
> > -	if (!types)
> > -		return priv->hw_rss_sup;
> > -	for (i = 0; i != RTE_DIM(in); ++i)
> > +	if (!types) {
> > +		if (!verbs_to_dpdk)
> > +			return priv->hw_rss_sup;
> > +		types = priv->hw_rss_sup;
> > +	}
> > +	for (i = 0; i != RTE_DIM(dpdk); ++i)
> >  		if (types & in[i]) {
> >  			seen |= types & in[i];
> >  			conv |= out[i];
> >  		}
> > -	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > +	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > +	    !(types & ~seen))
> >  		return conv;
> >  	rte_errno = ENOTSUP;
> >  	return (uint64_t)-1;
> >  }
> > 
> 
> 1. I wonder if [INNER] case is correct. Assuming we are given IBV_RX_HASH_INNER I think we should return with error ENOTSUP.
> However in the implementation above we return 0 as a valid DPDK RSS value.
> Returning a 0 value is also confusing since it is used in dpdk as "Best effort" RSS HF.

Well, this function performs exactly one job: convert recognized DPDK or
Verbs RSS hash types to the other format. Verbs's INNER is supported but has
no ETH_RSS equivalent type, hence is discarded.

> 2. Assuming we are given = IBV_RX_HASH_SRC_IPV4 alone (without IBV_RX_HASH_DST_IPV4) I think we should return with error ENOSUP since mlx4 does not support a hash based on SRC IPV4 alone.
> However in the implementation above we return a valid answer of all the flags from [IPV4], [IPV4_TCP] and [IPV4_UDP]. Can you please explain this logic?
> The same question repeats for all other "_DST_" only or "_SRC_" only alone constants.

This is done like that for the following reasons:

- Original code didn't do it either.

- Doing so would be overkill. Consider that the PMD has a list of known
  supported IBV fields but doesn't choose them on its own, the application
  does so through a different API without the ability to tell SRC and DST
  apart. Hence it's safe to toggle them together when dealing with DPDK
  applications.

> 3. Given "IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP" alone we are returning a DPDK reply which includes TCP flags + all IPV4 and IPV6 flags.  Can you please explain this logic?

Same as above basically. The DPDK API does not have a concept of TCP alone,
it's always on top of something (v4, v6, with and without fragments or
extended headers, see ETH_RSS_TCP). Therefore a pure TCP input on the
Verbs side issues all of them on the DPDK side.

> As mentioned I like the idea of one function to transform either direction between verbs and dpdk but please explain the current implementation following the questions above.
> 
> >  /**
> > - * Convert verbs RSS types to their DPDK equivalents.
> > - *
> > - * This function returns a group of RSS DPDK types given their equivalent
> > group
> > - * of verbs types.
> > - * For example both source IPv4 and destination IPv4 verbs types are
> > converted
> > - * into their equivalent RSS group types. If each of these verbs types existed
> > - * exclusively - no conversion would take place.
> > - *
> > - * @param types
> > - *   RSS hash types in verbs format.
> > - *
> > - * @return
> > - *   DPDK RSS hash fields supported by mlx4.
> > - */
> > -uint64_t
> > -mlx4_ibv_to_rss_types(uint64_t types)
> > -{
> > -	enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
> > -
> > -	static const uint64_t in[] = {
> > -		[IPV4] = MLX4_IBV_IPV4_HF,
> > -		[IPV6] = MLX4_IBV_IPV6_HF,
> > -		[IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
> > -		[IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
> > -		[IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
> > -		[IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
> > -	};
> > -	static const uint64_t out[RTE_DIM(in)] = {
> > -		[IPV4] = MLX4_RSS_IPV4_HF,
> > -		[IPV6] = MLX4_RSS_IPV6_HF,
> > -		[IPV4_TCP] = MLX4_RSS_IPV4_HF |
> > MLX4_RSS_IPV4_TCP_HF,
> > -		[IPV6_TCP] = MLX4_RSS_IPV6_HF |
> > MLX4_RSS_IPV6_TCP_HF,
> > -		[IPV4_UDP] = MLX4_RSS_IPV4_HF |
> > MLX4_RSS_IPV4_UDP_HF,
> > -		[IPV6_UDP] = MLX4_RSS_IPV6_HF |
> > MLX4_RSS_IPV6_UDP_HF,
> > -	};
> > -	uint64_t conv = 0;
> > -	unsigned int i;
> > -
> > -	for (i = 0; i != RTE_DIM(in); ++i)
> > -		if ((types & in[i]) == in[i])
> > -			conv |= out[i];
> > -	return conv;
> > -}
> > -
> > -/**
> >   * Merge Ethernet pattern item into flow rule handle.
> >   *
> >   * Additional mlx4-specific constraints on supported fields:
> > @@ -890,7 +825,7 @@ mlx4_flow_prepare(struct priv *priv,
> >  				goto exit_action_not_supported;
> >  			}
> >  			rte_errno = 0;
> > -			fields = mlx4_conv_rss_types(priv, rss->types);
> > +			fields = mlx4_conv_rss_types(priv, rss->types, 0);
> >  			if (fields == (uint64_t)-1 && rte_errno) {
> >  				msg = "unsupported RSS hash type
> > requested";
> >  				goto exit_action_not_supported;
> > diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
> > index 2e82903bd..2917ebe95 100644
> > --- a/drivers/net/mlx4/mlx4_flow.h
> > +++ b/drivers/net/mlx4/mlx4_flow.h
> > @@ -48,8 +48,8 @@ struct rte_flow {
> > 
> >  /* mlx4_flow.c */
> > 
> > -uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types); -uint64_t
> > mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
> > +uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
> > +			     int verbs_to_dpdk);
> >  int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);  void
> > mlx4_flow_clean(struct priv *priv);  int mlx4_filter_ctrl(struct rte_eth_dev
> > *dev,
> > --
> > 2.11.0

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions

[Shahaf Shuler]

Hi Adrien,

Please help me walkthrough this logic. Something doesn't end up correctly. 

Friday, May 18, 2018 12:49 PM, Adrien Mazarguil:
> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> Subject: Re: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > Cc: Ophir Munk <ophirmu@mellanox.com>
> > >   *
> > >   * @return
> > > - *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > > - *   otherwise and rte_errno is set.
> > > + *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > > + *   rte_errno is set.
> > >   */
> > >  uint64_t
> > > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > > +verbs_to_dpdk)
> > >  {


Let's assume:
Types = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4
And verbs_to_dpdk=1

> > > -	enum { IPV4, IPV6, TCP, UDP, };
> > > -	static const uint64_t in[] = {
> > > +	enum {
> > > +		INNER, IPV4, IPV6, TCP, UDP,
> > > +		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > > +	};
> > > +	static const uint64_t dpdk[] = {
> > > +		[INNER] = 0,
> > >  		[IPV4] = (ETH_RSS_IPV4 |
> > >  			  ETH_RSS_FRAG_IPV4 |
> > > -			  ETH_RSS_NONFRAG_IPV4_TCP |
> > > -			  ETH_RSS_NONFRAG_IPV4_UDP |
> > >  			  ETH_RSS_NONFRAG_IPV4_OTHER),
> > >  		[IPV6] = (ETH_RSS_IPV6 |
> > >  			  ETH_RSS_FRAG_IPV6 |
> > > -			  ETH_RSS_NONFRAG_IPV6_TCP |
> > > -			  ETH_RSS_NONFRAG_IPV6_UDP |
> > >  			  ETH_RSS_NONFRAG_IPV6_OTHER |
> > > -			  ETH_RSS_IPV6_EX |
> > > -			  ETH_RSS_IPV6_TCP_EX |
> > > -			  ETH_RSS_IPV6_UDP_EX),
> > > -		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > > -			 ETH_RSS_NONFRAG_IPV6_TCP |
> > > -			 ETH_RSS_IPV6_TCP_EX),
> > > -		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > > -			 ETH_RSS_NONFRAG_IPV6_UDP |
> > > -			 ETH_RSS_IPV6_UDP_EX),
> > > +			  ETH_RSS_IPV6_EX),
> > > +		[TCP] = 0,
> > > +		[UDP] = 0,
> > > +		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > > +		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > > +		[IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > > +			      ETH_RSS_IPV6_TCP_EX),
> > > +		[IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > > +			      ETH_RSS_IPV6_UDP_EX),
> > >  	};
> > > -	static const uint64_t out[RTE_DIM(in)] = {
> > > +	const uint64_t verbs[RTE_DIM(dpdk)] = {
> > > +		[INNER] = IBV_RX_HASH_INNER,
> > >  		[IPV4] = IBV_RX_HASH_SRC_IPV4 |
> IBV_RX_HASH_DST_IPV4,
> > >  		[IPV6] = IBV_RX_HASH_SRC_IPV6 |
> IBV_RX_HASH_DST_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_UDP] = verbs[IPV6] | verbs[UDP],
> > >  	};
> > > +	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > > +	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> > >  	uint64_t seen = 0;
> > >  	uint64_t conv = 0;
> > >  	unsigned int i;
> > >
> > > -	if (!types)

Types != 0 , we skip. 

> > > -		return priv->hw_rss_sup;
> > > -	for (i = 0; i != RTE_DIM(in); ++i)
> > > +	if (!types) {
> > > +		if (!verbs_to_dpdk)
> > > +			return priv->hw_rss_sup;
> > > +		types = priv->hw_rss_sup;
> > > +	}
> > > +	for (i = 0; i != RTE_DIM(dpdk); ++i)
> > >  		if (types & in[i]) {
> > >  			seen |= types & in[i];
> > >  			conv |= out[i];
> > >  		}


After this loop:
seen = (IBV_RX_HASH_SRC_PORT_IPV4 | IBV_RX_HASH_DST_IPV4)
conv = dpdk[IPV4] | dpdk[IPV4_TCP] | dpdk[IPV4_UDP]

> > > -	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > > +	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > > +	    !(types & ~seen))

All types were seen && verbs_to_dpdk hence returning that PMD support RSS based on IPV4 | TCPV4 | UDPV4
While according to the reported capabilities form kernel only IPV4 is supported. 

> > >  		return conv;
> > >  	rte_errno = ENOTSUP;
> > >  	return (uint64_t)-1;
> > >  }
> > >
> >

Re: [dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions

[Adrien Mazarguil]

On Mon, May 21, 2018 at 10:59:36AM +0000, Shahaf Shuler wrote:
> Hi Adrien,
> 
> Please help me walkthrough this logic. Something doesn't end up correctly. 
> 
> Friday, May 18, 2018 12:49 PM, Adrien Mazarguil:
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> > > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > Cc: Ophir Munk <ophirmu@mellanox.com>
> > > >   *
> > > >   * @return
> > > > - *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > > > - *   otherwise and rte_errno is set.
> > > > + *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > > > + *   rte_errno is set.
> > > >   */
> > > >  uint64_t
> > > > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > > > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > > > +verbs_to_dpdk)
> > > >  {
> 
> 
> Let's assume:
> Types = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4
> And verbs_to_dpdk=1
> 
> > > > -	enum { IPV4, IPV6, TCP, UDP, };
> > > > -	static const uint64_t in[] = {
> > > > +	enum {
> > > > +		INNER, IPV4, IPV6, TCP, UDP,
> > > > +		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > > > +	};
> > > > +	static const uint64_t dpdk[] = {
> > > > +		[INNER] = 0,
> > > >  		[IPV4] = (ETH_RSS_IPV4 |
> > > >  			  ETH_RSS_FRAG_IPV4 |
> > > > -			  ETH_RSS_NONFRAG_IPV4_TCP |
> > > > -			  ETH_RSS_NONFRAG_IPV4_UDP |
> > > >  			  ETH_RSS_NONFRAG_IPV4_OTHER),
> > > >  		[IPV6] = (ETH_RSS_IPV6 |
> > > >  			  ETH_RSS_FRAG_IPV6 |
> > > > -			  ETH_RSS_NONFRAG_IPV6_TCP |
> > > > -			  ETH_RSS_NONFRAG_IPV6_UDP |
> > > >  			  ETH_RSS_NONFRAG_IPV6_OTHER |
> > > > -			  ETH_RSS_IPV6_EX |
> > > > -			  ETH_RSS_IPV6_TCP_EX |
> > > > -			  ETH_RSS_IPV6_UDP_EX),
> > > > -		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > > > -			 ETH_RSS_NONFRAG_IPV6_TCP |
> > > > -			 ETH_RSS_IPV6_TCP_EX),
> > > > -		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > > > -			 ETH_RSS_NONFRAG_IPV6_UDP |
> > > > -			 ETH_RSS_IPV6_UDP_EX),
> > > > +			  ETH_RSS_IPV6_EX),
> > > > +		[TCP] = 0,
> > > > +		[UDP] = 0,
> > > > +		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > > > +		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > > > +		[IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > > > +			      ETH_RSS_IPV6_TCP_EX),
> > > > +		[IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > > > +			      ETH_RSS_IPV6_UDP_EX),
> > > >  	};
> > > > -	static const uint64_t out[RTE_DIM(in)] = {
> > > > +	const uint64_t verbs[RTE_DIM(dpdk)] = {
> > > > +		[INNER] = IBV_RX_HASH_INNER,
> > > >  		[IPV4] = IBV_RX_HASH_SRC_IPV4 |
> > IBV_RX_HASH_DST_IPV4,
> > > >  		[IPV6] = IBV_RX_HASH_SRC_IPV6 |
> > IBV_RX_HASH_DST_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_UDP] = verbs[IPV6] | verbs[UDP],
> > > >  	};
> > > > +	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > > > +	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> > > >  	uint64_t seen = 0;
> > > >  	uint64_t conv = 0;
> > > >  	unsigned int i;
> > > >
> > > > -	if (!types)
> 
> Types != 0 , we skip. 
> 
> > > > -		return priv->hw_rss_sup;
> > > > -	for (i = 0; i != RTE_DIM(in); ++i)
> > > > +	if (!types) {
> > > > +		if (!verbs_to_dpdk)
> > > > +			return priv->hw_rss_sup;
> > > > +		types = priv->hw_rss_sup;
> > > > +	}
> > > > +	for (i = 0; i != RTE_DIM(dpdk); ++i)
> > > >  		if (types & in[i]) {
> > > >  			seen |= types & in[i];
> > > >  			conv |= out[i];
> > > >  		}
> 
> 
> After this loop:
> seen = (IBV_RX_HASH_SRC_PORT_IPV4 | IBV_RX_HASH_DST_IPV4)
> conv = dpdk[IPV4] | dpdk[IPV4_TCP] | dpdk[IPV4_UDP]

Now I understand the mistake.

> > > > -	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > > > +	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > > > +	    !(types & ~seen))
> 
> All types were seen && verbs_to_dpdk hence returning that PMD support RSS based on IPV4 | TCPV4 | UDPV4
> While according to the reported capabilities form kernel only IPV4 is supported. 

Indeed, although this combination never occurs in practice, that's why I
didn't see any issue at first. I'll submit a fixed version. Thanks.

> > > >  		return conv;
> > > >  	rte_errno = ENOTSUP;
> > > >  	return (uint64_t)-1;
> > > >  }
> > > >
> > >

-- 
Adrien Mazarguil
6WIND

[dpdk-dev] [PATCH v2 1/2] net/mlx4: fix inadequate default in RSS converter

[Adrien Mazarguil]

Below commit documents 0 as a value standing for a default set of RSS hash
types, however the mlx4 PMD doesn't interpret it correctly and still uses
its own internal special value for that (-1).

Also, its function prototype was not updated.

Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
Fixes: 1d173da83ef2 ("net/mlx4: fix default RSS hash fields")

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

--

This is a rework of "net/mlx4: fix useless default in RSS converter" [1].

Not a candidate for stable anymore since DPDK 18.02 does not include the
flow API RSS rework.

[1] http://dpdk.org/ml/archives/dev/2018-May/100285.html
---
 drivers/net/mlx4/mlx4_flow.c | 6 +++---
 drivers/net/mlx4/mlx4_flow.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index 202779f7d..ebc9eeb8b 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -114,7 +114,7 @@ struct mlx4_drop {
  * Convert DPDK RSS hash types to their Verbs equivalent.
  *
  * This function returns the supported (default) set when @p types has
- * special value (uint64_t)-1.
+ * special value 0.
  *
  * @param priv
  *   Pointer to private structure.
@@ -160,7 +160,7 @@ mlx4_conv_rss_types(struct priv *priv, uint64_t types)
 	uint64_t conv = 0;
 	unsigned int i;
 
-	if (types == (uint64_t)-1)
+	if (!types)
 		return priv->hw_rss_sup;
 	for (i = 0; i != RTE_DIM(in); ++i)
 		if (types & in[i]) {
@@ -1384,7 +1384,7 @@ mlx4_flow_internal(struct priv *priv, struct rte_flow_error *error)
 	struct rte_flow_action_rss action_rss = {
 		.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
 		.level = 0,
-		.types = -1,
+		.types = 0,
 		.key_len = MLX4_RSS_HASH_KEY_SIZE,
 		.queue_num = queues,
 		.key = mlx4_rss_hash_key_default,
diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
index d1f1611eb..2e82903bd 100644
--- a/drivers/net/mlx4/mlx4_flow.h
+++ b/drivers/net/mlx4/mlx4_flow.h
@@ -48,7 +48,7 @@ struct rte_flow {
 
 /* mlx4_flow.c */
 
-uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t rss_hf);
+uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types);
 uint64_t mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
 int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);
 void mlx4_flow_clean(struct priv *priv);
-- 
2.11.0

[dpdk-dev] [PATCH v2 2/2] net/mlx4: refactor RSS conversion functions

[Adrien Mazarguil]

Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
functions"), this PMD includes two similar-looking functions that convert
RSS hash fields between Verbs and DPDK formats.

This patch refactors them as a single two-way function and gets rid of
redundant helper macros.

Note the loss of the "static" keyword on the out[] (now verbs[]) array
introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations in
function") is what prevents the reliance on macro definitions for static
initializers at the expense of a few extra instructions. An acceptable
trade-off given this function is not involved in data plane operations.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Ophir Munk <ophirmu@mellanox.com>

--

v2 changes:

- Split hash types on the DPDK side and added corresponding Verbs entries
  for a proper 1:1 mapping between them.
- Replaced logical AND ("any of") with exact match in loop on array
  entries.
- The combination of these changes address the issue where e.g. enabling
  only IPv4 on the Verbs side return IPv4 + TCP + UDP on the DPDK side of
  the conversion.
---
 drivers/net/mlx4/mlx4_ethdev.c |   3 +-
 drivers/net/mlx4/mlx4_flow.c   | 170 ++++++++++++------------------------
 drivers/net/mlx4/mlx4_flow.h   |   4 +-
 3 files changed, 60 insertions(+), 117 deletions(-)

diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index 0a9c2e2f5..30deb3ef0 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 			ETH_LINK_SPEED_20G |
 			ETH_LINK_SPEED_40G |
 			ETH_LINK_SPEED_56G;
-	info->flow_type_rss_offloads =
-		mlx4_ibv_to_rss_types(priv->hw_rss_sup);
+	info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);
 }
 
 /**
diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
index ebc9eeb8b..ddb7108dc 100644
--- a/drivers/net/mlx4/mlx4_flow.c
+++ b/drivers/net/mlx4/mlx4_flow.c
@@ -42,41 +42,6 @@
 #include "mlx4_rxtx.h"
 #include "mlx4_utils.h"
 
-/** IBV supported RSS hash functions combinations */
-#define MLX4_IBV_IPV4_HF ( \
-	IBV_RX_HASH_SRC_IPV4 | \
-	IBV_RX_HASH_DST_IPV4)
-#define MLX4_IBV_IPV6_HF ( \
-	IBV_RX_HASH_SRC_IPV6 | \
-	IBV_RX_HASH_DST_IPV6)
-#define MLX4_IBV_TCP_HF ( \
-	IBV_RX_HASH_SRC_PORT_TCP | \
-	IBV_RX_HASH_DST_PORT_TCP)
-#define MLX4_IBV_UDP_HF ( \
-	IBV_RX_HASH_SRC_PORT_UDP | \
-	IBV_RX_HASH_DST_PORT_UDP)
-
-/** Supported RSS hash functions combinations */
-#define MLX4_RSS_IPV4_HF ( \
-	ETH_RSS_IPV4 | \
-	ETH_RSS_FRAG_IPV4 | \
-	ETH_RSS_NONFRAG_IPV4_OTHER)
-#define MLX4_RSS_IPV6_HF ( \
-	ETH_RSS_IPV6 | \
-	ETH_RSS_FRAG_IPV6 | \
-	ETH_RSS_NONFRAG_IPV6_OTHER | \
-	ETH_RSS_IPV6_EX)
-#define MLX4_RSS_IPV4_TCP_HF ( \
-	ETH_RSS_NONFRAG_IPV4_TCP)
-#define MLX4_RSS_IPV6_TCP_HF ( \
-	ETH_RSS_NONFRAG_IPV6_TCP | \
-	ETH_RSS_IPV6_TCP_EX)
-#define MLX4_RSS_IPV4_UDP_HF ( \
-	ETH_RSS_NONFRAG_IPV4_UDP)
-#define MLX4_RSS_IPV6_UDP_HF ( \
-	ETH_RSS_NONFRAG_IPV6_UDP | \
-	ETH_RSS_IPV6_UDP_EX)
-
 /** Static initializer for a list of subsequent item types. */
 #define NEXT_ITEM(...) \
 	(const enum rte_flow_item_type []){ \
@@ -111,7 +76,7 @@ struct mlx4_drop {
 };
 
 /**
- * Convert DPDK RSS hash types to their Verbs equivalent.
+ * Convert supported RSS hash field types between DPDK and Verbs formats.
  *
  * This function returns the supported (default) set when @p types has
  * special value 0.
@@ -119,106 +84,85 @@ struct mlx4_drop {
  * @param priv
  *   Pointer to private structure.
  * @param types
- *   Hash types in DPDK format (see struct rte_eth_rss_conf).
+ *   Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
+ *   rte_eth_rss_conf) or Verbs format.
+ * @param verbs_to_dpdk
+ *   A zero value converts @p types from DPDK to Verbs, a nonzero value
+ *   performs the reverse operation.
  *
  * @return
- *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
- *   otherwise and rte_errno is set.
+ *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
+ *   rte_errno is set.
  */
 uint64_t
-mlx4_conv_rss_types(struct priv *priv, uint64_t types)
+mlx4_conv_rss_types(struct priv *priv, uint64_t types, int verbs_to_dpdk)
 {
-	enum { IPV4, IPV6, TCP, UDP, };
-	static const uint64_t in[] = {
-		[IPV4] = (ETH_RSS_IPV4 |
-			  ETH_RSS_FRAG_IPV4 |
-			  ETH_RSS_NONFRAG_IPV4_TCP |
-			  ETH_RSS_NONFRAG_IPV4_UDP |
-			  ETH_RSS_NONFRAG_IPV4_OTHER),
-		[IPV6] = (ETH_RSS_IPV6 |
-			  ETH_RSS_FRAG_IPV6 |
-			  ETH_RSS_NONFRAG_IPV6_TCP |
-			  ETH_RSS_NONFRAG_IPV6_UDP |
-			  ETH_RSS_NONFRAG_IPV6_OTHER |
-			  ETH_RSS_IPV6_EX |
-			  ETH_RSS_IPV6_TCP_EX |
-			  ETH_RSS_IPV6_UDP_EX),
-		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
-			 ETH_RSS_NONFRAG_IPV6_TCP |
-			 ETH_RSS_IPV6_TCP_EX),
-		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
-			 ETH_RSS_NONFRAG_IPV6_UDP |
-			 ETH_RSS_IPV6_UDP_EX),
+	enum {
+		INNER,
+		IPV4, IPV4_1, IPV4_2, IPV6, IPV6_1, IPV6_2, IPV6_3,
+		TCP, UDP,
+		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_TCP_1, IPV6_UDP, IPV6_UDP_1,
+	};
+	static const uint64_t dpdk[] = {
+		[INNER] = 0,
+		[IPV4] = ETH_RSS_IPV4,
+		[IPV4_1] = ETH_RSS_FRAG_IPV4,
+		[IPV4_2] = ETH_RSS_NONFRAG_IPV4_OTHER,
+		[IPV6] = ETH_RSS_IPV6,
+		[IPV6_1] = ETH_RSS_FRAG_IPV6,
+		[IPV6_2] = ETH_RSS_NONFRAG_IPV6_OTHER,
+		[IPV6_3] = ETH_RSS_IPV6_EX,
+		[TCP] = 0,
+		[UDP] = 0,
+		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
+		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
+		[IPV6_TCP] = ETH_RSS_NONFRAG_IPV6_TCP,
+		[IPV6_TCP_1] = ETH_RSS_IPV6_TCP_EX,
+		[IPV6_UDP] = ETH_RSS_NONFRAG_IPV6_UDP,
+		[IPV6_UDP_1] = ETH_RSS_IPV6_UDP_EX,
 	};
-	static const uint64_t out[RTE_DIM(in)] = {
+	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],
 	};
+	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
+	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
 	uint64_t seen = 0;
 	uint64_t conv = 0;
 	unsigned int i;
 
-	if (!types)
-		return priv->hw_rss_sup;
-	for (i = 0; i != RTE_DIM(in); ++i)
-		if (types & in[i]) {
+	if (!types) {
+		if (!verbs_to_dpdk)
+			return priv->hw_rss_sup;
+		types = priv->hw_rss_sup;
+	}
+	for (i = 0; i != RTE_DIM(dpdk); ++i)
+		if (in[i] && (types & in[i]) == in[i]) {
 			seen |= types & in[i];
 			conv |= out[i];
 		}
-	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
+	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
+	    !(types & ~seen))
 		return conv;
 	rte_errno = ENOTSUP;
 	return (uint64_t)-1;
 }
 
 /**
- * Convert verbs RSS types to their DPDK equivalents.
- *
- * This function returns a group of RSS DPDK types given their equivalent group
- * of verbs types.
- * For example both source IPv4 and destination IPv4 verbs types are converted
- * into their equivalent RSS group types. If each of these verbs types existed
- * exclusively - no conversion would take place.
- *
- * @param types
- *   RSS hash types in verbs format.
- *
- * @return
- *   DPDK RSS hash fields supported by mlx4.
- */
-uint64_t
-mlx4_ibv_to_rss_types(uint64_t types)
-{
-	enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
-
-	static const uint64_t in[] = {
-		[IPV4] = MLX4_IBV_IPV4_HF,
-		[IPV6] = MLX4_IBV_IPV6_HF,
-		[IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
-		[IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
-		[IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
-		[IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
-	};
-	static const uint64_t out[RTE_DIM(in)] = {
-		[IPV4] = MLX4_RSS_IPV4_HF,
-		[IPV6] = MLX4_RSS_IPV6_HF,
-		[IPV4_TCP] = MLX4_RSS_IPV4_HF | MLX4_RSS_IPV4_TCP_HF,
-		[IPV6_TCP] = MLX4_RSS_IPV6_HF | MLX4_RSS_IPV6_TCP_HF,
-		[IPV4_UDP] = MLX4_RSS_IPV4_HF | MLX4_RSS_IPV4_UDP_HF,
-		[IPV6_UDP] = MLX4_RSS_IPV6_HF | MLX4_RSS_IPV6_UDP_HF,
-	};
-	uint64_t conv = 0;
-	unsigned int i;
-
-	for (i = 0; i != RTE_DIM(in); ++i)
-		if ((types & in[i]) == in[i])
-			conv |= out[i];
-	return conv;
-}
-
-/**
  * Merge Ethernet pattern item into flow rule handle.
  *
  * Additional mlx4-specific constraints on supported fields:
@@ -890,7 +834,7 @@ mlx4_flow_prepare(struct priv *priv,
 				goto exit_action_not_supported;
 			}
 			rte_errno = 0;
-			fields = mlx4_conv_rss_types(priv, rss->types);
+			fields = mlx4_conv_rss_types(priv, rss->types, 0);
 			if (fields == (uint64_t)-1 && rte_errno) {
 				msg = "unsupported RSS hash type requested";
 				goto exit_action_not_supported;
diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
index 2e82903bd..2917ebe95 100644
--- a/drivers/net/mlx4/mlx4_flow.h
+++ b/drivers/net/mlx4/mlx4_flow.h
@@ -48,8 +48,8 @@ struct rte_flow {
 
 /* mlx4_flow.c */
 
-uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types);
-uint64_t mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
+uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
+			     int verbs_to_dpdk);
 int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);
 void mlx4_flow_clean(struct priv *priv);
 int mlx4_filter_ctrl(struct rte_eth_dev *dev,
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2 1/2] net/mlx4: fix inadequate default in RSS converter

[Shahaf Shuler]

Monday, May 21, 2018 6:50 PM, Adrien Mazarguil:
> Subject: [PATCH v2 1/2] net/mlx4: fix inadequate default in RSS converter
> 
> Below commit documents 0 as a value standing for a default set of RSS hash
> types, however the mlx4 PMD doesn't interpret it correctly and still uses its
> own internal special value for that (-1).
> 
> Also, its function prototype was not updated.
> 
> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
> Fixes: 1d173da83ef2 ("net/mlx4: fix default RSS hash fields")
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 

Series applied to next-net-mlx, thanks. 

Re: [dpdk-dev] [PATCH v2 2/2] net/mlx4: refactor RSS conversion functions

[Ferruh Yigit]

On 5/21/2018 4:50 PM, Adrien Mazarguil wrote:
> Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> functions"), this PMD includes two similar-looking functions that convert
> RSS hash fields between Verbs and DPDK formats.
> 
> This patch refactors them as a single two-way function and gets rid of
> redundant helper macros.
> 
> Note the loss of the "static" keyword on the out[] (now verbs[]) array
> introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations in
> function") is what prevents the reliance on macro definitions for static
> initializers at the expense of a few extra instructions. An acceptable
> trade-off given this function is not involved in data plane operations.
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Ophir Munk <ophirmu@mellanox.com>

<...>

> @@ -119,106 +84,85 @@ struct mlx4_drop {
>   * @param priv
>   *   Pointer to private structure.
>   * @param types
> - *   Hash types in DPDK format (see struct rte_eth_rss_conf).
> + *   Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
> + *   rte_eth_rss_conf) or Verbs format.
> + * @param verbs_to_dpdk
> + *   A zero value converts @p types from DPDK to Verbs, a nonzero value
> + *   performs the reverse operation.
>   *
>   * @return
> - *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> - *   otherwise and rte_errno is set.
> + *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> + *   rte_errno is set.
>   */
>  uint64_t
> -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int verbs_to_dpdk)
>  {
> -	enum { IPV4, IPV6, TCP, UDP, };
> -	static const uint64_t in[] = {
> -		[IPV4] = (ETH_RSS_IPV4 |
> -			  ETH_RSS_FRAG_IPV4 |
> -			  ETH_RSS_NONFRAG_IPV4_TCP |
> -			  ETH_RSS_NONFRAG_IPV4_UDP |
> -			  ETH_RSS_NONFRAG_IPV4_OTHER),
> -		[IPV6] = (ETH_RSS_IPV6 |
> -			  ETH_RSS_FRAG_IPV6 |
> -			  ETH_RSS_NONFRAG_IPV6_TCP |
> -			  ETH_RSS_NONFRAG_IPV6_UDP |
> -			  ETH_RSS_NONFRAG_IPV6_OTHER |
> -			  ETH_RSS_IPV6_EX |
> -			  ETH_RSS_IPV6_TCP_EX |
> -			  ETH_RSS_IPV6_UDP_EX),
> -		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> -			 ETH_RSS_NONFRAG_IPV6_TCP |
> -			 ETH_RSS_IPV6_TCP_EX),
> -		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> -			 ETH_RSS_NONFRAG_IPV6_UDP |
> -			 ETH_RSS_IPV6_UDP_EX),
> +	enum {
> +		INNER,
> +		IPV4, IPV4_1, IPV4_2, IPV6, IPV6_1, IPV6_2, IPV6_3,
> +		TCP, UDP,
> +		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_TCP_1, IPV6_UDP, IPV6_UDP_1,
> +	};
> +	static const uint64_t dpdk[] = {
> +		[INNER] = 0,
> +		[IPV4] = ETH_RSS_IPV4,
> +		[IPV4_1] = ETH_RSS_FRAG_IPV4,
> +		[IPV4_2] = ETH_RSS_NONFRAG_IPV4_OTHER,
> +		[IPV6] = ETH_RSS_IPV6,
> +		[IPV6_1] = ETH_RSS_FRAG_IPV6,
> +		[IPV6_2] = ETH_RSS_NONFRAG_IPV6_OTHER,
> +		[IPV6_3] = ETH_RSS_IPV6_EX,
> +		[TCP] = 0,
> +		[UDP] = 0,
> +		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> +		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> +		[IPV6_TCP] = ETH_RSS_NONFRAG_IPV6_TCP,
> +		[IPV6_TCP_1] = ETH_RSS_IPV6_TCP_EX,
> +		[IPV6_UDP] = ETH_RSS_NONFRAG_IPV6_UDP,
> +		[IPV6_UDP_1] = ETH_RSS_IPV6_UDP_EX,
>  	};
> -	static const uint64_t out[RTE_DIM(in)] = {
> +	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],

This gives following build error with ICC [1]. Is it guarantied that in above
assignment the executing order will be the same order with code?


[1]
.../dpdk/drivers/net/mlx4/mlx4_flow.c(135): error #592: variable "verbs" is used
before its value is set
                [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],
>  	};

<...>

Re: [dpdk-dev] [PATCH v2 2/2] net/mlx4: refactor RSS conversion functions

[Adrien Mazarguil]

On Tue, May 22, 2018 at 10:41:35AM +0100, Ferruh Yigit wrote:
> On 5/21/2018 4:50 PM, Adrien Mazarguil wrote:
> > Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> > functions"), this PMD includes two similar-looking functions that convert
> > RSS hash fields between Verbs and DPDK formats.
> > 
> > This patch refactors them as a single two-way function and gets rid of
> > redundant helper macros.
> > 
> > Note the loss of the "static" keyword on the out[] (now verbs[]) array
> > introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations in
> > function") is what prevents the reliance on macro definitions for static
> > initializers at the expense of a few extra instructions. An acceptable
> > trade-off given this function is not involved in data plane operations.
<snip>
> > -	static const uint64_t out[RTE_DIM(in)] = {
> > +	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],
> 
> This gives following build error with ICC [1]. Is it guarantied that in above
> assignment the executing order will be the same order with code?
> 
> [1]
> .../dpdk/drivers/net/mlx4/mlx4_flow.c(135): error #592: variable "verbs" is used
> before its value is set
>                 [IPV4_TCP] = verbs[IPV4] | verbs[TCP],

Didn't see this error with GCC nor clang (compilation and validation OK with
both), however ICC is correct, initialization order is not guaranteed
here. This shortcut is unsafe, I'll have to come up with something else.

-- 
Adrien Mazarguil
6WIND

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

[Adrien Mazarguil]

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

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

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

[Ferruh Yigit]

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?

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

[Adrien Mazarguil]

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!

-- 
Adrien Mazarguil
6WIND