[PATCH net v2 0/2] selftests: drv-net: ring_reconfig: handle devices without channel config

[PATCH net v2 0/2] selftests: drv-net: ring_reconfig: handle devices without channel config

[Bilal Yazıcıoğlu]

ring_reconfig.py assumes that a device which supports ring parameters
also supports changing its channels. Drivers without a get_channels op,
such as e1000e, return -EOPNOTSUPP for the channel query, and drivers
with get_channels but no set_channels, such as vmxnet3 and r8169,
return it for every channel change. The resulting NlError makes the
tests fail instead of skipping them.

On a ThinkPad T480 with an I219-V (e1000e) "ethtool -g" works while
"ethtool -l" returns "Operation not supported", and running the test
functions through the in-tree YNL library against the device shows
the three failures.

Patch 1 skips channels() on such devices and lets ringparam() continue
without the optional channel reduction, patch 2 skips
reconfig_tx_stall(). Only EOPNOTSUPP is handled; any other error still
fails, and restores are only dropped when nothing was changed. The
patches are split because the affected code came from two different
commits.

Testing: there is no remote endpoint for the T480, so the full test
was not run. With v1, the test functions were run against the real
device under ksft_run with a stub environment and all ethtool set
requests blocked: before the series all three fail, after it two are
skipped and the channel helper returns after the query. v2 was not
run on hardware; its new code is only reached after a successful
channel query, which never happens on e1000e. Drivers with
get_channels but no set_channels were not available; that part, and
devices which support changing channels, were checked against an
emulated ethtool family (unsupported query or change, other errors,
drivers rejecting only some layouts, failing restores) to check that
other errors still fail, restores still run after a change, and
behaviour with channel support is unchanged. pylint and ruff report
nothing new. Not tested: the full test with NETIF and a remote
endpoint, and the rest of ringparam() on e1000e (see the note in
patch 1 about ring size clamping).

I found this while looking for kernel tests I could validate on my
ThinkPad T480. I investigated and validated the issue on that
hardware, defined the scope of the fix, reviewed and refined the
implementation, and extended the regression tests used to validate
it.

This work was AI-assisted: OpenAI Codex helped with the initial
source analysis, and Claude Code drafted the implementation,
changelogs and test tooling. I take responsibility for the submitted
changes.

Changes in v2:
- patch 1: also handle -EOPNOTSUPP from the channel change in
  _configure_min_ring_cnt(), for drivers with get_channels but no
  set_channels (reported by the Sashiko review of v1)
- patch 1: skip channels() when every channel layout it tries is
  rejected with -EOPNOTSUPP, dropping the restore of the unchanged
  counts
- patch 2: skip reconfig_tx_stall() when enabling a second combined
  channel is rejected with -EOPNOTSUPP, dropping the restore for it
v1: https://lore.kernel.org/all/20261106150509.30458-1-ahmetbilalyazicioglu@gmail.com/

Bilal Yazıcıoğlu (2):
  selftests: drv-net: ring_reconfig: handle devices without channel
    config
  selftests: drv-net: ring_reconfig: skip tx stall test without channel
    config

 .../selftests/drivers/net/ring_reconfig.py    | 65 ++++++++++++++++---
 1 file changed, 57 insertions(+), 8 deletions(-)


base-commit: 19a227903e65c7a9c179156485660a4e017575fe
-- 
2.56.0

[PATCH net v2 1/2] selftests: drv-net: ring_reconfig: handle devices without channel config

[Bilal Yazıcıoğlu]

Drivers which don't implement the ethtool get_channels op (e.g.
e1000e) make ETHTOOL_MSG_CHANNELS_GET fail with -EOPNOTSUPP, and
drivers which implement get_channels but not set_channels (e.g.
vmxnet3, r8169) make ETHTOOL_MSG_CHANNELS_SET fail the same way. YNL
raises that as an NlError, so ring_reconfig reports failures on such
devices:

 - channels() fails on its first channels_get call or, if the device
   can't change channels, when restoring the original counts;
 - ringparam() fails in _configure_min_ring_cnt() before any ring
   size is tested. The helper only reduces the channel count to
   speed up the reconfiguration, ringparam() does not depend on it.

Skip channels() when the channel query is not supported, or when every
channel layout it tries is rejected with -EOPNOTSUPP; nothing was
changed in that case, so the restore is dropped. Return early from
_configure_min_ring_cnt() when the channel count can't be queried or
changed, logging that it was left unchanged. Other netlink errors are
still reported as failures.

The issue was found through AI-assisted source code analysis
(OpenAI Codex) and confirmed on a ThinkPad T480 with an Intel I219-V
(e1000e, 8086:15d8): "ethtool -l" fails with "Operation not
supported" while "ethtool -g" works, and running the test functions
with the in-tree YNL library against the device fails with NlError
95 (EOPNOTSUPP). The get_channels-only case was reported by the
Sashiko AI review of v1.

No remote endpoint was available, so the full test was not run. On
the same machine, an earlier version of this patch with the same
channel query handling was run under ksft_run with a stub
environment and all ethtool set requests blocked: channels() was
skipped and _configure_min_ring_cnt() returned after the channel
query. The handling of a rejected channel change, and devices which
support changing channels, were only checked against an emulated
ethtool family.

Fixes: ecca75ae5ae6 ("selftests: drv-net: replace the nsim ring test with a drv-net one")
Signed-off-by: Bilal Yazıcıoğlu <ahmetbilalyazicioglu@gmail.com>
---

Notes:
    Note, not part of this fix: with this patch ringparam() gets past
    _configure_min_ring_cnt() on e1000e and reaches the ring size loop.
    e1000e clamps ring sizes below 64 instead of rejecting them, so the
    check that the requested minimum was applied is expected to fail on
    that driver. I could not run that part (it needs a remote endpoint),
    and whether the test or the driver should change is a separate
    question.

 .../selftests/drivers/net/ring_reconfig.py    | 45 +++++++++++++++++--
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/ring_reconfig.py b/tools/testing/selftests/drivers/net/ring_reconfig.py
index 6682d4badfb..c7491154d1d 100755
--- a/tools/testing/selftests/drivers/net/ring_reconfig.py
+++ b/tools/testing/selftests/drivers/net/ring_reconfig.py
@@ -5,6 +5,7 @@
 Test channel and ring size configuration via ethtool (-L / -G).
 """
 
+import errno
 import socket
 import struct
 import time
@@ -26,6 +27,16 @@ MIN_TX_RING = 32
 MAX_TX_RING = 1024
 
 
+def _channels_get_or_skip(cfg, ehdr):
+    """ Get the channel config, skip the test if the device has none. """
+    try:
+        return cfg.eth.channels_get(ehdr)
+    except NlError as e:
+        if e.error == errno.EOPNOTSUPP:
+            raise KsftSkipEx("device does not support channel config") from e
+        raise
+
+
 def channels(cfg) -> None:
     """
     Twiddle channel counts in various combinations of parameters.
@@ -33,7 +44,7 @@ def channels(cfg) -> None:
     if the config is accepted and crashes.
     """
     ehdr = {'header':{'dev-index': cfg.ifindex}}
-    chans = cfg.eth.channels_get(ehdr)
+    chans = _channels_get_or_skip(cfg, ehdr)
 
     all_keys = ["rx", "tx", "combined"]
     mixes = [{"combined"}, {"rx", "tx"}, {"rx", "combined"}, {"tx", "combined"},
@@ -47,21 +58,26 @@ def channels(cfg) -> None:
             supported.add(key)
             restore |= {key + "-count": chans[key + "-count"]}
 
-    defer(cfg.eth.channels_set, ehdr | restore)
+    restore_chans = defer(cfg.eth.channels_set, ehdr | restore)
 
     def test_config(config):
+        applied = False
         try:
             cfg.eth.channels_set(ehdr | config)
+            applied = True
             get = cfg.eth.channels_get(ehdr)
             for k, v in config.items():
                 ksft_eq(get.get(k, 0), v)
         except NlError as e:
             failed.append(mix)
+            if e.error == errno.EOPNOTSUPP and not applied:
+                unsupported.append(mix)
             ksft_pr("Can't set", config, e)
         else:
             ksft_pr("Okay", config)
 
     failed = []
+    unsupported = []
     for mix in mixes:
         if not mix.issubset(supported):
             continue
@@ -72,6 +88,13 @@ def channels(cfg) -> None:
             config[key + "-count"] = 1 if key in mix else 0
         test_config(config)
 
+    # Every config was rejected as unsupported, so nothing was changed:
+    # the device reports channels but can't change them
+    attempted = [m for m in mixes if m.issubset(supported)]
+    if attempted and unsupported == attempted:
+        restore_chans.cancel()
+        raise KsftSkipEx("device does not support changing channels")
+
     for mix in mixes:
         if not mix.issubset(supported):
             continue
@@ -88,7 +111,14 @@ def channels(cfg) -> None:
 def _configure_min_ring_cnt(cfg) -> None:
     """ Try to configure a single Rx/Tx ring. """
     ehdr = {'header':{'dev-index': cfg.ifindex}}
-    chans = cfg.eth.channels_get(ehdr)
+    try:
+        chans = cfg.eth.channels_get(ehdr)
+    except NlError as e:
+        # Reducing the channel count is only an optimization
+        if e.error == errno.EOPNOTSUPP:
+            ksft_pr("Channel configuration unsupported; continuing ring test")
+            return
+        raise
 
     all_keys = ["rx-count", "tx-count", "combined-count"]
     restore = {}
@@ -107,7 +137,14 @@ def _configure_min_ring_cnt(cfg) -> None:
         # looks like we're already on 1 channel
         return
 
-    cfg.eth.channels_set(ehdr | config)
+    try:
+        cfg.eth.channels_set(ehdr | config)
+    except NlError as e:
+        # Some drivers report channels but can't change them
+        if e.error == errno.EOPNOTSUPP:
+            ksft_pr("Channel configuration unsupported; continuing ring test")
+            return
+        raise
     defer(cfg.eth.channels_set, ehdr | restore)
 
 
-- 
2.56.0

[PATCH net v2 2/2] selftests: drv-net: ring_reconfig: skip tx stall test without channel config

[Bilal Yazıcıoğlu]

reconfig_tx_stall() needs at least two combined channels and already
skips devices which don't report enough of them. Two kinds of devices
make it fail instead:

 - drivers without the ethtool get_channels op (e.g. e1000e) fail the
   initial channels_get with -EOPNOTSUPP;
 - drivers with get_channels but no set_channels which report fewer
   active than maximum combined channels fail the attempt to enable a
   second one with -EOPNOTSUPP, and then the restore registered for it.

Skip the test in both cases. When the change is rejected with
-EOPNOTSUPP nothing was changed, so its restore is dropped. Other
netlink errors are still reported as failures, and the restore is
still registered before the change as before.

The issue was found through AI-assisted source code analysis
(OpenAI Codex) and confirmed on a ThinkPad T480 with an Intel I219-V
(e1000e, 8086:15d8). As for the previous patch, an earlier version
with the same channel query handling was run against that device
with a stub environment and ethtool set requests blocked, and the
test was skipped. The set_channels case was only checked against an
emulated ethtool family.

Fixes: c98610c2eb70 ("selftests: drv-net: Test queue stall upon reconfig")
Signed-off-by: Bilal Yazıcıoğlu <ahmetbilalyazicioglu@gmail.com>
---
 .../selftests/drivers/net/ring_reconfig.py    | 20 +++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/ring_reconfig.py b/tools/testing/selftests/drivers/net/ring_reconfig.py
index c7491154d1d..d6e84550d67 100755
--- a/tools/testing/selftests/drivers/net/ring_reconfig.py
+++ b/tools/testing/selftests/drivers/net/ring_reconfig.py
@@ -37,6 +37,18 @@ def _channels_get_or_skip(cfg, ehdr):
         raise
 
 
+def _channels_set_or_skip(cfg, req, restore):
+    """ Set the channel config, skip the test if the device can't. """
+    undo = defer(cfg.eth.channels_set, restore)
+    try:
+        cfg.eth.channels_set(req)
+    except NlError as e:
+        if e.error == errno.EOPNOTSUPP:
+            undo.cancel()
+            raise KsftSkipEx("device does not support changing channels") from e
+        raise
+
+
 def channels(cfg) -> None:
     """
     Twiddle channel counts in various combinations of parameters.
@@ -338,16 +350,16 @@ def reconfig_tx_stall(cfg) -> None:
     other_queue = 0
 
     ehdr = {'header': {'dev-index': cfg.ifindex}}
-    chans = cfg.eth.channels_get(ehdr)
+    chans = _channels_get_or_skip(cfg, ehdr)
 
     if "combined-max" not in chans:
         raise KsftSkipEx("device does not support combined channels")
     if chans.get("combined-max", 0) < 2:
         raise KsftSkipEx("device does not support 2+ combined channels")
     if chans["combined-count"] < 2:
-        defer(cfg.eth.channels_set,
-              ehdr | {"combined-count": chans["combined-count"]})
-        cfg.eth.channels_set(ehdr | {"combined-count": 2})
+        _channels_set_or_skip(
+            cfg, ehdr | {"combined-count": 2},
+            ehdr | {"combined-count": chans["combined-count"]})
 
     rings = cfg.eth.rings_get(ehdr)
     if 'rx' not in rings or 'tx' not in rings:
-- 
2.56.0