[dpdk-dev] [PATCH v1] net/ice: fix wrong RSS hash update

[dpdk-dev] [PATCH v1] net/ice: fix wrong RSS hash update

[Wenjun Wu]

This patch removes redundant judgment statements to disable RSS
when RSS hash function configured is not supported.

Fixes: 4717a12cfaf1 ("net/ice: initialize and update RSS based on user config")
Cc: stable@dpdk.org

Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index f43b2e0b2..a84b3d3c0 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -4461,9 +4461,6 @@ ice_rss_hash_update(struct rte_eth_dev *dev,
 	if (status)
 		return status;
 
-	if (rss_conf->rss_hf == 0)
-		return 0;
-
 	/* RSS hash configuration */
 	ice_rss_hash_set(pf, rss_conf->rss_hf);
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH v1] net/ice: fix wrong RSS hash update

[Yang, Qiming]

> -----Original Message-----
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Sent: Tuesday, March 2, 2021 13:31
> To: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; stable@dpdk.org
> Subject: [PATCH v1] net/ice: fix wrong RSS hash update
> 
> This patch removes redundant judgment statements to disable RSS when RSS
> hash function configured is not supported.
> 
> Fixes: 4717a12cfaf1 ("net/ice: initialize and update RSS based on user config")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> ---
>  drivers/net/ice/ice_ethdev.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
> f43b2e0b2..a84b3d3c0 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -4461,9 +4461,6 @@ ice_rss_hash_update(struct rte_eth_dev *dev,
>  	if (status)
>  		return status;
> 
> -	if (rss_conf->rss_hf == 0)
> -		return 0;
> -
Why need to delete this code? It's a code clean to avoid to do more judgement in the next funxtion.

>  	/* RSS hash configuration */
>  	ice_rss_hash_set(pf, rss_conf->rss_hf);
> 
> --
> 2.25.1

Re: [dpdk-dev] [PATCH v1] net/ice: fix wrong RSS hash update

[Wu, Wenjun1]

Hi Qiming, 

There are two consequences if skipping the state rss_conf->rss_hf == 0.

1. The function " port config all rss none" means to disable RSS. When rss_conf->rss_hf == 0, the function will not take affect, which does not conform to the description in dpdk doc:
	The none option is equivalent to the --disable-rss command-line option.

2. Some ptypes are not supported by CVL. When rss_conf->rss_hf == 0, users cannot predict the consequences when setting RSS with these unsupported ptypes. The RSS may be disabled, or with no change, which is not what we want to see.

So delete these codes may be better.

Regards,
Wenjun

-----Original Message-----
From: Yang, Qiming <qiming.yang@intel.com> 
Sent: Wednesday, March 3, 2021 2:47 PM
To: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: stable@dpdk.org
Subject: RE: [PATCH v1] net/ice: fix wrong RSS hash update



> -----Original Message-----
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Sent: Tuesday, March 2, 2021 13:31
> To: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; stable@dpdk.org
> Subject: [PATCH v1] net/ice: fix wrong RSS hash update
> 
> This patch removes redundant judgment statements to disable RSS when 
> RSS hash function configured is not supported.
> 
> Fixes: 4717a12cfaf1 ("net/ice: initialize and update RSS based on user 
> config")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> ---
>  drivers/net/ice/ice_ethdev.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_ethdev.c 
> b/drivers/net/ice/ice_ethdev.c index
> f43b2e0b2..a84b3d3c0 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -4461,9 +4461,6 @@ ice_rss_hash_update(struct rte_eth_dev *dev,
>  	if (status)
>  		return status;
> 
> -	if (rss_conf->rss_hf == 0)
> -		return 0;
> -
Why need to delete this code? It's a code clean to avoid to do more judgement in the next funxtion.

>  	/* RSS hash configuration */
>  	ice_rss_hash_set(pf, rss_conf->rss_hf);
> 
> --
> 2.25.1