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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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],
>  	};

<...>