[PATCH net v2 0/2] bnxt: Fix failure to report RSS context in ntuple rule

[PATCH net v2 0/2] bnxt: Fix failure to report RSS context in ntuple rule

[Daniel Xu]

This patchset fixes a bug where bnxt driver was failing to report that
an ntuple rule is redirecting to an RSS context. First commit is the
fix, then second commit extends selftests to detect if other/new drivers
are compliant with ntuple/rss_ctx API.

Daniel Xu (2):
  bnxt_en: ethtool: Supply ntuple rss context action
  selftests: drv-net: rss_ctx: Add test for ntuple rule

 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  8 ++++++--
 tools/testing/selftests/drivers/net/hw/rss_ctx.py | 13 ++++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.46.0

[PATCH net v2 1/2] bnxt_en: ethtool: Supply ntuple rss context action

[Daniel Xu]

Commit 2f4f9fe5bf5f ("bnxt_en: Support adding ntuple rules on RSS
contexts") added support for redirecting to an RSS context as an ntuple
rule action. However, it forgot to update the ETHTOOL_GRXCLSRULE
codepath. This caused `ethtool -n` to always report the action as
"Action: Direct to queue 0" which is wrong.

Fix by teaching bnxt driver to report the RSS context when applicable.

Fixes: 2f4f9fe5bf5f ("bnxt_en: Support adding ntuple rules on RSS contexts")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index cfd2c65b1c90..a218802befa8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1187,10 +1187,14 @@ static int bnxt_grxclsrule(struct bnxt *bp, struct ethtool_rxnfc *cmd)
 		}
 	}
 
-	if (fltr->base.flags & BNXT_ACT_DROP)
+	if (fltr->base.flags & BNXT_ACT_DROP) {
 		fs->ring_cookie = RX_CLS_FLOW_DISC;
-	else
+	} else if (fltr->base.flags & BNXT_ACT_RSS_CTX) {
+		fs->flow_type |= FLOW_RSS;
+		cmd->rss_context = fltr->base.fw_vnic_id;
+	} else {
 		fs->ring_cookie = fltr->base.rxq;
+	}
 	rc = 0;
 
 fltr_err:
-- 
2.46.0

[PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rule

[Daniel Xu]

Extend the rss_ctx test suite to test that an ntuple action that
redirects to an RSS context contains that information in `ethtool -n`.
Otherwise the output from ethtool is highly deceiving. This test helps
ensure drivers are compliant with the API.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 tools/testing/selftests/drivers/net/hw/rss_ctx.py | 13 ++++++++++++-
 1 file changed, 12 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 29995586993c..4fa158f37155 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -3,7 +3,8 @@
 
 import datetime
 import random
-from lib.py import ksft_run, ksft_pr, ksft_exit, ksft_eq, ksft_ne, ksft_ge, ksft_lt
+import re
+from lib.py import ksft_run, ksft_pr, ksft_exit, ksft_eq, ksft_ne, ksft_ge, ksft_lt, ksft_true
 from lib.py import NetDrvEpEnv
 from lib.py import EthtoolFamily, NetdevFamily
 from lib.py import KsftSkipEx, KsftFailEx
@@ -96,6 +97,14 @@ def _send_traffic_check(cfg, port, name, params):
                 f"traffic on inactive queues ({name}): " + str(cnts))
 
 
+def _ntuple_rule_check(cfg, rule_id, ctx_id):
+    """Check that ntuple rule references RSS context ID"""
+    text = ethtool(f"-n {cfg.ifname} rule {rule_id}").stdout
+    pattern = f"RSS Context ID: {ctx_id}"
+    ksft_true(re.search(pattern, text, re.IGNORECASE),
+              "RSS context not referenced in ntuple rule")
+
+
 def test_rss_key_indir(cfg):
     """Test basics like updating the main RSS key and indirection table."""
 
@@ -433,6 +442,8 @@ def test_rss_context(cfg, ctx_cnt=1, create_with_cfg=None):
         ntuple = ethtool_create(cfg, "-N", flow)
         defer(ethtool, f"-N {cfg.ifname} delete {ntuple}")
 
+        _ntuple_rule_check(cfg, ntuple, ctx_id)
+
     for i in range(ctx_cnt):
         _send_traffic_check(cfg, ports[i], f"context {i}",
                             { 'target': (2+i*2, 3+i*2),
-- 
2.46.0

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rule

[Edward Cree]

On 12/11/2024 02:23, Daniel Xu wrote:
> Extend the rss_ctx test suite to test that an ntuple action that
> redirects to an RSS context contains that information in `ethtool -n`.
> Otherwise the output from ethtool is highly deceiving. This test helps
> ensure drivers are compliant with the API.
> 
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
...
> +def _ntuple_rule_check(cfg, rule_id, ctx_id):
> +    """Check that ntuple rule references RSS context ID"""
> +    text = ethtool(f"-n {cfg.ifname} rule {rule_id}").stdout
> +    pattern = f"RSS Context ID: {ctx_id}"
> +    ksft_true(re.search(pattern, text, re.IGNORECASE),

This won't match the output from your ethtool patch, because that
 removes the colon.  Probably want to wait until the patch is
 finalised and then write a regex that matches both versions.

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rule

[Jakub Kicinski]

On Mon, 11 Nov 2024 19:23:31 -0700 Daniel Xu wrote:
> Extend the rss_ctx test suite to test that an ntuple action that
> redirects to an RSS context contains that information in `ethtool -n`.
> Otherwise the output from ethtool is highly deceiving. This test helps
> ensure drivers are compliant with the API.

Looks like it doesn't apply, please base the v3 on net rather than
net-next, I'll deal with the merge.

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rule

[Daniel Xu]

Hi Ed,

On Tue, Nov 12, 2024, at 3:10 AM, Edward Cree wrote:
> On 12/11/2024 02:23, Daniel Xu wrote:
>> Extend the rss_ctx test suite to test that an ntuple action that
>> redirects to an RSS context contains that information in `ethtool -n`.
>> Otherwise the output from ethtool is highly deceiving. This test helps
>> ensure drivers are compliant with the API.
>> 
>> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ...
>> +def _ntuple_rule_check(cfg, rule_id, ctx_id):
>> +    """Check that ntuple rule references RSS context ID"""
>> +    text = ethtool(f"-n {cfg.ifname} rule {rule_id}").stdout
>> +    pattern = f"RSS Context ID: {ctx_id}"
>> +    ksft_true(re.search(pattern, text, re.IGNORECASE),
>
> This won't match the output from your ethtool patch, because that
>  removes the colon.  Probably want to wait until the patch is
>  finalised and then write a regex that matches both versions.

Argh, I meant to have wildcards here. But yeah, makes sense, will wait
until the other patch is finalized.

Thanks,
Daniel 

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rule

[Daniel Xu]

Hi Jakub,

On Tue, Nov 12, 2024, at 5:44 PM, Jakub Kicinski wrote:
> On Mon, 11 Nov 2024 19:23:31 -0700 Daniel Xu wrote:
>> Extend the rss_ctx test suite to test that an ntuple action that
>> redirects to an RSS context contains that information in `ethtool -n`.
>> Otherwise the output from ethtool is highly deceiving. This test helps
>> ensure drivers are compliant with the API.
>
> Looks like it doesn't apply, please base the v3 on net rather than
> net-next, I'll deal with the merge.

Sorry about that. Will think on how to make my workflow not involve
manually retyping patches between different machines...

Re: [PATCH net v2 2/2] selftests: drv-net: rss_ctx: Add test for ntuple rule

[Jakub Kicinski]

On Tue, 12 Nov 2024 17:51:50 -0800 Daniel Xu wrote:
> > This won't match the output from your ethtool patch, because that
> >  removes the colon.  Probably want to wait until the patch is
> >  finalised and then write a regex that matches both versions.  
> 
> Argh, I meant to have wildcards here. But yeah, makes sense, will wait
> until the other patch is finalized.

Keep in mind Michal scans ethtool patches once every week or two,
so there may be a bit of a delay there.