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:
 <DM6PR11MB4236FE8CEF8EC610B2525EF283622@DM6PR11MB4236.namprd11.prod.outlook.com>
Date: Wed, 18 Sep 2024 10:32:40 +0000
From: <Mohan.Prasad@...rochip.com>
To: <andrew@...n.ch>
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

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.