linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v2 0/2] Fix ntuple rules targeting default RSS
@ 2025-06-09 12:02 Gal Pressman
  2025-06-09 12:02 ` [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0 Gal Pressman
  2025-06-09 12:02 ` [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context Gal Pressman
  0 siblings, 2 replies; 10+ messages in thread
From: Gal Pressman @ 2025-06-09 12:02 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, netdev
  Cc: Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Gal Pressman

This series addresses a regression in ethtool flow steering where rules
targeting the default RSS context (context 0) were incorrectly rejected.

The default RSS context always exists but is not stored in the rss_ctx
xarray like additional contexts. The current validation logic was
checking for the existence of context 0 in this array, causing valid
flow steering rules to be rejected.

This prevented configurations such as:
- High priority rules directing specific traffic to the default context
- Low priority catch-all rules directing remaining traffic to additional
  contexts

Patch 1 fixes the validation logic to skip the existence check for
context 0.

Patch 2 adds a selftest that verifies this behavior.

Changelog -
v1->v2: https://lore.kernel.org/all/20250225071348.509432-1-gal@nvidia.com/
* Reworded commit message.
* Added a selftest.

Gal Pressman (2):
  net: ethtool: Don't check if RSS context exists in case of context 0
  selftests: drv-net: rss_ctx: Add test for ntuple rules targeting
    default RSS context

 net/ethtool/ioctl.c                           |  3 +-
 .../selftests/drivers/net/hw/rss_ctx.py       | 59 ++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

-- 
2.40.1


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

* [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0
  2025-06-09 12:02 [PATCH net v2 0/2] Fix ntuple rules targeting default RSS Gal Pressman
@ 2025-06-09 12:02 ` Gal Pressman
  2025-06-09 15:38   ` Subbaraya Sundeep
  2025-06-09 22:41   ` Jakub Kicinski
  2025-06-09 12:02 ` [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context Gal Pressman
  1 sibling, 2 replies; 10+ messages in thread
From: Gal Pressman @ 2025-06-09 12:02 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, netdev
  Cc: Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Gal Pressman, Tariq Toukan, Nimrod Oren

Context 0 (default context) always exists, there is no need to check
whether it exists or not when adding a flow steering rule.

The existing check fails when creating a flow steering rule for context
0 as it is not stored in the rss_ctx xarray.

For example:
$ ethtool --config-ntuple eth2 flow-type tcp4 dst-ip 194.237.147.23 dst-port 19983 context 0 loc 618
rmgr: Cannot insert RX class rule: Invalid argument
Cannot insert classification rule

An example usecase for this could be:
- A high-priority rule (loc 0) directing specific port traffic to
  context 0.
- A low-priority rule (loc 1) directing all other TCP traffic to context
  1.

Fixes: de7f7582dff2 ("net: ethtool: prevent flow steering to RSS contexts which don't exist")
Cc: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Nimrod Oren <noren@nvidia.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
Reviewed-by: Joe Damato <jdamato@fastly.com>
---
 net/ethtool/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 39ec920f5de7..71c828d0bf31 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1083,7 +1083,8 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
 		    ethtool_get_flow_spec_ring(info.fs.ring_cookie))
 			return -EINVAL;
 
-		if (!xa_load(&dev->ethtool->rss_ctx, info.rss_context))
+		if (info.rss_context &&
+		    !xa_load(&dev->ethtool->rss_ctx, info.rss_context))
 			return -EINVAL;
 	}
 
-- 
2.40.1


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

* [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context
  2025-06-09 12:02 [PATCH net v2 0/2] Fix ntuple rules targeting default RSS Gal Pressman
  2025-06-09 12:02 ` [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0 Gal Pressman
@ 2025-06-09 12:02 ` Gal Pressman
  2025-06-10 21:32   ` Jakub Kicinski
  1 sibling, 1 reply; 10+ messages in thread
From: Gal Pressman @ 2025-06-09 12:02 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, netdev
  Cc: Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Gal Pressman, Nimrod Oren

Add test_rss_default_context_rule() to verify that ntuple rules can
correctly direct traffic to the default RSS context (context 0).

The test creates two ntuple rules with explicit location priorities:
- A high-priority rule (loc 0) directing specific port traffic to
  context 0.
- A low-priority rule (loc 1) directing all other TCP traffic to context
  1.

This validates that:
1. Rules targeting the default context function properly.
2. Traffic steering works as expected when mixing default and
   additional RSS contexts.

The test was written by AI, and reviewed by humans.

Cc: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Nimrod Oren <noren@nvidia.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 59 ++++++++++++++++++-
 1 file changed, 58 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 ca60ae325c22..6aef8f643ca4 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -747,6 +747,62 @@ def test_rss_ntuple_addition(cfg):
                                                           'noise' : (0,) })
 
 
+def test_rss_default_context_rule(cfg):
+    """
+    Allocate a port, direct this port to context 0, then create a new RSS
+    context and steer all TCP traffic to it (context 1).  Verify that:
+      * Traffic to the specific port continues to use queues of the main
+        context (0/1).
+      * Traffic to any other TCP port is redirected to the new context
+        (queues 2/3).
+    """
+
+    require_ntuple(cfg)
+
+    queue_cnt = len(_get_rx_cnts(cfg))
+    if queue_cnt < 4:
+        try:
+            ksft_pr(f"Increasing queue count {queue_cnt} -> 4")
+            ethtool(f"-L {cfg.ifname} combined 4")
+            defer(ethtool, f"-L {cfg.ifname} combined {queue_cnt}")
+        except:
+            raise KsftSkipEx("Not enough queues for the test")
+
+    # Use queues 0 and 1 for the main context
+    ethtool(f"-X {cfg.ifname} equal 2")
+    defer(ethtool, f"-X {cfg.ifname} default")
+
+    # Create a new RSS context that uses queues 2 and 3
+    ctx_id = ethtool_create(cfg, "-X", "context new start 2 equal 2")
+    defer(ethtool, f"-X {cfg.ifname} context {ctx_id} delete")
+
+    # Generic low-priority rule: redirect all TCP traffic to the new context.
+    # Give it an explicit higher location number (lower priority).
+    flow_generic = f"flow-type tcp{cfg.addr_ipver} dst-ip {cfg.addr} context {ctx_id} loc 1"
+    ethtool(f"-N {cfg.ifname} {flow_generic}")
+    defer(ethtool, f"-N {cfg.ifname} delete 1")
+
+    # Specific high-priority rule for a random port that should stay on context 0.
+    # Assign loc 0 so it is evaluated before the generic rule.
+    port_main = rand_port()
+    flow_main = f"flow-type tcp{cfg.addr_ipver} dst-ip {cfg.addr} dst-port {port_main} context 0 loc 0"
+    ethtool(f"-N {cfg.ifname} {flow_main}")
+    defer(ethtool, f"-N {cfg.ifname} delete 0")
+
+    _ntuple_rule_check(cfg, 1, ctx_id)
+
+    # Verify that traffic matching the specific rule still goes to queues 0/1
+    _send_traffic_check(cfg, port_main, "context 0",
+                        { 'target': (0, 1),
+                          'empty' : (2, 3) })
+
+    # And that traffic for any other port is steered to the new context
+    port_other = rand_port()
+    _send_traffic_check(cfg, port_other, f"context {ctx_id}",
+                        { 'target': (2, 3),
+                          'noise' : (0, 1) })
+
+
 def main() -> None:
     with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
         cfg.context_cnt = None
@@ -760,7 +816,8 @@ def main() -> None:
                   test_rss_context_overlap, test_rss_context_overlap2,
                   test_rss_context_out_of_order, test_rss_context4_create_with_cfg,
                   test_flow_add_context_missing,
-                  test_delete_rss_context_busy, test_rss_ntuple_addition],
+                  test_delete_rss_context_busy, test_rss_ntuple_addition,
+                  test_rss_default_context_rule],
                  args=(cfg, ))
     ksft_exit()
 
-- 
2.40.1


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

* Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0
  2025-06-09 12:02 ` [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0 Gal Pressman
@ 2025-06-09 15:38   ` Subbaraya Sundeep
  2025-06-09 22:41   ` Jakub Kicinski
  1 sibling, 0 replies; 10+ messages in thread
From: Subbaraya Sundeep @ 2025-06-09 15:38 UTC (permalink / raw)
  To: Gal Pressman
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, netdev, Andrew Lunn, Simon Horman, Shuah Khan,
	Joe Damato, linux-kselftest, Tariq Toukan, Nimrod Oren

On 2025-06-09 at 12:02:49, Gal Pressman (gal@nvidia.com) wrote:
> Context 0 (default context) always exists, there is no need to check
> whether it exists or not when adding a flow steering rule.
> 
> The existing check fails when creating a flow steering rule for context
> 0 as it is not stored in the rss_ctx xarray.
> 
> For example:
> $ ethtool --config-ntuple eth2 flow-type tcp4 dst-ip 194.237.147.23 dst-port 19983 context 0 loc 618
> rmgr: Cannot insert RX class rule: Invalid argument
> Cannot insert classification rule
> 
> An example usecase for this could be:
> - A high-priority rule (loc 0) directing specific port traffic to
>   context 0.
> - A low-priority rule (loc 1) directing all other TCP traffic to context
>   1.
> 
> Fixes: de7f7582dff2 ("net: ethtool: prevent flow steering to RSS contexts which don't exist")
> Cc: Jakub Kicinski <kuba@kernel.org>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> Reviewed-by: Nimrod Oren <noren@nvidia.com>
> Signed-off-by: Gal Pressman <gal@nvidia.com>
> Reviewed-by: Joe Damato <jdamato@fastly.com>

Reviewed-by: Subbaraya Sundeep <sbhatta@marvell.com>

Thanks,
Sundeep

> ---
>  net/ethtool/ioctl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 39ec920f5de7..71c828d0bf31 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1083,7 +1083,8 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
>  		    ethtool_get_flow_spec_ring(info.fs.ring_cookie))
>  			return -EINVAL;
>  
> -		if (!xa_load(&dev->ethtool->rss_ctx, info.rss_context))
> +		if (info.rss_context &&
> +		    !xa_load(&dev->ethtool->rss_ctx, info.rss_context))
>  			return -EINVAL;
>  	}
>  
> -- 
> 2.40.1
> 

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

* Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0
  2025-06-09 12:02 ` [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0 Gal Pressman
  2025-06-09 15:38   ` Subbaraya Sundeep
@ 2025-06-09 22:41   ` Jakub Kicinski
  2025-06-10  5:58     ` Gal Pressman
  1 sibling, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2025-06-09 22:41 UTC (permalink / raw)
  To: Gal Pressman
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Andrew Lunn, netdev,
	Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Tariq Toukan, Nimrod Oren

On Mon, 9 Jun 2025 15:02:49 +0300 Gal Pressman wrote:
> Cc: Jakub Kicinski <kuba@kernel.org>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> Reviewed-by: Nimrod Oren <noren@nvidia.com>
> Signed-off-by: Gal Pressman <gal@nvidia.com>
> Reviewed-by: Joe Damato <jdamato@fastly.com>

Did you receive a user report for this change in behavior?

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

* Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0
  2025-06-09 22:41   ` Jakub Kicinski
@ 2025-06-10  5:58     ` Gal Pressman
  2025-06-10 21:33       ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Gal Pressman @ 2025-06-10  5:58 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Andrew Lunn, netdev,
	Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Tariq Toukan, Nimrod Oren

On 10/06/2025 1:41, Jakub Kicinski wrote:
> On Mon, 9 Jun 2025 15:02:49 +0300 Gal Pressman wrote:
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>> Reviewed-by: Nimrod Oren <noren@nvidia.com>
>> Signed-off-by: Gal Pressman <gal@nvidia.com>
>> Reviewed-by: Joe Damato <jdamato@fastly.com>
> 
> Did you receive a user report for this change in behavior?

No.

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

* Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context
  2025-06-09 12:02 ` [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context Gal Pressman
@ 2025-06-10 21:32   ` Jakub Kicinski
  2025-06-11  5:06     ` Gal Pressman
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2025-06-10 21:32 UTC (permalink / raw)
  To: Gal Pressman
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Andrew Lunn, netdev,
	Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Nimrod Oren

On Mon, 9 Jun 2025 15:02:50 +0300 Gal Pressman wrote:
> The test was written by AI, and reviewed by humans.

Please use pylint and avoid adding new warnings:

tools/testing/selftests/drivers/net/hw/rss_ctx.py:788:0: C0301: Line too long (103/100) (line-too-long)
tools/testing/selftests/drivers/net/hw/rss_ctx.py:769:12: W0707: Consider explicitly re-raising using 'except Exception as exc' and 'raise KsftSkipEx('Not enough queues for the test') from exc' (raise-missing-from)
-- 
pw-bot: cr

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

* Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0
  2025-06-10  5:58     ` Gal Pressman
@ 2025-06-10 21:33       ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2025-06-10 21:33 UTC (permalink / raw)
  To: Gal Pressman
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Andrew Lunn, netdev,
	Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Tariq Toukan, Nimrod Oren

On Tue, 10 Jun 2025 08:58:52 +0300 Gal Pressman wrote:
> On 10/06/2025 1:41, Jakub Kicinski wrote:
> > On Mon, 9 Jun 2025 15:02:49 +0300 Gal Pressman wrote:  
> >> Cc: Jakub Kicinski <kuba@kernel.org>
> >> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> >> Reviewed-by: Nimrod Oren <noren@nvidia.com>
> >> Signed-off-by: Gal Pressman <gal@nvidia.com>
> >> Reviewed-by: Joe Damato <jdamato@fastly.com>  
> > 
> > Did you receive a user report for this change in behavior?  
> 
> No.

Please make this clear in the commit message.

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

* Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context
  2025-06-10 21:32   ` Jakub Kicinski
@ 2025-06-11  5:06     ` Gal Pressman
  2025-06-11 15:52       ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Gal Pressman @ 2025-06-11  5:06 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Andrew Lunn, netdev,
	Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Nimrod Oren

On 11/06/2025 0:32, Jakub Kicinski wrote:
> On Mon, 9 Jun 2025 15:02:50 +0300 Gal Pressman wrote:
>> The test was written by AI, and reviewed by humans.
> 
> Please use pylint and avoid adding new warnings:

Ack.

> 
> tools/testing/selftests/drivers/net/hw/rss_ctx.py:788:0: C0301: Line too long (103/100) (line-too-long)

Are you sure you want to blindly break this line?
It's a single string.

> tools/testing/selftests/drivers/net/hw/rss_ctx.py:769:12: W0707: Consider explicitly re-raising using 'except Exception as exc' and 'raise KsftSkipEx('Not enough queues for the test') from exc' (raise-missing-from)

Cool, didn't know python can do that.

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

* Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context
  2025-06-11  5:06     ` Gal Pressman
@ 2025-06-11 15:52       ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2025-06-11 15:52 UTC (permalink / raw)
  To: Gal Pressman
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Andrew Lunn, netdev,
	Andrew Lunn, Simon Horman, Shuah Khan, Joe Damato,
	linux-kselftest, Nimrod Oren

On Wed, 11 Jun 2025 08:06:38 +0300 Gal Pressman wrote:
> > tools/testing/selftests/drivers/net/hw/rss_ctx.py:788:0: C0301: Line too long (103/100) (line-too-long)  
> 
> Are you sure you want to blindly break this line?
> It's a single string.

Yeah, if it's a string leave it be. This one is just a check, the other
one as a warning. Unhelpfully I don't see an option in pylint to add the
bad lines to the report so its a bit hard to tell at a glance. The long
strings and the lack of doc string in the main function are the two
checks that we are definitely okay to ignore.

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

end of thread, other threads:[~2025-06-11 15:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-09 12:02 [PATCH net v2 0/2] Fix ntuple rules targeting default RSS Gal Pressman
2025-06-09 12:02 ` [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0 Gal Pressman
2025-06-09 15:38   ` Subbaraya Sundeep
2025-06-09 22:41   ` Jakub Kicinski
2025-06-10  5:58     ` Gal Pressman
2025-06-10 21:33       ` Jakub Kicinski
2025-06-09 12:02 ` [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context Gal Pressman
2025-06-10 21:32   ` Jakub Kicinski
2025-06-11  5:06     ` Gal Pressman
2025-06-11 15:52       ` Jakub Kicinski

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