[PATCH net-next v2 0/5] selftests: drv-net: rss_ctx: more tests

[PATCH net-next v2 0/5] selftests: drv-net: rss_ctx: more tests

[Jakub Kicinski]

Add a few more tests for RSS.

v2:
 - update the commit messages
 - add a comment in patch 2
v1: https://lore.kernel.org/all/20240705015725.680275-1-kuba@kernel.org/

Jakub Kicinski (5):
  selftests: drv-net: rss_ctx: fix cleanup in the basic test
  selftests: drv-net: rss_ctx: factor out send traffic and check
  selftests: drv-net: rss_ctx: test queue changes vs user RSS config
  selftests: drv-net: rss_ctx: check behavior of indirection table
    resizing
  selftests: drv-net: rss_ctx: test flow rehashing without impacting
    traffic

 .../selftests/drivers/net/hw/rss_ctx.py       | 214 ++++++++++++++++--
 1 file changed, 189 insertions(+), 25 deletions(-)

-- 
2.45.2

[PATCH net-next v2 1/5] selftests: drv-net: rss_ctx: fix cleanup in the basic test

[Jakub Kicinski]

The basic test may fail without resetting the RSS indir table.
Use the .exec() method to run cleanup early since we re-test
with traffic that returning to default state works.
While at it reformat the doc a tiny bit.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/hw/rss_ctx.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index 475f2a63fcd5..de2a55c0f35c 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -64,9 +64,8 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
 
 
 def test_rss_key_indir(cfg):
-    """
-    Test basics like updating the main RSS key and indirection table.
-    """
+    """Test basics like updating the main RSS key and indirection table."""
+
     if len(_get_rx_cnts(cfg)) < 2:
         KsftSkipEx("Device has only one queue (or doesn't support queue stats)")
 
@@ -89,6 +88,7 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
 
     # Set the indirection table
     ethtool(f"-X {cfg.ifname} equal 2")
+    reset_indir = defer(ethtool, f"-X {cfg.ifname} default")
     data = get_rss(cfg)
     ksft_eq(0, min(data['rss-indirection-table']))
     ksft_eq(1, max(data['rss-indirection-table']))
@@ -104,7 +104,7 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     ksft_eq(sum(cnts[2:]), 0, "traffic on unused queues: " + str(cnts))
 
     # Restore, and check traffic gets spread again
-    ethtool(f"-X {cfg.ifname} default")
+    reset_indir.exec()
 
     cnts = _get_rx_cnts(cfg)
     GenerateTraffic(cfg).wait_pkts_and_stop(20000)
-- 
2.45.2

[PATCH net-next v2 2/5] selftests: drv-net: rss_ctx: factor out send traffic and check

[Jakub Kicinski]

Wrap up sending traffic and checking in which queues it landed
in a helper.

The method used for testing is to send a lot of iperf traffic
and check which queues received the most packets. Those should
be the queues where we expect iperf to land - either because we
installed a filter for the port iperf uses, or we didn't and
expect it to use context 0.

Contexts get disjoint queue sets, but the main context (AKA context 0)
may receive some background traffic (noise).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: add a comment
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 58 +++++++++++++------
 1 file changed, 39 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index de2a55c0f35c..ede8eee1d9a9 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -63,6 +63,30 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     return queue_stats
 
 
+def _send_traffic_check(cfg, port, name, params):
+    # params is a dict with 3 possible keys:
+    #  - "target": required, which queues we expect to get iperf traffic
+    #  - "empty": optional, which queues should see no traffic at all
+    #  - "noise": optional, which queues we expect to see low traffic;
+    #             used for queues of the main context, since some background
+    #             OS activity may use those queues while we're testing
+    # the value for each is a list, or some other iterable containing queue ids.
+
+    cnts = _get_rx_cnts(cfg)
+    GenerateTraffic(cfg, port=port).wait_pkts_and_stop(20000)
+    cnts = _get_rx_cnts(cfg, prev=cnts)
+
+    directed = sum(cnts[i] for i in params['target'])
+
+    ksft_ge(directed, 20000, f"traffic on {name}: " + str(cnts))
+    if params.get('noise'):
+        ksft_lt(sum(cnts[i] for i in params['noise']), directed / 2,
+                "traffic on other queues:" + str(cnts))
+    if params.get('empty'):
+        ksft_eq(sum(cnts[i] for i in params['empty']), 0,
+                "traffic on inactive queues: " + str(cnts))
+
+
 def test_rss_key_indir(cfg):
     """Test basics like updating the main RSS key and indirection table."""
 
@@ -170,15 +194,10 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
         defer(ethtool, f"-N {cfg.ifname} delete {ntuple}")
 
     for i in range(ctx_cnt):
-        cnts = _get_rx_cnts(cfg)
-        GenerateTraffic(cfg, port=ports[i]).wait_pkts_and_stop(20000)
-        cnts = _get_rx_cnts(cfg, prev=cnts)
-
-        directed = sum(cnts[2+i*2:4+i*2])
-
-        ksft_lt(sum(cnts[ :2]), directed / 2, "traffic on main context:" + str(cnts))
-        ksft_ge(directed, 20000, f"traffic on context {i}: " + str(cnts))
-        ksft_eq(sum(cnts[2:2+i*2] + cnts[4+i*2:]), 0, "traffic on other contexts: " + str(cnts))
+        _send_traffic_check(cfg, ports[i], f"context {i}",
+                            { 'target': (2+i*2, 3+i*2),
+                              'noise': (0, 1),
+                              'empty': list(range(2, 2+i*2)) + list(range(4+i*2, 2+2*ctx_cnt)) })
 
     if requested_ctx_cnt != ctx_cnt:
         raise KsftSkipEx(f"Tested only {ctx_cnt} contexts, wanted {requested_ctx_cnt}")
@@ -230,18 +249,19 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
 
     def check_traffic():
         for i in range(ctx_cnt):
-            cnts = _get_rx_cnts(cfg)
-            GenerateTraffic(cfg, port=ports[i]).wait_pkts_and_stop(20000)
-            cnts = _get_rx_cnts(cfg, prev=cnts)
-
             if ctx[i]:
-                directed = sum(cnts[2+i*2:4+i*2])
-                ksft_lt(sum(cnts[ :2]), directed / 2, "traffic on main context:" + str(cnts))
-                ksft_ge(directed, 20000, f"traffic on context {i}: " + str(cnts))
-                ksft_eq(sum(cnts[2:2+i*2] + cnts[4+i*2:]), 0, "traffic on other contexts: " + str(cnts))
+                expected = {
+                    'target': (2+i*2, 3+i*2),
+                    'noise': (0, 1),
+                    'empty': list(range(2, 2+i*2)) + list(range(4+i*2, 2+2*ctx_cnt))
+                }
             else:
-                ksft_ge(sum(cnts[ :2]), 20000, "traffic on main context:" + str(cnts))
-                ksft_eq(sum(cnts[2: ]),     0, "traffic on other contexts: " + str(cnts))
+                expected = {
+                    'target': (0, 1),
+                    'empty':  range(2, 2+2*ctx_cnt)
+                }
+
+            _send_traffic_check(cfg, ports[i], f"context {i}", expected)
 
     # Use queues 0 and 1 for normal traffic
     ethtool(f"-X {cfg.ifname} equal 2")
-- 
2.45.2

[PATCH net-next v2 3/5] selftests: drv-net: rss_ctx: test queue changes vs user RSS config

[Jakub Kicinski]

By default main RSS table should change to include all queues.
When user sets a specific RSS config the driver should preserve it,
even when queue count changes. Driver should refuse to deactivate
queues used in the user-set RSS config.

For additional contexts driver should still refuse to deactivate
queues in use. Whether the contexts should get resized like
context 0 when queue count increases is a bit unclear. I anticipate
most driver today don't do that. Since main use case for additional
contexts is to set the indir table - it doesn't seem worthwhile to
care about behavior of the default table too much. Don't test that.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 81 ++++++++++++++++++-
 1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index ede8eee1d9a9..177abfd06412 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -137,6 +137,80 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     ksft_lt(sum(cnts[:2]), sum(cnts[2:]), "traffic distributed: " + str(cnts))
 
 
+def test_rss_queue_reconfigure(cfg, main_ctx=True):
+    """Make sure queue changes can't override requested RSS config.
+
+    By default main RSS table should change to include all queues.
+    When user sets a specific RSS config the driver should preserve it,
+    even when queue count changes. Driver should refuse to deactivate
+    queues used in the user-set RSS config.
+    """
+
+    if not main_ctx:
+        require_ntuple(cfg)
+
+    # Start with 4 queues, an arbitrary known number.
+    try:
+        qcnt = len(_get_rx_cnts(cfg))
+        ethtool(f"-L {cfg.ifname} combined 4")
+        defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
+    except:
+        raise KsftSkipEx("Not enough queues for the test or qstat not supported")
+
+    if main_ctx:
+        ctx_id = 0
+        ctx_ref = ""
+    else:
+        ctx_id = ethtool_create(cfg, "-X", "context new")
+        ctx_ref = f"context {ctx_id}"
+        defer(ethtool, f"-X {cfg.ifname} {ctx_ref} delete")
+
+    # Indirection table should be distributing to all queues.
+    data = get_rss(cfg, context=ctx_id)
+    ksft_eq(0, min(data['rss-indirection-table']))
+    ksft_eq(3, max(data['rss-indirection-table']))
+
+    # Increase queues, indirection table should be distributing to all queues.
+    # It's unclear whether tables of additional contexts should be reset, too.
+    if main_ctx:
+        ethtool(f"-L {cfg.ifname} combined 5")
+        data = get_rss(cfg)
+        ksft_eq(0, min(data['rss-indirection-table']))
+        ksft_eq(4, max(data['rss-indirection-table']))
+        ethtool(f"-L {cfg.ifname} combined 4")
+
+    # Configure the table explicitly
+    port = rand_port()
+    ethtool(f"-X {cfg.ifname} {ctx_ref} weight 1 0 0 1")
+    if main_ctx:
+        other_key = 'empty'
+        defer(ethtool, f"-X {cfg.ifname} default")
+    else:
+        other_key = 'noise'
+        flow = f"flow-type tcp{cfg.addr_ipver} dst-port {port} context {ctx_id}"
+        ntuple = ethtool_create(cfg, "-N", flow)
+        defer(ethtool, f"-N {cfg.ifname} delete {ntuple}")
+
+    _send_traffic_check(cfg, port, ctx_ref, { 'target': (0, 3),
+                                              other_key: (1, 2) })
+
+    # We should be able to increase queues, but table should be left untouched
+    ethtool(f"-L {cfg.ifname} combined 5")
+    data = get_rss(cfg, context=ctx_id)
+    ksft_eq({0, 3}, set(data['rss-indirection-table']))
+
+    _send_traffic_check(cfg, port, ctx_ref, { 'target': (0, 3),
+                                              other_key: (1, 2, 4) })
+
+    # Setting queue count to 3 should fail, queue 3 is used
+    try:
+        ethtool(f"-L {cfg.ifname} combined 3")
+    except CmdExitFailure:
+        pass
+    else:
+        raise Exception(f"Driver didn't prevent us from deactivating a used queue (context {ctx_id})")
+
+
 def test_rss_context(cfg, ctx_cnt=1, create_with_cfg=None):
     """
     Test separating traffic into RSS contexts.
@@ -215,6 +289,10 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     test_rss_context(cfg, 4, create_with_cfg=True)
 
 
+def test_rss_context_queue_reconfigure(cfg):
+    test_rss_queue_reconfigure(cfg, main_ctx=False)
+
+
 def test_rss_context_out_of_order(cfg, ctx_cnt=4):
     """
     Test separating traffic into RSS contexts.
@@ -366,8 +444,9 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
         cfg.netdevnl = NetdevFamily()
 
-        ksft_run([test_rss_key_indir,
+        ksft_run([test_rss_key_indir, test_rss_queue_reconfigure,
                   test_rss_context, test_rss_context4, test_rss_context32,
+                  test_rss_context_queue_reconfigure,
                   test_rss_context_overlap, test_rss_context_overlap2,
                   test_rss_context_out_of_order, test_rss_context4_create_with_cfg],
                  args=(cfg, ))
-- 
2.45.2

[PATCH net-next v2 4/5] selftests: drv-net: rss_ctx: check behavior of indirection table resizing

[Jakub Kicinski]

Some devices dynamically increase and decrease the size of the RSS
indirection table based on the number of enabled queues.
When that happens driver must maintain the balance of entries
(preferably duplicating the smaller table).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 37 ++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index 177abfd06412..38a871220bff 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -5,7 +5,7 @@ import datetime
 import random
 from lib.py import ksft_run, ksft_pr, ksft_exit, ksft_eq, ksft_ge, ksft_lt
 from lib.py import NetDrvEpEnv
-from lib.py import NetdevFamily
+from lib.py import EthtoolFamily, NetdevFamily
 from lib.py import KsftSkipEx
 from lib.py import rand_port
 from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
@@ -211,6 +211,39 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
         raise Exception(f"Driver didn't prevent us from deactivating a used queue (context {ctx_id})")
 
 
+def test_rss_resize(cfg):
+    """Test resizing of the RSS table.
+
+    Some devices dynamically increase and decrease the size of the RSS
+    indirection table based on the number of enabled queues.
+    When that happens driver must maintain the balance of entries
+    (preferably duplicating the smaller table).
+    """
+
+    channels = cfg.ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
+    ch_max = channels['combined-max']
+    qcnt = channels['combined-count']
+
+    if ch_max < 2:
+        raise KsftSkipEx(f"Not enough queues for the test: {ch_max}")
+
+    ethtool(f"-L {cfg.ifname} combined 2")
+    defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
+
+    ethtool(f"-X {cfg.ifname} weight 1 7")
+    defer(ethtool, f"-X {cfg.ifname} default")
+
+    ethtool(f"-L {cfg.ifname} combined {ch_max}")
+    data = get_rss(cfg)
+    ksft_eq(0, min(data['rss-indirection-table']))
+    ksft_eq(1, max(data['rss-indirection-table']))
+
+    ksft_eq(7,
+            data['rss-indirection-table'].count(1) /
+            data['rss-indirection-table'].count(0),
+            f"Table imbalance after resize: {data['rss-indirection-table']}")
+
+
 def test_rss_context(cfg, ctx_cnt=1, create_with_cfg=None):
     """
     Test separating traffic into RSS contexts.
@@ -442,9 +475,11 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
 
 def main() -> None:
     with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
+        cfg.ethnl = EthtoolFamily()
         cfg.netdevnl = NetdevFamily()
 
         ksft_run([test_rss_key_indir, test_rss_queue_reconfigure,
+                  test_rss_resize,
                   test_rss_context, test_rss_context4, test_rss_context32,
                   test_rss_context_queue_reconfigure,
                   test_rss_context_overlap, test_rss_context_overlap2,
-- 
2.45.2

[PATCH net-next v2 5/5] selftests: drv-net: rss_ctx: test flow rehashing without impacting traffic

[Jakub Kicinski]

Some workloads may want to rehash the flows in response to an imbalance.
Most effective way to do that is changing the RSS key. Check that changing
the key does not cause link flaps or traffic disruption.

Disrupting traffic for key update is not incorrect, but makes the key
update unusable for rehashing under load.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index 38a871220bff..931dbc36ca43 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -244,6 +244,36 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
             f"Table imbalance after resize: {data['rss-indirection-table']}")
 
 
+def test_hitless_key_update(cfg):
+    """Test that flows may be rehashed without impacting traffic.
+
+    Some workloads may want to rehash the flows in response to an imbalance.
+    Most effective way to do that is changing the RSS key. Check that changing
+    the key does not cause link flaps or traffic disruption.
+
+    Disrupting traffic for key update is not a bug, but makes the key
+    update unusable for rehashing under load.
+    """
+    data = get_rss(cfg)
+    key_len = len(data['rss-hash-key'])
+
+    key = _rss_key_rand(key_len)
+
+    tgen = GenerateTraffic(cfg)
+    try:
+        errors0, carrier0 = get_drop_err_sum(cfg)
+        t0 = datetime.datetime.now()
+        ethtool(f"-X {cfg.ifname} hkey " + _rss_key_str(key))
+        t1 = datetime.datetime.now()
+        errors1, carrier1 = get_drop_err_sum(cfg)
+    finally:
+        tgen.wait_pkts_and_stop(5000)
+
+    ksft_lt((t1 - t0).total_seconds(), 0.2)
+    ksft_eq(errors1 - errors1, 0)
+    ksft_eq(carrier1 - carrier0, 0)
+
+
 def test_rss_context(cfg, ctx_cnt=1, create_with_cfg=None):
     """
     Test separating traffic into RSS contexts.
@@ -479,7 +509,7 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
         cfg.netdevnl = NetdevFamily()
 
         ksft_run([test_rss_key_indir, test_rss_queue_reconfigure,
-                  test_rss_resize,
+                  test_rss_resize, test_hitless_key_update,
                   test_rss_context, test_rss_context4, test_rss_context32,
                   test_rss_context_queue_reconfigure,
                   test_rss_context_overlap, test_rss_context_overlap2,
-- 
2.45.2

Re: [PATCH net-next v2 0/5] selftests: drv-net: rss_ctx: more tests

[Willem de Bruijn]

Jakub Kicinski wrote:
> Add a few more tests for RSS.
> 
> v2:
>  - update the commit messages
>  - add a comment in patch 2
> v1: https://lore.kernel.org/all/20240705015725.680275-1-kuba@kernel.org/
> 
> Jakub Kicinski (5):
>   selftests: drv-net: rss_ctx: fix cleanup in the basic test
>   selftests: drv-net: rss_ctx: factor out send traffic and check
>   selftests: drv-net: rss_ctx: test queue changes vs user RSS config
>   selftests: drv-net: rss_ctx: check behavior of indirection table
>     resizing
>   selftests: drv-net: rss_ctx: test flow rehashing without impacting
>     traffic
> 

For the series:

Reviewed-by: Willem de Bruijn <willemb@google.com>

Minor typo in 3/5: "most driver[s] today"

Re: [PATCH net-next v2 0/5] selftests: drv-net: rss_ctx: more tests

[patchwork-bot+netdevbpf]

Hello:

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

On Mon,  8 Jul 2024 14:36:22 -0700 you wrote:
> Add a few more tests for RSS.
> 
> v2:
>  - update the commit messages
>  - add a comment in patch 2
> v1: https://lore.kernel.org/all/20240705015725.680275-1-kuba@kernel.org/
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/5] selftests: drv-net: rss_ctx: fix cleanup in the basic test
    https://git.kernel.org/netdev/net-next/c/a0aab7d7c860
  - [net-next,v2,2/5] selftests: drv-net: rss_ctx: factor out send traffic and check
    https://git.kernel.org/netdev/net-next/c/847aa551fa78
  - [net-next,v2,3/5] selftests: drv-net: rss_ctx: test queue changes vs user RSS config
    https://git.kernel.org/netdev/net-next/c/e2c9703d424e
  - [net-next,v2,4/5] selftests: drv-net: rss_ctx: check behavior of indirection table resizing
    https://git.kernel.org/netdev/net-next/c/7e3e5b0bc51d
  - [net-next,v2,5/5] selftests: drv-net: rss_ctx: test flow rehashing without impacting traffic
    https://git.kernel.org/netdev/net-next/c/933048fec4dd

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