Hi Brett,
Thank you for the review comments.
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On 9/3/2024 3:15 PM, Mohan Prasad J wrote:
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > Add testcase for checking speed and duplex states for lan743x network
> > driver.
> > Testcase comprises of varying the network speed and duplex state to
> > 10/100/1000Mbps and half/full via ethtool.
> >
> > Signed-off-by: Mohan Prasad J <mohan.prasad@xxxxxxxxxxxxx>
> > ---
> > .../net/hw/microchip/lan743x/lan743x.py | 33 +++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git
> > a/tools/testing/selftests/drivers/net/hw/microchip/lan743x/lan743x.py
> > b/tools/testing/selftests/drivers/net/hw/microchip/lan743x/lan743x.py
> > index f1ad97dc2..59f0be2a7 100755
> > ---
> > a/tools/testing/selftests/drivers/net/hw/microchip/lan743x/lan743x.py
> > +++ b/tools/testing/selftests/drivers/net/hw/microchip/lan743x/lan743x
> > +++ .py
> > @@ -36,12 +36,45 @@ def verify_autonegotiation(ifname: str,
> expected_state: str) -> None:
> > actual_state = autoneg_match.group(1)
> > 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 speeed 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"""
> > + output = ethtool(f"{ifname}")
>
> How does "output" get used here?
"output" is not used anywhere, I will remove this in the next version.
>
> > + 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()
>
> Should you check speed/duplex from both sysfs and ethtool? Maybe that's
> overkill.
I will remove the ethtool dependency here in the next version.
>
> Thanks,
>
> Brett
>
> > +
> > + ksft_eq(actual_speed, expected_speed)
> > + ksft_eq(actual_duplex, expected_duplex)
> > +
> > def test_autonegotiation(cfg) -> None:
> > for state in ["off", "on"]:
> > set_autonegotiation_state(cfg.ifname, state)
> > time.sleep(5)
> > verify_autonegotiation(cfg.ifname, state)
> >
> > +def test_network_speed(cfg) -> None:
> > + speeds = ["10", "100", "1000"]
> > + duplex_modes = ["half", "full"]
> > +
> > + for speed in speeds:
> > + for duplex in duplex_modes:
> > + # Skip for speed = 1000Mbps, duplex = Half
> > + if speed == "1000" and duplex == "half":
> > + continue
> > + set_speed_and_duplex(cfg.ifname, speed, duplex)
> > + time.sleep(5)
> > + verify_speed_and_duplex(cfg.ifname, speed, duplex)
> > + > def main() -> None:
> > with NetDrvEpEnv(__file__) as cfg:
> > ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg,))
> > --
> > 2.43.0
> >
