On 2023/10/12 1:39, Stephen Hemminger wrote:
> On Wed, 11 Oct 2023 17:27:27 +0800
> Jie Hai <haij...@huawei.com> wrote:
>
>> Currently, rte_eth_rss_conf supports configuring and querying
>> RSS hash functions, rss key and it's length, but not RSS hash
>> algorithm.
>>
>> The structure ``rte_eth_rss_conf`` is extended by adding a new
>> field "algorithm". This represents the RSS algorithms to apply.
>> The following API will be affected:
>> - rte_eth_dev_configure
>> - rte_eth_dev_rss_hash_update
>> - rte_eth_dev_rss_hash_conf_get
>>
>> If the value of "algorithm" used for configuration is a gibberish
>> value, report the error and return. Do the same for
>> rte_eth_dev_rss_hash_update and rte_eth_dev_configure.
>>
>> To check whether the drivers report valid "algorithm", it is set
>> to default value before querying.
>>
>> Signed-off-by: Jie Hai <haij...@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdo...@huawei.com>
>> ---
>> doc/guides/rel_notes/release_23_11.rst | 2 ++
>> lib/ethdev/rte_ethdev.c | 17 ++++++++++++++++
>> lib/ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++
>> lib/ethdev/rte_flow.c | 1 -
>> lib/ethdev/rte_flow.h | 28 ++------------------------
>> 5 files changed, 48 insertions(+), 27 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_23_11.rst
>> b/doc/guides/rel_notes/release_23_11.rst
>> index e13d57728071..92a445ab2ed3 100644
>> --- a/doc/guides/rel_notes/release_23_11.rst
>> +++ b/doc/guides/rel_notes/release_23_11.rst
>> @@ -197,6 +197,8 @@ ABI Changes
>> fields, to move ``rxq`` and ``txq`` fields, to change the size of
>> ``reserved1`` and ``reserved2`` fields.
>>
>> +* ethdev: Added "algorithm" field to ``rte_eth_rss_conf`` structure for RSS
>> + hash algorithm.
>>
>> Known Issues
>> ------------
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 18a4b950b184..2eda1b8072e5 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -1464,6 +1464,14 @@ 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.algorithm >=
>> RTE_ETH_HASH_FUNCTION_MAX) {
>> + RTE_ETHDEV_LOG(ERR,
>> + "Ethdev port_id=%u invalid RSS algorithm:
>> 0x%"PRIx64"\n",
>> + port_id, dev_conf->rx_adv_conf.rss_conf.algorithm);
>> + ret = -EINVAL;
>> + goto rollback;
>> + }
>> +
>
> Rather than having every driver check the algorithm, why not handle this like
> other features in DPDK (which may mean API/ABI changes).
>
> Add a field rss_algo_capa which is bit field of available RSS functions.
> Then the check for algorithm can be done in generic code. There a couple
> of reserved fields that could be used.
+1 for add a field
But there are two ways to config rss: ethdev-ops, ethdev-rteflow-ops, should
distinguish them ? or just define for ethdev-ops ?
>
> It would mean updating all the drivers once with the capa field but
> would provide way for application to know what fields are possible.
>
> It has proved to be a problem in later ABI changes if a maximum value
> is exposed. I.e don't expose RTE_ETH_HASH_FUNCTION_MAX.
+1
>
> .
>
