From: Sunil Kumar Kori <skori@marvell.com>
To: Robin Jarry <rjarry@redhat.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
Thomas Monjalon <thomas@monjalon.net>,
Jerin Jacob <jerinjacobk@gmail.com>
Subject: RE: [EXT] Re: [PATCH v2 1/1] usertools/rss: add CNXK RSS key
Date: Wed, 18 Oct 2023 07:26:13 +0000 [thread overview]
Message-ID: <CO6PR18MB38606F28E6A1E05D712B9232B4D5A@CO6PR18MB3860.namprd18.prod.outlook.com> (raw)
In-Reply-To: <CWAPHPBXISEE.GJJOOCCOX8K5@redhat.com>
> -----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
next prev parent reply other threads:[~2023-10-18 7:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-09 16:30 [PATCH] " skori
2023-10-09 16:36 ` [PATCH v2 1/1] " skori
2023-10-10 5:47 ` Jerin Jacob
2023-10-17 12:08 ` Thomas Monjalon
2023-10-17 12:17 ` Robin Jarry
2023-10-18 7:26 ` Sunil Kumar Kori [this message]
2023-10-18 9:14 ` [EXT] " Thomas Monjalon
2023-10-18 9:18 ` Robin Jarry
2023-10-18 10:01 ` Sunil Kumar Kori
2023-10-25 12:47 ` Robin Jarry
2023-10-18 7:29 ` [PATCH v3 " skori
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO6PR18MB38606F28E6A1E05D712B9232B4D5A@CO6PR18MB3860.namprd18.prod.outlook.com \
--to=skori@marvell.com \
--cc=dev@dpdk.org \
--cc=jerinjacobk@gmail.com \
--cc=rjarry@redhat.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).