[v2,1/1] usertools/rss: add CNXK RSS key
Checks
Commit Message
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(-)
Comments
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>
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,
> + )
> +)
, 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
> -----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
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.
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.
> -----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.
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?
@@ -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").
""",