Re: [dpdk-dev] [PATCH] net/mlx5: fix RSS flow action hash type selection

[Yongseok Koh <yskoh@mellanox.com>]

> On Aug 12, 2018, at 4:14 AM, Shahaf Shuler <shahafs@mellanox.com> wrote:
> 
> On the code after the below commits, the criteria to select the IPV4 or
> IPV6 hash functions was the existence of some ETH_RSS_IPV4 RSS types on
> the flow rule.
> 
> The check is wrong. For example ETH_RSS_NONFRAG_IPV4_TCP will not select
> the IPV4 hash which will cause the packet to be spread in a bad way.
> 
> Fix it by adding the corresponding types needed for each hash selection.
> 
> Fixes: 592f05b29a25 ("net/mlx5: add RSS flow action")
> Fixes: fd0b70316bca ("net/mlx5: support inner RSS computation")
> Cc: stable@dpdk.org
> Cc: nelio.laranjeiro@6wind.com
> Cc: orika@mellanox.com
> 
> Reported-by: Yaroslav Brustinov <ybrustin@cisco.com>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
> 
> Few notes:
> 1. this patch should be backported to 18.08 stable
> 2. There is more work planned in 18.11 for the flow engine.
>   The work should be on top of this fix.
> 
> ---
> drivers/net/mlx5/mlx5_flow.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index ca4625b699..da96932da5 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1053,6 +1053,8 @@ mlx5_flow_item_ipv4(const struct rte_flow_item *item, struct rte_flow *flow,
> 		mlx5_flow_verbs_hashfields_adjust
> 			(flow, tunnel,
> 			 (ETH_RSS_IPV4 | ETH_RSS_FRAG_IPV4 |
> +			  ETH_RSS_NONFRAG_IPV4_TCP |
> +			  ETH_RSS_NONFRAG_IPV4_UDP |
> 			  ETH_RSS_NONFRAG_IPV4_OTHER),
> 			 (IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4));
> 		flow->cur_verbs->attr->priority = MLX5_PRIORITY_MAP_L3;
> @@ -1188,7 +1190,11 @@ mlx5_flow_item_ipv6(const struct rte_flow_item *item, struct rte_flow *flow,
> 	if (size <= flow_size) {
> 		mlx5_flow_verbs_hashfields_adjust
> 			(flow, tunnel,
> -			 (ETH_RSS_IPV6 | ETH_RSS_NONFRAG_IPV6_OTHER),
> +			 (ETH_RSS_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 |
> +			  ETH_RSS_FRAG_IPV6 | ETH_RSS_NONFRAG_IPV6_OTHER),

ETH_RSS_NONFRAG_IPV6_OTHER appears twice.
And hope to see the same order as in rte_ethdev.h

Thanks,
Yongseok

> 			 (IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6));
> 		flow->cur_verbs->attr->priority = MLX5_PRIORITY_MAP_L3;
> 		mlx5_flow_spec_verbs_add(flow, &ipv6, size);
> -- 
> 2.12.0
>