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

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

v2: Many improvements to the selftest in patch #4

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       | 102 +++++++++++++++++-
 4 files changed, 126 insertions(+), 19 deletions(-)

-- 
2.51.0

[PATCH net v2 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 8419d1eb4035..64832289e18d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10827,12 +10827,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 v2 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 64832289e18d..6fae0490bc1c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6259,7 +6259,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;
@@ -10837,7 +10837,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 f88e7769a838..e2d52841fa86 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1425,6 +1425,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 068e191ede19..8fd0e06ef2a7 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;
 	}
@@ -1468,11 +1468,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 v2 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 6fae0490bc1c..6ec173a25332 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6156,6 +6156,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;
 
@@ -6177,7 +6178,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 |
@@ -6212,6 +6216,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;
@@ -6263,6 +6270,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);
 		}
@@ -6273,6 +6281,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);
@@ -6311,6 +6320,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);
@@ -6365,7 +6375,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 e2d52841fa86..fd7201283364 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1424,9 +1424,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 8fd0e06ef2a7..f8aacd2822f9 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 v2 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 fitlers. 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>
---
v2: Many improvements suggested by Jakub:
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       | 102 +++++++++++++++++-
 1 file changed, 101 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 ed7e405682f0..f72162ee9dc3 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -4,11 +4,13 @@
 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
 
@@ -809,6 +811,102 @@ 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:
+            raise KsftSkipEx("Not enough queues for the test")
+
+    ethtool(f"-X {cfg.ifname} equal 2")
+    defer(ethtool, f"-X {cfg.ifname} default")
+
+    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:
+        if pre_down:
+            ip(f"link set dev {cfg.ifname} up")
+            raise KsftSkipEx("Create context not supported with interface down")
+        raise
+
+    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")
+
+    ip(f"link set dev {cfg.ifname} up")
+
+    import subprocess
+    max_wait = 10
+    ping_success = False
+    remote_addr = cfg.remote_addr_v[cfg.addr_ipver]
+    for attempt in range(max_wait):
+        try:
+            result = subprocess.run(['ping', '-c', '1', '-W', '1', remote_addr],
+                                  capture_output=True, timeout=2)
+            if result.returncode == 0:
+                ping_success = True
+                break
+        except Exception:
+            pass
+        time.sleep(1)
+
+    if not ping_success:
+        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):
+    test_rss_context_persist_ifupdown(cfg, pre_down=False)
+
+
+def test_rss_context_persist_ifdown_and_create(cfg):
+    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 +921,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: [PATCH net v2 4/4] selftests: drv-net: rss_ctx: test RSS contexts persist after ifdown/up

[Bobby Eshleman]

On Thu, Feb 05, 2026 at 09:29:30PM -0800, Michael Chan wrote:
> 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 fitlers. 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>
> ---
> v2: Many improvements suggested by Jakub:
> 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       | 102 +++++++++++++++++-
>  1 file changed, 101 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 ed7e405682f0..f72162ee9dc3 100755
> --- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
> +++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
> @@ -4,11 +4,13 @@
>  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
>  
> @@ -809,6 +811,102 @@ 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:
> +            raise KsftSkipEx("Not enough queues for the test")
> +
> +    ethtool(f"-X {cfg.ifname} equal 2")
> +    defer(ethtool, f"-X {cfg.ifname} default")
> +
> +    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:
> +        if pre_down:
> +            ip(f"link set dev {cfg.ifname} up")
> +            raise KsftSkipEx("Create context not supported with interface down")
> +        raise
> +
> +    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")
> +
> +    ip(f"link set dev {cfg.ifname} up")
> +
> +    import subprocess
> +    max_wait = 10
> +    ping_success = False
> +    remote_addr = cfg.remote_addr_v[cfg.addr_ipver]
> +    for attempt in range(max_wait):
> +        try:
> +            result = subprocess.run(['ping', '-c', '1', '-W', '1', remote_addr],
> +                                  capture_output=True, timeout=2)
> +            if result.returncode == 0:
> +                ping_success = True
> +                break

This might be wanting to use utils.py's cmd()?

> +        except Exception:
> +            pass

> +        time.sleep(1)
> +
> +    if not ping_success:
> +        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):
> +    test_rss_context_persist_ifupdown(cfg, pre_down=False)
> +
> +
> +def test_rss_context_persist_ifdown_and_create(cfg):
> +    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 +921,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: [PATCH net v2 4/4] selftests: drv-net: rss_ctx: test RSS contexts persist after ifdown/up

[Jakub Kicinski]

On Thu,  5 Feb 2026 21:29:30 -0800 Michael Chan wrote:
> diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py

Please use:

 ruff check $file
 pylint --disable=R $file

and fix the new warnings.

This file predates my discovery of these tools so there are some
warnings already, but let's not add more.

> +@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:
> +            raise KsftSkipEx("Not enough queues for the test")
> +
> +    ethtool(f"-X {cfg.ifname} equal 2")
> +    defer(ethtool, f"-X {cfg.ifname} default")
> +
> +    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:
> +        if pre_down:
> +            ip(f"link set dev {cfg.ifname} up")

You should use defer for the ifup. You can save it and run it with
.exec():

	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:
		raise KsftSkipEx("Create context not supported with interface down")

	....

	if not pre_down:
		ip(f"link set dev {cfg.ifname} down")
	ifup.exec()

this ifup.exec() also removes ifup from the "defer queue".

> +            raise KsftSkipEx("Create context not supported with interface down")
> +        raise
> +
> +    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")
> +
> +    ip(f"link set dev {cfg.ifname} up")

You can add here:

	wait_file(f"/sys/class/net/{cfg.ifname}/carrier",
		  lambda x: x.strip() == "1")   

to wait for carrier before running the ping loop.

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

[Pavan Chebbi]

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

> > +
> > +    import subprocess
> > +    max_wait = 10
> > +    ping_success = False
> > +    remote_addr = cfg.remote_addr_v[cfg.addr_ipver]
> > +    for attempt in range(max_wait):
> > +        try:
> > +            result = subprocess.run(['ping', '-c', '1', '-W', '1', remote_addr],
> > +                                  capture_output=True, timeout=2)
> > +            if result.returncode == 0:
> > +                ping_success = True
> > +                break
>
> This might be wanting to use utils.py's cmd()?

I checked and tried using the cmd() and it indeed eliminates a lot of
lines here. But the test itself becomes a little unreliable.
I often see TimeoutError: [Errno Wait for file contents failed]
/sys/class/net/ens2f0np0/carrier from the wait_file (Jakub asked me to
use wait_file)
Also, even when the test succeeds, after the suite exits, I cannot
ping the remote host for a long time. In contrast, my manual loop
worked 100pc of the times I tested.
I am using the command like below:

   for _ in range(10):
        if cmd(f"ping -c 1 -W 1 {remote_addr}", fail=False, timeout=2).ret == 0:
            break
        time.sleep(1)
    else:
        raise KsftSkipEx("Cannot reach remote host after interface up")

I am not a py expert so is there something I am missing?

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

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

[Jakub Kicinski]

On Sun, 8 Feb 2026 21:56:39 +0530 Pavan Chebbi wrote:
> > > +
> > > +    import subprocess
> > > +    max_wait = 10
> > > +    ping_success = False
> > > +    remote_addr = cfg.remote_addr_v[cfg.addr_ipver]
> > > +    for attempt in range(max_wait):
> > > +        try:
> > > +            result = subprocess.run(['ping', '-c', '1', '-W', '1', remote_addr],
> > > +                                  capture_output=True, timeout=2)
> > > +            if result.returncode == 0:
> > > +                ping_success = True
> > > +                break  
> >
> > This might be wanting to use utils.py's cmd()?  
> 
> I checked and tried using the cmd() and it indeed eliminates a lot of
> lines here. But the test itself becomes a little unreliable.
> I often see TimeoutError: [Errno Wait for file contents failed]
> /sys/class/net/ens2f0np0/carrier from the wait_file (Jakub asked me to
> use wait_file)

Does the link take more than the default 5 sec to come up?
wait_file() has a deadline param that should let us wait longer

> Also, even when the test succeeds, after the suite exits, I cannot
> ping the remote host for a long time. In contrast, my manual loop
> worked 100pc of the times I tested.
> I am using the command like below:
> 
>    for _ in range(10):
>         if cmd(f"ping -c 1 -W 1 {remote_addr}", fail=False, timeout=2).ret == 0:
>             break
>         time.sleep(1)
>     else:
>         raise KsftSkipEx("Cannot reach remote host after interface up")
> 
> I am not a py expert so is there something I am missing?

I don't think there's anything Python related here :(
Is it possible that NIC reports carrier before it can actually send
packets? Maybe the wait_file() gives ping has a higher chance of failing
with carrier up which makes the neighbor resolution failure more sticky?

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

[Pavan Chebbi]

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

On Wed, Feb 11, 2026 at 7:16 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 8 Feb 2026 21:56:39 +0530 Pavan Chebbi wrote:
> > > > +
> > > > +    import subprocess
> > > > +    max_wait = 10
> > > > +    ping_success = False
> > > > +    remote_addr = cfg.remote_addr_v[cfg.addr_ipver]
> > > > +    for attempt in range(max_wait):
> > > > +        try:
> > > > +            result = subprocess.run(['ping', '-c', '1', '-W', '1', remote_addr],
> > > > +                                  capture_output=True, timeout=2)
> > > > +            if result.returncode == 0:
> > > > +                ping_success = True
> > > > +                break
> > >
> > > This might be wanting to use utils.py's cmd()?
> >
> > I checked and tried using the cmd() and it indeed eliminates a lot of
> > lines here. But the test itself becomes a little unreliable.
> > I often see TimeoutError: [Errno Wait for file contents failed]
> > /sys/class/net/ens2f0np0/carrier from the wait_file (Jakub asked me to
> > use wait_file)
>
> Does the link take more than the default 5 sec to come up?
> wait_file() has a deadline param that should let us wait longer

I did try with increased deadline. But I don't think it's helping.
>
> > Also, even when the test succeeds, after the suite exits, I cannot
> > ping the remote host for a long time. In contrast, my manual loop
> > worked 100pc of the times I tested.
> > I am using the command like below:
> >
> >    for _ in range(10):
> >         if cmd(f"ping -c 1 -W 1 {remote_addr}", fail=False, timeout=2).ret == 0:
> >             break
> >         time.sleep(1)
> >     else:
> >         raise KsftSkipEx("Cannot reach remote host after interface up")
> >
> > I am not a py expert so is there something I am missing?
>
> I don't think there's anything Python related here :(

What I wanted to check was that if the cmd() invokes some shell and
has some kind overhead that could affect timing..

> Is it possible that NIC reports carrier before it can actually send
> packets? Maybe the wait_file() gives ping has a higher chance of failing
> with carrier up which makes the neighbor resolution failure more sticky?

I am not sure. I am willing to send the manual ping loop version as I
have high confidence on that.
If you or Bobby insist I must use cmd() then I can spend more time on that..

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

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

[Jakub Kicinski]

On Wed, 11 Feb 2026 09:28:22 +0530 Pavan Chebbi wrote:
> On Wed, Feb 11, 2026 at 7:16 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > I checked and tried using the cmd() and it indeed eliminates a lot of
> > > lines here. But the test itself becomes a little unreliable.
> > > I often see TimeoutError: [Errno Wait for file contents failed]
> > > /sys/class/net/ens2f0np0/carrier from the wait_file (Jakub asked me to
> > > use wait_file)  
> >
> > Does the link take more than the default 5 sec to come up?
> > wait_file() has a deadline param that should let us wait longer  
> 
> I did try with increased deadline. But I don't think it's helping.

If you check the carrier manually in a second terminal while it's
running -- do you see it go to 1? The helper keeps the file open
and rewinds to the start, I think that may not work with sysfs :S
Maybe we need:

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 85884f3e827b..24e74475315d 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -294,11 +294,10 @@ GLOBAL_DEFER_ARMED = False
     """
     end = time.monotonic() + deadline
 
-    with open(fname, "r", encoding=encoding) as fp:
-        while True:
+    while True:
+        with open(fname, "r", encoding=encoding) as fp:
             if test_fn(fp.read()):
                 break
-            fp.seek(0)
             if time.monotonic() > end:
                 raise TimeoutError("Wait for file contents failed", fname)
             time.sleep(sleep)



> > > Also, even when the test succeeds, after the suite exits, I cannot
> > > ping the remote host for a long time. In contrast, my manual loop
> > > worked 100pc of the times I tested.
> > > I am using the command like below:
> > >
> > >    for _ in range(10):
> > >         if cmd(f"ping -c 1 -W 1 {remote_addr}", fail=False, timeout=2).ret == 0:
> > >             break
> > >         time.sleep(1)
> > >     else:
> > >         raise KsftSkipEx("Cannot reach remote host after interface up")
> > >
> > > I am not a py expert so is there something I am missing?  
> >
> > I don't think there's anything Python related here :(  
> 
> What I wanted to check was that if the cmd() invokes some shell and
> has some kind overhead that could affect timing..
> 
> > Is it possible that NIC reports carrier before it can actually send
> > packets? Maybe the wait_file() gives ping has a higher chance of failing
> > with carrier up which makes the neighbor resolution failure more sticky?  
> 
> I am not sure. I am willing to send the manual ping loop version as I
> have high confidence on that.
> If you or Bobby insist I must use cmd() then I can spend more time on that..

cmd() doesn't do any magic, it basically does:

	proc = subprocess.Popen(comm, shell=shell,
				stdout=subprocess.PIPE, stderr=subprocess.PIPE,
				pass_fds=pass_fds, env=env)
	proc.communicate(timeout)

So pretty much the same code as your subprocess.run().

It has support for communicating with extra fds to 

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

[Pavan Chebbi]

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

On Wed, Feb 11, 2026 at 9:47 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 11 Feb 2026 09:28:22 +0530 Pavan Chebbi wrote:
> > On Wed, Feb 11, 2026 at 7:16 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > > I checked and tried using the cmd() and it indeed eliminates a lot of
> > > > lines here. But the test itself becomes a little unreliable.
> > > > I often see TimeoutError: [Errno Wait for file contents failed]
> > > > /sys/class/net/ens2f0np0/carrier from the wait_file (Jakub asked me to
> > > > use wait_file)
> > >
> > > Does the link take more than the default 5 sec to come up?
> > > wait_file() has a deadline param that should let us wait longer
> >
> > I did try with increased deadline. But I don't think it's helping.
>
> If you check the carrier manually in a second terminal while it's
> running -- do you see it go to 1? The helper keeps the file open

Thanks for the nudge. I think I get the real issue now. It's not the
cmd() but ifup.exec()
I am using :
ifup = defer(ip, f"link set dev {cfg.ifname} up")
and later
ifup.exec()

I see that exec() takes unusually long time for carrier to go to 1.
Upto 20s and sometimes still fails.
Same with both direct exec or deferred. And this explains why I cannot
ping for long time after suite exists. Thats because I rely on defer()
to bring the interface up after bnxt exits with carrier 0 for the
second test.
If replace the exec with direct call to ip(f"link set dev {cfg.ifname}
up") I see it works reliably.

<-->

>
> cmd() doesn't do any magic, it basically does:
>
>         proc = subprocess.Popen(comm, shell=shell,
>                                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
>                                 pass_fds=pass_fds, env=env)
>         proc.communicate(timeout)
>
> So pretty much the same code as your subprocess.run().
>
> It has support for communicating with extra fds to

Yes, sorry, this was a red herring for me

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

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

[Pavan Chebbi]

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

On Wed, Feb 11, 2026 at 10:45 AM Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> On Wed, Feb 11, 2026 at 9:47 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 11 Feb 2026 09:28:22 +0530 Pavan Chebbi wrote:
> > > On Wed, Feb 11, 2026 at 7:16 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > > > I checked and tried using the cmd() and it indeed eliminates a lot of
> > > > > lines here. But the test itself becomes a little unreliable.
> > > > > I often see TimeoutError: [Errno Wait for file contents failed]
> > > > > /sys/class/net/ens2f0np0/carrier from the wait_file (Jakub asked me to
> > > > > use wait_file)
> > > >
> > > > Does the link take more than the default 5 sec to come up?
> > > > wait_file() has a deadline param that should let us wait longer
> > >
> > > I did try with increased deadline. But I don't think it's helping.
> >
> > If you check the carrier manually in a second terminal while it's
> > running -- do you see it go to 1? The helper keeps the file open
>
> Thanks for the nudge. I think I get the real issue now. It's not the
> cmd() but ifup.exec()
> I am using :
> ifup = defer(ip, f"link set dev {cfg.ifname} up")
> and later
> ifup.exec()
>
> I see that exec() takes unusually long time for carrier to go to 1.
> Upto 20s and sometimes still fails.
> Same with both direct exec or deferred. And this explains why I cannot
> ping for long time after suite exists. Thats because I rely on defer()
> to bring the interface up after bnxt exits with carrier 0 for the
> second test.
> If replace the exec with direct call to ip(f"link set dev {cfg.ifname}
> up") I see it works reliably.
>

My experiments are pointing to 'self.cancel()' sometimes taking a lot
of time as part of the exec().
I don't know but I feel the new test code itself should be OK. I can
send a v3 and you can run it once too?

> <-->
>
> >
> > cmd() doesn't do any magic, it basically does:
> >
> >         proc = subprocess.Popen(comm, shell=shell,
> >                                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
> >                                 pass_fds=pass_fds, env=env)
> >         proc.communicate(timeout)
> >
> > So pretty much the same code as your subprocess.run().
> >
> > It has support for communicating with extra fds to
>
> Yes, sorry, this was a red herring for me

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

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

[Jakub Kicinski]

On Wed, 11 Feb 2026 13:29:06 +0530 Pavan Chebbi wrote:
> > > If you check the carrier manually in a second terminal while it's
> > > running -- do you see it go to 1? The helper keeps the file open  
> >
> > Thanks for the nudge. I think I get the real issue now. It's not the
> > cmd() but ifup.exec()
> > I am using :
> > ifup = defer(ip, f"link set dev {cfg.ifname} up")
> > and later
> > ifup.exec()
> >
> > I see that exec() takes unusually long time for carrier to go to 1.
> > Upto 20s and sometimes still fails.
> > Same with both direct exec or deferred. And this explains why I cannot
> > ping for long time after suite exists. Thats because I rely on defer()
> > to bring the interface up after bnxt exits with carrier 0 for the
> > second test.
> > If replace the exec with direct call to ip(f"link set dev {cfg.ifname}
> > up") I see it works reliably.
> 
> My experiments are pointing to 'self.cancel()' sometimes taking a lot
> of time as part of the exec().
> I don't know but I feel the new test code itself should be OK. I can
> send a v3 and you can run it once too?

Could you reply to this thread with what the version of the patch where
you see defer() takes a long time? We use defer all over the place..

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

[Pavan Chebbi]

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

On Wed, Feb 11, 2026 at 10:01 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 11 Feb 2026 13:29:06 +0530 Pavan Chebbi wrote:
> > > > If you check the carrier manually in a second terminal while it's
> > > > running -- do you see it go to 1? The helper keeps the file open
> > >
> > > Thanks for the nudge. I think I get the real issue now. It's not the
> > > cmd() but ifup.exec()
> > > I am using :
> > > ifup = defer(ip, f"link set dev {cfg.ifname} up")
> > > and later
> > > ifup.exec()
> > >
> > > I see that exec() takes unusually long time for carrier to go to 1.
> > > Upto 20s and sometimes still fails.
> > > Same with both direct exec or deferred. And this explains why I cannot
> > > ping for long time after suite exists. Thats because I rely on defer()
> > > to bring the interface up after bnxt exits with carrier 0 for the
> > > second test.
> > > If replace the exec with direct call to ip(f"link set dev {cfg.ifname}
> > > up") I see it works reliably.
> >
> > My experiments are pointing to 'self.cancel()' sometimes taking a lot
> > of time as part of the exec().
> > I don't know but I feel the new test code itself should be OK. I can
> > send a v3 and you can run it once too?
>
> Could you reply to this thread with what the version of the patch where
> you see defer() takes a long time? We use defer all over the place..

Oh it is still in my sandbox. It's after I incorporate the comments I
received on the v2.
You should see it in v3. Let me ask Michael to send it.

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

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

[Jakub Kicinski]

On Wed, 11 Feb 2026 22:08:39 +0530 Pavan Chebbi wrote:
> > > My experiments are pointing to 'self.cancel()' sometimes taking a lot
> > > of time as part of the exec().
> > > I don't know but I feel the new test code itself should be OK. I can
> > > send a v3 and you can run it once too?  
> >
> > Could you reply to this thread with what the version of the patch where
> > you see defer() takes a long time? We use defer all over the place..  
> 
> Oh it is still in my sandbox. It's after I incorporate the comments I
> received on the v2.
> You should see it in v3. Let me ask Michael to send it.

I'm asking for the version that doesn't work / causes trouble.
Presumably that's not the version you want Michael to submit?

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

[Pavan Chebbi]

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

On Wed, Feb 11, 2026 at 10:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 11 Feb 2026 22:08:39 +0530 Pavan Chebbi wrote:
> > > > My experiments are pointing to 'self.cancel()' sometimes taking a lot
> > > > of time as part of the exec().
> > > > I don't know but I feel the new test code itself should be OK. I can
> > > > send a v3 and you can run it once too?
> > >
> > > Could you reply to this thread with what the version of the patch where
> > > you see defer() takes a long time? We use defer all over the place..
> >
> > Oh it is still in my sandbox. It's after I incorporate the comments I
> > received on the v2.
> > You should see it in v3. Let me ask Michael to send it.
>
> I'm asking for the version that doesn't work / causes trouble.

It's not in the list yet. v2, which is the latest version, does not
use exec/defer to bring the interface back up.

> Presumably that's not the version you want Michael to submit?

My submission is that the test code that will come in v3 is going to
be correct, except that sometimes I see a delay in getting the
interface back up, specifically when I use exec() or defer() for if
up. And I am being certain that the delay is not caused because of any
mistake I have made in the new tests..

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

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

[Pavan Chebbi]

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

On Wed, Feb 11, 2026 at 10:53 PM Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> On Wed, Feb 11, 2026 at 10:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 11 Feb 2026 22:08:39 +0530 Pavan Chebbi wrote:
> > > > > My experiments are pointing to 'self.cancel()' sometimes taking a lot
> > > > > of time as part of the exec().
> > > > > I don't know but I feel the new test code itself should be OK. I can
> > > > > send a v3 and you can run it once too?
> > > >
> > > > Could you reply to this thread with what the version of the patch where
> > > > you see defer() takes a long time? We use defer all over the place..
> > >
> > > Oh it is still in my sandbox. It's after I incorporate the comments I
> > > received on the v2.
> > > You should see it in v3. Let me ask Michael to send it.
> >
> > I'm asking for the version that doesn't work / causes trouble.
>

Ok, I now get what you are asking for. I will send the reply here with
the patch soon. thanks.

> It's not in the list yet. v2, which is the latest version, does not
> use exec/defer to bring the interface back up.
>
> > Presumably that's not the version you want Michael to submit?
>
> My submission is that the test code that will come in v3 is going to
> be correct, except that sometimes I see a delay in getting the
> interface back up, specifically when I use exec() or defer() for if
> up. And I am being certain that the delay is not caused because of any
> mistake I have made in the new tests..

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

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

[Pavan Chebbi]

On Wed, Feb 11, 2026 at 10:01 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> >
> > My experiments are pointing to 'self.cancel()' sometimes taking a lot
> > of time as part of the exec().
> > I don't know but I feel the new test code itself should be OK. I can
> > send a v3 and you can run it once too?
>
> Could you reply to this thread with what the version of the patch where
> you see defer() takes a long time? We use defer all over the place..

Please find the patch with which I see exec() and defer() often taking a
lot of time to perform ifup..
---
 .../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.47.3

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

[Jakub Kicinski]

On Thu, 12 Feb 2026 00:12:50 -0800 Pavan Chebbi wrote:
> +    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]

I don't see anything wrong here :(

The diff I shared to re-open the carrier file instead of using seek()
doesn't make a difference?

If it doesn't please use the version you shared here, even if it's
unstable right now. I'll try to find the time to investigate what's
wrong with the test infra. Better that than accumulating workarounds.

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

[Pavan Chebbi]

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

On Fri, Feb 13, 2026 at 7:22 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 12 Feb 2026 00:12:50 -0800 Pavan Chebbi wrote:
> > +    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]
>
> I don't see anything wrong here :(
>
> The diff I shared to re-open the carrier file instead of using seek()
> doesn't make a difference?

No, it did not make any difference.

>
> If it doesn't please use the version you shared here, even if it's
> unstable right now. I'll try to find the time to investigate what's
> wrong with the test infra. Better that than accumulating workarounds.

Sounds good. Thanks for checking.

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

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 12 Feb 2026 00:12:50 -0800 you wrote:
> On Wed, Feb 11, 2026 at 10:01 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > >
> > > My experiments are pointing to 'self.cancel()' sometimes taking a lot
> > > of time as part of the exec().
> > > I don't know but I feel the new test code itself should be OK. I can
> > > send a v3 and you can run it once too?
> >
> > Could you reply to this thread with what the version of the patch where
> > you see defer() takes a long time? We use defer all over the place..
> 
> [...]

Here is the summary with links:
  - [1/1] Re: [PATCH net v2 4/4] selftests: drv-net: rss_ctx: test RSS contexts persist after ifdown/up
    https://git.kernel.org/netdev/net/c/ce5a0f4612db

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html