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

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

[Jakub Kicinski]

Add a few more tests. I think they work. Commit 5d350dc3429b ("bnxt_en:
Fix the resource check condition for RSS contexts") does not appear
to have fixed bnxt for me :(

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       | 206 +++++++++++++++---
 1 file changed, 181 insertions(+), 25 deletions(-)

-- 
2.45.2

[PATCH net-next 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.
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

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

[Willem de Bruijn]

Jakub Kicinski wrote:
> The basic test may fail without resetting the RSS indir table.
> 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()

When is this explicit exec needed?

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

[Jakub Kicinski]

On Sat, 06 Jul 2024 09:53:41 -0400 Willem de Bruijn wrote:
> > @@ -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()  
> 
> When is this explicit exec needed?

When you want to run the cleanup _now_.

We construct the cleanup as soon as we allocate the resource,
it stays on the deferred list in case some exception makes us abort,
but the test may want to free the resource or reconfigure it further
as part of the test, in which case it can run .exec() (or cancel() to
discard the clean up without running it).

Here we do:
 1. constrain RSS
 2. run traffic (to check we're hitting expected queues)
 3. reset RSS
 4. run traffic (to check we're hitting all queues)

so step 3 runs the cleanup of step 1 explicitly.

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

[Willem de Bruijn]

Jakub Kicinski wrote:
> On Sat, 06 Jul 2024 09:53:41 -0400 Willem de Bruijn wrote:
> > > @@ -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()  
> > 
> > When is this explicit exec needed?
> 
> When you want to run the cleanup _now_.
> 
> We construct the cleanup as soon as we allocate the resource,
> it stays on the deferred list in case some exception makes us abort,
> but the test may want to free the resource or reconfigure it further
> as part of the test, in which case it can run .exec() (or cancel() to
> discard the clean up without running it).
> 
> Here we do:
>  1. constrain RSS
>  2. run traffic (to check we're hitting expected queues)
>  3. reset RSS
>  4. run traffic (to check we're hitting all queues)
> 
> so step 3 runs the cleanup of step 1 explicitly.

Thanks. I was wondering why this test calls it explicitly, while
others do not. Had overlooked step 4, which requires the reset.

[PATCH net-next 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.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 50 ++++++++++++-------
 1 file changed, 31 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..a95842baef99 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -63,6 +63,22 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     return queue_stats
 
 
+def _send_traffic_check(cfg, port, name, params):
+    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 +186,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 +241,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': range(2),
+                    '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 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.

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 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, ))
-- 
2.45.2

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

[Willem de Bruijn]

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

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

[Jakub Kicinski]

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)

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

[Willem de Bruijn]

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'

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

[Jakub Kicinski]

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

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

[Willem de Bruijn]

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.

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

[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 a bug, 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       | 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 fbc234d6c395..f42807e39e0d 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
@@ -203,6 +203,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.
@@ -434,9 +467,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

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

[Willem de Bruijn]

Jakub Kicinski wrote:
> 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.

This is the commit message of patch 5/5
 

[PATCH net-next 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 f42807e39e0d..3c47ddc509e7 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -236,6 +236,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.
@@ -471,7 +501,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 0/5] selftests: drv-net: rss_ctx: more tests

[Pavan Chebbi]

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

On Fri, Jul 5, 2024 at 7:27 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Add a few more tests. I think they work. Commit 5d350dc3429b ("bnxt_en:
> Fix the resource check condition for RSS contexts") does not appear
> to have fixed bnxt for me :(
The commit 5d350dc3429b won't fix the out_of_order test. That is
failing because bnxt sees netif_is_rxfh_configured() as false when the
test sends set_rxfh for reducing the main context's RSS. I am yet to
get to the bottom of it. The above commit is still required to keep
main context sane when we are adding/deleting other ctxs.

>
> 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       | 206 +++++++++++++++---
>  1 file changed, 181 insertions(+), 25 deletions(-)
>
> --
> 2.45.2
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]