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

[Sunil Kumar Kori]

> -----Original Message-----
> From: Robin Jarry <rjarry at redhat.com>
> Sent: Tuesday, October 17, 2023 5:47 PM
> To: Sunil Kumar Kori <skori at marvell.com>
> Cc: dev at dpdk.org; Thomas Monjalon <thomas at monjalon.net>; Jerin Jacob
> <jerinjacobk at 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 at 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 at 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