[PATCH net v3 0/4] bnxt_en: Fix RSS context and ntuple filter issues

[PATCH net v3 0/4] bnxt_en: Fix RSS context and ntuple filter issues

[Michael Chan]

The first patch fixes the problem of ifup failing if one or more RSS
contexts were previously created.  The 2nd patch is a cleanup patch
required by the 3rd patch which fixes ntuple filter deletion errors
in ifdown state.  The 4th patch adds self tests to cover these failure
cases.

v3: More improvements to the selftest in patch #4

v2: Many improvements to the selftest in patch #4
https://lore.kernel.org/netdev/20260206052930.1882740-1-michael.chan@broadcom.com/

v1: https://lore.kernel.org/netdev/20260129061646.1417185-1-michael.chan@broadcom.com/

Pavan Chebbi (4):
  bnxt_en: Fix RSS context delete logic
  bnxt_en: Don't overload fw_vnic_id for RSS context's filters
  bnxt_en: Fix deleting of Ntuple filters
  selftests: drv-net: rss_ctx: test RSS contexts persist after ifdown/up

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  28 +++--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   3 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  12 +--
 .../selftests/drivers/net/hw/rss_ctx.py       | 100 +++++++++++++++++-
 4 files changed, 123 insertions(+), 20 deletions(-)

-- 
2.51.0

[PATCH net v3 1/4] bnxt_en: Fix RSS context delete logic

[Michael Chan]

From: Pavan Chebbi <pavan.chebbi@broadcom.com>

We need to free the corresponding RSS context VNIC
in FW everytime an RSS context is deleted in driver.
Commit 667ac333dbb7 added a check to delete the VNIC
in FW only when netif_running() is true to help delete
RSS contexts with interface down.

Having that condition will make the driver leak VNICs
in FW whenever close() happens with active RSS contexts.
On the subsequent open(), as part of RSS context restoration,
we will end up trying to create extra VNICs for which we
did not make any reservation. FW can fail this request,
thereby making us lose active RSS contexts.

Suppose an RSS context is deleted already and we try to
process a delete request again, then the HWRM functions
will check for validity of the request and they simply
return if the resource is already freed. So, even for
delete-when-down cases, netif_running() check is not
necessary.

Remove the netif_running() condition check when deleting
an RSS context.

Reported-by: Jakub Kicinski <kicinski@meta.com>
Fixes: 667ac333dbb7 ("eth: bnxt: allow deleting RSS contexts when the device is down")
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index fb45e1dd1dd7..7768d9753e2d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10889,12 +10889,10 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
 	struct bnxt_ntuple_filter *ntp_fltr;
 	int i;
 
-	if (netif_running(bp->dev)) {
-		bnxt_hwrm_vnic_free_one(bp, &rss_ctx->vnic);
-		for (i = 0; i < BNXT_MAX_CTX_PER_VNIC; i++) {
-			if (vnic->fw_rss_cos_lb_ctx[i] != INVALID_HW_RING_ID)
-				bnxt_hwrm_vnic_ctx_free_one(bp, vnic, i);
-		}
+	bnxt_hwrm_vnic_free_one(bp, &rss_ctx->vnic);
+	for (i = 0; i < BNXT_MAX_CTX_PER_VNIC; i++) {
+		if (vnic->fw_rss_cos_lb_ctx[i] != INVALID_HW_RING_ID)
+			bnxt_hwrm_vnic_ctx_free_one(bp, vnic, i);
 	}
 	if (!all)
 		return;
-- 
2.51.0

[PATCH net v3 2/4] bnxt_en: Don't overload fw_vnic_id for RSS context's filters

[Michael Chan]

From: Pavan Chebbi <pavan.chebbi@broadcom.com>

Currently the ntuple filters installed for an RSS context
use the ntuple filter's base member 'fw_vnic_id' to save the
RSS context id. This is making the fw_vnic_id overloaded when it
is being used with RSS context. This is very confusing.

Make explicit provision to save rss_ctx_id in the ntuple filter
so that we can avoid overloading fw_vnic_id.
Also remove unnecessary initialization of the id to 0.

This cleanup is necessary for the next patch that will remove
the fw_vnic_id field.

Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 4 ++--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         | 1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 5 ++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7768d9753e2d..8fc15a8353fc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6287,7 +6287,7 @@ bnxt_cfg_rfs_ring_tbl_idx(struct bnxt *bp,
 		struct bnxt_vnic_info *vnic;
 
 		ctx = xa_load(&bp->dev->ethtool->rss_ctx,
-			      fltr->base.fw_vnic_id);
+			      fltr->base.rss_ctx_id);
 		if (ctx) {
 			rss_ctx = ethtool_rxfh_context_priv(ctx);
 			vnic = &rss_ctx->vnic;
@@ -10899,7 +10899,7 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
 
 	list_for_each_entry_safe(usr_fltr, tmp, &bp->usr_fltr_list, list) {
 		if ((usr_fltr->flags & BNXT_ACT_RSS_CTX) &&
-		    usr_fltr->fw_vnic_id == rss_ctx->index) {
+		    usr_fltr->rss_ctx_id == rss_ctx->index) {
 			ntp_fltr = container_of(usr_fltr,
 						struct bnxt_ntuple_filter,
 						base);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 9a41b9e0423c..6a18a1973fd5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1435,6 +1435,7 @@ struct bnxt_filter_base {
 	u16			sw_id;
 	u16			rxq;
 	u16			fw_vnic_id;
+	u16			rss_ctx_id;
 	u16			vf_idx;
 	unsigned long		state;
 #define BNXT_FLTR_VALID		0
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 53a83b6680c4..d0363f410ddb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1219,7 +1219,7 @@ static int bnxt_grxclsrule(struct bnxt *bp, struct ethtool_rxnfc *cmd)
 		fs->ring_cookie = RX_CLS_FLOW_DISC;
 	} else if (fltr->base.flags & BNXT_ACT_RSS_CTX) {
 		fs->flow_type |= FLOW_RSS;
-		cmd->rss_context = fltr->base.fw_vnic_id;
+		cmd->rss_context = fltr->base.rss_ctx_id;
 	} else {
 		fs->ring_cookie = fltr->base.rxq;
 	}
@@ -1469,11 +1469,10 @@ static int bnxt_add_ntuple_cls_rule(struct bnxt *bp,
 	if (fs->flow_type & FLOW_RSS) {
 		struct bnxt_rss_ctx *rss_ctx;
 
-		new_fltr->base.fw_vnic_id = 0;
 		new_fltr->base.flags |= BNXT_ACT_RSS_CTX;
 		rss_ctx = bnxt_get_rss_ctx_from_index(bp, cmd->rss_context);
 		if (rss_ctx) {
-			new_fltr->base.fw_vnic_id = rss_ctx->index;
+			new_fltr->base.rss_ctx_id = rss_ctx->index;
 		} else {
 			rc = -EINVAL;
 			goto ntuple_err;
-- 
2.51.0

[PATCH net v3 3/4] bnxt_en: Fix deleting of Ntuple filters

[Michael Chan]

From: Pavan Chebbi <pavan.chebbi@broadcom.com>

Ntuple filters can be deleted when the interface
is down. The current code blindly sends the filter
delete command to FW. When the interface is down, all
the VNICs are deleted in the FW. When the VNIC is
freed in the FW, all the associated filters are also
freed. We need not send the free command explicitly.
Sending such command will generate FW error in the
dmesg.

In order to fix this, save a pointer to the corresponding
VNIC of an Ntuple filter in the ntuple filter's base
structure. Use this pointer to check if the VNIC is
already freed whenever we are freeing the Ntuple filters.
Now that we have access to the vnic itself, remove the
redundant reference to fw_vnic_id from all places.

Fixes: 8336a974f37d ("bnxt_en: Save user configured filters in a lookup list")
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 14 ++++++++++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  7 +++----
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8fc15a8353fc..1f05ecf367fe 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6184,6 +6184,7 @@ int bnxt_hwrm_l2_filter_alloc(struct bnxt *bp, struct bnxt_l2_filter *fltr)
 {
 	struct hwrm_cfa_l2_filter_alloc_output *resp;
 	struct hwrm_cfa_l2_filter_alloc_input *req;
+	u16 dst_id = INVALID_HW_RING_ID;
 	u16 target_id = 0xffff;
 	int rc;
 
@@ -6205,7 +6206,10 @@ int bnxt_hwrm_l2_filter_alloc(struct bnxt *bp, struct bnxt_l2_filter *fltr)
 	if (!BNXT_CHIP_TYPE_NITRO_A0(bp))
 		req->flags |=
 			cpu_to_le32(CFA_L2_FILTER_ALLOC_REQ_FLAGS_OUTERMOST);
-	req->dst_id = cpu_to_le16(fltr->base.fw_vnic_id);
+
+	if (fltr->base.vnic)
+		dst_id = fltr->base.vnic->fw_vnic_id;
+	req->dst_id = cpu_to_le16(dst_id);
 	req->enables =
 		cpu_to_le32(CFA_L2_FILTER_ALLOC_REQ_ENABLES_L2_ADDR |
 			    CFA_L2_FILTER_ALLOC_REQ_ENABLES_DST_ID |
@@ -6240,6 +6244,9 @@ int bnxt_hwrm_cfa_ntuple_filter_free(struct bnxt *bp,
 	int rc;
 
 	set_bit(BNXT_FLTR_FW_DELETED, &fltr->base.state);
+	if (fltr->base.vnic->fw_vnic_id == INVALID_HW_RING_ID)
+		return 0;
+
 	rc = hwrm_req_init(bp, req, HWRM_CFA_NTUPLE_FILTER_FREE);
 	if (rc)
 		return rc;
@@ -6291,6 +6298,7 @@ bnxt_cfg_rfs_ring_tbl_idx(struct bnxt *bp,
 		if (ctx) {
 			rss_ctx = ethtool_rxfh_context_priv(ctx);
 			vnic = &rss_ctx->vnic;
+			fltr->base.vnic = vnic;
 
 			req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
 		}
@@ -6301,6 +6309,7 @@ bnxt_cfg_rfs_ring_tbl_idx(struct bnxt *bp,
 		u32 enables;
 
 		vnic = &bp->vnic_info[BNXT_VNIC_NTUPLE];
+		fltr->base.vnic = vnic;
 		req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
 		enables = CFA_NTUPLE_FILTER_ALLOC_REQ_ENABLES_RFS_RING_TBL_IDX;
 		req->enables |= cpu_to_le32(enables);
@@ -6339,6 +6348,7 @@ int bnxt_hwrm_cfa_ntuple_filter_alloc(struct bnxt *bp,
 		bnxt_cfg_rfs_ring_tbl_idx(bp, req, fltr);
 	} else {
 		vnic = &bp->vnic_info[fltr->base.rxq + 1];
+		fltr->base.vnic = vnic;
 		req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
 	}
 	req->enables |= cpu_to_le32(BNXT_NTP_FLTR_FLAGS);
@@ -6393,7 +6403,7 @@ static int bnxt_hwrm_set_vnic_filter(struct bnxt *bp, u16 vnic_id, u16 idx,
 	if (IS_ERR(fltr))
 		return PTR_ERR(fltr);
 
-	fltr->base.fw_vnic_id = bp->vnic_info[vnic_id].fw_vnic_id;
+	fltr->base.vnic = &bp->vnic_info[vnic_id];
 	rc = bnxt_hwrm_l2_filter_alloc(bp, fltr);
 	if (rc)
 		bnxt_del_l2_filter(bp, fltr);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 6a18a1973fd5..ae4d01445cea 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1434,9 +1434,9 @@ struct bnxt_filter_base {
 #define BNXT_ACT_RSS_CTX	0x10
 	u16			sw_id;
 	u16			rxq;
-	u16			fw_vnic_id;
 	u16			rss_ctx_id;
 	u16			vf_idx;
+	struct bnxt_vnic_info	*vnic;
 	unsigned long		state;
 #define BNXT_FLTR_VALID		0
 #define BNXT_FLTR_INSERTED	1
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index d0363f410ddb..861965ca2e1e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1267,9 +1267,9 @@ static int bnxt_add_l2_cls_rule(struct bnxt *bp,
 	u8 vf = ethtool_get_flow_spec_ring_vf(fs->ring_cookie);
 	struct ethhdr *h_ether = &fs->h_u.ether_spec;
 	struct ethhdr *m_ether = &fs->m_u.ether_spec;
+	struct bnxt_vnic_info *vnic = NULL;
 	struct bnxt_l2_filter *fltr;
 	struct bnxt_l2_key key;
-	u16 vnic_id;
 	u8 flags;
 	int rc;
 
@@ -1291,17 +1291,16 @@ static int bnxt_add_l2_cls_rule(struct bnxt *bp,
 
 	if (vf) {
 		flags = BNXT_ACT_FUNC_DST;
-		vnic_id = 0xffff;
 		vf--;
 	} else {
 		flags = BNXT_ACT_RING_DST;
-		vnic_id = bp->vnic_info[ring + 1].fw_vnic_id;
+		vnic = &bp->vnic_info[ring + 1];
 	}
 	fltr = bnxt_alloc_new_l2_filter(bp, &key, flags);
 	if (IS_ERR(fltr))
 		return PTR_ERR(fltr);
 
-	fltr->base.fw_vnic_id = vnic_id;
+	fltr->base.vnic = vnic;
 	fltr->base.rxq = ring;
 	fltr->base.vf_idx = vf;
 	rc = bnxt_hwrm_l2_filter_alloc(bp, fltr);
-- 
2.51.0

[PATCH net v3 4/4] selftests: drv-net: rss_ctx: test RSS contexts persist after ifdown/up

[Michael Chan]

From: Pavan Chebbi <pavan.chebbi@broadcom.com>

Add a test to verify that RSS contexts persist across interface
down/up along with their associated Ntuple filters. Another test
that creates contexts/rules keeping interface down and test their
persistence is also added.

Tested on bnxt_en:

 TAP version 13
 1..1
 # timeout set to 0
 # selftests: drivers/net/hw: rss_ctx.py
 # TAP version 13
 # 1..2
 # ok 1 rss_ctx.test_rss_context_persist_create_and_ifdown
 # ok 2 rss_ctx.test_rss_context_persist_ifdown_and_create # SKIP Create context not supported with interface down
 # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:1 error:0

Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v3: More improvements suggested by Jakub:
1. Used defer() and exec() for the if up
2. Wait for carrier before trying the ping to start traffic tests
3. Replace manual subprocess.run() ping loop with cmd()
4. Fixed all ruff and pylint warnings in the newly added code

v2: Many improvements suggested by Jakub:
https://lore.kernel.org/netdev/20260206052930.1882740-5-michael.chan@broadcom.com/
1. simplified the tests to have only 2 contexts, use one of the contexts
to test ntuple filters' persistence also 
2. used @ksft_disruptive for the test
3. used netlink API to get rss contexts

v1: https://lore.kernel.org/netdev/20260129061646.1417185-5-michael.chan@broadcom.com/
---
 .../selftests/drivers/net/hw/rss_ctx.py       | 100 +++++++++++++++++-
 1 file changed, 98 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index ed7e405682f0..b9b7527c2c6b 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -4,13 +4,15 @@
 import datetime
 import random
 import re
+import time
 from lib.py import ksft_run, ksft_pr, ksft_exit
 from lib.py import ksft_eq, ksft_ne, ksft_ge, ksft_in, ksft_lt, ksft_true, ksft_raises
 from lib.py import NetDrvEpEnv
 from lib.py import EthtoolFamily, NetdevFamily
 from lib.py import KsftSkipEx, KsftFailEx
+from lib.py import ksft_disruptive
 from lib.py import rand_port
-from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
+from lib.py import cmd, ethtool, ip, defer, GenerateTraffic, CmdExitFailure, wait_file
 
 
 def _rss_key_str(key):
@@ -809,6 +811,98 @@ def test_rss_default_context_rule(cfg):
                           'noise' : (0, 1) })
 
 
+@ksft_disruptive
+def test_rss_context_persist_ifupdown(cfg, pre_down=False):
+    """
+    Test that RSS contexts and their associated ntuple filters persist across
+    an interface down/up cycle.
+
+    """
+
+    require_ntuple(cfg)
+
+    qcnt = len(_get_rx_cnts(cfg))
+    if qcnt < 6:
+        try:
+            ethtool(f"-L {cfg.ifname} combined 6")
+            defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
+        except Exception as exc:
+            raise KsftSkipEx("Not enough queues for the test") from exc
+
+    ethtool(f"-X {cfg.ifname} equal 2")
+    defer(ethtool, f"-X {cfg.ifname} default")
+
+    ifup = defer(ip, f"link set dev {cfg.ifname} up")
+    if pre_down:
+        ip(f"link set dev {cfg.ifname} down")
+
+    try:
+        ctx1_id = ethtool_create(cfg, "-X", "context new start 2 equal 2")
+        defer(ethtool, f"-X {cfg.ifname} context {ctx1_id} delete")
+    except CmdExitFailure as exc:
+        raise KsftSkipEx("Create context not supported with interface down") from exc
+
+    ctx2_id = ethtool_create(cfg, "-X", "context new start 4 equal 2")
+    defer(ethtool, f"-X {cfg.ifname} context {ctx2_id} delete")
+
+    port_ctx2 = rand_port()
+    flow = f"flow-type tcp{cfg.addr_ipver} dst-ip {cfg.addr} dst-port {port_ctx2} context {ctx2_id}"
+    ntuple_id = ethtool_create(cfg, "-N", flow)
+    defer(ethtool, f"-N {cfg.ifname} delete {ntuple_id}")
+
+    if not pre_down:
+        ip(f"link set dev {cfg.ifname} down")
+    ifup.exec()
+
+    wait_file(f"/sys/class/net/{cfg.ifname}/carrier",
+        lambda x: x.strip() == "1", deadline=20)
+
+    remote_addr = cfg.remote_addr_v[cfg.addr_ipver]
+    for _ in range(10):
+        if cmd(f"ping -c 1 -W 1 {remote_addr}", fail=False).ret == 0:
+            break
+        time.sleep(1)
+    else:
+        raise KsftSkipEx("Cannot reach remote host after interface up")
+
+    ctxs = cfg.ethnl.rss_get({'header': {'dev-name': cfg.ifname}}, dump=True)
+
+    data1 = [c for c in ctxs if c.get('context') == ctx1_id]
+    ksft_eq(len(data1), 1, f"Context {ctx1_id} should persist after ifup")
+
+    data2 = [c for c in ctxs if c.get('context') == ctx2_id]
+    ksft_eq(len(data2), 1, f"Context {ctx2_id} should persist after ifup")
+
+    _ntuple_rule_check(cfg, ntuple_id, ctx2_id)
+
+    cnts = _get_rx_cnts(cfg)
+    GenerateTraffic(cfg).wait_pkts_and_stop(20000)
+    cnts = _get_rx_cnts(cfg, prev=cnts)
+
+    main_traffic = sum(cnts[0:2])
+    ksft_ge(main_traffic, 18000, f"Main context traffic distribution: {cnts}")
+    ksft_lt(sum(cnts[2:6]), 500, f"Other context queues should be mostly empty: {cnts}")
+
+    _send_traffic_check(cfg, port_ctx2, f"context {ctx2_id}",
+                        {'target': (4, 5),
+                         'noise': (0, 1),
+                         'empty': (2, 3)})
+
+
+def test_rss_context_persist_create_and_ifdown(cfg):
+    """
+    Create RSS contexts then cycle the interface down and up.
+    """
+    test_rss_context_persist_ifupdown(cfg, pre_down=False)
+
+
+def test_rss_context_persist_ifdown_and_create(cfg):
+    """
+    Bring interface down first, then create RSS contexts and bring up.
+    """
+    test_rss_context_persist_ifupdown(cfg, pre_down=True)
+
+
 def main() -> None:
     with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
         cfg.context_cnt = None
@@ -823,7 +917,9 @@ def main() -> None:
                   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_rss_default_context_rule],
+                  test_rss_default_context_rule,
+                  test_rss_context_persist_create_and_ifdown,
+                  test_rss_context_persist_ifdown_and_create],
                  args=(cfg, ))
     ksft_exit()
 
-- 
2.51.0

Re: [net,v3,3/4] bnxt_en: Fix deleting of Ntuple filters

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
bnxt_en: Fix deleting of Ntuple filters

This commit fixes an issue where ntuple filters deleted when the
interface is down would cause FW errors. It saves a vnic pointer in the
filter's base structure to check if the VNIC is already freed.

Fixes: 8336a974f37d ("bnxt_en: Save user configured filters in a lookup list")

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 8fc15a8353fc..1f05ecf367fe 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

[ ... ]

> @@ -6240,6 +6244,9 @@ int bnxt_hwrm_cfa_ntuple_filter_free(struct bnxt *bp,
>  	int rc;
>
>  	set_bit(BNXT_FLTR_FW_DELETED, &fltr->base.state);
> +	if (fltr->base.vnic->fw_vnic_id == INVALID_HW_RING_ID)
> +		return 0;
> +

Does this code dereference fltr->base.vnic without checking if it's NULL?

For ntuple drop filters created with ring_cookie == RX_CLS_FLOW_DISC, the
code sets the BNXT_ACT_DROP flag. Looking at bnxt_hwrm_cfa_ntuple_filter_alloc:

int bnxt_hwrm_cfa_ntuple_filter_alloc(struct bnxt *bp,
                                      struct bnxt_ntuple_filter *fltr)
{
	...
	if (fltr->base.flags & BNXT_ACT_DROP) {
		req->flags =
			cpu_to_le32(CFA_NTUPLE_FILTER_ALLOC_REQ_FLAGS_DROP);
	} else if (bp->fw_cap & BNXT_FW_CAP_CFA_RFS_RING_TBL_IDX_V2) {
		bnxt_cfg_rfs_ring_tbl_idx(bp, req, fltr);
	} else {
		vnic = &bp->vnic_info[fltr->base.rxq + 1];
		fltr->base.vnic = vnic;
		req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
	}

The BNXT_ACT_DROP case sets the drop flag but doesn't initialize
fltr->base.vnic. When that filter is later freed, this code dereferences
the NULL vnic pointer.

[ ... ]

> @@ -6339,6 +6348,7 @@ int bnxt_hwrm_cfa_ntuple_filter_alloc(struct bnxt *bp,
>  		bnxt_cfg_rfs_ring_tbl_idx(bp, req, fltr);
>  	} else {
>  		vnic = &bp->vnic_info[fltr->base.rxq + 1];
> +		fltr->base.vnic = vnic;
>  		req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
>  	}

Looking at the if-else structure here:

	if (fltr->base.flags & BNXT_ACT_DROP) {
		req->flags =
			cpu_to_le32(CFA_NTUPLE_FILTER_ALLOC_REQ_FLAGS_DROP);
	} else if (bp->fw_cap & BNXT_FW_CAP_CFA_RFS_RING_TBL_IDX_V2) {
		bnxt_cfg_rfs_ring_tbl_idx(bp, req, fltr);
	} else {
		vnic = &bp->vnic_info[fltr->base.rxq + 1];
		fltr->base.vnic = vnic;
		req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
	}

Should the BNXT_ACT_DROP case also set fltr->base.vnic?

Drop filters don't have a destination vnic, so the code doesn't set one.
But the free path in bnxt_hwrm_cfa_ntuple_filter_free now dereferences
fltr->base.vnic without a NULL check, which would crash when freeing drop
filters.

Either the drop case should explicitly set fltr->base.vnic = NULL and the
free path should check for NULL, or drop filters should be assigned a dummy
vnic. Currently the code leaves vnic uninitialized and then dereferences it.

[ ... ]

Re: [net,v3,3/4] bnxt_en: Fix deleting of Ntuple filters

[Michael Chan]

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

On Fri, Feb 13, 2026 at 8:06 AM Simon Horman <horms@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> bnxt_en: Fix deleting of Ntuple filters
>
> This commit fixes an issue where ntuple filters deleted when the
> interface is down would cause FW errors. It saves a vnic pointer in the
> filter's base structure to check if the VNIC is already freed.
>
> Fixes: 8336a974f37d ("bnxt_en: Save user configured filters in a lookup list")
>
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index 8fc15a8353fc..1f05ecf367fe 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>
> [ ... ]
>
> > @@ -6240,6 +6244,9 @@ int bnxt_hwrm_cfa_ntuple_filter_free(struct bnxt *bp,
> >       int rc;
> >
> >       set_bit(BNXT_FLTR_FW_DELETED, &fltr->base.state);
> > +     if (fltr->base.vnic->fw_vnic_id == INVALID_HW_RING_ID)
> > +             return 0;
> > +
>
> Does this code dereference fltr->base.vnic without checking if it's NULL?

Yes, it's a valid issue.  We'll respin.  Thanks.

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