linux-kernel - RE: [PATCH net-next v2 2/3] selftests: nic_basic_tests: Add selftest case for speed and duplex state checks

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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <DM6PR11MB42366C3098D9DB2C5319B99C836C2@DM6PR11MB4236.namprd11.prod.outlook.com>
Date: Fri, 20 Sep 2024 04:56:57 +0000
From: <Mohan.Prasad@...rochip.com>
To: <willemdebruijn.kernel@...il.com>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
	<andrew@...n.ch>, <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 2/3] selftests: nic_basic_tests: Add selftest
 case for speed and duplex state checks

Hi Willem,

Thanks for the review comments.

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Mohan Prasad J wrote:
> > Add selftest case for testing the speed and duplex state of local NIC
> > driver and the partner based on the supported link modes obtained from
> > the ethtool. Speed and duplex states are varied and verified using
> > ethtool.
> >
> > Signed-off-by: Mohan Prasad J <mohan.prasad@...rochip.com>
> > ---
> >  .../drivers/net/hw/nic_basic_tests.py         | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >
> > diff --git a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > index 27f780032..ff46f2406 100644
> > --- a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > +++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > @@ -42,6 +42,14 @@ from lib.py import ethtool  """Global variables"""
> >  common_link_modes = []
> >
> > +def check_autonegotiation(ifname: str) -> None:
> > +    autoneg = get_ethtool_content(ifname, "Supports auto-negotiation:")
> > +    partner_autoneg = get_ethtool_content(ifname, "Link partner
> > +advertised auto-negotiation:")
> > +
> > +    """Check if auto-neg supported by local and partner NIC"""
> > +    if autoneg[0] != "Yes" or partner_autoneg[0] != "Yes":
> > +        raise KsftSkipEx(f"Interface {ifname} or partner does not
> > + support auto-negotiation")
> > +
> >  def get_ethtool_content(ifname: str, field: str):
> >      capture = False
> >      content = []
> > @@ -112,6 +120,25 @@ def verify_autonegotiation(ifname: str,
> expected_state: str) -> None:
> >
> >      ksft_eq(actual_state, expected_state)
> >
> > +def set_speed_and_duplex(ifname: str, speed: str, duplex: str) -> None:
> > +    """Set the speed and duplex state for the interface"""
> > +    process = ethtool(f"--change {ifname} speed {speed} duplex
> > +{duplex} autoneg on")
> > +
> > +    if process.ret != 0:
> > +        raise KsftFailEx(f"Not able to set speed and duplex parameters for
> {ifname}")
> > +    ksft_pr(f"Speed: {speed} Mbps, Duplex: {duplex} set for
> > + Interface: {ifname}")
> > +
> > +def verify_speed_and_duplex(ifname: str, expected_speed: str,
> expected_duplex: str) -> None:
> > +    verify_link_up(ifname)
> > +    """Verifying the speed and duplex state for the interface"""
> > +    with open(f"/sys/class/net/{ifname}/speed", "r") as fp:
> > +        actual_speed = fp.read().strip()
> > +    with open(f"/sys/class/net/{ifname}/duplex", "r") as fp:
> > +        actual_duplex = fp.read().strip()
> > +
> > +    ksft_eq(actual_speed, expected_speed)
> > +    ksft_eq(actual_duplex, expected_duplex)
> > +
> >  def test_link_modes(cfg) -> None:
> >      global common_link_modes
> >      link_modes = get_ethtool_content(cfg.ifname, "Supported link
> > modes:") @@ -136,6 +163,25 @@ def test_autonegotiation(cfg) -> None:
> >      else:
> >          raise KsftSkipEx(f"Auto-Negotiation is not supported for
> > interface {cfg.ifname}")
> >
> > +def test_network_speed(cfg) -> None:
> > +    check_autonegotiation(cfg.ifname)
> > +    if not common_link_modes:
> > +        KsftSkipEx("No common link modes exist")
> > +    speeds, duplex_modes = get_speed_duplex(common_link_modes)
> > +
> > +    if speeds and duplex_modes and len(speeds) == len(duplex_modes):
> > +        for idx in range(len(speeds)):
> > +            speed = speeds[idx]
> > +            duplex = duplex_modes[idx]
> > +            set_speed_and_duplex(cfg.ifname, speed, duplex)
> > +            time.sleep(sleep_time)
> > +            verify_speed_and_duplex(cfg.ifname, speed, duplex)
> > +    else:
> > +        if not speeds or not duplex_modes:
> > +            KsftSkipEx(f"No supported speeds or duplex modes found for
> interface {cfg.ifname}")
> > +        else:
> > +            KsftSkipEx("Mismatch in the number of speeds and duplex
> > + modes")
> > +
> 
> Do these tests reset configuration to their original state?

Currently these tests do not reset configuration to their original state.
However resetting would be a good idea, I will either do it here (or) at the end of the tests.

> 
> More high level: basic test is not very descriptive. Can they have a more
> precise name? Perhaps link layer operations or link layer config?

I will have a check on the naming and update it accordingly.