[06/16] net/hns3: separate the setting of hash algorithm
Checks
Commit Message
From: Huisong Li <lihuisong@huawei.com>
Currently, the setting of hash algorithm comes from the
default configuration in driver and the rte_flow interface.
The hash algorithm that is set to hardware in both ways is
saved in hw->rss_info.conf.func.
But the 'func' in struct rte_flow_action_rss is usually used
in rte flow interface. And the ethdev ops interface may also
set hash algorithm in the future. It is not appropriate and
is a little messy for ethdev ops interface and driver default
configuration to use struct rte_flow_action_rss. So we have
to separate the RSS configuration from ethdev ops and rte
flow interface to make codes more easier to maintain.
This patch separates hash algorithm by following ways:
1) 'hash_algo' in struct hns3_rss_conf is used for ethdev ops
interface or default configuration in driver.
2) Add a 'rte_flow_hash_algo' field in struct hns3_rss_conf
to save algorithm from rte flow interface. The main reasons
are as follows:
Currently, only the last rule is used to restore the rte
flow rule. If 'func' in RSS action is 'DEFAULT', it means
that this rule doesn't modify algorithm and driver need to
save current algorithm for restoring algorithm during reset
phase.
Fixes: c37ca66f2b27 ("net/hns3: support RSS")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
drivers/net/hns3/hns3_flow.c | 60 +++++++++++++++++++++---------------
drivers/net/hns3/hns3_rss.c | 14 +--------
drivers/net/hns3/hns3_rss.h | 1 +
3 files changed, 37 insertions(+), 38 deletions(-)
Comments
On 3/10/2023 9:35 AM, Dongdong Liu wrote:
> From: Huisong Li <lihuisong@huawei.com>
>
> Currently, the setting of hash algorithm comes from the
> default configuration in driver and the rte_flow interface.
> The hash algorithm that is set to hardware in both ways is
> saved in hw->rss_info.conf.func.
>
> But the 'func' in struct rte_flow_action_rss is usually used
> in rte flow interface. And the ethdev ops interface may also
> set hash algorithm in the future. It is not appropriate and
> is a little messy for ethdev ops interface and driver default
> configuration to use struct rte_flow_action_rss. So we have
> to separate the RSS configuration from ethdev ops and rte
> flow interface to make codes more easier to maintain.
>
Agree that it is not ideal to have two different ways for same/similar
control path functionality in ethdev.
+Ori to discuss if this is a common problem and can be resolved in
ethdev layer.
I can see some of remaining patches are related to this ethdev / flow
API RSS separation. I will continue with this set, but I believe it is
better if this issue addressed in higher level.
> This patch separates hash algorithm by following ways:
> 1) 'hash_algo' in struct hns3_rss_conf is used for ethdev ops
> interface or default configuration in driver.
> 2) Add a 'rte_flow_hash_algo' field in struct hns3_rss_conf
> to save algorithm from rte flow interface. The main reasons
> are as follows:
> Currently, only the last rule is used to restore the rte
> flow rule. If 'func' in RSS action is 'DEFAULT', it means
> that this rule doesn't modify algorithm and driver need to
> save current algorithm for restoring algorithm during reset
> phase.
>
> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> Cc: stable@dpdk.org
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
<...>
在 2023/3/11 3:36, Ferruh Yigit 写道:
> On 3/10/2023 9:35 AM, Dongdong Liu wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Currently, the setting of hash algorithm comes from the
>> default configuration in driver and the rte_flow interface.
>> The hash algorithm that is set to hardware in both ways is
>> saved in hw->rss_info.conf.func.
>>
>> But the 'func' in struct rte_flow_action_rss is usually used
>> in rte flow interface. And the ethdev ops interface may also
>> set hash algorithm in the future. It is not appropriate and
>> is a little messy for ethdev ops interface and driver default
>> configuration to use struct rte_flow_action_rss. So we have
>> to separate the RSS configuration from ethdev ops and rte
>> flow interface to make codes more easier to maintain.
>>
> Agree that it is not ideal to have two different ways for same/similar
> control path functionality in ethdev.
>
> +Ori to discuss if this is a common problem and can be resolved in
> ethdev layer.
This patchset is aimed to decouple the configuration API and the
structure used by
ethdev ops and rte flow API in driver. I think this can be ignored.
>
> I can see some of remaining patches are related to this ethdev / flow
> API RSS separation. I will continue with this set, but I believe it is
> better if this issue addressed in higher level.
As far as I know, the priority of rte flow hash rule is higher than
ethdev ops by default,
and there are many other rules.
But the implementation guide documentation about it is not clear.
It may be better if we can clarify the documentation to guide driver coding.
>> This patch separates hash algorithm by following ways:
>> 1) 'hash_algo' in struct hns3_rss_conf is used for ethdev ops
>> interface or default configuration in driver.
>> 2) Add a 'rte_flow_hash_algo' field in struct hns3_rss_conf
>> to save algorithm from rte flow interface. The main reasons
>> are as follows:
>> Currently, only the last rule is used to restore the rte
>> flow rule. If 'func' in RSS action is 'DEFAULT', it means
>> that this rule doesn't modify algorithm and driver need to
>> save current algorithm for restoring algorithm during reset
>> phase.
>>
>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> <...>
>
> .
On 3/29/2023 2:58 AM, lihuisong (C) wrote:
>
> 在 2023/3/11 3:36, Ferruh Yigit 写道:
>> On 3/10/2023 9:35 AM, Dongdong Liu wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Currently, the setting of hash algorithm comes from the
>>> default configuration in driver and the rte_flow interface.
>>> The hash algorithm that is set to hardware in both ways is
>>> saved in hw->rss_info.conf.func.
>>>
>>> But the 'func' in struct rte_flow_action_rss is usually used
>>> in rte flow interface. And the ethdev ops interface may also
>>> set hash algorithm in the future. It is not appropriate and
>>> is a little messy for ethdev ops interface and driver default
>>> configuration to use struct rte_flow_action_rss. So we have
>>> to separate the RSS configuration from ethdev ops and rte
>>> flow interface to make codes more easier to maintain.
>>>
>> Agree that it is not ideal to have two different ways for same/similar
>> control path functionality in ethdev.
>>
>> +Ori to discuss if this is a common problem and can be resolved in
>> ethdev layer.
> This patchset is aimed to decouple the configuration API and the
> structure used by
> ethdev ops and rte flow API in driver. I think this can be ignored.
>>
>> I can see some of remaining patches are related to this ethdev / flow
>> API RSS separation. I will continue with this set, but I believe it is
>> better if this issue addressed in higher level.
> As far as I know, the priority of rte flow hash rule is higher than
> ethdev ops by default,
> and there are many other rules.
> But the implementation guide documentation about it is not clear.
> It may be better if we can clarify the documentation to guide driver
> coding.
I am not aware of any priority between flow API hash rule and ethdev
APIs, Ori & Andrew may comment better. And +1 to document this.
>>> This patch separates hash algorithm by following ways:
>>> 1) 'hash_algo' in struct hns3_rss_conf is used for ethdev ops
>>> interface or default configuration in driver.
>>> 2) Add a 'rte_flow_hash_algo' field in struct hns3_rss_conf
>>> to save algorithm from rte flow interface. The main reasons
>>> are as follows:
>>> Currently, only the last rule is used to restore the rte
>>> flow rule. If 'func' in RSS action is 'DEFAULT', it means
>>> that this rule doesn't modify algorithm and driver need to
>>> save current algorithm for restoring algorithm during reset
>>> phase.
>>>
>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> <...>
>>
>> .
@@ -1438,30 +1438,40 @@ hns3_disable_rss(struct hns3_hw *hw)
}
static int
-hns3_parse_rss_algorithm(struct hns3_hw *hw, enum rte_eth_hash_function *func,
+hns3_parse_rss_algorithm(struct hns3_hw *hw, enum rte_eth_hash_function func,
uint8_t *hash_algo)
{
- enum rte_eth_hash_function algo_func = *func;
- switch (algo_func) {
- case RTE_ETH_HASH_FUNCTION_DEFAULT:
- /* Keep *hash_algo as what it used to be */
- algo_func = hw->rss_info.conf.func;
- break;
- case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
- *hash_algo = HNS3_RSS_HASH_ALGO_TOEPLITZ;
- break;
- case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
- *hash_algo = HNS3_RSS_HASH_ALGO_SIMPLE;
- break;
- case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
- *hash_algo = HNS3_RSS_HASH_ALGO_SYMMETRIC_TOEP;
- break;
- default:
- hns3_err(hw, "Invalid RSS algorithm configuration(%d)",
- algo_func);
- return -EINVAL;
+ const uint8_t hash_func_map[] = {
+ [RTE_ETH_HASH_FUNCTION_DEFAULT] = HNS3_RSS_HASH_ALGO_TOEPLITZ,
+ [RTE_ETH_HASH_FUNCTION_TOEPLITZ] = HNS3_RSS_HASH_ALGO_TOEPLITZ,
+ [RTE_ETH_HASH_FUNCTION_SIMPLE_XOR] = HNS3_RSS_HASH_ALGO_SIMPLE,
+ [RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ] = HNS3_RSS_HASH_ALGO_SYMMETRIC_TOEP,
+ };
+ uint8_t key[HNS3_RSS_KEY_SIZE_MAX] = {0};
+ int ret;
+
+ if (func == RTE_ETH_HASH_FUNCTION_DEFAULT) {
+ ret = hns3_rss_get_algo_key(hw, hash_algo, key,
+ hw->rss_key_size);
+ if (ret != 0) {
+ hns3_err(hw, "fail to get current RSS hash algorithm, ret = %d",
+ ret);
+ return ret;
+ }
+
+ /*
+ * During the phase of reset recovery, the hash algorithm
+ * obtained from hardware may not be the one used(saved in
+ * rte_flow_hash_algo) when this rule is delivered.
+ */
+ if (__atomic_load_n(&hw->reset.resetting, __ATOMIC_RELAXED) &&
+ *hash_algo != hw->rss_info.rte_flow_hash_algo)
+ *hash_algo = hw->rss_info.rte_flow_hash_algo;
+
+ return 0;
}
- *func = algo_func;
+
+ *hash_algo = hash_func_map[func];
return 0;
}
@@ -1471,6 +1481,7 @@ hns3_hw_rss_hash_set(struct hns3_hw *hw, struct rte_flow_action_rss *rss_config)
{
uint8_t rss_key[HNS3_RSS_KEY_SIZE_MAX] = {0};
bool use_default_key = false;
+ uint8_t hash_algo;
int ret;
if (rss_config->key == NULL || rss_config->key_len != hw->rss_key_size) {
@@ -1480,18 +1491,17 @@ hns3_hw_rss_hash_set(struct hns3_hw *hw, struct rte_flow_action_rss *rss_config)
use_default_key = true;
}
- ret = hns3_parse_rss_algorithm(hw, &rss_config->func,
- &hw->rss_info.hash_algo);
+ ret = hns3_parse_rss_algorithm(hw, rss_config->func, &hash_algo);
if (ret)
return ret;
- ret = hns3_rss_set_algo_key(hw, hw->rss_info.hash_algo,
+ ret = hns3_rss_set_algo_key(hw, hash_algo,
use_default_key ? rss_key : rss_config->key,
hw->rss_key_size);
if (ret)
return ret;
- hw->rss_info.conf.func = rss_config->func;
+ hw->rss_info.rte_flow_hash_algo = hash_algo;
ret = hns3_set_rss_tuple_by_rss_hf(hw, rss_config->types);
if (ret)
@@ -1021,7 +1021,7 @@ hns3_rss_set_default_args(struct hns3_hw *hw)
uint16_t i;
/* Default hash algorithm */
- rss_cfg->conf.func = RTE_ETH_HASH_FUNCTION_TOEPLITZ;
+ rss_cfg->hash_algo = HNS3_RSS_HASH_ALGO_TOEPLITZ;
memcpy(rss_cfg->key, hns3_hash_key,
RTE_MIN(sizeof(hns3_hash_key), hw->rss_key_size));
@@ -1045,18 +1045,6 @@ hns3_config_rss(struct hns3_adapter *hns)
enum rte_eth_rx_mq_mode mq_mode = hw->data->dev_conf.rxmode.mq_mode;
- switch (hw->rss_info.conf.func) {
- case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
- hw->rss_info.hash_algo = HNS3_RSS_HASH_ALGO_SIMPLE;
- break;
- case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
- hw->rss_info.hash_algo = HNS3_RSS_HASH_ALGO_SYMMETRIC_TOEP;
- break;
- default:
- hw->rss_info.hash_algo = HNS3_RSS_HASH_ALGO_TOEPLITZ;
- break;
- }
-
ret = hns3_rss_set_algo_key(hw, rss_cfg->hash_algo,
hash_key, hw->rss_key_size);
if (ret)
@@ -42,6 +42,7 @@ struct hns3_rss_conf {
/* RSS parameters :algorithm, flow_types, key, queue */
struct rte_flow_action_rss conf;
uint8_t hash_algo; /* hash function type defined by hardware */
+ uint8_t rte_flow_hash_algo;
uint8_t key[HNS3_RSS_KEY_SIZE_MAX]; /* Hash key */
uint16_t rss_indirection_tbl[HNS3_RSS_IND_TBL_SIZE_MAX];
uint16_t queue[HNS3_RSS_QUEUES_BUFFER_NUM]; /* Queues indices to use */