netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx
@ 2025-04-25  2:20 David Wei
  2025-04-25  2:20 ` [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup David Wei
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: David Wei @ 2025-04-25  2:20 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni

Update io_uring zero copy receive selftest. Patch 1 does a requested
cleanup to use defer() for undoing ethtool actions during the test and
restoring the NIC under test back to its original state.

Patch 2 adds a required call to set hds_thresh to 0. This is needed for
the queue API.

Patch 3 adds a new test case for steering into RSS contexts. A real
application using io_uring zero copy receive relies on this working to
shard work across multiple queues. There seems to be some
differences/bugs with steering into RSS contexts and individual queues.

David Wei (3):
  io_uring/zcrx: selftests: switch to using defer() for cleanup
  io_uring/zcrx: selftests: set hds_thresh to 0
  io_uring/zcrx: selftests: add test case for rss ctx

 .../selftests/drivers/net/hw/iou-zcrx.py      | 122 ++++++++++++------
 1 file changed, 82 insertions(+), 40 deletions(-)

-- 
2.47.1


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

* [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup
  2025-04-25  2:20 [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx David Wei
@ 2025-04-25  2:20 ` David Wei
  2025-04-25 17:17   ` Simon Horman
  2025-04-25 22:46   ` Joe Damato
  2025-04-25  2:20 ` [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0 David Wei
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: David Wei @ 2025-04-25  2:20 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni

Switch to using defer() for putting the NIC back to the original state
prior to running the selftest.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 .../selftests/drivers/net/hw/iou-zcrx.py      | 61 +++++++++----------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
index 6a0378e06cab..698f29cfd7eb 100755
--- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
+++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
@@ -5,7 +5,7 @@ import re
 from os import path
 from lib.py import ksft_run, ksft_exit
 from lib.py import NetDrvEpEnv
-from lib.py import bkg, cmd, ethtool, wait_port_listen
+from lib.py import bkg, cmd, defer, ethtool, wait_port_listen
 
 
 def _get_rx_ring_entries(cfg):
@@ -34,22 +34,21 @@ def test_zcrx(cfg) -> None:
         raise KsftSkipEx('at least 2 combined channels required')
     rx_ring = _get_rx_ring_entries(cfg)
 
-    try:
-        ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
-        ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
-        ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
-        flow_rule_id = _set_flow_rule(cfg, combined_chans - 1)
 
-        rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1}"
-        tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 12840"
-        with bkg(rx_cmd, host=cfg.remote, exit_wait=True):
-            wait_port_listen(9999, proto="tcp", host=cfg.remote)
-            cmd(tx_cmd)
-    finally:
-        ethtool(f"-N {cfg.ifname} delete {flow_rule_id}", host=cfg.remote)
-        ethtool(f"-X {cfg.ifname} default", host=cfg.remote)
-        ethtool(f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
-        ethtool(f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
+    ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
+    defer(ethtool, f"-X {cfg.ifname} default", host=cfg.remote)
+    flow_rule_id = _set_flow_rule(cfg, combined_chans - 1)
+    defer(ethtool, f"-N {cfg.ifname} delete {flow_rule_id}", host=cfg.remote)
+
+    rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1}"
+    tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 12840"
+    with bkg(rx_cmd, host=cfg.remote, exit_wait=True):
+        wait_port_listen(9999, proto="tcp", host=cfg.remote)
+        cmd(tx_cmd)
 
 
 def test_zcrx_oneshot(cfg) -> None:
@@ -60,22 +59,20 @@ def test_zcrx_oneshot(cfg) -> None:
         raise KsftSkipEx('at least 2 combined channels required')
     rx_ring = _get_rx_ring_entries(cfg)
 
-    try:
-        ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
-        ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
-        ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
-        flow_rule_id = _set_flow_rule(cfg, combined_chans - 1)
-
-        rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1} -o 4"
-        tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 4096 -z 16384"
-        with bkg(rx_cmd, host=cfg.remote, exit_wait=True):
-            wait_port_listen(9999, proto="tcp", host=cfg.remote)
-            cmd(tx_cmd)
-    finally:
-        ethtool(f"-N {cfg.ifname} delete {flow_rule_id}", host=cfg.remote)
-        ethtool(f"-X {cfg.ifname} default", host=cfg.remote)
-        ethtool(f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
-        ethtool(f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
+    ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
+    defer(ethtool, f"-X {cfg.ifname} default", host=cfg.remote)
+    flow_rule_id = _set_flow_rule(cfg, combined_chans - 1)
+    defer(ethtool, f"-N {cfg.ifname} delete {flow_rule_id}", host=cfg.remote)
+
+    rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1} -o 4"
+    tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 4096 -z 16384"
+    with bkg(rx_cmd, host=cfg.remote, exit_wait=True):
+        wait_port_listen(9999, proto="tcp", host=cfg.remote)
+        cmd(tx_cmd)
 
 
 def main() -> None:
-- 
2.47.1


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

* [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0
  2025-04-25  2:20 [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx David Wei
  2025-04-25  2:20 ` [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup David Wei
@ 2025-04-25  2:20 ` David Wei
  2025-04-25 17:18   ` Simon Horman
  2025-04-25 22:50   ` Joe Damato
  2025-04-25  2:20 ` [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx David Wei
  2025-04-26  1:50 ` [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test " patchwork-bot+netdevbpf
  3 siblings, 2 replies; 16+ messages in thread
From: David Wei @ 2025-04-25  2:20 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni

Setting hds_thresh to 0 is required for queue reset.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 .../testing/selftests/drivers/net/hw/iou-zcrx.py | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
index 698f29cfd7eb..0b0b6a261159 100755
--- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
+++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
@@ -8,10 +8,11 @@ from lib.py import NetDrvEpEnv
 from lib.py import bkg, cmd, defer, ethtool, wait_port_listen
 
 
-def _get_rx_ring_entries(cfg):
+def _get_current_settings(cfg):
     output = ethtool(f"-g {cfg.ifname}", host=cfg.remote).stdout
-    values = re.findall(r'RX:\s+(\d+)', output)
-    return int(values[1])
+    rx_ring = re.findall(r'RX:\s+(\d+)', output)
+    hds_thresh = re.findall(r'HDS thresh:\s+(\d+)', output)
+    return (int(rx_ring[1]), int(hds_thresh[1]))
 
 
 def _get_combined_channels(cfg):
@@ -32,11 +33,12 @@ def test_zcrx(cfg) -> None:
     combined_chans = _get_combined_channels(cfg)
     if combined_chans < 2:
         raise KsftSkipEx('at least 2 combined channels required')
-    rx_ring = _get_rx_ring_entries(cfg)
-
+    (rx_ring, hds_thresh) = _get_current_settings(cfg)
 
     ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
     defer(ethtool, f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} hds-thresh 0", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} hds-thresh {hds_thresh}", host=cfg.remote)
     ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
     defer(ethtool, f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
     ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
@@ -57,10 +59,12 @@ def test_zcrx_oneshot(cfg) -> None:
     combined_chans = _get_combined_channels(cfg)
     if combined_chans < 2:
         raise KsftSkipEx('at least 2 combined channels required')
-    rx_ring = _get_rx_ring_entries(cfg)
+    (rx_ring, hds_thresh) = _get_current_settings(cfg)
 
     ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
     defer(ethtool, f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} hds-thresh 0", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} hds-thresh {hds_thresh}", host=cfg.remote)
     ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
     defer(ethtool, f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
     ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
-- 
2.47.1


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

* [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx
  2025-04-25  2:20 [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx David Wei
  2025-04-25  2:20 ` [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup David Wei
  2025-04-25  2:20 ` [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0 David Wei
@ 2025-04-25  2:20 ` David Wei
  2025-04-25 17:18   ` Simon Horman
  2025-04-25 22:55   ` Joe Damato
  2025-04-26  1:50 ` [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test " patchwork-bot+netdevbpf
  3 siblings, 2 replies; 16+ messages in thread
From: David Wei @ 2025-04-25  2:20 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni

RSS contexts are used to shard work across multiple queues for an
application using io_uring zero copy receive. Add a test case checking
that steering flows into an RSS context works.

Until I add multi-thread support to the selftest binary, this test case
only has 1 queue in the RSS context.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 .../selftests/drivers/net/hw/iou-zcrx.py      | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
index 0b0b6a261159..48b3d27cf472 100755
--- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
+++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
@@ -21,12 +21,25 @@ def _get_combined_channels(cfg):
     return int(values[1])
 
 
+def _create_rss_ctx(cfg, chans):
+    output = ethtool(f"-X {cfg.ifname} context new start {chans - 1} equal 1", host=cfg.remote).stdout
+    values = re.search(r'New RSS context is (\d+)', output).group(1)
+    ctx_id = int(values)
+    return (ctx_id, defer(ethtool, f"-X {cfg.ifname} delete context {ctx_id}", host=cfg.remote))
+
+
 def _set_flow_rule(cfg, chan):
     output = ethtool(f"-N {cfg.ifname} flow-type tcp6 dst-port 9999 action {chan}", host=cfg.remote).stdout
     values = re.search(r'ID (\d+)', output).group(1)
     return int(values)
 
 
+def _set_flow_rule_rss(cfg, chan):
+    output = ethtool(f"-N {cfg.ifname} flow-type tcp6 dst-port 9999 action {chan}", host=cfg.remote).stdout
+    values = re.search(r'ID (\d+)', output).group(1)
+    return int(values)
+
+
 def test_zcrx(cfg) -> None:
     cfg.require_ipver('6')
 
@@ -79,6 +92,34 @@ def test_zcrx_oneshot(cfg) -> None:
         cmd(tx_cmd)
 
 
+def test_zcrx_rss(cfg) -> None:
+    cfg.require_ipver('6')
+
+    combined_chans = _get_combined_channels(cfg)
+    if combined_chans < 2:
+        raise KsftSkipEx('at least 2 combined channels required')
+    (rx_ring, hds_thresh) = _get_current_settings(cfg)
+
+    ethtool(f"-G {cfg.ifname} tcp-data-split on", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} tcp-data-split auto", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} hds-thresh 0", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} hds-thresh {hds_thresh}", host=cfg.remote)
+    ethtool(f"-G {cfg.ifname} rx 64", host=cfg.remote)
+    defer(ethtool, f"-G {cfg.ifname} rx {rx_ring}", host=cfg.remote)
+    ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote)
+    defer(ethtool, f"-X {cfg.ifname} default", host=cfg.remote)
+
+    (ctx_id, delete_ctx) = _create_rss_ctx(cfg, combined_chans)
+    flow_rule_id = _set_flow_rule_rss(cfg, ctx_id)
+    defer(ethtool, f"-N {cfg.ifname} delete {flow_rule_id}", host=cfg.remote)
+
+    rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1}"
+    tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 12840"
+    with bkg(rx_cmd, host=cfg.remote, exit_wait=True):
+        wait_port_listen(9999, proto="tcp", host=cfg.remote)
+        cmd(tx_cmd)
+
+
 def main() -> None:
     with NetDrvEpEnv(__file__) as cfg:
         cfg.bin_local = path.abspath(path.dirname(__file__) + "/../../../drivers/net/hw/iou-zcrx")
-- 
2.47.1


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

* Re: [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup
  2025-04-25  2:20 ` [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup David Wei
@ 2025-04-25 17:17   ` Simon Horman
  2025-04-25 22:46   ` Joe Damato
  1 sibling, 0 replies; 16+ messages in thread
From: Simon Horman @ 2025-04-25 17:17 UTC (permalink / raw)
  To: David Wei
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Thu, Apr 24, 2025 at 07:20:47PM -0700, David Wei wrote:
> Switch to using defer() for putting the NIC back to the original state
> prior to running the selftest.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0
  2025-04-25  2:20 ` [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0 David Wei
@ 2025-04-25 17:18   ` Simon Horman
  2025-04-25 22:50   ` Joe Damato
  1 sibling, 0 replies; 16+ messages in thread
From: Simon Horman @ 2025-04-25 17:18 UTC (permalink / raw)
  To: David Wei
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Thu, Apr 24, 2025 at 07:20:48PM -0700, David Wei wrote:
> Setting hds_thresh to 0 is required for queue reset.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx
  2025-04-25  2:20 ` [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx David Wei
@ 2025-04-25 17:18   ` Simon Horman
  2025-04-25 22:55   ` Joe Damato
  1 sibling, 0 replies; 16+ messages in thread
From: Simon Horman @ 2025-04-25 17:18 UTC (permalink / raw)
  To: David Wei
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Thu, Apr 24, 2025 at 07:20:49PM -0700, David Wei wrote:
> RSS contexts are used to shard work across multiple queues for an
> application using io_uring zero copy receive. Add a test case checking
> that steering flows into an RSS context works.
> 
> Until I add multi-thread support to the selftest binary, this test case
> only has 1 queue in the RSS context.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>

Reviewed-by: Simon Horman <horms@kernel.org>


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

* Re: [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup
  2025-04-25  2:20 ` [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup David Wei
  2025-04-25 17:17   ` Simon Horman
@ 2025-04-25 22:46   ` Joe Damato
  1 sibling, 0 replies; 16+ messages in thread
From: Joe Damato @ 2025-04-25 22:46 UTC (permalink / raw)
  To: David Wei
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Thu, Apr 24, 2025 at 07:20:47PM -0700, David Wei wrote:
> Switch to using defer() for putting the NIC back to the original state
> prior to running the selftest.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  .../selftests/drivers/net/hw/iou-zcrx.py      | 61 +++++++++----------
>  1 file changed, 29 insertions(+), 32 deletions(-)
>

Reviewed-by: Joe Damato <jdamato@fastly.com>

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

* Re: [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0
  2025-04-25  2:20 ` [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0 David Wei
  2025-04-25 17:18   ` Simon Horman
@ 2025-04-25 22:50   ` Joe Damato
  2025-04-25 23:37     ` David Wei
  1 sibling, 1 reply; 16+ messages in thread
From: Joe Damato @ 2025-04-25 22:50 UTC (permalink / raw)
  To: David Wei
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Thu, Apr 24, 2025 at 07:20:48PM -0700, David Wei wrote:
> Setting hds_thresh to 0 is required for queue reset.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  .../testing/selftests/drivers/net/hw/iou-zcrx.py | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> index 698f29cfd7eb..0b0b6a261159 100755
> --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> @@ -8,10 +8,11 @@ from lib.py import NetDrvEpEnv
>  from lib.py import bkg, cmd, defer, ethtool, wait_port_listen
>  
>  
> -def _get_rx_ring_entries(cfg):
> +def _get_current_settings(cfg):
>      output = ethtool(f"-g {cfg.ifname}", host=cfg.remote).stdout
> -    values = re.findall(r'RX:\s+(\d+)', output)
> -    return int(values[1])
> +    rx_ring = re.findall(r'RX:\s+(\d+)', output)
> +    hds_thresh = re.findall(r'HDS thresh:\s+(\d+)', output)
> +    return (int(rx_ring[1]), int(hds_thresh[1]))

Makes me wonder if both of these values can be parsed from ethtool
JSON output instead of regexing the "regular" output. No reason in
particular; just vaguely feels like parsing JSON is safer somehow.

Reviewed-by: Joe Damato <jdamato@fastly.com>

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

* Re: [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx
  2025-04-25  2:20 ` [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx David Wei
  2025-04-25 17:18   ` Simon Horman
@ 2025-04-25 22:55   ` Joe Damato
  2025-04-25 23:38     ` David Wei
  1 sibling, 1 reply; 16+ messages in thread
From: Joe Damato @ 2025-04-25 22:55 UTC (permalink / raw)
  To: David Wei
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On Thu, Apr 24, 2025 at 07:20:49PM -0700, David Wei wrote:
> RSS contexts are used to shard work across multiple queues for an
> application using io_uring zero copy receive. Add a test case checking
> that steering flows into an RSS context works.
> 
> Until I add multi-thread support to the selftest binary, this test case
> only has 1 queue in the RSS context.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  .../selftests/drivers/net/hw/iou-zcrx.py      | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> index 0b0b6a261159..48b3d27cf472 100755
> --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> @@ -21,12 +21,25 @@ def _get_combined_channels(cfg):
>      return int(values[1])
>  
>  
> +    output = ethtool(f"-N {cfg.ifname} flow-type tcp6 dst-port 9999 action {chan}", host=cfg.remote).stdout
> +    rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1}"
> +    tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 12840"

I wonder if perhaps future cleanup work might use rand_port from
lib.py instead of hardcoding 9999 ?

Reviewed-by: Joe Damato <jdamato@fastly.com>

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

* Re: [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0
  2025-04-25 22:50   ` Joe Damato
@ 2025-04-25 23:37     ` David Wei
  2025-04-26  1:42       ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: David Wei @ 2025-04-25 23:37 UTC (permalink / raw)
  To: Joe Damato, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On 2025-04-25 15:50, Joe Damato wrote:
> On Thu, Apr 24, 2025 at 07:20:48PM -0700, David Wei wrote:
>> Setting hds_thresh to 0 is required for queue reset.
>>
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>>  .../testing/selftests/drivers/net/hw/iou-zcrx.py | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
>> index 698f29cfd7eb..0b0b6a261159 100755
>> --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
>> +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
>> @@ -8,10 +8,11 @@ from lib.py import NetDrvEpEnv
>>  from lib.py import bkg, cmd, defer, ethtool, wait_port_listen
>>  
>>  
>> -def _get_rx_ring_entries(cfg):
>> +def _get_current_settings(cfg):
>>      output = ethtool(f"-g {cfg.ifname}", host=cfg.remote).stdout
>> -    values = re.findall(r'RX:\s+(\d+)', output)
>> -    return int(values[1])
>> +    rx_ring = re.findall(r'RX:\s+(\d+)', output)
>> +    hds_thresh = re.findall(r'HDS thresh:\s+(\d+)', output)
>> +    return (int(rx_ring[1]), int(hds_thresh[1]))
> 
> Makes me wonder if both of these values can be parsed from ethtool
> JSON output instead of regexing the "regular" output. No reason in
> particular; just vaguely feels like parsing JSON is safer somehow.

Yeah I agree. JSON output isn't available for these ethtool commands as
support for that is quite patchy. If/once there is JSON output I'd be up
for switching to that.

> 
> Reviewed-by: Joe Damato <jdamato@fastly.com>

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

* Re: [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx
  2025-04-25 22:55   ` Joe Damato
@ 2025-04-25 23:38     ` David Wei
  2025-04-26  1:43       ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: David Wei @ 2025-04-25 23:38 UTC (permalink / raw)
  To: Joe Damato, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni

On 2025-04-25 15:55, Joe Damato wrote:
> On Thu, Apr 24, 2025 at 07:20:49PM -0700, David Wei wrote:
>> RSS contexts are used to shard work across multiple queues for an
>> application using io_uring zero copy receive. Add a test case checking
>> that steering flows into an RSS context works.
>>
>> Until I add multi-thread support to the selftest binary, this test case
>> only has 1 queue in the RSS context.
>>
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>>  .../selftests/drivers/net/hw/iou-zcrx.py      | 41 +++++++++++++++++++
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
>> index 0b0b6a261159..48b3d27cf472 100755
>> --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
>> +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
>> @@ -21,12 +21,25 @@ def _get_combined_channels(cfg):
>>      return int(values[1])
>>  
>>  
>> +    output = ethtool(f"-N {cfg.ifname} flow-type tcp6 dst-port 9999 action {chan}", host=cfg.remote).stdout
>> +    rx_cmd = f"{cfg.bin_remote} -s -p 9999 -i {cfg.ifname} -q {combined_chans - 1}"
>> +    tx_cmd = f"{cfg.bin_local} -c -h {cfg.remote_addr_v['6']} -p 9999 -l 12840"
> 
> I wonder if perhaps future cleanup work might use rand_port from
> lib.py instead of hardcoding 9999 ?

SG, I'll tackle this as a follow up.

> 
> Reviewed-by: Joe Damato <jdamato@fastly.com>

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

* Re: [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0
  2025-04-25 23:37     ` David Wei
@ 2025-04-26  1:42       ` Jakub Kicinski
  2025-04-26 18:00         ` David Wei
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-04-26  1:42 UTC (permalink / raw)
  To: David Wei
  Cc: Joe Damato, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni

On Fri, 25 Apr 2025 16:37:26 -0700 David Wei wrote:
> >> -def _get_rx_ring_entries(cfg):
> >> +def _get_current_settings(cfg):
> >>      output = ethtool(f"-g {cfg.ifname}", host=cfg.remote).stdout
> >> -    values = re.findall(r'RX:\s+(\d+)', output)
> >> -    return int(values[1])
> >> +    rx_ring = re.findall(r'RX:\s+(\d+)', output)
> >> +    hds_thresh = re.findall(r'HDS thresh:\s+(\d+)', output)
> >> +    return (int(rx_ring[1]), int(hds_thresh[1]))  
> > 
> > Makes me wonder if both of these values can be parsed from ethtool
> > JSON output instead of regexing the "regular" output. No reason in
> > particular; just vaguely feels like parsing JSON is safer somehow.  
> 
> Yeah I agree. JSON output isn't available for these ethtool commands as
> support for that is quite patchy. If/once there is JSON output I'd be up
> for switching to that.

Joe is right, gor -g JSON is there, IIRC the only one we use often that
doesn't have JSON is -l. You could also switch to YNL directly, fewer
dependencies. But probably not a blocker for this series.


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

* Re: [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx
  2025-04-25 23:38     ` David Wei
@ 2025-04-26  1:43       ` Jakub Kicinski
  0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2025-04-26  1:43 UTC (permalink / raw)
  To: David Wei
  Cc: Joe Damato, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni

On Fri, 25 Apr 2025 16:38:16 -0700 David Wei wrote:
> > I wonder if perhaps future cleanup work might use rand_port from
> > lib.py instead of hardcoding 9999 ?  
> 
> SG, I'll tackle this as a follow up.

Yes, please..

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

* Re: [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx
  2025-04-25  2:20 [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx David Wei
                   ` (2 preceding siblings ...)
  2025-04-25  2:20 ` [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx David Wei
@ 2025-04-26  1:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-04-26  1:50 UTC (permalink / raw)
  To: David Wei; +Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 24 Apr 2025 19:20:46 -0700 you wrote:
> Update io_uring zero copy receive selftest. Patch 1 does a requested
> cleanup to use defer() for undoing ethtool actions during the test and
> restoring the NIC under test back to its original state.
> 
> Patch 2 adds a required call to set hds_thresh to 0. This is needed for
> the queue API.
> 
> [...]

Here is the summary with links:
  - [net-next,v1,1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup
    https://git.kernel.org/netdev/net-next/c/43fd0054f356
  - [net-next,v1,2/3] io_uring/zcrx: selftests: set hds_thresh to 0
    https://git.kernel.org/netdev/net-next/c/4ce3ade36f25
  - [net-next,v1,3/3] io_uring/zcrx: selftests: add test case for rss ctx
    https://git.kernel.org/netdev/net-next/c/5c3524b031be

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0
  2025-04-26  1:42       ` Jakub Kicinski
@ 2025-04-26 18:00         ` David Wei
  0 siblings, 0 replies; 16+ messages in thread
From: David Wei @ 2025-04-26 18:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Joe Damato, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni

On 4/25/25 18:42, Jakub Kicinski wrote:
> On Fri, 25 Apr 2025 16:37:26 -0700 David Wei wrote:
>>>> -def _get_rx_ring_entries(cfg):
>>>> +def _get_current_settings(cfg):
>>>>       output = ethtool(f"-g {cfg.ifname}", host=cfg.remote).stdout
>>>> -    values = re.findall(r'RX:\s+(\d+)', output)
>>>> -    return int(values[1])
>>>> +    rx_ring = re.findall(r'RX:\s+(\d+)', output)
>>>> +    hds_thresh = re.findall(r'HDS thresh:\s+(\d+)', output)
>>>> +    return (int(rx_ring[1]), int(hds_thresh[1]))
>>>
>>> Makes me wonder if both of these values can be parsed from ethtool
>>> JSON output instead of regexing the "regular" output. No reason in
>>> particular; just vaguely feels like parsing JSON is safer somehow.
>>
>> Yeah I agree. JSON output isn't available for these ethtool commands as
>> support for that is quite patchy. If/once there is JSON output I'd be up
>> for switching to that.
> 
> Joe is right, gor -g JSON is there, IIRC the only one we use often that
> doesn't have JSON is -l. You could also switch to YNL directly, fewer
> dependencies. But probably not a blocker for this series.
> 

Oh, sorry. I'll change to parsing --json output too.

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

end of thread, other threads:[~2025-04-26 18:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-25  2:20 [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test for rss ctx David Wei
2025-04-25  2:20 ` [PATCH net-next v1 1/3] io_uring/zcrx: selftests: switch to using defer() for cleanup David Wei
2025-04-25 17:17   ` Simon Horman
2025-04-25 22:46   ` Joe Damato
2025-04-25  2:20 ` [PATCH net-next v1 2/3] io_uring/zcrx: selftests: set hds_thresh to 0 David Wei
2025-04-25 17:18   ` Simon Horman
2025-04-25 22:50   ` Joe Damato
2025-04-25 23:37     ` David Wei
2025-04-26  1:42       ` Jakub Kicinski
2025-04-26 18:00         ` David Wei
2025-04-25  2:20 ` [PATCH net-next v1 3/3] io_uring/zcrx: selftests: add test case for rss ctx David Wei
2025-04-25 17:18   ` Simon Horman
2025-04-25 22:55   ` Joe Damato
2025-04-25 23:38     ` David Wei
2025-04-26  1:43       ` Jakub Kicinski
2025-04-26  1:50 ` [PATCH net-next v1 0/3] io_uring/zcrx: fix selftests and add new test " patchwork-bot+netdevbpf

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