[1/5] ethdev: support setting and querying rss algorithm
Checks
Commit Message
From: Jie Hai <haijie1@huawei.com>
Currently, rte_eth_rss_conf supports configuring 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,
"func". This represents the RSS algorithms to apply. The following
API is affected:
- rte_eth_dev_configure
- rte_eth_dev_rss_hash_update
- rte_eth_dev_rss_hash_conf_get
To prevent configuration failures caused by incorrect func input, check
this parameter in advance. If it's incorrect, a warning is generated
and the default value is set. Do the same for rte_eth_dev_rss_hash_update
and rte_eth_dev_configure.
To check whether the drivers report the func field, it is set to default
value before querying.
Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
doc/guides/rel_notes/release_23_03.rst | 4 ++--
lib/ethdev/rte_ethdev.c | 18 ++++++++++++++++++
lib/ethdev/rte_ethdev.h | 5 +++++
3 files changed, 25 insertions(+), 2 deletions(-)
Comments
Hi,
On Wed, 15 Mar 2023, Dongdong Liu wrote:
> From: Jie Hai <haijie1@huawei.com>
>
> Currently, rte_eth_rss_conf supports configuring 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,
> "func". This represents the RSS algorithms to apply. The following
> API is affected:
> - rte_eth_dev_configure
> - rte_eth_dev_rss_hash_update
> - rte_eth_dev_rss_hash_conf_get
>
> To prevent configuration failures caused by incorrect func input, check
> this parameter in advance. If it's incorrect, a warning is generated
> and the default value is set. Do the same for rte_eth_dev_rss_hash_update
> and rte_eth_dev_configure.
>
> To check whether the drivers report the func field, it is set to default
> value before querying.
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
> doc/guides/rel_notes/release_23_03.rst | 4 ++--
> lib/ethdev/rte_ethdev.c | 18 ++++++++++++++++++
> lib/ethdev/rte_ethdev.h | 5 +++++
> 3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_notes/release_23_03.rst
> index af6f37389c..7879567427 100644
> --- a/doc/guides/rel_notes/release_23_03.rst
> +++ b/doc/guides/rel_notes/release_23_03.rst
> @@ -284,8 +284,8 @@ ABI Changes
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> -* No ABI change that would break compatibility with 22.11.
> -
> +* ethdev: Added "func" 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 4d03255683..db561026bd 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1368,6 +1368,15 @@ 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.func >= RTE_ETH_HASH_FUNCTION_MAX) {
> + RTE_ETHDEV_LOG(WARNING,
> + "Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
> + port_id, dev_conf->rx_adv_conf.rss_conf.func,
> + RTE_ETH_HASH_FUNCTION_DEFAULT);
> + dev->data->dev_conf.rx_adv_conf.rss_conf.func =
> + RTE_ETH_HASH_FUNCTION_DEFAULT;
I have no strong opinion, but, to me, this behaviour conceals
programming errors. For example, if an application intends
to enable hash algorithm A but, due to a programming error,
passes a gibberish value here, chances are the error will
end up unnoticed. Especially in case the application
sets the log level to such that warnings are omitted.
Why not just return the error the standard way?
> + }
> +
> /* Check if Rx RSS distribution is disabled but RSS hash is enabled. */
> if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
> (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
> @@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
> return -ENOTSUP;
> }
>
> + if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
> + RTE_ETHDEV_LOG(NOTICE,
> + "Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
> + port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
> + rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
> + }
> +
> if (*dev->dev_ops->rss_hash_update == NULL)
> return -ENOTSUP;
> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
> @@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
> return -EINVAL;
> }
>
> + rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
> +
> if (*dev->dev_ops->rss_hash_conf_get == NULL)
> return -ENOTSUP;
> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 99fe9e238b..5abe2cb36d 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -174,6 +174,7 @@ extern "C" {
>
> #include "rte_ethdev_trace_fp.h"
> #include "rte_dev_info.h"
> +#include "rte_flow.h"
>
> extern int rte_eth_dev_logtype;
>
> @@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
> * The *rss_hf* field of the *rss_conf* structure indicates the different
> * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
> * Supplying an *rss_hf* equal to zero disables the RSS feature.
> + *
> + * The *func* field of the *rss_conf* structure indicates the different
> + * types of hash algorithms applied by the RSS hashing.
Consider:
The *func* field of the *rss_conf* structure indicates the algorithm to
use when computing hash. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
the PMD to use its best-effort algorithm rather than a specific one.
> */
> struct rte_eth_rss_conf {
> uint8_t *rss_key; /**< If not NULL, 40-byte hash key. */
> uint8_t rss_key_len; /**< hash key length in bytes. */
> uint64_t rss_hf; /**< Hash functions to apply - see below. */
> + enum rte_eth_hash_function func; /**< Hash algorithm to apply. */
> };
>
> /*
> --
> 2.22.0
>
>
Thank you.
15/03/2023 12:00, Dongdong Liu:
> From: Jie Hai <haijie1@huawei.com>
> --- a/doc/guides/rel_notes/release_23_03.rst
> +++ b/doc/guides/rel_notes/release_23_03.rst
> -* No ABI change that would break compatibility with 22.11.
> -
> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
> + algorithm.
We cannot break ABI compatibility until 23.11.
Hi Ivan
Many thanks for your review.
On 2023/3/15 19:28, Ivan Malov wrote:
> Hi,
>
> On Wed, 15 Mar 2023, Dongdong Liu wrote:
>
>> From: Jie Hai <haijie1@huawei.com>
>>
>> Currently, rte_eth_rss_conf supports configuring 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,
>> "func". This represents the RSS algorithms to apply. The following
>> API is affected:
>> - rte_eth_dev_configure
>> - rte_eth_dev_rss_hash_update
>> - rte_eth_dev_rss_hash_conf_get
>>
>> To prevent configuration failures caused by incorrect func input, check
>> this parameter in advance. If it's incorrect, a warning is generated
>> and the default value is set. Do the same for rte_eth_dev_rss_hash_update
>> and rte_eth_dev_configure.
>>
>> To check whether the drivers report the func field, it is set to default
>> value before querying.
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> ---
>> doc/guides/rel_notes/release_23_03.rst | 4 ++--
>> lib/ethdev/rte_ethdev.c | 18 ++++++++++++++++++
>> lib/ethdev/rte_ethdev.h | 5 +++++
>> 3 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_23_03.rst
>> b/doc/guides/rel_notes/release_23_03.rst
>> index af6f37389c..7879567427 100644
>> --- a/doc/guides/rel_notes/release_23_03.rst
>> +++ b/doc/guides/rel_notes/release_23_03.rst
>> @@ -284,8 +284,8 @@ ABI Changes
>> Also, make sure to start the actual text at the margin.
>> =======================================================
>>
>> -* No ABI change that would break compatibility with 22.11.
>> -
>> +* ethdev: Added "func" 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 4d03255683..db561026bd 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -1368,6 +1368,15 @@ 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.func >=
>> RTE_ETH_HASH_FUNCTION_MAX) {
>> + RTE_ETHDEV_LOG(WARNING,
>> + "Ethdev port_id=%u invalid rss hash function (%u),
>> modified to default value (%u)\n",
>> + port_id, dev_conf->rx_adv_conf.rss_conf.func,
>> + RTE_ETH_HASH_FUNCTION_DEFAULT);
>> + dev->data->dev_conf.rx_adv_conf.rss_conf.func =
>> + RTE_ETH_HASH_FUNCTION_DEFAULT;
>
> I have no strong opinion, but, to me, this behaviour conceals
> programming errors. For example, if an application intends
> to enable hash algorithm A but, due to a programming error,
> passes a gibberish value here, chances are the error will
> end up unnoticed. Especially in case the application
> sets the log level to such that warnings are omitted.
Good point, will fix.
>
> Why not just return the error the standard way?
Aha, The original intention is not to break the ABI,
but I think it could not achieve that.
>
>> + }
>> +
>> /* Check if Rx RSS distribution is disabled but RSS hash is
>> enabled. */
>> if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
>> (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
>> @@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
>> return -ENOTSUP;
>> }
>>
>> + if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
>> + RTE_ETHDEV_LOG(NOTICE,
>> + "Ethdev port_id=%u invalid rss hash function (%u),
>> modified to default value (%u)\n",
>> + port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
>> + rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
>> + }
>> +
>> if (*dev->dev_ops->rss_hash_update == NULL)
>> return -ENOTSUP;
>> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
>> @@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>> return -EINVAL;
>> }
>>
>> + rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
>> +
>> if (*dev->dev_ops->rss_hash_conf_get == NULL)
>> return -ENOTSUP;
>> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 99fe9e238b..5abe2cb36d 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -174,6 +174,7 @@ extern "C" {
>>
>> #include "rte_ethdev_trace_fp.h"
>> #include "rte_dev_info.h"
>> +#include "rte_flow.h"
>>
>> extern int rte_eth_dev_logtype;
>>
>> @@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
>> * The *rss_hf* field of the *rss_conf* structure indicates the different
>> * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
>> * Supplying an *rss_hf* equal to zero disables the RSS feature.
>> + *
>> + * The *func* field of the *rss_conf* structure indicates the different
>> + * types of hash algorithms applied by the RSS hashing.
>
> Consider:
>
> The *func* field of the *rss_conf* structure indicates the algorithm to
> use when computing hash. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
> the PMD to use its best-effort algorithm rather than a specific one.
Look at some PMD drivers(i40e, hns3 etc), it seems the
RTE_ETH_HASH_FUNCTION_DEFAULT consider as no rss algorithm is set.
Thanks,
Dongdong
>
>> */
>> struct rte_eth_rss_conf {
>> uint8_t *rss_key; /**< If not NULL, 40-byte hash key. */
>> uint8_t rss_key_len; /**< hash key length in bytes. */
>> uint64_t rss_hf; /**< Hash functions to apply - see below. */
>> + enum rte_eth_hash_function func; /**< Hash algorithm to apply. */
>> };
>>
>> /*
>> --
>> 2.22.0
>>
>>
>
> Thank you.
>
> .
>
Hi Thomas
On 2023/3/15 21:43, Thomas Monjalon wrote:
> 15/03/2023 12:00, Dongdong Liu:
>> From: Jie Hai <haijie1@huawei.com>
>> --- a/doc/guides/rel_notes/release_23_03.rst
>> +++ b/doc/guides/rel_notes/release_23_03.rst
>> -* No ABI change that would break compatibility with 22.11.
>> -
>> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
>> + algorithm.
>
> We cannot break ABI compatibility until 23.11.
Got it. Thank you for reminding.
[PATCH 3/5] and [PATCH 4/5] do not relate with this ABI compatibility.
I will send them separately.
Thanks,
Dongdong
>
>
>
> .
>
Hi,
Thanks for responding and PSB.
On Thu, 16 Mar 2023, Dongdong Liu wrote:
> Hi Ivan
>
> Many thanks for your review.
>
> On 2023/3/15 19:28, Ivan Malov wrote:
>> Hi,
>>
>> On Wed, 15 Mar 2023, Dongdong Liu wrote:
>>
>>> From: Jie Hai <haijie1@huawei.com>
>>>
>>> Currently, rte_eth_rss_conf supports configuring 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,
>>> "func". This represents the RSS algorithms to apply. The following
>>> API is affected:
>>> - rte_eth_dev_configure
>>> - rte_eth_dev_rss_hash_update
>>> - rte_eth_dev_rss_hash_conf_get
>>>
>>> To prevent configuration failures caused by incorrect func input, check
>>> this parameter in advance. If it's incorrect, a warning is generated
>>> and the default value is set. Do the same for rte_eth_dev_rss_hash_update
>>> and rte_eth_dev_configure.
>>>
>>> To check whether the drivers report the func field, it is set to default
>>> value before querying.
>>>
>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>> ---
>>> doc/guides/rel_notes/release_23_03.rst | 4 ++--
>>> lib/ethdev/rte_ethdev.c | 18 ++++++++++++++++++
>>> lib/ethdev/rte_ethdev.h | 5 +++++
>>> 3 files changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/doc/guides/rel_notes/release_23_03.rst
>>> b/doc/guides/rel_notes/release_23_03.rst
>>> index af6f37389c..7879567427 100644
>>> --- a/doc/guides/rel_notes/release_23_03.rst
>>> +++ b/doc/guides/rel_notes/release_23_03.rst
>>> @@ -284,8 +284,8 @@ ABI Changes
>>> Also, make sure to start the actual text at the margin.
>>> =======================================================
>>>
>>> -* No ABI change that would break compatibility with 22.11.
>>> -
>>> +* ethdev: Added "func" 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 4d03255683..db561026bd 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -1368,6 +1368,15 @@ 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.func >=
>>> RTE_ETH_HASH_FUNCTION_MAX) {
>>> + RTE_ETHDEV_LOG(WARNING,
>>> + "Ethdev port_id=%u invalid rss hash function (%u),
>>> modified to default value (%u)\n",
>>> + port_id, dev_conf->rx_adv_conf.rss_conf.func,
>>> + RTE_ETH_HASH_FUNCTION_DEFAULT);
>>> + dev->data->dev_conf.rx_adv_conf.rss_conf.func =
>>> + RTE_ETH_HASH_FUNCTION_DEFAULT;
>>
>> I have no strong opinion, but, to me, this behaviour conceals
>> programming errors. For example, if an application intends
>> to enable hash algorithm A but, due to a programming error,
>> passes a gibberish value here, chances are the error will
>> end up unnoticed. Especially in case the application
>> sets the log level to such that warnings are omitted.
> Good point, will fix.
>>
>> Why not just return the error the standard way?
>
> Aha, The original intention is not to break the ABI,
> but I think it could not achieve that.
>>
>>> + }
>>> +
>>> /* Check if Rx RSS distribution is disabled but RSS hash is
>>> enabled. */
>>> if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
>>> (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
>>> @@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
>>> return -ENOTSUP;
>>> }
>>>
>>> + if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
>>> + RTE_ETHDEV_LOG(NOTICE,
>>> + "Ethdev port_id=%u invalid rss hash function (%u),
>>> modified to default value (%u)\n",
>>> + port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
>>> + rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
>>> + }
>>> +
>>> if (*dev->dev_ops->rss_hash_update == NULL)
>>> return -ENOTSUP;
>>> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
>>> @@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>>> return -EINVAL;
>>> }
>>>
>>> + rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
>>> +
>>> if (*dev->dev_ops->rss_hash_conf_get == NULL)
>>> return -ENOTSUP;
>>> ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>>> index 99fe9e238b..5abe2cb36d 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -174,6 +174,7 @@ extern "C" {
>>>
>>> #include "rte_ethdev_trace_fp.h"
>>> #include "rte_dev_info.h"
>>> +#include "rte_flow.h"
>>>
>>> extern int rte_eth_dev_logtype;
>>>
>>> @@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
>>> * The *rss_hf* field of the *rss_conf* structure indicates the different
>>> * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
>>> * Supplying an *rss_hf* equal to zero disables the RSS feature.
>>> + *
>>> + * The *func* field of the *rss_conf* structure indicates the different
>>> + * types of hash algorithms applied by the RSS hashing.
>>
>> Consider:
>>
>> The *func* field of the *rss_conf* structure indicates the algorithm to
>> use when computing hash. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
>> the PMD to use its best-effort algorithm rather than a specific one.
>
> Look at some PMD drivers(i40e, hns3 etc), it seems the
> RTE_ETH_HASH_FUNCTION_DEFAULT consider as no rss algorithm is set.
This does not seem to contradict the suggested description.
If they, however, treat this as "no RSS at all", then
perhaps it is a mistake, because if the user requests
Rx MQ mode "RSS" and selects algorithm DEFAULT, this
is clearly not the same as "no RSS". Not by a long
shot. Because for "no RSS" the user would have
passed MQ mode choice "NONE", I take it.
>
> Thanks,
> Dongdong
>>
>>> */
>>> struct rte_eth_rss_conf {
>>> uint8_t *rss_key; /**< If not NULL, 40-byte hash key. */
>>> uint8_t rss_key_len; /**< hash key length in bytes. */
>>> uint64_t rss_hf; /**< Hash functions to apply - see below. */
>>> + enum rte_eth_hash_function func; /**< Hash algorithm to apply. */
>>> };
>>>
>>> /*
>>> --
>>> 2.22.0
>>>
>>>
>>
>> Thank you.
>>
>> .
>>
>
Thank you.
On 3/16/2023 1:16 PM, Dongdong Liu wrote:
> Hi Thomas
> On 2023/3/15 21:43, Thomas Monjalon wrote:
>> 15/03/2023 12:00, Dongdong Liu:
>>> From: Jie Hai <haijie1@huawei.com>
>>> --- a/doc/guides/rel_notes/release_23_03.rst
>>> +++ b/doc/guides/rel_notes/release_23_03.rst
>>> -* No ABI change that would break compatibility with 22.11.
>>> -
>>> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for
>>> RSS hash
>>> + algorithm.
>>
>> We cannot break ABI compatibility until 23.11.
> Got it. Thank you for reminding.
>
Hi Dongdong,
Please remember to send a deprecation notice for this release.
Deprecation notice should be merged in this release so that it can be
applied in v23.11
> [PATCH 3/5] and [PATCH 4/5] do not relate with this ABI compatibility.
> I will send them separately.
>
> Thanks,
> Dongdong
>>
>>
>>
>> .
>>
Hi Ferruh
On 2023/6/3 4:19, Ferruh Yigit wrote:
> On 3/16/2023 1:16 PM, Dongdong Liu wrote:
>> Hi Thomas
>> On 2023/3/15 21:43, Thomas Monjalon wrote:
>>> 15/03/2023 12:00, Dongdong Liu:
>>>> From: Jie Hai <haijie1@huawei.com>
>>>> --- a/doc/guides/rel_notes/release_23_03.rst
>>>> +++ b/doc/guides/rel_notes/release_23_03.rst
>>>> -* No ABI change that would break compatibility with 22.11.
>>>> -
>>>> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for
>>>> RSS hash
>>>> + algorithm.
>>>
>>> We cannot break ABI compatibility until 23.11.
>> Got it. Thank you for reminding.
>>
>
> Hi Dongdong,
>
> Please remember to send a deprecation notice for this release.
> Deprecation notice should be merged in this release so that it can be
> applied in v23.11
Thanks for pointing that.
Will do.
Thanks,
Dongdong
>
>
>> [PATCH 3/5] and [PATCH 4/5] do not relate with this ABI compatibility.
>> I will send them separately.
>>
>> Thanks,
>> Dongdong
>>>
>>>
>>>
>>> .
>>>
>
> .
>
@@ -284,8 +284,8 @@ ABI Changes
Also, make sure to start the actual text at the margin.
=======================================================
-* No ABI change that would break compatibility with 22.11.
-
+* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
+ algorithm.
Known Issues
------------
@@ -1368,6 +1368,15 @@ 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.func >= RTE_ETH_HASH_FUNCTION_MAX) {
+ RTE_ETHDEV_LOG(WARNING,
+ "Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
+ port_id, dev_conf->rx_adv_conf.rss_conf.func,
+ RTE_ETH_HASH_FUNCTION_DEFAULT);
+ dev->data->dev_conf.rx_adv_conf.rss_conf.func =
+ RTE_ETH_HASH_FUNCTION_DEFAULT;
+ }
+
/* Check if Rx RSS distribution is disabled but RSS hash is enabled. */
if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
(dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
@@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
return -ENOTSUP;
}
+ if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
+ RTE_ETHDEV_LOG(NOTICE,
+ "Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
+ port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
+ rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
+ }
+
if (*dev->dev_ops->rss_hash_update == NULL)
return -ENOTSUP;
ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
@@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
return -EINVAL;
}
+ rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
+
if (*dev->dev_ops->rss_hash_conf_get == NULL)
return -ENOTSUP;
ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
@@ -174,6 +174,7 @@ extern "C" {
#include "rte_ethdev_trace_fp.h"
#include "rte_dev_info.h"
+#include "rte_flow.h"
extern int rte_eth_dev_logtype;
@@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
* The *rss_hf* field of the *rss_conf* structure indicates the different
* types of IPv4/IPv6 packets to which the RSS hashing must be applied.
* Supplying an *rss_hf* equal to zero disables the RSS feature.
+ *
+ * The *func* field of the *rss_conf* structure indicates the different
+ * types of hash algorithms applied by the RSS hashing.
*/
struct rte_eth_rss_conf {
uint8_t *rss_key; /**< If not NULL, 40-byte hash key. */
uint8_t rss_key_len; /**< hash key length in bytes. */
uint64_t rss_hf; /**< Hash functions to apply - see below. */
+ enum rte_eth_hash_function func; /**< Hash algorithm to apply. */
};
/*