[PATCH net v2 0/2] Fix ntuple rules targeting default RSS

[PATCH net v2 0/2] Fix ntuple rules targeting default RSS

[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

[PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0

[Gal Pressman]

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

Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0

[Subbaraya Sundeep]

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
> 

Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0

[Jakub Kicinski]

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?

Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0

[Gal Pressman]

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.

Re: [PATCH net v2 1/2] net: ethtool: Don't check if RSS context exists in case of context 0

[Jakub Kicinski]

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.

[PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context

[Gal Pressman]

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

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context

[Jakub Kicinski]

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

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context

[Gal Pressman]

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.

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rules targeting default RSS context

[Jakub Kicinski]

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.