[v1] net/ice: fix wrong RSS hash update

Message ID 20210302053105.63783-1-wenjun1.wu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [v1] net/ice: fix wrong RSS hash update |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot fail travis build: failed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-mellanox-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot success github build: passed

Commit Message

Wenjun Wu March 2, 2021, 5:31 a.m. UTC
  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(-)
  

Comments

Qiming Yang March 3, 2021, 6:46 a.m. UTC | #1
> -----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
  
Wenjun Wu March 3, 2021, 7:39 a.m. UTC | #2
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
  

Patch

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);