Re: [dpdk-dev] [PATCH] ethdev: check for Rx RSS distribution and RSS hash

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

On 4/27/2020 1:49 PM, Andrew Rybchenko wrote:
> On 4/27/20 3:34 PM, Lijun Ou wrote:
>> When rte api checks the Rx RSS distribution is enable but the RSS
>> hash is disabled, it will return an error.
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index 0854ef8..07734c4 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -1411,6 +1411,17 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>  		goto rollback;
>>  	}
>>  
>> +	/* Check if Rx RSS distribution is enable but RSS hash is disabled. */
>> +	if (((dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) != 0) &&
>> +	    !(dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_RSS_HASH)) {
>> +		RTE_ETHDEV_LOG(ERR,
>> +			"Ethdev port_id=%u config valid Rx mq_mode with RSS but %s offload is no-requested\n",
>> +			port_id,
>> +			rte_eth_dev_rx_offload_name(DEV_RX_OFFLOAD_RSS_HASH));
>> +		ret = -EINVAL;
>> +		goto rollback;
>> +	}
>> +
>>  	/*
>>  	 * Setup new number of RX/TX queues and reconfigure device.
>>  	 */
> 
> NACK. It is perfectly fine to do distribution, but do not need RSS hash
> information.
>  - ETH_MQ_RX_RSS_FLAG controls RSS hash calculation and distribution
>  - DEV_RX_OFFLOAD_RSS_HASH controls delivery of the hash value
>    itself from HW to SW
> 

Hi Lijun,

As Andrew described, now 'DEV_RX_OFFLOAD_RSS_HASH' controls to copy calculated
hash value to 'mbuf::hash::rss' or not. When rss hash copied to mubf,
'PKT_RX_RSS_HASH' flag of 'mbuf::ol_flags' set to notify the application that
hash value is valid. This was a performance optimization.

We know above because we were involved in development of it, if this is not
clean for third party, can you please amend your patch to clarify above behavior?

Thanks,
ferruh