[PATCH v12 04/12] net/tap: validate and setup parameters for BPF RSS

[Ferruh Yigit]

On 5/2/2024 10:31 PM, Stephen Hemminger wrote:
> The flow RSS support via BPF was not using the key, or
> hash type parameters. Which is good because they were never
> properly setup.
> 
> Fix the setup and validate the flow parameters, the BPF
> side gets fixed later.
> 
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
>

<...>

> @@ -2095,6 +2112,41 @@ static int rss_add_actions(struct rte_flow *flow, struct pmd_internals *pmd,
>  			(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>  			 "a nonzero RSS encapsulation level is not supported");
>  
> +	if (rss->queue_num == 0 || rss->queue_num >= TAP_MAX_QUEUES)
> +		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "invalid number of queues");
> +
> +	/* allow RSS key_len 0 in case of NULL (default) RSS key. */
> +	if (rss->key_len == 0) {
> +		if (rss->key != NULL)
> +			return rte_flow_error_set(error, ENOTSUP,
> +						  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> +						  &rss->key_len, "RSS hash key length 0");
> +		key_in = rss_hash_default_key;
> +	} else {
> +		if (rss->key_len != TAP_RSS_HASH_KEY_SIZE)
> +			return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +						  NULL, "RSS hash invalid key length");
> +		if (rss->key == NULL)
> +			return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +						  NULL, "RSS hash key is NULL");
> +		key_in = rss->key;
> +	}
>

Does it make sense to add above expectations (like if "key_len == 0"
default key used, or 'key_len' expectations, etc..) to the function comment?

<...>

> @@ -2109,8 +2161,11 @@ static int rss_add_actions(struct rte_flow *flow, struct pmd_internals *pmd,
>  	rss_entry.nb_queues = rss->queue_num;
>  	for (i = 0; i < rss->queue_num; i++)
>  		rss_entry.queues[i] = rss->queue[i];
> -	rss_entry.hash_fields =
> -		(1 << HASH_FIELD_IPV4_L3_L4) | (1 << HASH_FIELD_IPV6_L3_L4);
> +
> +	rss_entry.hash_fields = hash_type;
> +	rte_convert_rss_key((const uint32_t *)key_in, (uint32_t *)rss_entry.key,
> +			    TAP_RSS_HASH_KEY_SIZE);
> +
>

Why 'rte_convert_rss_key()' is required? Is this making an assumption on
the CPU architecture?