Hi Andrew, Thanks for the review comments. > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > > +def verify_link_up(ifname: str) -> None: > > + """Verify whether the link is up""" > > + with open(f"/sys/class/net/{ifname}/operstate", "r") as fp: > > + link_state = fp.read().strip() > > + > > + if link_state == "down": > > + raise KsftSkipEx(f"Link state of interface {ifname} is DOWN") > > + > > +def set_autonegotiation_state(ifname: str, state: str) -> None: > > + content = get_ethtool_content(ifname, "Supported link modes:") > > + speeds, duplex_modes = get_speed_duplex(content) > > + speed = speeds[0] > > + duplex = duplex_modes[0] > > + if not speed or not duplex: > > + KsftSkipEx("No speed or duplex modes found") > > + """Set the autonegotiation state for the interface""" > > + process = ethtool(f"-s {ifname} speed {speed} duplex {duplex} > > +autoneg {state}") > > > +def verify_autonegotiation(ifname: str, expected_state: str) -> None: > > + verify_link_up(ifname) > > + """Verifying the autonegotiation state""" > > + output = get_ethtool_content(ifname, "Auto-negotiation:") > > + actual_state = output[0] > > + > > + ksft_eq(actual_state, expected_state) > > + > > +def test_link_modes(cfg) -> None: > > + global common_link_modes > > + link_modes = get_ethtool_content(cfg.ifname, "Supported link modes:") > > + partner_link_modes = get_ethtool_content(cfg.ifname, "Link > > +partner advertised link modes:") > > + > > + if link_modes and partner_link_modes: > > + for idx1 in range(len(link_modes)): > > + for idx2 in range(len(partner_link_modes)): > > + if link_modes[idx1] == partner_link_modes[idx2]: > > + common_link_modes.append(link_modes[idx1]) > > + break > > + else: > > + raise KsftFailEx("No link modes available") > > + > > +def test_autonegotiation(cfg) -> None: > > + autoneg = get_ethtool_content(cfg.ifname, "Supports auto- > negotiation:") > > + if autoneg[0] == "Yes": > > + for state in ["off", "on"]: > > + set_autonegotiation_state(cfg.ifname, state) > > + time.sleep(sleep_time) > > + verify_autonegotiation(cfg.ifname, state) > > If i'm understanding this correctly, you test with autoneg off, and expect the > link to come up. That only works reliably if the link peer also has autoneg off, > and is using the same speed/duplex. > > What i guess is happening in your test setup is that the link peer is failing > autoneg and defaulting to 10/Half. But i don't think that is guaranteed by > 802.3. There are also a small number of devices which no longer support > 10/Half, they are likely to default to something higher. This is especially true > for datacenter NICs, they may start at 10G and go up from there. > > So i don't think this is a valid test. To really test autoneg off, you need to > configure both ends of the link. I will change the implementation to configure both the ends of the link appropriately in the next version.