[PATCH v8 02/10] lib/ethdev: check RSS key length

[Jie Hai]

On 2023/11/1 21:19, Ferruh Yigit wrote:
> On 11/1/2023 7:40 AM, Jie Hai wrote:
>> In rte_eth_dev_rss_hash_conf_get(), the "rss_key_len" should be
>> greater than or equal to the "hash_key_size" which get from
>> rte_eth_dev_info_get() API. And the "rss_key" should contain at
>> least "hash_key_size" bytes. If these requirements are not met,
>> the query unreliable.
>>
>> In rte_eth_dev_rss_hash_update() or rte_eth_dev_configure(), the
>> "rss_key_len" indicates the length of the "rss_key" in bytes of
>> the array pointed by "rss_key", it should be equal to the
>> "hash_key_size" if "rss_key" is not NULL.
>>
>> This patch checks "rss_key_len" in ethdev level.
>>
> 
> Can you please squash this patch and previous one, previous one
> clarifies the API and this one adds relevant checks, so they con be in
> some patch.
> 
> Can you also please update release notes, 'API Changes', explaining
> 'rss_conf.rss_key_len' needs to be provided by user for the case
> "conf.rss_key != NULL", it won't be taken as default 40 bytes anymore.
> 
Thanks.
Will update.
> 
>> Signed-off-by: Jie Hai <haijie1 at huawei.com>
>> ---
>>   lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index af23ac0ad00f..07bb35833ba6 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -1500,6 +1500,16 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>   		goto rollback;
>>   	}
>>   
>> +	if (dev_conf->rx_adv_conf.rss_conf.rss_key != NULL &&
>> +	    dev_conf->rx_adv_conf.rss_conf.rss_key_len < dev_info.hash_key_size) {
>>
> 
> Why check is "rss_key_len < dev_info.hash_key_size", is it allowed to
> have "rss_key_len > dev_info.hash_key_size"?
> 
> Shouldn't it enforce that "rss_key_len == dev_info.hash_key_size"?
> 
> 
Yes, should be equal, will correct.
>> +		RTE_ETHDEV_LOG(ERR,
>> +			"Ethdev port_id=%u invalid RSS key len: %u, valid value: %u\n",
>> +			port_id, dev_conf->rx_adv_conf.rss_conf.rss_key_len,
>> +			dev_info.hash_key_size);
>> +		ret = -EINVAL;
>> +		goto rollback;
>> +	}
>> +
>>   	/*
>>   	 * Setup new number of Rx/Tx queues and reconfigure device.
>>   	 */
>> @@ -4698,6 +4708,14 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
>>   		return -ENOTSUP;
>>   	}
>>   
>> +	if (rss_conf->rss_key != NULL &&
>> +	    rss_conf->rss_key_len != dev_info.hash_key_size) {
>> +		RTE_ETHDEV_LOG(ERR,
>> +			"Ethdev port_id=%u invalid RSS key len: %u, valid value: %u\n",
>> +			port_id, rss_conf->rss_key_len, dev_info.hash_key_size);
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (*dev->dev_ops->rss_hash_update == NULL)
>>   		return -ENOTSUP;
>>   	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
>> @@ -4712,6 +4730,7 @@ int
>>   rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>>   			      struct rte_eth_rss_conf *rss_conf)
>>   {
>> +	struct rte_eth_dev_info dev_info = { 0 };
>>   	struct rte_eth_dev *dev;
>>   	int ret;
>>   
>> @@ -4725,6 +4744,18 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>>   		return -EINVAL;
>>   	}
>>   
>> +	ret = rte_eth_dev_info_get(port_id, &dev_info);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	if (rss_conf->rss_key != NULL &&
>> +	    rss_conf->rss_key_len < dev_info.hash_key_size) {
>> +		RTE_ETHDEV_LOG(ERR,
>> +			"Ethdev port_id=%u invalid RSS key len: %u, should not be less than: %u\n",
>> +			port_id, rss_conf->rss_key_len, dev_info.hash_key_size);
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (*dev->dev_ops->rss_hash_conf_get == NULL)
>>   		return -ENOTSUP;
>>   	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
> 
> .