diff mbox series

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

Message ID 20240705015725.680275-4-kuba@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series selftests: drv-net: rss_ctx: more tests | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers fail 2 maintainers not CCed: linux-kselftest@vger.kernel.org shuah@kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 100 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-07-05--06-00 (tests: 695)

Commit Message

Jakub Kicinski July 5, 2024, 1:57 a.m. UTC
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.

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

Comments

Willem de Bruijn July 6, 2024, 2 p.m. UTC | #1
Jakub Kicinski wrote:
> 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.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

> +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) })

How come queues missing from the indir set in non-main context are not
empty (but noise)?

> +
> +    # 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})")
> +
> +
Jakub Kicinski July 8, 2024, 4:17 p.m. UTC | #2
On Sat, 06 Jul 2024 10:00:58 -0400 Willem de Bruijn wrote:
> > +    _send_traffic_check(cfg, port, ctx_ref, { 'target': (0, 3),
> > +                                              other_key: (1, 2) })  
> 
> How come queues missing from the indir set in non-main context are not
> empty (but noise)?

There may be background noise traffic on the main context.
If we are running iperf to the main context the noise will just add up
to the iperf traffic and all other queues should be completely idle.
If we're testing additional context we'll get only iperf traffic on
the target context, and all non-iperf noise stays on main context
(hence noise rather than empty)
Willem de Bruijn July 8, 2024, 4:56 p.m. UTC | #3
Jakub Kicinski wrote:
> On Sat, 06 Jul 2024 10:00:58 -0400 Willem de Bruijn wrote:
> > > +    _send_traffic_check(cfg, port, ctx_ref, { 'target': (0, 3),
> > > +                                              other_key: (1, 2) })  
> > 
> > How come queues missing from the indir set in non-main context are not
> > empty (but noise)?
> 
> There may be background noise traffic on the main context.
> If we are running iperf to the main context the noise will just add up
> to the iperf traffic and all other queues should be completely idle.
> If we're testing additional context we'll get only iperf traffic on
> the target context, and all non-iperf noise stays on main context
> (hence noise rather than empty)

That makes sense. Should the following be inverted then?

+    if main_ctx:
+        other_key = 'empty'
+        defer(ethtool, f"-X {cfg.ifname} default")
+    else:
+        other_key = 'noise'
Jakub Kicinski July 8, 2024, 8:04 p.m. UTC | #4
On Mon, 08 Jul 2024 12:56:01 -0400 Willem de Bruijn wrote:
> > There may be background noise traffic on the main context.
> > If we are running iperf to the main context the noise will just add up
> > to the iperf traffic and all other queues should be completely idle.
> > If we're testing additional context we'll get only iperf traffic on
> > the target context, and all non-iperf noise stays on main context
> > (hence noise rather than empty)  
> 
> That makes sense. Should the following be inverted then?
> 
> +    if main_ctx:
> +        other_key = 'empty'
> +        defer(ethtool, f"-X {cfg.ifname} default")
> +    else:
> +        other_key = 'noise'

No, unless I'm confused. if we're testing the main context the other
queues will be empty. Else we're testing other (additional) contexts
and queues outside those contexts will contain noise (the queues in 
the main context, specifically).
Willem de Bruijn July 8, 2024, 8:29 p.m. UTC | #5
Jakub Kicinski wrote:
> On Mon, 08 Jul 2024 12:56:01 -0400 Willem de Bruijn wrote:
> > > There may be background noise traffic on the main context.
> > > If we are running iperf to the main context the noise will just add up
> > > to the iperf traffic and all other queues should be completely idle.
> > > If we're testing additional context we'll get only iperf traffic on
> > > the target context, and all non-iperf noise stays on main context
> > > (hence noise rather than empty)  
> > 
> > That makes sense. Should the following be inverted then?
> > 
> > +    if main_ctx:
> > +        other_key = 'empty'
> > +        defer(ethtool, f"-X {cfg.ifname} default")
> > +    else:
> > +        other_key = 'noise'
> 
> No, unless I'm confused. if we're testing the main context the other
> queues will be empty. Else we're testing other (additional) contexts
> and queues outside those contexts will contain noise (the queues in 
> the main context, specifically).

Nope, I'm the one who was confused, of course :)

I for some reason assumed that the contexts had exclusive queue sets.
Rather than these being absolute queue indexes and overlapping with
main_ctx.

In which case, understood. Sorry for the noise.
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index a95842baef99..fbc234d6c395 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -129,6 +129,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.
@@ -207,6 +281,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.
@@ -358,8 +436,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, ))