ethtool: Dynamic RSS context indirection table resizing

Hi!

Some NICs (e.g. bnxt) change their RSS indirection table size based on the queue count, because the hardware table is a shared resource. The ethtool core locks ctx->indir_size at context creation, so drivers have to reject channel changes when RSS contexts exist.

This series adds resize helpers and wires them up in bnxt.

Patch 1 adds core helpers:
ethtool_rxfh_can_resize() - read-only validation ethtool_rxfh_resize() - fold/unfold a raw table in place ethtool_rxfh_ctxs_can_resize() - validate all non-default contexts ethtool_rxfh_ctxs_resize() - resize all non-default contexts,
with locking and RSS_NTF notifications

Patch 2 uses them in bnxt_set_channels(). Validation runs before bnxt_close_nic(); actual resize is deferred until after. RSS table size only changes on P5 chips with older firmware.

Patch 3 adds HW tests in rss_drv.py (devices without dynamic table sizing are skipped):
resize_periodic - fold/unfold with a non-default [3,2,1,0]xN
pattern, verifying exact content preservation (main + ctx)
resize_nonperiodic_reject - non-periodic table blocks channel
reduction, with an extra periodic context to exercise multi-context validation (main + ctx)
resize_nonperiodic_no_corruption - failed resize leaves table
contents and channel count unchanged (main + ctx)

Running the tests:

# On real hardware
sudo NETIF=eth0 ./rss_drv.py

Changes v3 -> v4:

- Rebased onto net-next
- Added Reviewed-by: from Michael
- Added missing Cc: to make the pwbots happier

Changes v2 -> v3:

- Changed ethtool_rxfh_can_resize() to return bool instead of int; true means resize is possible, false means it is not. Inverted callers accordingly. (Jakub)
- Added Tested-by from Pavan

Changes v1 -> v2:

- Dropped netdevsim support and netdevsim selftest (Jakub)
- Split ethtool_rxfh_contexts_resize_all() into separate validate (ethtool_rxfh_ctxs_can_resize) and apply (ethtool_rxfh_ctxs_resize) so drivers can validate before closing the device (Jakub)
- Shortened helper names (Jakub)
- Replaced scoped_guard(mutex) with explicit mutex_lock/unlock (Jakub)
- Removed defensive zero-size check, bare expressions instead of != 0 comparisons, ! instead of == 0 (Jakub)
- In bnxt, moved bnxt_check_rings() before RSS validation and deferred actual resize to after bnxt_close_nic() (Jakub, Michael)
- Added comment that RSS table size only changes on P5 chips with older firmware (Michael)
- Use non-default [3,2,1,0]xN pattern set via netlink to distinguish correct fold from driver resetting to defaults (Jakub)
- Check exact indirection table pattern, not just set(indir) (Jakub)
- Use ksft_raises() instead of try/except/else (Jakub)
- Removed queue_count=8 from NetDrvEnv (Jakub)
- Added ksft_variants to resize_nonperiodic_reject for ctx coverage
- Added extra periodic context in reject test for multi-context validation coverage
- Added resize_nonperiodic_no_corruption test

Open items:

- No user-controlled minimum table size yet. The plan is to record the user-provided indirection table length in the context (e.g. ctx->user_indir_size) and use it as a floor when folding: reject if new_size < user_indir_size. This way the user's original table size serves as an implicit minimum, preventing the driver from shrinking below what the user intended. Left for a follow-up.

Björn Töpel (3):
ethtool: Add RSS indirection table resize helpers
bnxt_en: Resize RSS contexts on channel count change
selftests: rss_drv: Add RSS indirection table resize tests

 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  36 +++-
 include/linux/ethtool.h                       |   4 +
 net/ethtool/common.c                          | 143 +++++++++++++++
 .../selftests/drivers/net/hw/rss_drv.py       | 163 +++++++++++++++++-
 4 files changed, 335 insertions(+), 11 deletions(-)

base-commit: 8f921f61005450589c0bc1a941a5ddde21d9aed9

2.53.0

The core locks ctx->indir_size when an RSS context is created. Some NICs (e.g. bnxt) change their indirection table size based on the channel count, because the hardware table is a shared resource. This forces drivers to reject channel changes when RSS contexts exist.

Add helpers to resize indirection tables:

ethtool_rxfh_can_resize() checks whether a table can be resized without modifying it.

ethtool_rxfh_resize() resizes a raw u32 table in place. Folding (shrink) requires the table to be periodic at the new size; non-periodic tables are rejected. Unfolding (grow) replicates the existing pattern. Sizes must be multiples of each other.

ethtool_rxfh_ctxs_can_resize() validates all non-default RSS contexts can be resized. ethtool_rxfh_ctxs_resize() applies the resize and sends ETHTOOL_MSG_RSS_NTF per resized context after releasing rss_lock.

No reallocation is needed because ethtool_rxfh_ctx_alloc() reserves space for rxfh_indir_space entries, and key_off is based on that maximum.

---
 include/linux/ethtool.h |   4 ++
 net/ethtool/common.c    | 143 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+)

index 83c375840835..e2c4aa0a3348 100644
@@ -214,6 +214,10 @@ static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
 }
 
 void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
+bool ethtool_rxfh_can_resize(const u32 *tbl, u32 old_size, u32 new_size);
+int ethtool_rxfh_resize(u32 *tbl, u32 old_size, u32 new_size);
+int ethtool_rxfh_ctxs_can_resize(struct net_device *dev, u32 new_indir_size);
+void ethtool_rxfh_ctxs_resize(struct net_device *dev, u32 new_indir_size);
 
 struct link_mode_info {
 	int	speed;

2.53.0

bnxt_set_channels() rejects channel changes that alter the RSS table size when IFF_RXFH_CONFIGURED is set, because non-default context sizes were locked at creation.

Replace the rejection with the new resize helpers. All validation runs before the device is closed; actual resize is deferred until after bnxt_close_nic():

1. ethtool_rxfh_can_resize() checks context 0.
2. ethtool_rxfh_ctxs_can_resize() validates all non-default contexts.
3. After bnxt_close_nic(), ethtool_rxfh_resize() applies context 0 changes, and ethtool_rxfh_ctxs_resize() resizes non-default contexts.

RSS table size only changes on P5 chips with older firmware; newer firmware always uses the largest table size.

When context 0 uses defaults (!IFF_RXFH_CONFIGURED), steps 1 and 3 are skipped; the driver regenerates the table via bnxt_set_dflt_rss_indir_tbl().

---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 36 +++++++++++++++----
 1 file changed, 29 insertions(+), 7 deletions(-)

index 48e8e3be70d3..dc3eb24c9c0b 100644
@@ -942,6 +942,7 @@ static int bnxt_set_channels(struct net_device *dev,
 {
 	struct bnxt *bp = netdev_priv(dev);
 	int req_tx_rings, req_rx_rings, tcs;
+	u32 new_tbl_size = 0, old_tbl_size;
 	bool sh = false;
 	int tx_xdp = 0;
 	int rc = 0;

@@ -977,19 +978,33 @@ static int bnxt_set_channels(struct net_device *dev,
 		tx_xdp = req_rx_rings;
 	}
 
-	if (bnxt_get_nr_rss_ctxs(bp, req_rx_rings) !=
-	    bnxt_get_nr_rss_ctxs(bp, bp->rx_nr_rings) &&
-	    (netif_is_rxfh_configured(dev) || bp->num_rss_ctx)) {
-		netdev_warn(dev, "RSS table size change required, RSS table entries must be default (with no additional RSS contexts present) to proceed\n");
-		return -EINVAL;
-	}
-
 	rc = bnxt_check_rings(bp, req_tx_rings, req_rx_rings, sh, tcs, tx_xdp);
 	if (rc) {
 		netdev_warn(dev, "Unable to allocate the requested rings\n");
 		return rc;
 	}
 
+	/* RSS table size only changes on P5 chips with older firmware;
+	 * newer firmware always uses the largest table size.
+	 */
+	if (bnxt_get_nr_rss_ctxs(bp, req_rx_rings) !=
+	    bnxt_get_nr_rss_ctxs(bp, bp->rx_nr_rings)) {
+		new_tbl_size = bnxt_get_nr_rss_ctxs(bp, req_rx_rings) *
+			       BNXT_RSS_TABLE_ENTRIES_P5;
+		old_tbl_size = bnxt_get_rxfh_indir_size(dev);
+
+		if (netif_is_rxfh_configured(dev) &&
+		    !ethtool_rxfh_can_resize(bp->rss_indir_tbl,
+					     old_tbl_size, new_tbl_size)) {
+			netdev_warn(dev, "RSS table resize not possible\n");
+			return -EINVAL;
+		}
+
+		rc = ethtool_rxfh_ctxs_can_resize(dev, new_tbl_size);
+		if (rc)
+			return rc;
+	}
+
 	if (netif_running(dev)) {
 		if (BNXT_PF(bp)) {
 			/* TODO CHIMP_FW: Send message to all VF's

@@ -999,6 +1014,13 @@ static int bnxt_set_channels(struct net_device *dev,
 		bnxt_close_nic(bp, true, false);
 	}
 
+	if (new_tbl_size) {
+		if (netif_is_rxfh_configured(dev))
+			ethtool_rxfh_resize(bp->rss_indir_tbl,
+					    old_tbl_size, new_tbl_size);
+		ethtool_rxfh_ctxs_resize(dev, new_tbl_size);
+	}
+
 	if (sh) {
 		bp->flags |= BNXT_FLAG_SHARED_RINGS;
 		bp->rx_nr_rings = channel->combined_count;

2.53.0

Add resize tests to rss_drv.py. Devices without dynamic table sizing are skipped via _require_dynamic_indir_size().

resize_periodic: set a periodic table (equal 4), shrink channels to fold, grow back to unfold. Check the exact pattern is preserved. Has main and non-default context variants.

resize_nonperiodic_reject: set a non-periodic table (equal N), verify that channel reduction is rejected.

---
 .../selftests/drivers/net/hw/rss_drv.py       | 163 +++++++++++++++++-
 1 file changed, 159 insertions(+), 4 deletions(-)

index 2d1a33189076..9763760f8306 100755
@@ -5,9 +5,9 @@
 Driver-related behavior tests for RSS.
 """
 
-from lib.py import ksft_run, ksft_exit, ksft_ge
-from lib.py import ksft_variants, KsftNamedVariant, KsftSkipEx
-from lib.py import defer, ethtool
+from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_ge
+from lib.py import ksft_variants, KsftNamedVariant, KsftSkipEx, ksft_raises
+from lib.py import defer, ethtool, CmdExitFailure
 from lib.py import EthtoolFamily, NlError
 from lib.py import NetDrvEnv
 

@@ -45,6 +45,18 @@ def _maybe_create_context(cfg, create_context):
     return ctx_id
 
 
+def _require_dynamic_indir_size(cfg, ch_max):
+    """Skip if the device does not dynamically size its indirection table."""
+    ethtool(f"-X {cfg.ifname} default")
+    ethtool(f"-L {cfg.ifname} combined 2")
+    small = len(_get_rss(cfg)['rss-indirection-table'])
+    ethtool(f"-L {cfg.ifname} combined {ch_max}")
+    large = len(_get_rss(cfg)['rss-indirection-table'])
+
+    if small == large:
+        raise KsftSkipEx("Device does not dynamically size indirection table")
+
+
 @ksft_variants([
     KsftNamedVariant("main", False),
     KsftNamedVariant("ctx", True),

@@ -76,11 +88,154 @@ def indir_size_4x(cfg, create_context):
         _test_rss_indir_size(cfg, test_max, context=ctx_id)
 
 
+@ksft_variants([
+    KsftNamedVariant("main", False),
+    KsftNamedVariant("ctx", True),
+])
+def resize_periodic(cfg, create_context):
+    """Test that a periodic indirection table survives channel changes.
+
+    Set a non-default periodic table ([3, 2, 1, 0] x N) via netlink,
+    reduce channels to trigger a fold, then increase to trigger an
+    unfold. Using a reversed pattern (instead of [0, 1, 2, 3]) ensures
+    the test can distinguish a correct fold from a driver that silently
+    resets the table to defaults. Verify the exact pattern is preserved
+    and the size tracks the channel count.
+    """
+    channels = cfg.ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
+    ch_max = channels.get('combined-max', 0)
+    qcnt = channels['combined-count']
+
+    if ch_max < 4:
+        raise KsftSkipEx(f"Not enough queues for the test: max={ch_max}")
+
+    defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
+
+    _require_dynamic_indir_size(cfg, ch_max)
+
+    ctx_id = _maybe_create_context(cfg, create_context)
+
+    # Set a non-default periodic pattern via netlink
+    rss = _get_rss(cfg, context=ctx_id)
+    orig_size = len(rss['rss-indirection-table'])
+    pattern = [3, 2, 1, 0] * (orig_size // 4)
+    req = {'header': {'dev-index': cfg.ifindex}, 'indir': pattern}
+    if ctx_id:
+        req['context'] = ctx_id
+    else:
+        defer(ethtool, f"-X {cfg.ifname} default")
+    cfg.ethnl.rss_set(req)
+
+    # Shrink — should fold
+    ethtool(f"-L {cfg.ifname} combined 4")
+    rss = _get_rss(cfg, context=ctx_id)
+    indir = rss['rss-indirection-table']
+
+    ksft_ge(orig_size, len(indir), "Table did not shrink")
+    ksft_eq(indir, [3, 2, 1, 0] * (len(indir) // 4),
+            "Folded table has wrong pattern")
+
+    # Grow back — should unfold
+    ethtool(f"-L {cfg.ifname} combined {ch_max}")
+    rss = _get_rss(cfg, context=ctx_id)
+    indir = rss['rss-indirection-table']
+
+    ksft_eq(len(indir), orig_size, "Table size not restored")
+    ksft_eq(indir, [3, 2, 1, 0] * (len(indir) // 4),
+            "Unfolded table has wrong pattern")
+
+
+@ksft_variants([
+    KsftNamedVariant("main", False),
+    KsftNamedVariant("ctx", True),
+])
+def resize_nonperiodic_reject(cfg, create_context):
+    """Test that a non-periodic table blocks channel reduction.
+
+    Set equal weight across all queues so the table is not periodic
+    at any smaller size, then verify channel reduction is rejected.
+    An additional context with a periodic table is created to verify
+    that validation catches the non-periodic one even when others
+    are fine.
+    """
+    channels = cfg.ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
+    ch_max = channels.get('combined-max', 0)
+    qcnt = channels['combined-count']
+
+    if ch_max < 4:
+        raise KsftSkipEx(f"Not enough queues for the test: max={ch_max}")
+
+    defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
+
+    _require_dynamic_indir_size(cfg, ch_max)
+
+    ctx_id = _maybe_create_context(cfg, create_context)
+    ctx_ref = f"context {ctx_id}" if ctx_id else ""
+
+    # Create an extra context with a periodic (foldable) table so that
+    # the validation must iterate all contexts to find the bad one.
+    extra_ctx = _maybe_create_context(cfg, True)
+    ethtool(f"-X {cfg.ifname} context {extra_ctx} equal 2")
+
+    ethtool(f"-X {cfg.ifname} {ctx_ref} equal {ch_max}")
+    if not create_context:
+        defer(ethtool, f"-X {cfg.ifname} default")
+
+    with ksft_raises(CmdExitFailure):
+        ethtool(f"-L {cfg.ifname} combined 2")
+
+
+@ksft_variants([
+    KsftNamedVariant("main", False),
+    KsftNamedVariant("ctx", True),
+])
+def resize_nonperiodic_no_corruption(cfg, create_context):
+    """Test that a failed resize does not corrupt table or channel count.
+
+    Set a non-periodic table, attempt a channel reduction (which must
+    fail), then verify both the indirection table contents and the
+    channel count are unchanged.
+    """
+    channels = cfg.ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
+    ch_max = channels.get('combined-max', 0)
+    qcnt = channels['combined-count']
+
+    if ch_max < 4:
+        raise KsftSkipEx(f"Not enough queues for the test: max={ch_max}")
+
+    defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
+
+    _require_dynamic_indir_size(cfg, ch_max)
+
+    ctx_id = _maybe_create_context(cfg, create_context)
+    ctx_ref = f"context {ctx_id}" if ctx_id else ""
+
+    ethtool(f"-X {cfg.ifname} {ctx_ref} equal {ch_max}")
+    if not create_context:
+        defer(ethtool, f"-X {cfg.ifname} default")
+
+    rss_before = _get_rss(cfg, context=ctx_id)
+
+    with ksft_raises(CmdExitFailure):
+        ethtool(f"-L {cfg.ifname} combined 2")
+
+    rss_after = _get_rss(cfg, context=ctx_id)
+    ksft_eq(rss_after['rss-indirection-table'],
+            rss_before['rss-indirection-table'],
+            "Indirection table corrupted after failed resize")
+
+    channels = cfg.ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
+    ksft_eq(channels['combined-count'], ch_max,
+            "Channel count changed after failed resize")
+
+
 def main() -> None:
     """ Ksft boiler plate main """
     with NetDrvEnv(__file__) as cfg:
         cfg.ethnl = EthtoolFamily()
-        ksft_run([indir_size_4x], args=(cfg, ))
+        ksft_run([indir_size_4x, resize_periodic,
+                  resize_nonperiodic_reject,
+                  resize_nonperiodic_no_corruption], args=(cfg, ))
     ksft_exit()
 
 

2.53.0

Sorry Bjorn, I was typing the explanation below and I realized that we may be violating user intent. We should already record the user_size from rss_set_prep_indir() as part of the context (and presumably some netdev state for the main context?) and don't allow shrinking the context below that mark..

Now for the broader audience - my understanding is that the RSS table is a precious resource for most if not all drivers.
The direction of this work is to allow user to explicitly specify what RSS table size they want. The Netlink API for RSS already allows users to send tables smaller than what the device reports (Netlink code just "replicates" the table). So if the user asks for X entries the driver should be able to allocate a table of any size as long as its a multiple of X. This series only support "global" resizing but it should be simple to add the state for the driver to mark in the context what table size it actually allocated. The only reason we haven't added the support for that is that it's quite hard to know how to implement it without docs for the FW/HW.. fbnic has fixed table sizes so we can't even prototype there :S

Please reply if you'd like to collaborate on adding support for the user-defined RSS context sizing.

Don't be sorry! Good catch, and glad you caught it now rather than after it landed.

I'll respin with user_size tracked in the context (and netdev state for context 0) as a lower bound, so resize never folds below the size the user explicitly configured.

WDYT?

Björn

Yes, keep in mind that AFAIR context 0 is not currently tracked in the xarray. So maybe add the size of the main / default context as a field in struct ethtool_netdev_state ? Or we can start tracking it (partially). IDK what's cleaner.