DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/2] usertools/rss: add driver abstractions
@ 2023-10-23  8:07 Robin Jarry
  2023-10-23  8:07 ` [PATCH 2/2] usertools/rss: add --info flag Robin Jarry
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Robin Jarry @ 2023-10-23  8:07 UTC (permalink / raw)
  To: dev; +Cc: skori, thomas, jerinjacobk, Robin Jarry

The default RETA size is not the same for all drivers. In some drivers
(mlx5), the RETA size may also be dependent on the number of RX queues.

Introduce a new DriverInfo abstraction for known keys. Define a simple
API to expose the RSS key and RETA size (based on the number of RX
queues).

Use that abstraction for all three known keys.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
 usertools/dpdk-rss-flows.py | 144 ++++++++++++++++++++++--------------
 1 file changed, 89 insertions(+), 55 deletions(-)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index 73821eb47125..dfdad33dde8a 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -154,60 +154,81 @@ def balanced_traffic(
 
 NO_PORT = (0,)
 
-# fmt: off
-# rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
-RSS_KEY_INTEL = bytes(
-    (
-        0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
-        0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
-        0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
-        0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
-        0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
-    )
-)
-# rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
-RSS_KEY_MLX = bytes(
-    (
-        0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
-        0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
-        0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
-        0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
-        0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
-    )
-)
-# rss_key_default, see drivers/net/i40e/i40e_ethdev.c
-# i40e is the only driver that takes 52 bytes keys
-RSS_KEY_I40E = bytes(
-    (
-        0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
-        0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
-        0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
-        0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
-        0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
-        0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
-        0x81, 0x15, 0x03, 0x66,
-    )
-)
-# fmt: on
-DEFAULT_DRIVER_KEYS = {
-    "intel": RSS_KEY_INTEL,
-    "mlx": RSS_KEY_MLX,
-    "i40e": RSS_KEY_I40E,
+
+class DriverInfo:
+    def __init__(self, key: bytes = None, reta_size: int = None):
+        self.__key = key
+        self.__reta_size = reta_size
+
+    def rss_key(self) -> bytes:
+        return self.__key
+
+    def reta_size(self, num_queues: int) -> int:
+        return self.__reta_size
+
+
+class MlxDriverInfo(DriverInfo):
+    def rss_key(self) -> bytes:
+        return bytes(
+            (
+                # fmt: off
+                # rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
+                0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
+                0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
+                0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
+                0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
+                0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
+                # fmt: on
+            )
+        )
+
+    def reta_size(self, num_queues: int) -> int:
+        if num_queues & (num_queues - 1) == 0:
+            # If the requested number of RX queues is power of two,
+            # use a table of this size.
+            return num_queues
+        # otherwise, use the maximum table size
+        return 512
+
+
+DEFAULT_DRIVERS = {
+    "intel": DriverInfo(
+        key=bytes(
+            (
+                # fmt: off
+                # rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
+                0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
+                0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
+                0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
+                0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
+                0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
+                # fmt: on
+            )
+        ),
+        reta_size=128,
+    ),
+    "mlx": MlxDriverInfo(),
+    "i40e": DriverInfo(
+        key=bytes(
+            (
+                # fmt: off
+                # rss_key_default, see drivers/net/i40e/i40e_ethdev.c
+                # i40e is the only driver that takes 52 bytes keys
+                0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
+                0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
+                0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
+                0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
+                0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
+                0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
+                0x81, 0x15, 0x03, 0x66,
+                # fmt: on
+            )
+        ),
+        reta_size=512,
+    ),
 }
 
 
-def rss_key(value):
-    if value in DEFAULT_DRIVER_KEYS:
-        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")
-        return key
-    except (TypeError, ValueError) as e:
-        raise argparse.ArgumentTypeError(str(e)) from e
-
-
 def port_range(value):
     try:
         if "-" in value:
@@ -296,8 +317,7 @@ def parse_args():
     parser.add_argument(
         "-k",
         "--rss-key",
-        default=RSS_KEY_INTEL,
-        type=rss_key,
+        default="intel",
         help="""
         The random 40-bytes key used to compute the RSS hash. This option
         supports either a well-known name or the hex value of the key
@@ -307,10 +327,9 @@ def parse_args():
     parser.add_argument(
         "-t",
         "--reta-size",
-        default=128,
         type=power_of_two,
         help="""
-        Size of the redirection table or "RETA" (default: 128).
+        Size of the redirection table or "RETA" (default: depends on driver).
         """,
     )
     parser.add_argument(
@@ -339,9 +358,24 @@ def parse_args():
         parser.error(
             f"{args.ip_src} and {args.ip_dst} don't have the same protocol version"
         )
+
+    if args.rss_key in DEFAULT_DRIVERS:
+        driver_info = DEFAULT_DRIVERS[args.rss_key]
+    else:
+        try:
+            key = binascii.unhexlify(args.rss_key)
+        except (TypeError, ValueError) as e:
+            parser.error(f"RSS_KEY: {e}")
+        driver_info = DriverInfo(key=key, reta_size=128)
+
+    if args.reta_size is None:
+        args.reta_size = driver_info.reta_size(args.rx_queues)
+
     if args.reta_size < args.rx_queues:
         parser.error("RETA_SIZE must be greater than or equal to RX_QUEUES")
 
+    args.rss_key = driver_info.rss_key()
+
     return args
 
 
-- 
2.41.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/2] usertools/rss: add --info flag
  2023-10-23  8:07 [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
@ 2023-10-23  8:07 ` Robin Jarry
  2023-11-20 13:28 ` [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Robin Jarry @ 2023-10-23  8:07 UTC (permalink / raw)
  To: dev; +Cc: skori, thomas, jerinjacobk, Robin Jarry

Add a flag to print the RSS key and RETA size that are used to compute
balanced traffic. Example:

 $ usertools/dpdk-rss-flows.py 4 1.0.0.0 2.2.0.0/24 -k mlx --info
 RSS key:     2cc681d15bdbf4f7fca28319db1a3e946b9e38d92c9c03d1ad9944a7d…
 RETA size:   4

 SRC_IP     DST_IP     QUEUE
 1.0.0.0    2.2.0.1    2
 1.0.0.0    2.2.0.2    0
 1.0.0.0    2.2.0.4    1
 1.0.0.0    2.2.0.6    3

The flag is only available with the default text output.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
 usertools/dpdk-rss-flows.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index dfdad33dde8a..cc5d4d22096f 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -351,6 +351,14 @@ def parse_args():
         Output in parseable JSON format.
         """,
     )
+    parser.add_argument(
+        "-i",
+        "--info",
+        action="store_true",
+        help="""
+        Print RETA size and RSS key above the results. Not available with --json.
+        """,
+    )
 
     args = parser.parse_args()
 
@@ -359,6 +367,9 @@ def parse_args():
             f"{args.ip_src} and {args.ip_dst} don't have the same protocol version"
         )
 
+    if args.json and args.info:
+        parser.error("--json and --info are mutually exclusive")
+
     if args.rss_key in DEFAULT_DRIVERS:
         driver_info = DEFAULT_DRIVERS[args.rss_key]
     else:
@@ -441,6 +452,11 @@ def main():
             cells.append(r)
         rows.append(tuple(cells))
 
+    if args.info:
+        print(f"RSS key:     {binascii.hexlify(args.rss_key).decode()}")
+        print(f"RETA size:   {args.reta_size}")
+        print()
+
     fmt = [f"%-{w}s" for w in widths]
     fmt[-1] = "%s"  # avoid trailing whitespace
     fmt = "    ".join(fmt)
-- 
2.41.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-10-23  8:07 [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
  2023-10-23  8:07 ` [PATCH 2/2] usertools/rss: add --info flag Robin Jarry
@ 2023-11-20 13:28 ` Robin Jarry
  2023-11-20 15:13   ` [EXT] " Sunil Kumar Kori
  2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Robin Jarry @ 2023-11-20 13:28 UTC (permalink / raw)
  To: dev; +Cc: skori, thomas, jerinjacobk

Hi, Jerin, Sunil Komar,

Could you have a look at this series and report if it brings the needed 
abstractions for integrating CNXK?

Thanks!


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [EXT] Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-20 13:28 ` [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
@ 2023-11-20 15:13   ` Sunil Kumar Kori
  0 siblings, 0 replies; 22+ messages in thread
From: Sunil Kumar Kori @ 2023-11-20 15:13 UTC (permalink / raw)
  To: Robin Jarry, dev; +Cc: thomas, jerinjacobk

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

Sure Robin,

I will review it and revert ASAP.



Regards

Sunil Kumar Kori



> -----Original Message-----

> From: Robin Jarry <rjarry@redhat.com>

> Sent: Monday, November 20, 2023 6:59 PM

> To: dev@dpdk.org

> Cc: Sunil Kumar Kori <skori@marvell.com>; thomas@monjalon.net;

> jerinjacobk@gmail.com

> Subject: [EXT] Re: [PATCH 1/2] usertools/rss: add driver abstractions

>

> External Email

>

> ----------------------------------------------------------------------

> Hi, Jerin, Sunil Komar,

>

> Could you have a look at this series and report if it brings the needed

> abstractions for integrating CNXK?

>

> Thanks!



[-- Attachment #2: Type: text/html, Size: 3153 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/3] usertools/rss: add driver abstractions
  2023-10-23  8:07 [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
  2023-10-23  8:07 ` [PATCH 2/2] usertools/rss: add --info flag Robin Jarry
  2023-11-20 13:28 ` [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
@ 2023-11-20 16:22 ` Robin Jarry
  2023-11-20 16:22   ` [PATCH v2 2/3] usertools/rss: add --info flag Robin Jarry
                     ` (3 more replies)
  2023-11-21  6:11 ` [EXT] [PATCH 1/2] " Sunil Kumar Kori
  2023-11-21 16:38 ` Stephen Hemminger
  4 siblings, 4 replies; 22+ messages in thread
From: Robin Jarry @ 2023-11-20 16:22 UTC (permalink / raw)
  To: dev

The default RETA size is not the same for all drivers. In some drivers
(mlx5), the RETA size may also be dependent on the number of RX queues.

Introduce a new DriverInfo abstraction for known keys. Define a simple
API to expose the RSS key and RETA size (based on the number of RX
queues).

Use that abstraction for all three known keys.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
 usertools/dpdk-rss-flows.py | 151 ++++++++++++++++++++++--------------
 1 file changed, 93 insertions(+), 58 deletions(-)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index 73821eb47125..241ee4fe28c6 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -154,60 +154,81 @@ def balanced_traffic(
 
 NO_PORT = (0,)
 
-# fmt: off
-# rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
-RSS_KEY_INTEL = bytes(
-    (
-        0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
-        0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
-        0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
-        0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
-        0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
-    )
-)
-# rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
-RSS_KEY_MLX = bytes(
-    (
-        0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
-        0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
-        0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
-        0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
-        0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
-    )
-)
-# rss_key_default, see drivers/net/i40e/i40e_ethdev.c
-# i40e is the only driver that takes 52 bytes keys
-RSS_KEY_I40E = bytes(
-    (
-        0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
-        0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
-        0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
-        0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
-        0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
-        0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
-        0x81, 0x15, 0x03, 0x66,
-    )
-)
-# fmt: on
-DEFAULT_DRIVER_KEYS = {
-    "intel": RSS_KEY_INTEL,
-    "mlx": RSS_KEY_MLX,
-    "i40e": RSS_KEY_I40E,
+
+class DriverInfo:
+    def __init__(self, key: bytes = None, reta_size: int = None):
+        self.__key = key
+        self.__reta_size = reta_size
+
+    def rss_key(self) -> bytes:
+        return self.__key
+
+    def reta_size(self, num_queues: int) -> int:
+        return self.__reta_size
+
+
+class MlxDriverInfo(DriverInfo):
+    def rss_key(self) -> bytes:
+        return bytes(
+            (
+                # fmt: off
+                # rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
+                0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
+                0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
+                0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
+                0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
+                0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
+                # fmt: on
+            )
+        )
+
+    def reta_size(self, num_queues: int) -> int:
+        if num_queues & (num_queues - 1) == 0:
+            # If the requested number of RX queues is power of two,
+            # use a table of this size.
+            return num_queues
+        # otherwise, use the maximum table size
+        return 512
+
+
+DEFAULT_DRIVERS = {
+    "intel": DriverInfo(
+        key=bytes(
+            (
+                # fmt: off
+                # rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
+                0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
+                0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
+                0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
+                0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
+                0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
+                # fmt: on
+            )
+        ),
+        reta_size=128,
+    ),
+    "mlx": MlxDriverInfo(),
+    "i40e": DriverInfo(
+        key=bytes(
+            (
+                # fmt: off
+                # rss_key_default, see drivers/net/i40e/i40e_ethdev.c
+                # i40e is the only driver that takes 52 bytes keys
+                0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
+                0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
+                0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
+                0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
+                0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
+                0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
+                0x81, 0x15, 0x03, 0x66,
+                # fmt: on
+            )
+        ),
+        reta_size=512,
+    ),
 }
 
 
-def rss_key(value):
-    if value in DEFAULT_DRIVER_KEYS:
-        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")
-        return key
-    except (TypeError, ValueError) as e:
-        raise argparse.ArgumentTypeError(str(e)) from e
-
-
 def port_range(value):
     try:
         if "-" in value:
@@ -296,21 +317,20 @@ def parse_args():
     parser.add_argument(
         "-k",
         "--rss-key",
-        default=RSS_KEY_INTEL,
-        type=rss_key,
-        help="""
-        The random 40-bytes key used to compute the RSS hash. This option
+        default="intel",
+        help=f"""
+        The random 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").
+        (well-known names: {', '.join(DEFAULT_DRIVERS)}, default: intel).
         """,
     )
     parser.add_argument(
         "-t",
         "--reta-size",
-        default=128,
         type=power_of_two,
         help="""
-        Size of the redirection table or "RETA" (default: 128).
+        Size of the redirection table or "RETA" (default: depends on driver if
+        using a well-known driver name, otherwise 128).
         """,
     )
     parser.add_argument(
@@ -339,9 +359,24 @@ def parse_args():
         parser.error(
             f"{args.ip_src} and {args.ip_dst} don't have the same protocol version"
         )
+
+    if args.rss_key in DEFAULT_DRIVERS:
+        driver_info = DEFAULT_DRIVERS[args.rss_key]
+    else:
+        try:
+            key = binascii.unhexlify(args.rss_key)
+        except (TypeError, ValueError) as e:
+            parser.error(f"RSS_KEY: {e}")
+        driver_info = DriverInfo(key=key, reta_size=128)
+
+    if args.reta_size is None:
+        args.reta_size = driver_info.reta_size(args.rx_queues)
+
     if args.reta_size < args.rx_queues:
         parser.error("RETA_SIZE must be greater than or equal to RX_QUEUES")
 
+    args.rss_key = driver_info.rss_key()
+
     return args
 
 
-- 
2.42.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 2/3] usertools/rss: add --info flag
  2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
@ 2023-11-20 16:22   ` Robin Jarry
  2023-11-21  6:15     ` [EXT] " Sunil Kumar Kori
  2023-11-20 16:22   ` [PATCH v2 3/3] usertools/rss: add CNXK well-known key Robin Jarry
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 22+ messages in thread
From: Robin Jarry @ 2023-11-20 16:22 UTC (permalink / raw)
  To: dev

Add a flag to print the RSS key and RETA size that are used to compute
balanced traffic. Example:

 $ usertools/dpdk-rss-flows.py -i 4 1.0.0.0 2.2.0.0/24 -k mlx
 RSS key:     2cc681d15bdbf4f7fca28319db1a3e946b9e38d92c9c03d1ad9944a7d…
 RETA size:   4

 SRC_IP     DST_IP     QUEUE
 1.0.0.0    2.2.0.1    2
 1.0.0.0    2.2.0.2    0
 1.0.0.0    2.2.0.4    1
 1.0.0.0    2.2.0.6    3

The flag is only available with the default text output.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
 usertools/dpdk-rss-flows.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index 241ee4fe28c6..fd225a697f08 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -352,6 +352,14 @@ def parse_args():
         Output in parseable JSON format.
         """,
     )
+    parser.add_argument(
+        "-i",
+        "--info",
+        action="store_true",
+        help="""
+        Print RETA size and RSS key above the results. Not available with --json.
+        """,
+    )
 
     args = parser.parse_args()
 
@@ -360,6 +368,9 @@ def parse_args():
             f"{args.ip_src} and {args.ip_dst} don't have the same protocol version"
         )
 
+    if args.json and args.info:
+        parser.error("--json and --info are mutually exclusive")
+
     if args.rss_key in DEFAULT_DRIVERS:
         driver_info = DEFAULT_DRIVERS[args.rss_key]
     else:
@@ -442,6 +453,11 @@ def main():
             cells.append(r)
         rows.append(tuple(cells))
 
+    if args.info:
+        print(f"RSS key:     {binascii.hexlify(args.rss_key).decode()}")
+        print(f"RETA size:   {args.reta_size}")
+        print()
+
     fmt = [f"%-{w}s" for w in widths]
     fmt[-1] = "%s"  # avoid trailing whitespace
     fmt = "    ".join(fmt)
-- 
2.42.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 3/3] usertools/rss: add CNXK well-known key
  2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
  2023-11-20 16:22   ` [PATCH v2 2/3] usertools/rss: add --info flag Robin Jarry
@ 2023-11-20 16:22   ` Robin Jarry
  2023-11-20 16:41     ` Stephen Hemminger
  2023-11-21  6:08     ` [EXT] " Sunil Kumar Kori
  2023-11-21  6:14   ` [EXT] [PATCH v2 1/3] usertools/rss: add driver abstractions Sunil Kumar Kori
  2023-11-22 23:43   ` Thomas Monjalon
  3 siblings, 2 replies; 22+ messages in thread
From: Robin Jarry @ 2023-11-20 16:22 UTC (permalink / raw)
  To: dev; +Cc: Sunil Kumar Kori

Add default RSS key for CNXK platforms. CNXK platform uses 48 bytes long
key for hash calculations and a default RETA size of 64.

Example:

  ~$ usertools/dpdk-rss-flows.py 8 28.0.0.0/24 40.0.0.0/24 -k cnxk
  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

Co-authored-by: Sunil Kumar Kori <skori@marvell.com>
Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
 usertools/dpdk-rss-flows.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
index fd225a697f08..be9b3d760c03 100755
--- a/usertools/dpdk-rss-flows.py
+++ b/usertools/dpdk-rss-flows.py
@@ -226,6 +226,23 @@ def reta_size(self, num_queues: int) -> int:
         ),
         reta_size=512,
     ),
+    "cnxk": DriverInfo(
+        key=bytes(
+            (
+                # fmt: off
+                # roc_nix_rss_key_default_fill, see drivers/common/cnxk/roc_nix_rss.c
+                # Marvell's cnxk NICs take 48 bytes keys
+                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
+            )
+        ),
+        reta_size=64,
+    ),
 }
 
 
-- 
2.42.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/3] usertools/rss: add CNXK well-known key
  2023-11-20 16:22   ` [PATCH v2 3/3] usertools/rss: add CNXK well-known key Robin Jarry
@ 2023-11-20 16:41     ` Stephen Hemminger
  2023-11-21  6:08     ` [EXT] " Sunil Kumar Kori
  1 sibling, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2023-11-20 16:41 UTC (permalink / raw)
  To: Robin Jarry; +Cc: dev, Sunil Kumar Kori

On Mon, 20 Nov 2023 17:22:59 +0100
Robin Jarry <rjarry@redhat.com> wrote:

> diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
> index fd225a697f08..be9b3d760c03 100755
> --- a/usertools/dpdk-rss-flows.py
> +++ b/usertools/dpdk-rss-flows.py
> @@ -226,6 +226,23 @@ def reta_size(self, num_queues: int) -> int:
>          ),
>          reta_size=512,
>      ),
> +    "cnxk": DriverInfo(
> +        key=bytes(
> +            (
> +                # fmt: off
> +                # roc_nix_rss_key_default_fill, see drivers/common/cnxk/roc_nix_rss.c
> +                # Marvell's cnxk NICs take 48 bytes keys
> +                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
> +            )
> +        ),
> +        reta_size=64,
> +    ),
>  }

Maybe an enhancement in future to expose the default key better.
Having it live in two places means adding new drivers has another
thing to worry about (and not documented).

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [EXT] [PATCH v2 3/3] usertools/rss: add CNXK well-known key
  2023-11-20 16:22   ` [PATCH v2 3/3] usertools/rss: add CNXK well-known key Robin Jarry
  2023-11-20 16:41     ` Stephen Hemminger
@ 2023-11-21  6:08     ` Sunil Kumar Kori
  1 sibling, 0 replies; 22+ messages in thread
From: Sunil Kumar Kori @ 2023-11-21  6:08 UTC (permalink / raw)
  To: Robin Jarry, dev

Thanks Robin for adding key for CNXK platform. 

Regards
Sunil Kumar Kori

> -----Original Message-----
> From: Robin Jarry <rjarry@redhat.com>
> Sent: Monday, November 20, 2023 9:53 PM
> To: dev@dpdk.org
> Cc: Sunil Kumar Kori <skori@marvell.com>
> Subject: [EXT] [PATCH v2 3/3] usertools/rss: add CNXK well-known key
> 
> External Email
> 
> ----------------------------------------------------------------------
> Add default RSS key for CNXK platforms. CNXK platform uses 48 bytes long
> key for hash calculations and a default RETA size of 64.
> 
> Example:
> 
>   ~$ usertools/dpdk-rss-flows.py 8 28.0.0.0/24 40.0.0.0/24 -k cnxk
>   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
> 
> Co-authored-by: Sunil Kumar Kori <skori@marvell.com>
> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> ---
>  usertools/dpdk-rss-flows.py | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
> index fd225a697f08..be9b3d760c03 100755
> --- a/usertools/dpdk-rss-flows.py
> +++ b/usertools/dpdk-rss-flows.py
> @@ -226,6 +226,23 @@ def reta_size(self, num_queues: int) -> int:
>          ),
>          reta_size=512,
>      ),
> +    "cnxk": DriverInfo(
> +        key=bytes(
> +            (
> +                # fmt: off
> +                # roc_nix_rss_key_default_fill, see
> drivers/common/cnxk/roc_nix_rss.c
> +                # Marvell's cnxk NICs take 48 bytes keys
> +                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
> +            )
> +        ),
> +        reta_size=64,
> +    ),
>  }
> 
Acked-by: Sunil Kumar Kori <skori@marvell.com>
> 
> --
> 2.42.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [EXT] [PATCH 1/2] usertools/rss: add driver abstractions
  2023-10-23  8:07 [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
                   ` (2 preceding siblings ...)
  2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
@ 2023-11-21  6:11 ` Sunil Kumar Kori
  2023-11-21 16:38 ` Stephen Hemminger
  4 siblings, 0 replies; 22+ messages in thread
From: Sunil Kumar Kori @ 2023-11-21  6:11 UTC (permalink / raw)
  To: Robin Jarry, dev; +Cc: thomas, jerinjacobk

> -----Original Message-----
> From: Robin Jarry <rjarry@redhat.com>
> Sent: Monday, October 23, 2023 1:37 PM
> To: dev@dpdk.org
> Cc: Sunil Kumar Kori <skori@marvell.com>; thomas@monjalon.net;
> jerinjacobk@gmail.com; Robin Jarry <rjarry@redhat.com>
> Subject: [EXT] [PATCH 1/2] usertools/rss: add driver abstractions
> 
> External Email
> 
> ----------------------------------------------------------------------
> The default RETA size is not the same for all drivers. In some drivers (mlx5),
> the RETA size may also be dependent on the number of RX queues.
> 
> Introduce a new DriverInfo abstraction for known keys. Define a simple API
> to expose the RSS key and RETA size (based on the number of RX queues).
> 
> Use that abstraction for all three known keys.
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> ---
>  usertools/dpdk-rss-flows.py | 144 ++++++++++++++++++++++--------------
>  1 file changed, 89 insertions(+), 55 deletions(-)
> 

Acked-by: Sunil Kumar Kori <skori@marvell.com>

> --
> 2.41.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [EXT] [PATCH v2 1/3] usertools/rss: add driver abstractions
  2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
  2023-11-20 16:22   ` [PATCH v2 2/3] usertools/rss: add --info flag Robin Jarry
  2023-11-20 16:22   ` [PATCH v2 3/3] usertools/rss: add CNXK well-known key Robin Jarry
@ 2023-11-21  6:14   ` Sunil Kumar Kori
  2023-11-22 23:43   ` Thomas Monjalon
  3 siblings, 0 replies; 22+ messages in thread
From: Sunil Kumar Kori @ 2023-11-21  6:14 UTC (permalink / raw)
  To: Robin Jarry, dev

> -----Original Message-----
> From: Robin Jarry <rjarry@redhat.com>
> Sent: Monday, November 20, 2023 9:53 PM
> To: dev@dpdk.org
> Subject: [EXT] [PATCH v2 1/3] usertools/rss: add driver abstractions
> 
> External Email
> 
> ----------------------------------------------------------------------
> The default RETA size is not the same for all drivers. In some drivers (mlx5),
> the RETA size may also be dependent on the number of RX queues.
> 
> Introduce a new DriverInfo abstraction for known keys. Define a simple API
> to expose the RSS key and RETA size (based on the number of RX queues).
> 
> Use that abstraction for all three known keys.
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> ---
>  usertools/dpdk-rss-flows.py | 151 ++++++++++++++++++++++--------------
>  1 file changed, 93 insertions(+), 58 deletions(-)
> 
> diff --git a/usertools/dpdk-rss-flows.py b/usertools/dpdk-rss-flows.py
> index 73821eb47125..241ee4fe28c6 100755
> --- a/usertools/dpdk-rss-flows.py
> +++ b/usertools/dpdk-rss-flows.py
> @@ -154,60 +154,81 @@ def balanced_traffic(
> 
>  NO_PORT = (0,)
> 
Acked-by: Sunil Kumar Kori <skori@marvell.com>
 
> --
> 2.42.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [EXT] [PATCH v2 2/3] usertools/rss: add --info flag
  2023-11-20 16:22   ` [PATCH v2 2/3] usertools/rss: add --info flag Robin Jarry
@ 2023-11-21  6:15     ` Sunil Kumar Kori
  0 siblings, 0 replies; 22+ messages in thread
From: Sunil Kumar Kori @ 2023-11-21  6:15 UTC (permalink / raw)
  To: Robin Jarry, dev

> -----Original Message-----
> From: Robin Jarry <rjarry@redhat.com>
> Sent: Monday, November 20, 2023 9:53 PM
> To: dev@dpdk.org
> Subject: [EXT] [PATCH v2 2/3] usertools/rss: add --info flag
> 
> External Email
> 
> ----------------------------------------------------------------------
> Add a flag to print the RSS key and RETA size that are used to compute
> balanced traffic. Example:
> 
>  $ usertools/dpdk-rss-flows.py -i 4 1.0.0.0 2.2.0.0/24 -k mlx
>  RSS key:
> 2cc681d15bdbf4f7fca28319db1a3e946b9e38d92c9c03d1ad9944a7d…
>  RETA size:   4
> 
>  SRC_IP     DST_IP     QUEUE
>  1.0.0.0    2.2.0.1    2
>  1.0.0.0    2.2.0.2    0
>  1.0.0.0    2.2.0.4    1
>  1.0.0.0    2.2.0.6    3
> 
> The flag is only available with the default text output.
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> ---
>  usertools/dpdk-rss-flows.py | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
Acked-by: Sunil Kumar Kori <skori@marvell.com>

> --
> 2.42.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-10-23  8:07 [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
                   ` (3 preceding siblings ...)
  2023-11-21  6:11 ` [EXT] [PATCH 1/2] " Sunil Kumar Kori
@ 2023-11-21 16:38 ` Stephen Hemminger
  2023-11-21 16:49   ` Thomas Monjalon
  2023-11-21 16:51   ` Jerin Jacob
  4 siblings, 2 replies; 22+ messages in thread
From: Stephen Hemminger @ 2023-11-21 16:38 UTC (permalink / raw)
  To: Robin Jarry; +Cc: dev, skori, thomas, jerinjacobk

On Mon, 23 Oct 2023 10:07:11 +0200
Robin Jarry <rjarry@redhat.com> wrote:

> +class DriverInfo:
> +    def __init__(self, key: bytes = None, reta_size: int = None):
> +        self.__key = key
> +        self.__reta_size = reta_size
> +
> +    def rss_key(self) -> bytes:
> +        return self.__key
> +
> +    def reta_size(self, num_queues: int) -> int:
> +        return self.__reta_size
> +
> +
> +class MlxDriverInfo(DriverInfo):
> +    def rss_key(self) -> bytes:
> +        return bytes(
> +            (
> +                # fmt: off
> +                # rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
> +                0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
> +                0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
> +                0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
> +                0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
> +                0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
> +                # fmt: on
> +            )
> +        )
> +
> +    def reta_size(self, num_queues: int) -> int:
> +        if num_queues & (num_queues - 1) == 0:
> +            # If the requested number of RX queues is power of two,
> +            # use a table of this size.
> +            return num_queues
> +        # otherwise, use the maximum table size
> +        return 512
> +
> +
> +DEFAULT_DRIVERS = {
> +    "intel": DriverInfo(
> +        key=bytes(
> +            (
> +                # fmt: off
> +                # rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
> +                0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> +                0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> +                0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> +                0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> +                0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> +                # fmt: on
> +            )
> +        ),
> +        reta_size=128,
> +    ),
> +    "mlx": MlxDriverInfo(),
> +    "i40e": DriverInfo(
> +        key=bytes(
> +            (
> +                # fmt: off
> +                # rss_key_default, see drivers/net/i40e/i40e_ethdev.c
> +                # i40e is the only driver that takes 52 bytes keys
> +                0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
> +                0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
> +                0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
> +                0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
> +                0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
> +                0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
> +                0x81, 0x15, 0x03, 0x66,
> +                # fmt: on
> +            )
> +        ),
> +        reta_size=512,
> +    ),
>  }

The tool should not need to have driver specific tables like this.
DPDK is already riddled with enough driver specific quirks..

That shows a flaw in the rss design, which should have been fixed.
I should have seen this in the earlier versions, would have rejected the patch.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-21 16:38 ` Stephen Hemminger
@ 2023-11-21 16:49   ` Thomas Monjalon
  2023-11-21 16:51   ` Jerin Jacob
  1 sibling, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2023-11-21 16:49 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Robin Jarry, dev, skori, jerinjacobk

21/11/2023 17:38, Stephen Hemminger:
> The tool should not need to have driver specific tables like this.
> DPDK is already riddled with enough driver specific quirks..
> 
> That shows a flaw in the rss design, which should have been fixed.
> I should have seen this in the earlier versions, would have rejected the patch.

What do you mean? You would like all drivers to use the same default key?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-21 16:38 ` Stephen Hemminger
  2023-11-21 16:49   ` Thomas Monjalon
@ 2023-11-21 16:51   ` Jerin Jacob
  2023-11-21 17:04     ` Stephen Hemminger
  1 sibling, 1 reply; 22+ messages in thread
From: Jerin Jacob @ 2023-11-21 16:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Robin Jarry, dev, skori, thomas

On Tue, Nov 21, 2023 at 10:08 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 23 Oct 2023 10:07:11 +0200
> Robin Jarry <rjarry@redhat.com> wrote:
>
> > +class DriverInfo:
> > +    def __init__(self, key: bytes = None, reta_size: int = None):
> > +        self.__key = key
> > +        self.__reta_size = reta_size
> > +
> > +    def rss_key(self) -> bytes:
> > +        return self.__key
> > +
> > +    def reta_size(self, num_queues: int) -> int:
> > +        return self.__reta_size
> > +
> > +
> > +class MlxDriverInfo(DriverInfo):
> > +    def rss_key(self) -> bytes:
> > +        return bytes(
> > +            (
> > +                # fmt: off
> > +                # rss_hash_default_key, see drivers/net/mlx5/mlx5_rxq.c
> > +                0x2c, 0xc6, 0x81, 0xd1, 0x5b, 0xdb, 0xf4, 0xf7,
> > +                0xfc, 0xa2, 0x83, 0x19, 0xdb, 0x1a, 0x3e, 0x94,
> > +                0x6b, 0x9e, 0x38, 0xd9, 0x2c, 0x9c, 0x03, 0xd1,
> > +                0xad, 0x99, 0x44, 0xa7, 0xd9, 0x56, 0x3d, 0x59,
> > +                0x06, 0x3c, 0x25, 0xf3, 0xfc, 0x1f, 0xdc, 0x2a,
> > +                # fmt: on
> > +            )
> > +        )
> > +
> > +    def reta_size(self, num_queues: int) -> int:
> > +        if num_queues & (num_queues - 1) == 0:
> > +            # If the requested number of RX queues is power of two,
> > +            # use a table of this size.
> > +            return num_queues
> > +        # otherwise, use the maximum table size
> > +        return 512
> > +
> > +
> > +DEFAULT_DRIVERS = {
> > +    "intel": DriverInfo(
> > +        key=bytes(
> > +            (
> > +                # fmt: off
> > +                # rss_intel_key, see drivers/net/ixgbe/ixgbe_rxtx.c
> > +                0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> > +                0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> > +                0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> > +                0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> > +                0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa,
> > +                # fmt: on
> > +            )
> > +        ),
> > +        reta_size=128,
> > +    ),
> > +    "mlx": MlxDriverInfo(),
> > +    "i40e": DriverInfo(
> > +        key=bytes(
> > +            (
> > +                # fmt: off
> > +                # rss_key_default, see drivers/net/i40e/i40e_ethdev.c
> > +                # i40e is the only driver that takes 52 bytes keys
> > +                0x44, 0x39, 0x79, 0x6b, 0xb5, 0x4c, 0x50, 0x23,
> > +                0xb6, 0x75, 0xea, 0x5b, 0x12, 0x4f, 0x9f, 0x30,
> > +                0xb8, 0xa2, 0xc0, 0x3d, 0xdf, 0xdc, 0x4d, 0x02,
> > +                0xa0, 0x8c, 0x9b, 0x33, 0x4a, 0xf6, 0x4a, 0x4c,
> > +                0x05, 0xc6, 0xfa, 0x34, 0x39, 0x58, 0xd8, 0x55,
> > +                0x7d, 0x99, 0x58, 0x3a, 0xe1, 0x38, 0xc9, 0x2e,
> > +                0x81, 0x15, 0x03, 0x66,
> > +                # fmt: on
> > +            )
> > +        ),
> > +        reta_size=512,
> > +    ),
> >  }
>
> The tool should not need to have driver specific tables like this.
> DPDK is already riddled with enough driver specific quirks..
>
> That shows a flaw in the rss design, which should have been fixed.

Every NIC's implements standard Toeplitz hash algorithm for RSS hash generation.
Only the initial SEED is different. It is not DPDK property, all the
HW like is that as
there is no standardization on initial SEED for hash.

> I should have seen this in the earlier versions, would have rejected the patch.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-21 16:51   ` Jerin Jacob
@ 2023-11-21 17:04     ` Stephen Hemminger
  2023-11-21 17:21       ` Jerin Jacob
  0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2023-11-21 17:04 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: Robin Jarry, dev, skori, thomas

On Tue, 21 Nov 2023 22:21:54 +0530
Jerin Jacob <jerinjacobk@gmail.com> wrote:

> > The tool should not need to have driver specific tables like this.
> > DPDK is already riddled with enough driver specific quirks..
> >
> > That shows a flaw in the rss design, which should have been fixed.  
> 
> Every NIC's implements standard Toeplitz hash algorithm for RSS hash generation.
> Only the initial SEED is different. It is not DPDK property, all the
> HW like is that as
> there is no standardization on initial SEED for hash.

The tool should be able to query the default key from driver, or the
build infrastructure should extract and generate the table.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-21 17:04     ` Stephen Hemminger
@ 2023-11-21 17:21       ` Jerin Jacob
  2023-11-21 17:27         ` Stephen Hemminger
  0 siblings, 1 reply; 22+ messages in thread
From: Jerin Jacob @ 2023-11-21 17:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Robin Jarry, dev, skori, thomas

On Tue, Nov 21, 2023 at 10:34 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 21 Nov 2023 22:21:54 +0530
> Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> > > The tool should not need to have driver specific tables like this.
> > > DPDK is already riddled with enough driver specific quirks..
> > >
> > > That shows a flaw in the rss design, which should have been fixed.
> >
> > Every NIC's implements standard Toeplitz hash algorithm for RSS hash generation.
> > Only the initial SEED is different. It is not DPDK property, all the
> > HW like is that as
> > there is no standardization on initial SEED for hash.
>
> The tool should be able to query the default key from driver, or the
> build infrastructure should extract and generate the table.

IMO, That may be too much of non-productive work to save a couple of
lines(seed array) of duplicate code between c and python.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-21 17:21       ` Jerin Jacob
@ 2023-11-21 17:27         ` Stephen Hemminger
  2023-11-22  8:28           ` Robin Jarry
  0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2023-11-21 17:27 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: Robin Jarry, dev, skori, thomas

On Tue, 21 Nov 2023 22:51:19 +0530
Jerin Jacob <jerinjacobk@gmail.com> wrote:

> On Tue, Nov 21, 2023 at 10:34 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Tue, 21 Nov 2023 22:21:54 +0530
> > Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >  
> > > > The tool should not need to have driver specific tables like this.
> > > > DPDK is already riddled with enough driver specific quirks..
> > > >
> > > > That shows a flaw in the rss design, which should have been fixed.  
> > >
> > > Every NIC's implements standard Toeplitz hash algorithm for RSS hash generation.
> > > Only the initial SEED is different. It is not DPDK property, all the
> > > HW like is that as
> > > there is no standardization on initial SEED for hash.  
> >
> > The tool should be able to query the default key from driver, or the
> > build infrastructure should extract and generate the table.  
> 
> IMO, That may be too much of non-productive work to save a couple of
> lines(seed array) of duplicate code between c and python.

But the tool is incomplete right now, it doesn't cover all the drivers.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-21 17:27         ` Stephen Hemminger
@ 2023-11-22  8:28           ` Robin Jarry
  2023-11-22  9:37             ` fengchengwen
  0 siblings, 1 reply; 22+ messages in thread
From: Robin Jarry @ 2023-11-22  8:28 UTC (permalink / raw)
  To: Stephen Hemminger, Jerin Jacob; +Cc: dev, skori, thomas

Stephen Hemminger, Nov 21, 2023 at 18:27:
> On Tue, 21 Nov 2023 22:51:19 +0530
> Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > IMO, That may be too much of non-productive work to save a couple of
> > lines(seed array) of duplicate code between c and python.
>
> But the tool is incomplete right now, it doesn't cover all the drivers.

I agree that having a standard API or mechanism for drivers to expose 
their default redirection table size and hash seed key would be nice. 
The script could have a dynamically generated section that is populated 
during the build based on selected drivers.

However, it would make the python script dependant on it and it would 
not be a standalone tool anymore. This is not a blocking issue but 
something to keep in mind.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-22  8:28           ` Robin Jarry
@ 2023-11-22  9:37             ` fengchengwen
  2023-11-22 23:27               ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: fengchengwen @ 2023-11-22  9:37 UTC (permalink / raw)
  To: Robin Jarry, Stephen Hemminger, Jerin Jacob; +Cc: dev, skori, thomas

On 2023/11/22 16:28, Robin Jarry wrote:
> Stephen Hemminger, Nov 21, 2023 at 18:27:
>> On Tue, 21 Nov 2023 22:51:19 +0530
>> Jerin Jacob <jerinjacobk@gmail.com> wrote:
>> > IMO, That may be too much of non-productive work to save a couple of
>> > lines(seed array) of duplicate code between c and python.
>>
>> But the tool is incomplete right now, it doesn't cover all the drivers.
> 
> I agree that having a standard API or mechanism for drivers to expose their default redirection table size and hash seed key would be nice. The script could have a dynamically generated section that is populated during the build based on selected drivers.
> 
> However, it would make the python script dependant on it and it would not be a standalone tool anymore. This is not a blocking issue but something to keep in mind.

The usertools/dpdk-pmdinfo.py will parse dpdk elf to get some information, We could do same thing in here.

> 
> .

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] usertools/rss: add driver abstractions
  2023-11-22  9:37             ` fengchengwen
@ 2023-11-22 23:27               ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2023-11-22 23:27 UTC (permalink / raw)
  To: Robin Jarry, Stephen Hemminger, Jerin Jacob, fengchengwen; +Cc: dev, skori

22/11/2023 10:37, fengchengwen:
> On 2023/11/22 16:28, Robin Jarry wrote:
> > Stephen Hemminger, Nov 21, 2023 at 18:27:
> >> On Tue, 21 Nov 2023 22:51:19 +0530
> >> Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >> > IMO, That may be too much of non-productive work to save a couple of
> >> > lines(seed array) of duplicate code between c and python.
> >>
> >> But the tool is incomplete right now, it doesn't cover all the drivers.
> > 
> > I agree that having a standard API or mechanism for drivers to expose their default redirection table size and hash seed key would be nice. The script could have a dynamically generated section that is populated during the build based on selected drivers.
> > 
> > However, it would make the python script dependant on it and it would not be a standalone tool anymore. This is not a blocking issue but something to keep in mind.
> 
> The usertools/dpdk-pmdinfo.py will parse dpdk elf to get some information, We could do same thing in here.

We can imagine different things to avoid listing few RSS keys in this script.
For now, this series is an improvement, so I take it.

Feel free to follow-up with more enhancements.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/3] usertools/rss: add driver abstractions
  2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
                     ` (2 preceding siblings ...)
  2023-11-21  6:14   ` [EXT] [PATCH v2 1/3] usertools/rss: add driver abstractions Sunil Kumar Kori
@ 2023-11-22 23:43   ` Thomas Monjalon
  3 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2023-11-22 23:43 UTC (permalink / raw)
  To: Robin Jarry; +Cc: dev

20/11/2023 17:22, Robin Jarry:
> The default RETA size is not the same for all drivers. In some drivers
> (mlx5), the RETA size may also be dependent on the number of RX queues.
> 
> Introduce a new DriverInfo abstraction for known keys. Define a simple
> API to expose the RSS key and RETA size (based on the number of RX
> queues).
> 
> Use that abstraction for all three known keys.
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>

Series applied with drivers sorted alphabetically.



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-11-22 23:43 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-23  8:07 [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
2023-10-23  8:07 ` [PATCH 2/2] usertools/rss: add --info flag Robin Jarry
2023-11-20 13:28 ` [PATCH 1/2] usertools/rss: add driver abstractions Robin Jarry
2023-11-20 15:13   ` [EXT] " Sunil Kumar Kori
2023-11-20 16:22 ` [PATCH v2 1/3] " Robin Jarry
2023-11-20 16:22   ` [PATCH v2 2/3] usertools/rss: add --info flag Robin Jarry
2023-11-21  6:15     ` [EXT] " Sunil Kumar Kori
2023-11-20 16:22   ` [PATCH v2 3/3] usertools/rss: add CNXK well-known key Robin Jarry
2023-11-20 16:41     ` Stephen Hemminger
2023-11-21  6:08     ` [EXT] " Sunil Kumar Kori
2023-11-21  6:14   ` [EXT] [PATCH v2 1/3] usertools/rss: add driver abstractions Sunil Kumar Kori
2023-11-22 23:43   ` Thomas Monjalon
2023-11-21  6:11 ` [EXT] [PATCH 1/2] " Sunil Kumar Kori
2023-11-21 16:38 ` Stephen Hemminger
2023-11-21 16:49   ` Thomas Monjalon
2023-11-21 16:51   ` Jerin Jacob
2023-11-21 17:04     ` Stephen Hemminger
2023-11-21 17:21       ` Jerin Jacob
2023-11-21 17:27         ` Stephen Hemminger
2023-11-22  8:28           ` Robin Jarry
2023-11-22  9:37             ` fengchengwen
2023-11-22 23:27               ` Thomas Monjalon

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).