From: Mohsin Bashir 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 Reviewed-by: Maxime Chevallier --- 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