Fix NIC passthrough test issues

vyadavmsft · vyadavmsft · commit bee6d409993c · 2026-01-29T22:41:05.000-08:00
- Add missing __init__.py for device_passthrough package
- Fix unsafe pool.devices[0] access with validation
- Fix class name typo: NetworkPerformace -&gt; NetworkPerformance
- Add validation for device_addr to prevent None usage
- Fix logic bug: validate only the node parameter, not all node spaces
- Remove unused parameters (log, log_path, variables) from functional test
- Update descriptions: replace 'sriov' with 'passthrough' for accuracy
- Improve error messages with more context
- Add timeout documentation comments
- Clarify server_ip vs interface_ip usage in common.py
- Add pool_type validation before dictionary access
- Move device_passthrough tests to correct location for test discovery

1. Identifies interface by PCI address first (before dhclient)
2. Brings interface UP with 'ip link set up'
3. Waits for carrier detection (30s timeout)
4. Then runs dhclient to get IP address

This ensures the physical link is established before DHCP configuration.
diff --git a/lisa/microsoft/testsuites/device_passthrough/__init__.py b/lisa/microsoft/testsuites/device_passthrough/__init__.py
@@ -0,0 +1,2 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT license.
diff --git a/lisa/microsoft/testsuites/device_passthrough/functional_tests.py b/lisa/microsoft/testsuites/device_passthrough/functional_tests.py
@@ -1,30 +1,28 @@
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT license.
-from pathlib import Path
-from typing import Any, Dict, cast
+from typing import TYPE_CHECKING, cast
 
-from lisa import (
-    Environment,
-    Logger,
-    Node,
-    TestCaseMetadata,
-    TestSuite,
-    TestSuiteMetadata,
-)
+from lisa import Environment, Node, TestCaseMetadata, TestSuite, TestSuiteMetadata
+from lisa.operating_system import Windows
 from lisa.sut_orchestrator import CLOUD_HYPERVISOR
-from lisa.sut_orchestrator.libvirt.ch_platform import CloudHypervisorPlatform
-from lisa.sut_orchestrator.libvirt.schema import BaseLibvirtNodeSchema
 from lisa.testsuite import TestResult, simple_requirement
 from lisa.tools import Lspci
 from lisa.util import LisaException, SkippedException
 
+if TYPE_CHECKING:
+    from lisa.sut_orchestrator.libvirt.ch_platform import CloudHypervisorPlatform
+
 
 @TestSuiteMetadata(
     area="device_passthrough",
     category="functional",
     description="""
     This test suite is for testing device passthrough functional tests.
     """,
+    requirement=simple_requirement(
+        supported_platform_type=[CLOUD_HYPERVISOR],
+        unsupported_os=[Windows],
+    ),
 )
 class DevicePassthroughFunctionalTests(TestSuite):
     @TestCaseMetadata(
@@ -65,46 +63,52 @@ class DevicePassthroughFunctionalTests(TestSuite):
     )
     def verify_device_passthrough_on_guest(
         self,
-        log: Logger,
         node: Node,
         environment: Environment,
-        log_path: Path,
         result: TestResult,
-        variables: Dict[str, Any],
     ) -> None:
         lspci = node.tools[Lspci]
-        platform = cast(CloudHypervisorPlatform, environment.platform)
+        platform = cast("CloudHypervisorPlatform", environment.platform)
         pool_vendor_device_map = {}
-        assert platform.platform_runbook.device_pools, "Device pool cant be empty"
+        assert platform.platform_runbook.device_pools, "Device pool can't be empty"
         for pool in platform.platform_runbook.device_pools:
             pool_type = str(pool.type.value)
+            if not pool.devices:
+                raise LisaException(f"No devices defined for pool type: {pool_type}")
             vendor_device_id = {
                 "vendor_id": pool.devices[0].vendor_id,
                 "device_id": pool.devices[0].device_id,
             }
             pool_vendor_device_map[pool_type] = vendor_device_id
 
-        assert environment.runbook.nodes_requirement, "requirement cant be empty"
-        for node_space in environment.runbook.nodes_requirement:
-            node_runbook: BaseLibvirtNodeSchema = node_space.get_extended_runbook(
-                BaseLibvirtNodeSchema, CLOUD_HYPERVISOR
+        # Get the node's runbook to check its passthrough requirements
+        # Import at runtime to avoid libvirt dependency on non-libvirt platforms
+        from lisa.sut_orchestrator.libvirt.schema import BaseLibvirtNodeSchema
+
+        node_runbook: "BaseLibvirtNodeSchema" = node.capability.get_extended_runbook(
+            BaseLibvirtNodeSchema, CLOUD_HYPERVISOR
+        )
+        if not node_runbook.device_passthrough:
+            raise SkippedException("No device-passthrough is set for node")
+
+        for req in node_runbook.device_passthrough:
+            pool_type = str(req.pool_type.value)
+            if pool_type not in pool_vendor_device_map:
+                raise LisaException(
+                    f"Pool type '{pool_type}' not found in platform device pools"
+                )
+            ven_dev_id_of_pool = pool_vendor_device_map[pool_type]
+            ven_id = ven_dev_id_of_pool["vendor_id"]
+            dev_id = ven_dev_id_of_pool["device_id"]
+            devices = lspci.get_devices_by_vendor_device_id(
+                vendor_id=ven_id,
+                device_id=dev_id,
+                force_run=True,
             )
-            if not node_runbook.device_passthrough:
-                raise SkippedException("No device-passthrough is set for node")
-            for req in node_runbook.device_passthrough:
-                pool_type = str(req.pool_type.value)
-                ven_dev_id_of_pool = pool_vendor_device_map[pool_type]
-                ven_id = ven_dev_id_of_pool["vendor_id"]
-                dev_id = ven_dev_id_of_pool["device_id"]
-                devices = lspci.get_devices_by_vendor_device_id(
-                    vendor_id=ven_id,
-                    device_id=dev_id,
-                    force_run=True,
+            if len(devices) < req.count:
+                raise LisaException(
+                    f"Passthrough device validation failed for "
+                    f"pool_type '{pool_type}': Found {len(devices)} "
+                    f"device(s) but expected {req.count}. "
+                    f"Vendor/Device ID: {ven_id}:{dev_id}"
                 )
-                if len(devices) < req.count:
-                    raise LisaException(
-                        f"Device count don't match, got total: {len(devices)} "
-                        f"As per runbook, Required device count: {req.count}, "
-                        f"for pool_type: {pool_type} having Vendor/Device ID as "
-                        f"vendor_id = {ven_id}, device_id={dev_id}"
-                    )
diff --git a/lisa/microsoft/testsuites/performance/common.py b/lisa/microsoft/testsuites/performance/common.py
@@ -301,6 +301,8 @@ def perf_tcp_pps(
     for port in ports:
         server_netperf.run_as_server(port, interface_ip=server_interface_ip)
     for port in ports:
+        # Use server.internal_address as target since netperf client needs
+        # the server's IP (which may differ from the interface it binds to)
         client_netperf.run_as_client_async(
             server_ip=server.internal_address,
             core_count=thread_count,
diff --git a/lisa/microsoft/testsuites/performance/networkperf_passthrough.py b/lisa/microsoft/testsuites/performance/networkperf_passthrough.py
@@ -2,7 +2,13 @@
 # Licensed under the MIT license.
 import re
 from functools import partial
-from typing import Any, Dict, Tuple, cast
+from typing import Any, Callable, Dict, List, Tuple, cast
+
+from microsoft.testsuites.performance.common import (
+    perf_iperf,
+    perf_ntttcp,
+    perf_tcp_pps,
+)
 
 from lisa import (
     Logger,
@@ -20,27 +26,22 @@
 from lisa.sut_orchestrator import CLOUD_HYPERVISOR
 from lisa.sut_orchestrator.libvirt.context import get_node_context
 from lisa.testsuite import TestResult
-from lisa.tools import Dhclient, Lspci, Sysctl
+from lisa.tools import Dhclient, Kill, Lspci, Sysctl
 from lisa.tools.iperf3 import (
     IPERF_TCP_BUFFER_LENGTHS,
     IPERF_TCP_CONCURRENCY,
     IPERF_UDP_BUFFER_LENGTHS,
     IPERF_UDP_CONCURRENCY,
 )
 from lisa.util import (
+    LisaException,
     SkippedException,
     constants,
     find_group_in_lines,
     find_groups_in_lines,
 )
 from lisa.util.logger import get_logger
 from lisa.util.parallel import run_in_parallel
-from microsoft.testsuites.performance.common import (
-    cleanup_process,
-    perf_iperf,
-    perf_ntttcp,
-    perf_tcp_pps,
-)
 
 
 @TestSuiteMetadata(
@@ -50,11 +51,21 @@
     This test suite is to validate linux network performance
     for various NIC passthrough scenarios.
     """,
+    requirement=simple_requirement(
+        supported_platform_type=[CLOUD_HYPERVISOR],
+        unsupported_os=[Windows],
+    ),
 )
-class NetworkPerformace(TestSuite):
+class NetworkPerformance(TestSuite):
+    # Timeout values:
+    # TIMEOUT: 12000s (3.3 hrs) - accounts for test execution + network setup overhead
+    # PPS_TIMEOUT: 3000s (50 min) - shorter for PPS tests which are less intensive
     TIMEOUT = 12000
     PPS_TIMEOUT = 3000
 
+    # Track baremetal host nodes for cleanup
+    _baremetal_hosts: list[RemoteNode] = []
+
     # Network device passthrough tests between host and guest
     @TestCaseMetadata(
         description="""
@@ -239,7 +250,7 @@ def perf_tcp_ntttcp_passthrough_host_guest(
 
     @TestCaseMetadata(
         description="""
-        This test case uses ntttcp to test sriov tcp network throughput.
+        This test case uses ntttcp to test passthrough udp network throughput.
         """,
         priority=3,
         timeout=TIMEOUT,
@@ -423,7 +434,8 @@ def perf_tcp_max_pps_passthrough_two_guest(self, result: TestResult) -> None:
 
     @TestCaseMetadata(
         description="""
-        This test case uses ntttcp to test sriov tcp network throughput.
+        This test case uses ntttcp to test passthrough tcp network throughput
+        between two guest VMs.
         """,
         priority=3,
         timeout=TIMEOUT,
@@ -458,7 +470,8 @@ def perf_tcp_ntttcp_passthrough_two_guest(self, result: TestResult) -> None:
 
     @TestCaseMetadata(
         description="""
-        This test case uses ntttcp to test sriov tcp network throughput.
+        This test case uses ntttcp to test passthrough udp network throughput
+        between two guest VMs.
         """,
         priority=3,
         timeout=TIMEOUT,
@@ -500,14 +513,6 @@ def _configure_passthrough_nic_for_node(
         if not ctx.passthrough_devices:
             raise SkippedException("No passthrough devices found for node")
 
-        # Configure the nw interface on guest
-        node.tools[Dhclient].run(
-            force_run=True,
-            sudo=True,
-            expected_exit_code=0,
-            expected_exit_code_failure_message="dhclient run failed",
-        )
-
         lspci = node.tools[Lspci]
         pci_devices = lspci.get_devices_by_type(
             constants.DEVICE_TYPE_SRIOV, force_run=True
@@ -521,8 +526,14 @@ def _configure_passthrough_nic_for_node(
                 device_addr = device.slot
                 break
 
+        if device_addr is None:
+            raise LisaException(
+                f"No non-virtio passthrough device found. "
+                f"Available devices: {[d.slot for d in pci_devices]}"
+            )
+
         # Get the interface name
-        err_msg: str = "Can't find interface from PCI address"
+        err_msg: str = f"Can't find interface from PCI address: {device_addr}"
         device_path = node.execute(
             cmd=(
                 "find /sys/class/net/*/device/subsystem/devices"
@@ -542,6 +553,38 @@ def _configure_passthrough_nic_for_node(
         interface_name = interface_name_raw.get("INTERFACE_NAME", "")
         assert interface_name, "Can not find interface name"
 
+        # Bring the interface up before configuring it
+        node.execute(
+            cmd=f"ip link set {interface_name} up",
+            sudo=True,
+            expected_exit_code=0,
+            expected_exit_code_failure_message=(
+                f"Failed to bring up interface {interface_name}"
+            ),
+        )
+
+        # Wait for carrier (link up) - some NICs take time to negotiate
+        # Exit code 124 = timeout (no carrier), which is acceptable if DHCP works anyway
+        carrier_result = node.execute(
+            cmd=f"timeout 30 sh -c 'until cat /sys/class/net/{interface_name}/carrier"
+            f" 2>/dev/null | grep -q 1; do sleep 1; done'",
+            sudo=True,
+            shell=True,
+        )
+        if carrier_result.exit_code == 124:
+            node.log.warning(
+                f"Interface {interface_name} carrier not detected after 30s. "
+                f"Proceeding with DHCP anyway - may fail if no physical link."
+            )
+        elif carrier_result.exit_code != 0:
+            raise LisaException(
+                f"Failed to check carrier on {interface_name}: "
+                f"exit code {carrier_result.exit_code}"
+            )
+
+        # Configure the nw interface on guest with dhclient for the specific interface
+        node.tools[Dhclient].renew(interface_name)
+
         # Get the interface ip
         err_msg = f"Failed to get interface details for: {interface_name}"
         interface_details = node.execute(
@@ -557,7 +600,11 @@ def _configure_passthrough_nic_for_node(
             single_line=False,
         )
         passthrough_nic_ip = interface_ip.get("INTERFACE_IP", "")
-        assert passthrough_nic_ip, "Can not find interface IP"
+        if not passthrough_nic_ip:
+            raise LisaException(
+                f"Failed to get IP for passthrough interface '{interface_name}'. "
+                f"Interface details: {interface_details[:200]}"
+            )
 
         test_node = cast(RemoteNode, node)
         test_node.internal_address = passthrough_nic_ip
@@ -568,9 +615,14 @@ def _get_host_as_server(self, variables: Dict[str, Any]) -> RemoteNode:
         ip = variables.get("baremetal_host_ip", "")
         username = variables.get("baremetal_host_username", "")
         passwd = variables.get("baremetal_host_password", "")
+        private_key = variables.get("baremetal_host_private_key_file", "")
 
-        if not (ip and username and passwd):
-            raise SkippedException("Server-Node details are not provided")
+        if not (ip and username and (passwd or private_key)):
+            raise SkippedException(
+                "Server-Node details are not provided. Required: "
+                "baremetal_host_ip, baremetal_host_username, and either "
+                "baremetal_host_password or baremetal_host_private_key_file"
+            )
 
         server = RemoteNode(
             runbook=schema.Node(name="baremetal-host"),
@@ -584,7 +636,11 @@ def _get_host_as_server(self, variables: Dict[str, Any]) -> RemoteNode:
             public_port=22,
             username=username,
             password=passwd,
+            private_key_file=private_key,
         )
+        # Track baremetal host for cleanup
+        if server not in self._baremetal_hosts:
+            self._baremetal_hosts.append(server)
         return server
 
     def _get_host_nic_name(self, node: RemoteNode) -> str:
@@ -620,26 +676,28 @@ def _get_host_nic_name(self, node: RemoteNode) -> str:
     def after_case(self, log: Logger, **kwargs: Any) -> None:
         environment: Environment = kwargs.pop("environment")
 
+        # Combine environment nodes and baremetal host nodes for cleanup
+        all_nodes = list(environment.nodes.list())
+        if self._baremetal_hosts:
+            all_nodes.extend(self._baremetal_hosts)
+
         # use these cleanup functions
-        def do_process_cleanup(process: str) -> None:
-            cleanup_process(environment, process)
+        def do_process_cleanup(process: str, node: Node) -> None:
+            # Kill the process on the specific node
+            kill = node.tools[Kill]
+            kill.by_name(process, ignore_not_exist=True)
 
         def do_sysctl_cleanup(node: Node) -> None:
             node.tools[Sysctl].reset()
 
-        # to run parallel cleanup of processes and sysctl settings
-        run_in_parallel(
-            [
-                partial(do_process_cleanup, x)
-                for x in [
-                    "lagscope",
-                    "netperf",
-                    "netserver",
-                    "ntttcp",
-                    "iperf3",
-                ]
-            ]
-        )
-        run_in_parallel(
-            [partial(do_sysctl_cleanup, x) for x in environment.nodes.list()]
-        )
+        # to run parallel cleanup of processes on all nodes
+        cleanup_tasks: List[Callable[[], None]] = []
+        for process in ["lagscope", "netperf", "netserver", "ntttcp", "iperf3"]:
+            for node in all_nodes:
+                cleanup_tasks.append(partial(do_process_cleanup, process, node))
+
+        run_in_parallel(cleanup_tasks)
+        run_in_parallel([partial(do_sysctl_cleanup, x) for x in all_nodes])
+
+        # Clear the baremetal hosts list after cleanup
+        self._baremetal_hosts.clear()
diff --git a/lisa/tools/iperf3.py b/lisa/tools/iperf3.py
diff --git a/lisa/tools/netperf.py b/lisa/tools/netperf.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+# Copyright (c) Microsoft Corporation.`
	`2`	`+# Licensed under the MIT license.`