Nic: allow bind and unbind by device uuid and sysfs path

[mcgov (mcgov)]

Part 2 of 9 of a stacked series that reworks the DPDK SRIOV hot plug tests. Stacked on #4654, review only the last commit.

• Nics.bind and Nics.unbind required a NicInfo, so a device that is not represented in the node's nic list (for example a VF that has just been hot removed) could not be rebound. They now accept either a NicInfo or a raw device uuid string plus the driver sysfs path.
• get_nic_by_subnet skips nics without an assigned ip address instead of raising while parsing an empty address.

Key Test Cases:
verify_dpdk_sriov_rescind_failover_send_only|verify_dpdk_build_netvsc|verify_dpdk_build_failsafe

Impacted LISA Features:
Sriov, NetworkInterface

Tested Azure Marketplace Images:

• canonical 0001-com-ubuntu-server-jammy 22_04-lts latest
• redhat rhel 9_5 latest

[Copilot]

Pull request overview

This PR updates the NIC helper APIs to support DPDK SR-IOV hot-plug scenarios where a device may no longer exist in the node’s NIC list, by allowing bind/unbind operations using a raw device id and driver sysfs path. It also makes subnet-to-NIC lookup more robust by skipping NICs that currently have no IP assigned.

Changes:

• Update get_nic_by_subnet() to skip NICs without an IP address instead of raising during parsing.
• Extend Nics.bind() / Nics.unbind() to accept either a NicInfo or a raw device id (str) and driver sysfs path.
• Ensure bind() continues to update NicInfo.driver_sysfs_path and NicInfo.module_name when a NicInfo is provided.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/nic.py:11

• Minor: Union is imported but not used. If the intention is to support passing either NicInfo or a raw UUID (as described in the PR), update the affected type hints/signatures accordingly; otherwise remove the unused import to avoid lint/type-check noise.

from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union

lisa/nic.py:375

• Minor: bind() updates nic.module_name before the bind is attempted. If bind_by_uuid() raises (e.g. bind path missing), nic will incorrectly claim it is bound to the new module. Set module_name only after the bind succeeds.

    def bind(self, nic: "NicInfo", driver_module_path: str) -> None:
        nic.driver_sysfs_path = PurePosixPath(driver_module_path)
        nic.module_name = nic.driver_sysfs_path.name
        self.bind_by_uuid(nic.dev_uuid, driver_module_path)

lisa/nic.py:357

• Major: if not nic.driver_sysfs_path: will never be true because NicInfo initializes driver_sysfs_path to PurePosixPath("") (which stringifies to . and is truthy). This can prevent get_nic_driver() from running when the sysfs path is effectively unset, leading to unbind attempts against ./unbind.

        ip = self._node.tools[Ip]
        if not nic.driver_sysfs_path:
            self.get_nic_driver(nic.name)

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/nic.py:343

• The new comment says this NIC has no “subnet assigned”, but the code is actually checking whether an IP address is present (nic.ip_addr). This is misleading when reading/debugging subnet matching logic.

            # Skip this check if the object does not have a subnet assigned.
            if not nic.ip_addr:
                continue

lisa/nic.py:358

• NicInfo.driver_sysfs_path defaults to PurePosixPath(""), which normalizes to . and is always truthy. The current if not nic.driver_sysfs_path: condition will never run, so unbind() may try to unbind using ./unbind (and now will always fail the new path-exists assertion) if the driver path wasn't populated. Check explicitly for the default . value (or use None) before calling get_nic_driver.

Also, the PR description says unbind supports raw UUID+sysfs path; the implementation adds unbind_by_uuid for that. Adding a short docstring here helps clarify the intended usage.

    def unbind(self, nic: "NicInfo") -> None:
        ip = self._node.tools[Ip]
        if not nic.driver_sysfs_path:
            self.get_nic_driver(nic.name)
        # if the device is active, set to down before unbind

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lisa/nic.py:356

• The PR description says Nics.bind/Nics.unbind now accept either a NicInfo or a raw device UUID + sysfs path, but the signatures here still require NicInfo and the new UUID-based API is exposed via bind_by_uuid/unbind_by_uuid instead. Please either update the PR description to match the actual API, or adjust bind/unbind to accept the described inputs.

    def unbind(self, nic: "NicInfo") -> None:
        ip = self._node.tools[Ip]
        if not nic.driver_sysfs_path:

lisa/nic.py:343

• The new comment says the NIC has no "subnet assigned", but the check is actually for a missing IP address. Wording this as "no IP address assigned" makes the intent clearer and matches the condition being evaluated.

            # Skip this check if the object does not have a subnet assigned.
            if not nic.ip_addr:
                continue

lisa/nic.py:357

• if not nic.driver_sysfs_path will never be true because PurePosixPath objects are always truthy (and the default is effectively PurePosixPath('.')). This means get_nic_driver() won't run even when the sysfs path hasn't been populated, and unbind may use an invalid driver path.

        ip = self._node.tools[Ip]
        if not nic.driver_sysfs_path:
            self.get_nic_driver(nic.name)

lisa/nic.py:369

• The assertion messages for missing bind/unbind sysfs paths don't provide any guidance on what value is expected or how to troubleshoot. Please include actionable context (e.g. expected /sys/bus/.../drivers/<driver>/bind paths) so failures are easier to diagnose.

        assert_that(ls.path_exists(str(unbind_path), sudo=True)).described_as(
            f"Unbind path {unbind_path} does not exist."
        ).is_true()