[PATCH net-next v2 0/3] nic_basic_tests: Add selftest for doing basic tests of NIC driver

[PATCH net-next v2 0/3] nic_basic_tests: Add selftest for doing basic tests of NIC driver

[Mohan Prasad J]

The series of patches are for doing basic tests of NIC driver.
Test comprises checks for auto-negotiation, speed,
duplex state and throughput between local NIC and partner.
Tools such as ethtool, iperf3 are used.

Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
---
Changes in v2:
- Changed the hardcoded implementation of speed, duplex states,
  throughput to generic values, in order to support all type
  of NIC drivers.
- Test executes based on the supported link modes between local
  NIC driver and partner.
- Instead of lan743x directory, selftest file is now placed in
  /selftests/drivers/net/hw.
---
Mohan Prasad J (3):
  selftests: nic_basic_tests: Add selftest file for basic tests of NIC
  selftests: nic_basic_tests: Add selftest case for speed and duplex
    state checks
  selftests: nic_basic_tests: Add selftest case for throughput check

 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../drivers/net/hw/nic_basic_tests.py         | 230 ++++++++++++++++++
 2 files changed, 231 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/net/hw/nic_basic_tests.py

-- 
2.43.0

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

[Mohan Prasad J]

Add selftest file to test basic features of a NIC driver.
Tests for link modes, auto-negotiation are placed.
Selftest makes use of ksft modules and ethtool.
Add selftest file in the Makefile.

Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
---
 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../drivers/net/hw/nic_basic_tests.py         | 145 ++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/net/hw/nic_basic_tests.py

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index c9f2f48fc..9f105227c 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -10,6 +10,7 @@ TEST_PROGS = \
 	hw_stats_l3.sh \
 	hw_stats_l3_gre.sh \
 	loopback.sh \
+	nic_basic_tests.py \
 	pp_alloc_fail.py \
 	rss_ctx.py \
 	#
diff --git a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
new file mode 100644
index 000000000..27f780032
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
@@ -0,0 +1,145 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+#Introduction:
+#This file has basic tests for generic NIC drivers.
+#The test comprises of auto-negotiation, speed and duplex checks.
+#Also has tests to check the throughput
+#
+#Setup:
+#Connect the DUT PC with NIC card to partner pc back via ethernet medium of your choice(RJ45, T1)
+#
+#        DUT PC                                              Partner PC
+#┌───────────────────────┐                         ┌──────────────────────────┐
+#│                       │                         │                          │
+#│                       │                         │                          │
+#│           ┌───────────┐                         │                          │
+#│           │DUT NIC    │         Eth             │                          │
+#│           │Interface ─┼─────────────────────────┼─    any eth Interface    │
+#│           └───────────┘                         │                          │
+#│                       │                         │                          │
+#│                       │                         │                          │
+#└───────────────────────┘                         └──────────────────────────┘
+#
+#Configurations:
+# Change the below configuration based on your hw needs.
+# """Default values"""
+sleep_time = 5 #time taken to wait for transitions to happen, in seconds.
+test_duration = 5  #performance test duration for the throughput check, in seconds.
+throughput_threshold = 0.8 #percentage of throughput required to pass the throughput
+
+import time
+import os
+import re
+import configparser
+import json
+from lib.py import ksft_run, ksft_exit, ksft_pr, ksft_eq
+from lib.py import KsftFailEx, KsftSkipEx
+from lib.py import NetDrvEpEnv
+from lib.py import cmd
+from lib.py import ethtool
+
+"""Global variables"""
+common_link_modes = []
+
+def get_ethtool_content(ifname: str, field: str):
+    capture = False
+    content = []
+
+    """Get the ethtool content for the interface"""
+    process = ethtool(f"{ifname}")
+    if process.ret != 0:
+        raise KsftSkipEx(f"Error while getting the ethtool content for interface {ifname}")
+    lines = process.stdout.splitlines()
+
+    """Retrieve the content of the field"""
+    for line in lines:
+        if field in line:
+            capture = True
+            data = line.split(":")[1].strip()
+            content.extend(data.split())
+            continue
+
+        if capture:
+            if ":" in line:
+                break;
+            if line.strip():
+                content.extend(line.strip().split())
+    if capture == False:
+        raise KsftSkipEx(f"Field \"{field}\" not found in ethtool output")
+    return content
+
+def get_speed_duplex(content):
+    speed = []
+    duplex = []
+    """Check the link modes"""
+    for data in content:
+        parts = data.split('/')
+        speed_value = re.match(r'\d+', parts[0])
+        if speed_value:
+            speed.append(speed_value.group())
+        else:
+            raise KsftSkipEx(f"No speed value found for interface {ifname}")
+        duplex.append(parts[1].lower())
+    return speed, duplex
+
+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}")
+    if process.ret != 0:
+        raise KsftFailEx(f"Not able to set autoneg parameter for {ifname}")
+    ksft_pr(f"Autoneg set as {state} for {ifname}")
+
+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)
+    else:
+        raise KsftSkipEx(f"Auto-Negotiation is not supported for interface {cfg.ifname}")
+
+def main() -> None:
+    with NetDrvEpEnv(__file__) as cfg:
+        ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg,))
+    ksft_exit()
+
+if __name__ == "__main__":
+    main()
-- 
2.43.0

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

[Andrew Lunn]

On Tue, Sep 17, 2024 at 08:04:07AM +0530, Mohan Prasad J wrote:
> Add selftest file to test basic features of a NIC driver.
> Tests for link modes, auto-negotiation are placed.
> Selftest makes use of ksft modules and ethtool.
> Add selftest file in the Makefile.

Thanks for reworking this.

> +++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> @@ -0,0 +1,145 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +#Introduction:
> +#This file has basic tests for generic NIC drivers.
> +#The test comprises of auto-negotiation, speed and duplex checks.
> +#Also has tests to check the throughput
> +#
> +#Setup:
> +#Connect the DUT PC with NIC card to partner pc back via ethernet medium of your choice(RJ45, T1)
> +#
> +#        DUT PC                                              Partner PC
> +#┌───────────────────────┐                         ┌──────────────────────────┐
> +#│                       │                         │                          │
> +#│                       │                         │                          │
> +#│           ┌───────────┐                         │                          │
> +#│           │DUT NIC    │         Eth             │                          │
> +#│           │Interface ─┼─────────────────────────┼─    any eth Interface    │
> +#│           └───────────┘                         │                          │
> +#│                       │                         │                          │
> +#│                       │                         │                          │
> +#└───────────────────────┘                         └──────────────────────────┘
> +#
> +#Configurations:
> +# Change the below configuration based on your hw needs.
> +# """Default values"""
> +sleep_time = 5 #time taken to wait for transitions to happen, in seconds.
> +test_duration = 5  #performance test duration for the throughput check, in seconds.
> +throughput_threshold = 0.8 #percentage of throughput required to pass the throughput
> +
> +import time
> +import os
> +import re
> +import configparser
> +import json
> +from lib.py import ksft_run, ksft_exit, ksft_pr, ksft_eq
> +from lib.py import KsftFailEx, KsftSkipEx
> +from lib.py import NetDrvEpEnv
> +from lib.py import cmd
> +from lib.py import ethtool
> +
> +"""Global variables"""
> +common_link_modes = []
> +
> +def get_ethtool_content(ifname: str, field: str):
> +    capture = False
> +    content = []
> +
> +    """Get the ethtool content for the interface"""
> +    process = ethtool(f"{ifname}")
> +    if process.ret != 0:
> +        raise KsftSkipEx(f"Error while getting the ethtool content for interface {ifname}")
> +    lines = process.stdout.splitlines()
> +
> +    """Retrieve the content of the field"""
> +    for line in lines:
> +        if field in line:
> +            capture = True
> +            data = line.split(":")[1].strip()
> +            content.extend(data.split())
> +            continue

Since you have batteries included python:

ethtool --json enp2s0
[sudo] password for andrew: 
[ {
        "ifname": "enp2s0",
        "supported-ports": [ "TP","MII" ],
        "supported-link-modes": [ "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/Full" ],
        "supported-pause-frame-use": "Symmetric Receive-only",
        "supports-auto-negotiation": true,
        "supported-fec-modes": [ ],
        "advertised-link-modes": [ "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/Full" ],
        "advertised-pause-frame-use": "Symmetric Receive-only",
        "advertised-auto-negotiation": true,
        "advertised-fec-modes": [ ],
        "auto-negotiation": false,
        "master-slave-cfg": "preferred slave",
        "master-slave-status": "unknown",
        "port": "Twisted Pair",
        "phyad": 0,
        "transceiver": "external",
        "supports-wake-on": "pumbg",
        "wake-on": "d",
        "link-detected": false
    } ]

You can use a json library to do all the parsing for you.

> +def get_speed_duplex(content):
> +    speed = []
> +    duplex = []
> +    """Check the link modes"""
> +    for data in content:
> +        parts = data.split('/')
> +        speed_value = re.match(r'\d+', parts[0])
> +        if speed_value:
> +            speed.append(speed_value.group())
> +        else:
> +            raise KsftSkipEx(f"No speed value found for interface {ifname}")
> +        duplex.append(parts[1].lower())
> +    return speed, duplex
> +
> +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}")
> +    if process.ret != 0:
> +        raise KsftFailEx(f"Not able to set autoneg parameter for {ifname}")
> +    ksft_pr(f"Autoneg set as {state} for {ifname}")
> +
> +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)

One thing you could do here is look at "advertised-link-modes". You
would expect it to list a single mode, matching the speed/duplex
you requested.

	Andrew

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

[Mohan.Prasad]

Hello Andrew,

Thanks for the review comments.

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Tue, Sep 17, 2024 at 08:04:07AM +0530, Mohan Prasad J wrote:
> > Add selftest file to test basic features of a NIC driver.
> > Tests for link modes, auto-negotiation are placed.
> > Selftest makes use of ksft modules and ethtool.
> > Add selftest file in the Makefile.
> 
> Thanks for reworking this.
> 
> > +++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > @@ -0,0 +1,145 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +#Introduction:
> > +#This file has basic tests for generic NIC drivers.
> > +#The test comprises of auto-negotiation, speed and duplex checks.
> > +#Also has tests to check the throughput #
> > +#Setup:
> > +#Connect the DUT PC with NIC card to partner pc back via ethernet
> > +medium of your choice(RJ45, T1) #
> > +#        DUT PC                                              Partner PC
> > +#┌───────────────────────┐
> ┌──────────────────────────┐
> > +#│                       │                         │                          │
> > +#│                       │                         │                          │
> > +#│           ┌───────────┐                         │                          │
> > +#│           │DUT NIC    │         Eth             │                          │
> > +#│           │Interface ─┼─────────────────────────┼─    any eth
> Interface    │
> > +#│           └───────────┘                         │                          │
> > +#│                       │                         │                          │
> > +#│                       │                         │                          │
> > +#└───────────────────────┘
> └──────────────────────────┘
> > +#
> > +#Configurations:
> > +# Change the below configuration based on your hw needs.
> > +# """Default values"""
> > +sleep_time = 5 #time taken to wait for transitions to happen, in seconds.
> > +test_duration = 5  #performance test duration for the throughput check, in
> seconds.
> > +throughput_threshold = 0.8 #percentage of throughput required to pass
> > +the throughput
> > +
> > +import time
> > +import os
> > +import re
> > +import configparser
> > +import json
> > +from lib.py import ksft_run, ksft_exit, ksft_pr, ksft_eq from lib.py
> > +import KsftFailEx, KsftSkipEx from lib.py import NetDrvEpEnv from
> > +lib.py import cmd from lib.py import ethtool
> > +
> > +"""Global variables"""
> > +common_link_modes = []
> > +
> > +def get_ethtool_content(ifname: str, field: str):
> > +    capture = False
> > +    content = []
> > +
> > +    """Get the ethtool content for the interface"""
> > +    process = ethtool(f"{ifname}")
> > +    if process.ret != 0:
> > +        raise KsftSkipEx(f"Error while getting the ethtool content for interface
> {ifname}")
> > +    lines = process.stdout.splitlines()
> > +
> > +    """Retrieve the content of the field"""
> > +    for line in lines:
> > +        if field in line:
> > +            capture = True
> > +            data = line.split(":")[1].strip()
> > +            content.extend(data.split())
> > +            continue
> 
> Since you have batteries included python:
> 
> ethtool --json enp2s0
> [sudo] password for andrew:
> [ {
>         "ifname": "enp2s0",
>         "supported-ports": [ "TP","MII" ],
>         "supported-link-modes": [
> "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/
> Full" ],
>         "supported-pause-frame-use": "Symmetric Receive-only",
>         "supports-auto-negotiation": true,
>         "supported-fec-modes": [ ],
>         "advertised-link-modes": [
> "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/
> Full" ],
>         "advertised-pause-frame-use": "Symmetric Receive-only",
>         "advertised-auto-negotiation": true,
>         "advertised-fec-modes": [ ],
>         "auto-negotiation": false,
>         "master-slave-cfg": "preferred slave",
>         "master-slave-status": "unknown",
>         "port": "Twisted Pair",
>         "phyad": 0,
>         "transceiver": "external",
>         "supports-wake-on": "pumbg",
>         "wake-on": "d",
>         "link-detected": false
>     } ]
> 
> You can use a json library to do all the parsing for you.

I tried running the --json option with the ethtool ("ethtool --json enp9s0"), however I am not getting the above output.
Instead it always throws "ethtool: bad command line argument(s)"
I am figuring out what might be missing (or any suggestions would be helpful).

> 
> > +def get_speed_duplex(content):
> > +    speed = []
> > +    duplex = []
> > +    """Check the link modes"""
> > +    for data in content:
> > +        parts = data.split('/')
> > +        speed_value = re.match(r'\d+', parts[0])
> > +        if speed_value:
> > +            speed.append(speed_value.group())
> > +        else:
> > +            raise KsftSkipEx(f"No speed value found for interface {ifname}")
> > +        duplex.append(parts[1].lower())
> > +    return speed, duplex
> > +
> > +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}")
> > +    if process.ret != 0:
> > +        raise KsftFailEx(f"Not able to set autoneg parameter for {ifname}")
> > +    ksft_pr(f"Autoneg set as {state} for {ifname}")
> > +
> > +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)
> 
> One thing you could do here is look at "advertised-link-modes". You would
> expect it to list a single mode, matching the speed/duplex you requested.

I will change it in the next version.

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

[Andrew Lunn]

> > Since you have batteries included python:
> > 
> > ethtool --json enp2s0
> > [sudo] password for andrew:
> > [ {
> >         "ifname": "enp2s0",
> >         "supported-ports": [ "TP","MII" ],
> >         "supported-link-modes": [
> > "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/
> > Full" ],
> >         "supported-pause-frame-use": "Symmetric Receive-only",
> >         "supports-auto-negotiation": true,
> >         "supported-fec-modes": [ ],
> >         "advertised-link-modes": [
> > "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/
> > Full" ],
> >         "advertised-pause-frame-use": "Symmetric Receive-only",
> >         "advertised-auto-negotiation": true,
> >         "advertised-fec-modes": [ ],
> >         "auto-negotiation": false,
> >         "master-slave-cfg": "preferred slave",
> >         "master-slave-status": "unknown",
> >         "port": "Twisted Pair",
> >         "phyad": 0,
> >         "transceiver": "external",
> >         "supports-wake-on": "pumbg",
> >         "wake-on": "d",
> >         "link-detected": false
> >     } ]
> > 
> > You can use a json library to do all the parsing for you.
> 
> I tried running the --json option with the ethtool ("ethtool --json enp9s0"), however I am not getting the above output.
> Instead it always throws "ethtool: bad command line argument(s)"
> I am figuring out what might be missing (or any suggestions would be helpful).

Are you using real ethtool, or busybox? What version of ethtool? I'm
using 6.10, but it looks like JSON support was added somewhere around
5.10.

	Andrew

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

[Mohan.Prasad]

Hello Andrew,

Thank you for the suggestion.

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> > > Since you have batteries included python:
> > >
> > > ethtool --json enp2s0
> > > [sudo] password for andrew:
> > > [ {
> > >         "ifname": "enp2s0",
> > >         "supported-ports": [ "TP","MII" ],
> > >         "supported-link-modes": [
> > > "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000b
> > > aseT/
> > > Full" ],
> > >         "supported-pause-frame-use": "Symmetric Receive-only",
> > >         "supports-auto-negotiation": true,
> > >         "supported-fec-modes": [ ],
> > >         "advertised-link-modes": [
> > > "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000b
> > > aseT/
> > > Full" ],
> > >         "advertised-pause-frame-use": "Symmetric Receive-only",
> > >         "advertised-auto-negotiation": true,
> > >         "advertised-fec-modes": [ ],
> > >         "auto-negotiation": false,
> > >         "master-slave-cfg": "preferred slave",
> > >         "master-slave-status": "unknown",
> > >         "port": "Twisted Pair",
> > >         "phyad": 0,
> > >         "transceiver": "external",
> > >         "supports-wake-on": "pumbg",
> > >         "wake-on": "d",
> > >         "link-detected": false
> > >     } ]
> > >
> > > You can use a json library to do all the parsing for you.
> >
> > I tried running the --json option with the ethtool ("ethtool --json enp9s0"),
> however I am not getting the above output.
> > Instead it always throws "ethtool: bad command line argument(s)"
> > I am figuring out what might be missing (or any suggestions would be
> helpful).
> 
> Are you using real ethtool, or busybox? What version of ethtool? I'm using
> 6.10, but it looks like JSON support was added somewhere around 5.10.

I have been using ethtool 6.7, updating to ethtool 6.10 solved the problem.

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

[Andrew Lunn]

On Thu, Sep 19, 2024 at 10:44:11AM +0000, Mohan.Prasad@microchip.com wrote:
> Hello Andrew,
> 
> Thank you for the suggestion.
> 
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> > 
> > > > Since you have batteries included python:
> > > >
> > > > ethtool --json enp2s0
> > > > [sudo] password for andrew:
> > > > [ {
> > > >         "ifname": "enp2s0",
> > > >         "supported-ports": [ "TP","MII" ],
> > > >         "supported-link-modes": [
> > > > "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000b
> > > > aseT/
> > > > Full" ],
> > > >         "supported-pause-frame-use": "Symmetric Receive-only",
> > > >         "supports-auto-negotiation": true,
> > > >         "supported-fec-modes": [ ],
> > > >         "advertised-link-modes": [
> > > > "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000b
> > > > aseT/
> > > > Full" ],
> > > >         "advertised-pause-frame-use": "Symmetric Receive-only",
> > > >         "advertised-auto-negotiation": true,
> > > >         "advertised-fec-modes": [ ],
> > > >         "auto-negotiation": false,
> > > >         "master-slave-cfg": "preferred slave",
> > > >         "master-slave-status": "unknown",
> > > >         "port": "Twisted Pair",
> > > >         "phyad": 0,
> > > >         "transceiver": "external",
> > > >         "supports-wake-on": "pumbg",
> > > >         "wake-on": "d",
> > > >         "link-detected": false
> > > >     } ]
> > > >
> > > > You can use a json library to do all the parsing for you.
> > >
> > > I tried running the --json option with the ethtool ("ethtool --json enp9s0"),
> > however I am not getting the above output.
> > > Instead it always throws "ethtool: bad command line argument(s)"
> > > I am figuring out what might be missing (or any suggestions would be
> > helpful).
> > 
> > Are you using real ethtool, or busybox? What version of ethtool? I'm using
> > 6.10, but it looks like JSON support was added somewhere around 5.10.
> 
> I have been using ethtool 6.7, updating to ethtool 6.10 solved the problem.

It would be good to gracefully handle this. Have the test fail with a
human readable error indicating ethtool is too old, rather than just
throwing an exception etc.

Digging through the git history, it seems like 6.10 was actually the
first version that supported this:

commit bd1341cd2146bfb89e1239546299102339acbf4d
Author: Fabian Pfitzner <f.pfitzner@pengutronix.de>
Date:   Fri Jul 19 10:55:44 2024 +0200

    add json support for base command
    
    Most subcommands already implement json support for their output. The
    base command (without supplying any subcommand) still lacks this
    option. This patch implments the needed changes to get json output,
    which is printed via "ethtool --json [iface]"
    
    The following design decision were made during implementation:
    - json values like Yes/No are printed as true/false
    - values that are "Unknown" are not printed at all
    - all other json values are not changed
    - keys are printed in lowercase with dashes in between
    
    Signed-off-by: Fabian Pfitzner <f.pfitzner@pengutronix.de>

	Andrew

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

[Andrew Lunn]

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

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

[Mohan.Prasad]

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.

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

[Andrew Lunn]

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

That would be good, but it does make the test and the test setup a lot
more complex. I would suggest keeping such two target tests
separate. Also, look to see if there are other such tests using two
targets, and keep the basic configuration the same, IP address
configuration, how to SSH between them etc. I'm not too familiar with
the test framework. Maybe this is all a solved problem and there are
helpers to use?

	Andrew

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

[Andrew Lunn]

On Tue, Sep 17, 2024 at 08:04:07AM +0530, Mohan Prasad J wrote:
> Add selftest file to test basic features of a NIC driver.
> Tests for link modes, auto-negotiation are placed.
> Selftest makes use of ksft modules and ethtool.
> Add selftest file in the Makefile.
> 
> Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
> ---
>  .../testing/selftests/drivers/net/hw/Makefile |   1 +
>  .../drivers/net/hw/nic_basic_tests.py         | 145 ++++++++++++++++++
>  2 files changed, 146 insertions(+)
>  create mode 100644 tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> 
> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> index c9f2f48fc..9f105227c 100644
> --- a/tools/testing/selftests/drivers/net/hw/Makefile
> +++ b/tools/testing/selftests/drivers/net/hw/Makefile
> @@ -10,6 +10,7 @@ TEST_PROGS = \
>  	hw_stats_l3.sh \
>  	hw_stats_l3_gre.sh \
>  	loopback.sh \
> +	nic_basic_tests.py \
>  	pp_alloc_fail.py \
>  	rss_ctx.py \
>  	#
> diff --git a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> new file mode 100644
> index 000000000..27f780032
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> @@ -0,0 +1,145 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +#Introduction:
> +#This file has basic tests for generic NIC drivers.
> +#The test comprises of auto-negotiation, speed and duplex checks.
> +#Also has tests to check the throughput
> +#
> +#Setup:
> +#Connect the DUT PC with NIC card to partner pc back via ethernet medium of your choice(RJ45, T1)
> +#
> +#        DUT PC                                              Partner PC
> +#┌───────────────────────┐                         ┌──────────────────────────┐
> +#│                       │                         │                          │
> +#│                       │                         │                          │
> +#│           ┌───────────┐                         │                          │
> +#│           │DUT NIC    │         Eth             │                          │
> +#│           │Interface ─┼─────────────────────────┼─    any eth Interface    │
> +#│           └───────────┘                         │                          │
> +#│                       │                         │                          │
> +#│                       │                         │                          │
> +#└───────────────────────┘                         └──────────────────────────┘
> +#
> +#Configurations:
> +# Change the below configuration based on your hw needs.
> +# """Default values"""
> +sleep_time = 5 #time taken to wait for transitions to happen, in seconds.
> +test_duration = 5  #performance test duration for the throughput check, in seconds.
> +throughput_threshold = 0.8 #percentage of throughput required to pass the throughput
> +
> +import time
> +import os
> +import re
> +import configparser
> +import json
> +from lib.py import ksft_run, ksft_exit, ksft_pr, ksft_eq
> +from lib.py import KsftFailEx, KsftSkipEx
> +from lib.py import NetDrvEpEnv
> +from lib.py import cmd
> +from lib.py import ethtool
> +
> +"""Global variables"""
> +common_link_modes = []

Software engineers have a dislike for global variables. In this patch,
it is not even used. Please consider replacing it by passing it as a
parameter, or turn the code into a class and make it part of self.

> +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])

You can use the power of python here.

        "supported-link-modes": [ "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/Full" ],
	"link-partner-modes": [ "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/Full" ],

convert the list into a set, and then use 'and'.

	common_modes = set(josn['supported-link-modes']) and set(json['link-partner-modes'])

	Andrew

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

[Mohan.Prasad]

Hi Andrew,

Thank you for the review comments.

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Tue, Sep 17, 2024 at 08:04:07AM +0530, Mohan Prasad J wrote:
> > Add selftest file to test basic features of a NIC driver.
> > Tests for link modes, auto-negotiation are placed.
> > Selftest makes use of ksft modules and ethtool.
> > Add selftest file in the Makefile.
> >
> > Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
> > ---
> >  .../testing/selftests/drivers/net/hw/Makefile |   1 +
> >  .../drivers/net/hw/nic_basic_tests.py         | 145 ++++++++++++++++++
> >  2 files changed, 146 insertions(+)
> >  create mode 100644
> > tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> >
> > diff --git a/tools/testing/selftests/drivers/net/hw/Makefile
> > b/tools/testing/selftests/drivers/net/hw/Makefile
> > index c9f2f48fc..9f105227c 100644
> > --- a/tools/testing/selftests/drivers/net/hw/Makefile
> > +++ b/tools/testing/selftests/drivers/net/hw/Makefile
> > @@ -10,6 +10,7 @@ TEST_PROGS = \
> >       hw_stats_l3.sh \
> >       hw_stats_l3_gre.sh \
> >       loopback.sh \
> > +     nic_basic_tests.py \
> >       pp_alloc_fail.py \
> >       rss_ctx.py \
> >       #
> > diff --git a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > new file mode 100644
> > index 000000000..27f780032
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> > @@ -0,0 +1,145 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +#Introduction:
> > +#This file has basic tests for generic NIC drivers.
> > +#The test comprises of auto-negotiation, speed and duplex checks.
> > +#Also has tests to check the throughput #
> > +#Setup:
> > +#Connect the DUT PC with NIC card to partner pc back via ethernet
> > +medium of your choice(RJ45, T1) #
> > +#        DUT PC                                              Partner PC
> > +#┌───────────────────────┐
> ┌──────────────────────────┐
> > +#│                       │                         │                          │
> > +#│                       │                         │                          │
> > +#│           ┌───────────┐                         │                          │
> > +#│           │DUT NIC    │         Eth             │                          │
> > +#│           │Interface ─┼─────────────────────────┼─    any eth
> Interface    │
> > +#│           └───────────┘                         │                          │
> > +#│                       │                         │                          │
> > +#│                       │                         │                          │
> > +#└───────────────────────┘
> └──────────────────────────┘
> > +#
> > +#Configurations:
> > +# Change the below configuration based on your hw needs.
> > +# """Default values"""
> > +sleep_time = 5 #time taken to wait for transitions to happen, in seconds.
> > +test_duration = 5  #performance test duration for the throughput check, in
> seconds.
> > +throughput_threshold = 0.8 #percentage of throughput required to pass
> > +the throughput
> > +
> > +import time
> > +import os
> > +import re
> > +import configparser
> > +import json
> > +from lib.py import ksft_run, ksft_exit, ksft_pr, ksft_eq from lib.py
> > +import KsftFailEx, KsftSkipEx from lib.py import NetDrvEpEnv from
> > +lib.py import cmd from lib.py import ethtool
> > +
> > +"""Global variables"""
> > +common_link_modes = []
> 
> Software engineers have a dislike for global variables. In this patch, it is not
> even used. Please consider replacing it by passing it as a parameter, or turn
> the code into a class and make it part of self.

I will change this in the next version.

> 
> > +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])
> 
> You can use the power of python here.
> 
>         "supported-link-modes": [
> "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/
> Full" ],
>         "link-partner-modes": [
> "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/
> Full" ],
> 
> convert the list into a set, and then use 'and'.
> 
>         common_modes = set(josn['supported-link-modes']) and set(json['link-
> partner-modes'])

I am figuring out the way to get the ethtool output in json format.
Once successful, I will change this implementation as suggested in next version.

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

[Mohan Prasad J]

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@microchip.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")
+
 def main() -> None:
     with NetDrvEpEnv(__file__) as cfg:
         ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg,))
-- 
2.43.0

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

[Willem de Bruijn]

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@microchip.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?

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

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

[Mohan.Prasad]

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@microchip.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.

[PATCH net-next v2 3/3] selftests: nic_basic_tests: Add selftest case for throughput check

[Mohan Prasad J]

Add selftest case to check the send and receive throughput.
Supported link modes between local NIC driver and partner
are varied. Then send and receive throughput is captured
and verified. Test uses iperf3 tool.

Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
---
 .../drivers/net/hw/nic_basic_tests.py         | 39 +++++++++++++++++++
 1 file changed, 39 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 ff46f2406..ec1f5b6a2 100644
--- a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
+++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
@@ -182,6 +182,45 @@ def test_network_speed(cfg) -> None:
         else:
             KsftSkipEx("Mismatch in the number of speeds and duplex modes")
 
+def test_tcp_throughput(cfg) -> None:
+    check_autonegotiation(cfg.ifname)
+    if not common_link_modes:
+        KsftSkipEx("No common link modes found")
+    cfg.require_cmd("iperf3", remote=True)
+    """iperf3 server to be run in the partner pc"""
+    speeds, duplex_modes = get_speed_duplex(common_link_modes)
+    """Test duration in seconds"""
+    duration = test_duration
+    target_ip = cfg.remote_addr
+
+    for idx in range(len(speeds)-1, -1, -1):
+        set_speed_and_duplex(cfg.ifname, speeds[idx], duplex_modes[idx])
+        time.sleep(sleep_time)
+        verify_link_up(cfg.ifname)
+        send_command=f"iperf3 -c {target_ip} -t {duration} --json"
+        receive_command=f"iperf3 -c {target_ip} -t {duration} --reverse --json"
+        send_result = cmd(send_command)
+        receive_result = cmd(receive_command)
+        if send_result.ret != 0 or receive_result.ret != 0:
+            raise KsftSkipEx("No server is running")
+
+        send_output = send_result.stdout
+        receive_output = receive_result.stdout
+
+        send_data = json.loads(send_output)
+        receive_data = json.loads(receive_output)
+        """Convert throughput to Mbps"""
+        send_throughput = round(send_data['end']['sum_sent']['bits_per_second'] / 1e6, 2)
+        receive_throughput = round(receive_data['end']['sum_received']['bits_per_second'] / 1e6, 2)
+
+        ksft_pr(f"Send throughput: {send_throughput} Mbps, Receive throughput: {receive_throughput} Mbps")
+        """Check whether throughput is not below the threshold (default:80% of set speed)"""
+        threshold = float(speeds[idx]) * float(throughput_threshold)
+        if send_throughput < threshold:
+            raise KsftFailEx("Send throughput is below threshold")
+        elif receive_throughput < threshold:
+            raise KsftFailEx("Receive throughput is below threshold")
+
 def main() -> None:
     with NetDrvEpEnv(__file__) as cfg:
         ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg,))
-- 
2.43.0