linux-kernel - Re: [PATCH net-next v2 1/3] selftests: nic_basic_tests: Add selftest file for basic tests of NIC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0dd4130e-b06e-42e1-8f36-2589c18c4762@lunn.ch>
Date: Tue, 17 Sep 2024 17:48:08 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Mohan Prasad J <mohan.prasad@...rochip.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
	edumazet@...gle.com, pabeni@...hat.com, shuah@...nel.org,
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
	horms@...nel.org, brett.creeley@....com, rosenp@...il.com,
	UNGLinuxDriver@...rochip.com, willemb@...gle.com
Subject: Re: [PATCH net-next v2 1/3] selftests: nic_basic_tests: Add selftest
 file for basic tests of NIC

> +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.

	Andrew