[PATCH] usertools/rss: add CNXK RSS key

[PATCH] usertools/rss: add CNXK RSS key

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

This patch adds RSS key for CNXK platforms. CNXK platform uses
48 bytes long key for hash calculations.

For the same patch also updates help mesaages to provide range
information for supporting NICs/platforms.

Also CNXK uses reta size as 64 so to get correct offset to retrieve
queue index, user must pass reta_size option as 64 i.e. -t 64.

Examples:
$ ./dpdk-rss-flows.py -k cnxk 8 28.0.0.0/24 40.0.0.0/24 -t 64
SRC_IP      DST_IP       QUEUE
28.0.0.1    40.0.0.1     7
28.0.0.1    40.0.0.2     2
28.0.0.1    40.0.0.3     4
28.0.0.1    40.0.0.7     1
28.0.0.1    40.0.0.8     3
28.0.0.1    40.0.0.9     5
28.0.0.1    40.0.0.10    0
28.0.0.1    40.0.0.11    6

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
Change-Id: I631639d74c0bf91759f8aa188836019b4990caf2
---
 usertools/dpdk-rss-flows.py | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index 73821eb471..b6edd7a2e0 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -188,11 +188,24 @@ def balanced_traffic(
         0x81, 0x15, 0x03, 0x66,
     )
 )
+# rss_key_default, see drivers/net/cnxk/cnxk_flow.c
+# Marvell's cnxk NICs take 48 bytes keys
+RSS_KEY_CNXK = bytes(
+    (
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+    )
+)
 # fmt: on
 DEFAULT_DRIVER_KEYS = {
     "intel": RSS_KEY_INTEL,
     "mlx": RSS_KEY_MLX,
     "i40e": RSS_KEY_I40E,
+    "cnxk": RSS_KEY_CNXK,
 }
 
 
@@ -202,7 +215,7 @@ def rss_key(value):
     try:
         key = binascii.unhexlify(value)
         if len(key) not in (40, 52):
-            raise argparse.ArgumentTypeError("The key must be 40 or 52 bytes long")
+            raise argparse.ArgumentTypeError("The key must be 40 to 52 bytes long")
         return key
     except (TypeError, ValueError) as e:
         raise argparse.ArgumentTypeError(str(e)) from e
@@ -299,7 +312,7 @@ def parse_args():
         default=RSS_KEY_INTEL,
         type=rss_key,
         help="""
-        The random 40-bytes key used to compute the RSS hash. This option
+        The random 40 to 52 bytes key used to compute the RSS hash. This option
         supports either a well-known name or the hex value of the key
         (well-known names: "intel", "mlx", default: "intel").
         """,
-- 
2.25.1

[PATCH v2 1/1] usertools/rss: add CNXK RSS key

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

This patch adds RSS key for CNXK platforms. CNXK platform uses
48 bytes long key for hash calculations.

For the same patch also updates help mesaages to provide range
information for supporting NICs/platforms.

Also CNXK uses reta size as 64 so to get correct offset to retrieve
queue index, user must pass reta_size option as 64 i.e. -t 64.

Examples:
$ ./dpdk-rss-flows.py -k cnxk 8 28.0.0.0/24 40.0.0.0/24 -t 64
SRC_IP      DST_IP       QUEUE
28.0.0.1    40.0.0.1     7
28.0.0.1    40.0.0.2     2
28.0.0.1    40.0.0.3     4
28.0.0.1    40.0.0.7     1
28.0.0.1    40.0.0.8     3
28.0.0.1    40.0.0.9     5
28.0.0.1    40.0.0.10    0
28.0.0.1    40.0.0.11    6

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v1..v2:
 - Fix checkpatch errors.

 usertools/dpdk-rss-flows.py | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index 73821eb471..b6edd7a2e0 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -188,11 +188,24 @@ def balanced_traffic(
         0x81, 0x15, 0x03, 0x66,
     )
 )
+# rss_key_default, see drivers/net/cnxk/cnxk_flow.c
+# Marvell's cnxk NICs take 48 bytes keys
+RSS_KEY_CNXK = bytes(
+    (
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+    )
+)
 # fmt: on
 DEFAULT_DRIVER_KEYS = {
     "intel": RSS_KEY_INTEL,
     "mlx": RSS_KEY_MLX,
     "i40e": RSS_KEY_I40E,
+    "cnxk": RSS_KEY_CNXK,
 }
 
 
@@ -202,7 +215,7 @@ def rss_key(value):
     try:
         key = binascii.unhexlify(value)
         if len(key) not in (40, 52):
-            raise argparse.ArgumentTypeError("The key must be 40 or 52 bytes long")
+            raise argparse.ArgumentTypeError("The key must be 40 to 52 bytes long")
         return key
     except (TypeError, ValueError) as e:
         raise argparse.ArgumentTypeError(str(e)) from e
@@ -299,7 +312,7 @@ def parse_args():
         default=RSS_KEY_INTEL,
         type=rss_key,
         help="""
-        The random 40-bytes key used to compute the RSS hash. This option
+        The random 40 to 52 bytes key used to compute the RSS hash. This option
         supports either a well-known name or the hex value of the key
         (well-known names: "intel", "mlx", default: "intel").
         """,
-- 
2.25.1

Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Jerin Jacob]

On Mon, Oct 9, 2023 at 10:06 PM <skori@marvell.com> wrote:
>
> From: Sunil Kumar Kori <skori@marvell.com>
>
> This patch adds RSS key for CNXK platforms. CNXK platform uses
> 48 bytes long key for hash calculations.
>
> For the same patch also updates help mesaages to provide range
> information for supporting NICs/platforms.
>
> Also CNXK uses reta size as 64 so to get correct offset to retrieve
> queue index, user must pass reta_size option as 64 i.e. -t 64.
>
> Examples:
> $ ./dpdk-rss-flows.py -k cnxk 8 28.0.0.0/24 40.0.0.0/24 -t 64
> SRC_IP      DST_IP       QUEUE
> 28.0.0.1    40.0.0.1     7
> 28.0.0.1    40.0.0.2     2
> 28.0.0.1    40.0.0.3     4
> 28.0.0.1    40.0.0.7     1
> 28.0.0.1    40.0.0.8     3
> 28.0.0.1    40.0.0.9     5
> 28.0.0.1    40.0.0.10    0
> 28.0.0.1    40.0.0.11    6
>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>

Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Thomas Monjalon]

Adding the script author, Robin.

09/10/2023 18:36, skori@marvell.com:
> From: Sunil Kumar Kori <skori@marvell.com>
> 
> This patch adds RSS key for CNXK platforms. CNXK platform uses
> 48 bytes long key for hash calculations.
> 
> For the same patch also updates help mesaages to provide range
> information for supporting NICs/platforms.
> 
> Also CNXK uses reta size as 64 so to get correct offset to retrieve
> queue index, user must pass reta_size option as 64 i.e. -t 64.

Can it be something automatic instead of asking the user to pass -t 64?

> +# rss_key_default, see drivers/net/cnxk/cnxk_flow.c

I don't see rss_key_default in drivers/net/cnxk/cnxk_flow.c

> +# Marvell's cnxk NICs take 48 bytes keys
> +RSS_KEY_CNXK = bytes(
> +    (
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +    )
> +)

Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Robin Jarry]

, Oct 09, 2023 at 18:36:
> From: Sunil Kumar Kori <skori@marvell.com>
>
> This patch adds RSS key for CNXK platforms. CNXK platform uses
> 48 bytes long key for hash calculations.
>
> For the same patch also updates help mesaages to provide range
> information for supporting NICs/platforms.
>
> Also CNXK uses reta size as 64 so to get correct offset to retrieve
> queue index, user must pass reta_size option as 64 i.e. -t 64.

I think we should add some driver abstraction that contains the required 
key length and default reta size. Instead of requiring the user to guess 
the correct values. Is that something you could do?

>
> Examples:
> $ ./dpdk-rss-flows.py -k cnxk 8 28.0.0.0/24 40.0.0.0/24 -t 64
> SRC_IP      DST_IP       QUEUE
> 28.0.0.1    40.0.0.1     7
> 28.0.0.1    40.0.0.2     2
> 28.0.0.1    40.0.0.3     4
> 28.0.0.1    40.0.0.7     1
> 28.0.0.1    40.0.0.8     3
> 28.0.0.1    40.0.0.9     5
> 28.0.0.1    40.0.0.10    0
> 28.0.0.1    40.0.0.11    6
>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
> v1..v2:
>  - Fix checkpatch errors.

Hi Sunil,

>  usertools/dpdk-rss-flows.py | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
> index 73821eb471..b6edd7a2e0 100755
> --- a/usertools/dpdk-rss-flows.py
> +++ b/usertools/dpdk-rss-flows.py
> @@ -188,11 +188,24 @@ def balanced_traffic(
>          0x81, 0x15, 0x03, 0x66,
>      )
>  )
> +# rss_key_default, see drivers/net/cnxk/cnxk_flow.c

Are you referring to roc_nix_rss_key_default_fill in 
drivers/common/cnxk/roc_nix_rss.c?

> +# Marvell's cnxk NICs take 48 bytes keys
> +RSS_KEY_CNXK = bytes(
> +    (
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> +    )
> +)
>  # fmt: on
>  DEFAULT_DRIVER_KEYS = {
>      "intel": RSS_KEY_INTEL,
>      "mlx": RSS_KEY_MLX,
>      "i40e": RSS_KEY_I40E,
> +    "cnxk": RSS_KEY_CNXK,
>  }
>  
>  
> @@ -202,7 +215,7 @@ def rss_key(value):
>      try:
>          key = binascii.unhexlify(value)
>          if len(key) not in (40, 52):
> -            raise argparse.ArgumentTypeError("The key must be 40 or 52 bytes long")
> +            raise argparse.ArgumentTypeError("The key must be 40 to 52 bytes long")

You are not changing the length test, so passing a 48 bytes key will 
trigger an error.

>          return key
>      except (TypeError, ValueError) as e:
>          raise argparse.ArgumentTypeError(str(e)) from e
> @@ -299,7 +312,7 @@ def parse_args():
>          default=RSS_KEY_INTEL,
>          type=rss_key,
>          help="""
> -        The random 40-bytes key used to compute the RSS hash. This option
> +        The random 40 to 52 bytes key used to compute the RSS hash. This option
>          supports either a well-known name or the hex value of the key
>          (well-known names: "intel", "mlx", default: "intel").
>          """,
> -- 
> 2.25.1



-- 
Robin Jarry
Principal Software Engineer
Red Hat, Telco/NFV

RE: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Sunil Kumar Kori]

> -----Original Message-----
> From: Robin Jarry <rjarry@redhat.com>
> Sent: Tuesday, October 17, 2023 5:47 PM
> To: Sunil Kumar Kori <skori@marvell.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob
> <jerinjacobk@gmail.com>
> Subject: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key
> 
> External Email
> 
> ----------------------------------------------------------------------
> , Oct 09, 2023 at 18:36:
> > From: Sunil Kumar Kori <skori@marvell.com>
> >
> > This patch adds RSS key for CNXK platforms. CNXK platform uses
> > 48 bytes long key for hash calculations.
> >
> > For the same patch also updates help mesaages to provide range
> > information for supporting NICs/platforms.
> >
> > Also CNXK uses reta size as 64 so to get correct offset to retrieve
> > queue index, user must pass reta_size option as 64 i.e. -t 64.
> 
> I think we should add some driver abstraction that contains the required key
> length and default reta size. Instead of requiring the user to guess the correct
> values. Is that something you could do?
> 
Okay but in either case i.e. -t option or driver abstraction, user must know the reta size and key size before configuring.
So  I am not sure that how adding driver abstraction will help to solve this issue unless/until its documented somewhere. 

So for current release, I am planning to go this version as it is because we are close.
Later on we can think of it and add required support. 
Please provide input on it. 

> >
> > Examples:
> > $ ./dpdk-rss-flows.py -k cnxk 8 28.0.0.0/24 40.0.0.0/24 -t 64
> > SRC_IP      DST_IP       QUEUE
> > 28.0.0.1    40.0.0.1     7
> > 28.0.0.1    40.0.0.2     2
> > 28.0.0.1    40.0.0.3     4
> > 28.0.0.1    40.0.0.7     1
> > 28.0.0.1    40.0.0.8     3
> > 28.0.0.1    40.0.0.9     5
> > 28.0.0.1    40.0.0.10    0
> > 28.0.0.1    40.0.0.11    6
> >
> > Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> > ---
> > v1..v2:
> >  - Fix checkpatch errors.
> 
> Hi Sunil,
> 
> >  usertools/dpdk-rss-flows.py | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
> > index 73821eb471..b6edd7a2e0 100755
> > --- a/usertools/dpdk-rss-flows.py
> > +++ b/usertools/dpdk-rss-flows.py
> > @@ -188,11 +188,24 @@ def balanced_traffic(
> >          0x81, 0x15, 0x03, 0x66,
> >      )
> >  )
> > +# rss_key_default, see drivers/net/cnxk/cnxk_flow.c
> 
> Are you referring to roc_nix_rss_key_default_fill in
> drivers/common/cnxk/roc_nix_rss.c?
> 
Yes, that is the correct file name. Will fix in next version. 

> > +# Marvell's cnxk NICs take 48 bytes keys
> > +RSS_KEY_CNXK = bytes(
> > +    (
> > +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> > +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> > +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> > +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> > +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> > +        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
> > +    )
> > +)
> >  # fmt: on
> >  DEFAULT_DRIVER_KEYS = {
> >      "intel": RSS_KEY_INTEL,
> >      "mlx": RSS_KEY_MLX,
> >      "i40e": RSS_KEY_I40E,
> > +    "cnxk": RSS_KEY_CNXK,
> >  }
> >
> >
> > @@ -202,7 +215,7 @@ def rss_key(value):
> >      try:
> >          key = binascii.unhexlify(value)
> >          if len(key) not in (40, 52):
> > -            raise argparse.ArgumentTypeError("The key must be 40 or 52 bytes
> long")
> > +            raise argparse.ArgumentTypeError("The key must be 40 to 52 bytes
> long")
> 
> You are not changing the length test, so passing a 48 bytes key will
> trigger an error.
> 
Ack. Will fix in next version.

> >          return key
> >      except (TypeError, ValueError) as e:
> >          raise argparse.ArgumentTypeError(str(e)) from e
> > @@ -299,7 +312,7 @@ def parse_args():
> >          default=RSS_KEY_INTEL,
> >          type=rss_key,
> >          help="""
> > -        The random 40-bytes key used to compute the RSS hash. This option
> > +        The random 40 to 52 bytes key used to compute the RSS hash. This
> option
> >          supports either a well-known name or the hex value of the key
> >          (well-known names: "intel", "mlx", default: "intel").
> >          """,
> > --
> > 2.25.1
> 
> 
> 
> --
> Robin Jarry
> Principal Software Engineer
> Red Hat, Telco/NFV

[PATCH v3 1/1] usertools/rss: add CNXK RSS key

[skori]

From: Sunil Kumar Kori <skori@marvell.com>

This patch adds RSS key for CNXK platforms. CNXK platform uses
48 bytes long key for hash calculations.

For the same patch also updates help mesaages to provide range
information for supporting NICs/platforms.

Also CNXK uses reta size as 64 so to get correct offset to retrieve
queue index, user must pass reta_size option as 64 i.e. -t 64.

Examples:
$ ./dpdk-rss-flows.py -k cnxk 8 28.0.0.0/24 40.0.0.0/24 -t 64
SRC_IP      DST_IP       QUEUE
28.0.0.1    40.0.0.1     7
28.0.0.1    40.0.0.2     2
28.0.0.1    40.0.0.3     4
28.0.0.1    40.0.0.7     1
28.0.0.1    40.0.0.8     3
28.0.0.1    40.0.0.9     5
28.0.0.1    40.0.0.10    0
28.0.0.1    40.0.0.11    6

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
v2..v3:
 - Fix key size range check.
 - Fix default key size file name.

v1..v2:
 - Fix checkpatch errors.

 usertools/dpdk-rss-flows.py | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index 73821eb471..937f9e1927 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -188,11 +188,24 @@ def balanced_traffic(
         0x81, 0x15, 0x03, 0x66,
     )
 )
+# default_key, see drivers/common/cnxk/roc_nix_rss.c
+# Marvell's cnxk NICs take 48 bytes keys
+RSS_KEY_CNXK = bytes(
+    (
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+        0xfe, 0xed, 0x0b, 0xad, 0xfe, 0xed, 0x0b, 0xad,
+    )
+)
 # fmt: on
 DEFAULT_DRIVER_KEYS = {
     "intel": RSS_KEY_INTEL,
     "mlx": RSS_KEY_MLX,
     "i40e": RSS_KEY_I40E,
+    "cnxk": RSS_KEY_CNXK,
 }
 
 
@@ -201,8 +214,8 @@ def rss_key(value):
         return DEFAULT_DRIVER_KEYS[value]
     try:
         key = binascii.unhexlify(value)
-        if len(key) not in (40, 52):
-            raise argparse.ArgumentTypeError("The key must be 40 or 52 bytes long")
+        if len(key) not in range(40, 52):
+            raise argparse.ArgumentTypeError("The key must be 40 to 52 bytes long")
         return key
     except (TypeError, ValueError) as e:
         raise argparse.ArgumentTypeError(str(e)) from e
@@ -299,7 +312,7 @@ def parse_args():
         default=RSS_KEY_INTEL,
         type=rss_key,
         help="""
-        The random 40-bytes key used to compute the RSS hash. This option
+        The random 40 to 52 bytes key used to compute the RSS hash. This option
         supports either a well-known name or the hex value of the key
         (well-known names: "intel", "mlx", default: "intel").
         """,
-- 
2.25.1

Re: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Thomas Monjalon]

18/10/2023 09:26, Sunil Kumar Kori:
> From: Robin Jarry <rjarry@redhat.com>
> > From: Sunil Kumar Kori <skori@marvell.com>
> > >
> > > This patch adds RSS key for CNXK platforms. CNXK platform uses
> > > 48 bytes long key for hash calculations.
> > >
> > > For the same patch also updates help mesaages to provide range
> > > information for supporting NICs/platforms.
> > >
> > > Also CNXK uses reta size as 64 so to get correct offset to retrieve
> > > queue index, user must pass reta_size option as 64 i.e. -t 64.
> > 
> > I think we should add some driver abstraction that contains the required key
> > length and default reta size. Instead of requiring the user to guess the correct
> > values. Is that something you could do?
> > 
> Okay but in either case i.e. -t option or driver abstraction, user must know the reta size and key size before configuring.
> So  I am not sure that how adding driver abstraction will help to solve this issue unless/until its documented somewhere.

You can start with an option to get the size printed, depending on driver name.

> So for current release, I am planning to go this version as it is because we are close.
> Later on we can think of it and add required support. 
> Please provide input on it.

Please provide a more user friendly experience in this release.

Re: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Robin Jarry]

Thomas Monjalon, Oct 18, 2023 at 11:14:
> 18/10/2023 09:26, Sunil Kumar Kori:
> > From: Robin Jarry <rjarry@redhat.com>
> > > From: Sunil Kumar Kori <skori@marvell.com>
> > > >
> > > > This patch adds RSS key for CNXK platforms. CNXK platform uses
> > > > 48 bytes long key for hash calculations.
> > > >
> > > > For the same patch also updates help mesaages to provide range
> > > > information for supporting NICs/platforms.
> > > >
> > > > Also CNXK uses reta size as 64 so to get correct offset to retrieve
> > > > queue index, user must pass reta_size option as 64 i.e. -t 64.
> > > 
> > > I think we should add some driver abstraction that contains the required key
> > > length and default reta size. Instead of requiring the user to guess the correct
> > > values. Is that something you could do?
> > > 
> > Okay but in either case i.e. -t option or driver abstraction, user must know the reta size and key size before configuring.
> > So  I am not sure that how adding driver abstraction will help to solve this issue unless/until its documented somewhere.
>
> You can start with an option to get the size printed, depending on driver name.
>
> > So for current release, I am planning to go this version as it is because we are close.
> > Later on we can think of it and add required support. 
> > Please provide input on it.
>
> Please provide a more user friendly experience in this release.

I could have a shot at it since it may involve some refactoring. Also, 
existing supported drivers will benefit from it. This does not seem like 
it is directly related to CNXK.

RE: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Sunil Kumar Kori]

> -----Original Message-----
> From: Robin Jarry <rjarry@redhat.com>
> Sent: Wednesday, October 18, 2023 2:48 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Sunil Kumar Kori
> <skori@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinjacobk@gmail.com>
> Subject: Re: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key
> 
> Thomas Monjalon, Oct 18, 2023 at 11:14:
> > 18/10/2023 09:26, Sunil Kumar Kori:
> > > From: Robin Jarry <rjarry@redhat.com>
> > > > From: Sunil Kumar Kori <skori@marvell.com>
> > > > >
> > > > > This patch adds RSS key for CNXK platforms. CNXK platform uses
> > > > > 48 bytes long key for hash calculations.
> > > > >
> > > > > For the same patch also updates help mesaages to provide range
> > > > > information for supporting NICs/platforms.
> > > > >
> > > > > Also CNXK uses reta size as 64 so to get correct offset to
> > > > > retrieve queue index, user must pass reta_size option as 64 i.e. -t 64.
> > > >
> > > > I think we should add some driver abstraction that contains the
> > > > required key length and default reta size. Instead of requiring
> > > > the user to guess the correct values. Is that something you could do?
> > > >
> > > Okay but in either case i.e. -t option or driver abstraction, user must
> know the reta size and key size before configuring.
> > > So  I am not sure that how adding driver abstraction will help to solve this
> issue unless/until its documented somewhere.
> >
> > You can start with an option to get the size printed, depending on driver
> name.
> >
> > > So for current release, I am planning to go this version as it is because we
> are close.
> > > Later on we can think of it and add required support.
> > > Please provide input on it.
> >
> > Please provide a more user friendly experience in this release.
> 
> I could have a shot at it since it may involve some refactoring. Also, existing
> supported drivers will benefit from it. This does not seem like it is directly
> related to CNXK.
Sure, Thanks. 

Re: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key

[Robin Jarry]

Sunil Kumar Kori, Oct 18, 2023 at 12:01:
> > I could have a shot at it since it may involve some refactoring. 
> > Also, existing supported drivers will benefit from it. This does not 
> > seem like it is directly related to CNXK.
>
> Sure, Thanks.

Hi Sunil,

I have sent a patch that should allow you to define the default key and 
RETA size for the CNXK driver.

http://patches.dpdk.org/project/dpdk/patch/20231023080710.240402-3-rjarry@redhat.com/

It would probably make sense to apply my patch first and rebase yours on 
top of it.

Thomas, what do you think?