[PATCH net v3] selftests: drv-net: pause: Avoid cleanup failures for unsupported link settings

[PATCH net v3] selftests: drv-net: pause: Avoid cleanup failures for unsupported link settings

[Mohsin Bashir]

From: Mohsin Bashir <hmohsin@meta.com>

pause_setup() saves the initial link settings before tests run. Devices
that report link settings but do not support changing them can reject even
a no-op restore with EOPNOTSUPP. That turns pause tests that use
pause_setup() into cleanup failures.

Fix this by queuing link-setting restores only after a link-setting
request has succeeded. Keep pause_test_support coverage for devices that
cannot change link settings by falling back to forced MAC pause on the
current link. Run the extra link autoneg on/off checks only when those
link setting requests are accepted. Tests that force link autoneg off,
including pause_autoneg_link_autoneg(), now queue their own autoneg-on
restore after the force request succeeds.

Results on fbnic:

  Before: # Totals: pass:0 fail:4 xfail:0 xpass:0 skip:24 error:0
  After:  # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:24 error:0

Fixes: cbd8a536cd6d ("selftests: drv-net: Introduce a selftest for ethtool flow control")
Signed-off-by: Mohsin Bashir <hmohsin@meta.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
Changelog:
v3:
- Drop the remaining unconditional autoneg-on restore from pause_setup()
  and only queue link-setting restores after a link-setting request succeeds.
- Since pause_setup() no longer restores autoneg on, make pause_test_support()
  and pause_autoneg_link_autoneg() queue their own autoneg-on restores after
  they successfully force the link.

v2: https://lore.kernel.org/netdev/20261103190622.3981970-1-mohsin.bashr@gmail.com
v1: https://lore.kernel.org/netdev/20261030173516.1002250-1-mohsin.bashr@gmail.com
---
 .../testing/selftests/drivers/net/hw/pause.py | 56 ++++++++++---------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/pause.py b/tools/testing/selftests/drivers/net/hw/pause.py
index ca4a55e59c30..c3398c7452e0 100755
--- a/tools/testing/selftests/drivers/net/hw/pause.py
+++ b/tools/testing/selftests/drivers/net/hw/pause.py
@@ -402,16 +402,13 @@ def pause_setup(cfg):
     if "auto-negotiation" not in link:
         raise KsftSkipEx(f"{cfg.ifname} doesn't report its link modes")
 
-    if link["auto-negotiation"]:
-        defer(cmd, f"ethtool -s {cfg.ifname} autoneg on")
-    elif "speed" in link and "duplex" in link:
-        defer(cmd, f"ethtool -s {cfg.ifname} autoneg off speed {link['speed']} "
-                   f"duplex {link['duplex'].lower()}")
-
     # Local interface admin up, link aneg on
     ip(f"link set {cfg.ifname} up")
     if link["supports-auto-negotiation"] and not link["auto-negotiation"]:
         ethtool(f"-s {cfg.ifname} autoneg on")
+        if "speed" in link and "duplex" in link:
+            defer(cmd, f"ethtool -s {cfg.ifname} autoneg off speed {link['speed']} "
+                       f"duplex {link['duplex'].lower()}")
 
     # Get remote pause params
     ret, params = ethtool_ret(f"-a {cfg.remote_ifname}", host=cfg.remote)
@@ -426,18 +423,14 @@ def pause_setup(cfg):
     if "auto-negotiation" not in link:
         raise KsftSkipEx(f"{cfg.remote_ifname} doesn't report its link modes")
 
-    if link["auto-negotiation"]:
-        defer(cmd, f"ethtool -s {cfg.remote_ifname} autoneg on",
-              host=cfg.remote)
-    elif "speed" in link and "duplex" in link:
-        defer(cmd, f"ethtool -s {cfg.remote_ifname} autoneg off "
-                   f"speed {link['speed']} duplex {link['duplex'].lower()}",
-                   host=cfg.remote)
-
     # Remote interface admin up, link aneg on
     ip(f"link set {cfg.remote_ifname} up", host=cfg.remote)
     if link["supports-auto-negotiation"] and not link["auto-negotiation"]:
         ethtool(f"-s {cfg.remote_ifname} autoneg on", host=cfg.remote)
+        if "speed" in link and "duplex" in link:
+            defer(cmd, f"ethtool -s {cfg.remote_ifname} autoneg off "
+                       f"speed {link['speed']} duplex {link['duplex'].lower()}",
+                       host=cfg.remote)
 
     wait_for_link(cfg)
 
@@ -470,9 +463,12 @@ def pause_test_support(cfg, pauseparams):
 
     forced = forced_link_settings(cfg)
     _, supported = get_local_pause_supported(cfg)
+    link_config_tested = False
 
     # We check that what we can configure in the pause params matches what we
-    # support under various contditions : Link aneg on/off, pause aneg on/off
+    # support under various conditions: link aneg on/off, and pause aneg
+    # on/off. If no link configuration change is supported, fall back to
+    # checking forced MAC pause on the current link.
     ret, _ = ethtool_ret(f"-s {cfg.ifname} autoneg on", is_get = False)
     if ret != 0:
         ksft_pr("link autoneg on refused, not tested")
@@ -481,18 +477,23 @@ def pause_test_support(cfg, pauseparams):
                                "link autoneg on")
         expect_pauseparams_set(cfg, pauseparams, True, supported,
                                "link autoneg on")
+        link_config_tested = True
 
     if not forced:
         ksft_pr("link speed unknown, the forced link is not tested")
-        return
-
-    ret, _ = ethtool_ret(f"-s {cfg.ifname} autoneg off {forced}", is_get = False)
-    if ret != 0:
-        ksft_pr(f"link autoneg off {forced} refused, not tested")
-        return
-
-    expect_pauseparams_set(cfg, pauseparams, False, supported, "link autoneg off")
-    expect_pauseparams_set(cfg, pauseparams, True, supported, "link autoneg off")
+    else:
+        ret, _ = ethtool_ret(f"-s {cfg.ifname} autoneg off {forced}", is_get = False)
+        if ret != 0:
+            ksft_pr(f"link autoneg off {forced} refused, not tested")
+        else:
+            if link_config_tested:
+                defer(cmd, f"ethtool -s {cfg.ifname} autoneg on")
+            expect_pauseparams_set(cfg, pauseparams, False, supported, "link autoneg off")
+            expect_pauseparams_set(cfg, pauseparams, True, supported, "link autoneg off")
+            link_config_tested = True
+
+    if not link_config_tested:
+        expect_pauseparams_set(cfg, pauseparams, False, supported, "current link")
 
 @ksft_variants(pauseparams_variants)
 @ksft_disruptive
@@ -902,11 +903,14 @@ def pause_autoneg_link_autoneg(cfg):
                          is_get=False, host=cfg.remote)
     if ret != 0:
         raise KsftSkipEx(f"Can't force the peer's link at {forced}")
+    remote_restore = defer(cmd, f"ethtool -s {cfg.remote_ifname} autoneg on",
+                           host=cfg.remote)
 
     ret, _ = ethtool_ret(f"-s {cfg.ifname} autoneg off {forced}",
                          is_get=False)
     if ret != 0:
         raise KsftSkipEx(f"Can't force the link at {forced}")
+    local_restore = defer(cmd, f"ethtool -s {cfg.ifname} autoneg on")
 
     try:
         wait_for_aneg(cfg)
@@ -917,8 +921,8 @@ def pause_autoneg_link_autoneg(cfg):
     check_local_pauseparams(cfg, True, rx, tx)
 
     # Re-enable autoneg
-    ethtool(f"-s {cfg.remote_ifname} autoneg on", host=cfg.remote)
-    ethtool(f"-s {cfg.ifname} autoneg on")
+    remote_restore.exec()
+    local_restore.exec()
     wait_for_aneg(cfg)
 
     # Pause autoneg must still be on
-- 
2.53.0-Meta