[2/2] qede: make driver accept bigger rss tables

Message ID 20210221111637.31193-3-irusskikh@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series qede: 2020-02 minor fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-abi-testing warning Testing issues

Commit Message

Igor Russkikh Feb. 21, 2021, 11:16 a.m. UTC
  We found some dpdk applications blindly pass fixed side rss hash tables,
and do not check driver/device capabilities.

Moreover, many other drivers do not do such a strong check as well.

So here we fix it, making qede accept any size rss_key. For larger key
tables we just crop it with notice trace message.

CC: stable@dpdk.org
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/qede/qede_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Jerin Jacob March 8, 2021, 7:02 p.m. UTC | #1
On Sun, Feb 21, 2021 at 4:47 PM Igor Russkikh <irusskikh@marvell.com> wrote:
>
> We found some dpdk applications blindly pass fixed side rss hash tables,
> and do not check driver/device capabilities.
>
> Moreover, many other drivers do not do such a strong check as well.
>
> So here we fix it, making qede accept any size rss_key. For larger key
> tables we just crop it with notice trace message.
>
> CC: stable@dpdk.org
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>

HI @Rasesh Mody  @Shahed Shaikh

Could you review this series?


> ---
>  drivers/net/qede/qede_ethdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index ab5f5b106..7363d98f2 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -2139,8 +2139,8 @@ int qede_rss_hash_update(struct rte_eth_dev *eth_dev,
>                 /* RSS hash key */
>                 if (key) {
>                         if (len > (ECORE_RSS_KEY_SIZE * sizeof(uint32_t))) {
> -                               DP_ERR(edev, "RSS key length exceeds limit\n");
> -                               return -EINVAL;
> +                               len = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
> +                               DP_NOTICE(edev, false, "RSS key length exceeds limit\n");
>                         }
>                         DP_INFO(edev, "Applying user supplied hash key\n");
>                         rss_params.update_rss_key = 1;
> --
> 2.25.1
>
  
Devendra Singh Rawat March 11, 2021, 9:28 a.m. UTC | #2
> -----Original Message-----
> From: Igor Russkikh <irusskikh@marvell.com>
> Sent: Sunday, February 21, 2021 4:47 PM
> To: dev@dpdk.org
> Cc: Rasesh Mody <rmody@marvell.com>; Devendra Singh Rawat
> <dsinghrawat@marvell.com>; Igor Russkikh <irusskikh@marvell.com>;
> stable@dpdk.org
> Subject: [PATCH 2/2] qede: make driver accept bigger rss tables
> 
> We found some dpdk applications blindly pass fixed side rss hash tables, and
> do not check driver/device capabilities.
> 
> Moreover, many other drivers do not do such a strong check as well.
> 
> So here we fix it, making qede accept any size rss_key. For larger key tables we
> just crop it with notice trace message.
> 
> CC: stable@dpdk.org
> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
> ---
>  drivers/net/qede/qede_ethdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index ab5f5b106..7363d98f2 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -2139,8 +2139,8 @@ int qede_rss_hash_update(struct rte_eth_dev
> *eth_dev,
>  		/* RSS hash key */
>  		if (key) {
>  			if (len > (ECORE_RSS_KEY_SIZE * sizeof(uint32_t))) {
> -				DP_ERR(edev, "RSS key length exceeds
> limit\n");
> -				return -EINVAL;
> +				len = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
> +				DP_NOTICE(edev, false, "RSS key length
> exceeds limit\n");

IMO, it will be better if the log message also states that RSS key is trimmed to 'len' size.

Thanks,
Devendra
  
Igor Russkikh March 19, 2021, 9:48 a.m. UTC | #3
> IMO, it will be better if the log message also states that RSS key is trimmed to 'len' size.

Resent v2, thanks.

Devendra, please take these two patches into our internal codebase as well.

Igor
  
Devendra Singh Rawat March 22, 2021, 5:22 a.m. UTC | #4
Sure Igor, I will do that.

Thanks,
Devendra

> -----Original Message-----
> From: Igor Russkikh <irusskikh@marvell.com>
> Sent: Friday, March 19, 2021 3:18 PM
> To: Devendra Singh Rawat <dsinghrawat@marvell.com>; dev@dpdk.org
> Cc: Rasesh Mody <rmody@marvell.com>; stable@dpdk.org
> Subject: RE: [PATCH 2/2] qede: make driver accept bigger rss tables
> 
> 
> > IMO, it will be better if the log message also states that RSS key is trimmed to
> 'len' size.
> 
> Resent v2, thanks.
> 
> Devendra, please take these two patches into our internal codebase as well.
> 
> Igor
  

Patch

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index ab5f5b106..7363d98f2 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -2139,8 +2139,8 @@  int qede_rss_hash_update(struct rte_eth_dev *eth_dev,
 		/* RSS hash key */
 		if (key) {
 			if (len > (ECORE_RSS_KEY_SIZE * sizeof(uint32_t))) {
-				DP_ERR(edev, "RSS key length exceeds limit\n");
-				return -EINVAL;
+				len = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
+				DP_NOTICE(edev, false, "RSS key length exceeds limit\n");
 			}
 			DP_INFO(edev, "Applying user supplied hash key\n");
 			rss_params.update_rss_key = 1;