|SUCCESS| [v3] dts: clean cryptodev environment after a test run

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163977

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-13

Reviewing patch: dts: clean cryptodev environment after a test run

git sha: 8ee2df9da125 (referenced in Fixes tag)
Author: Andrew Bailey <abailey at iol.unh.edu>
Subject: [PATCH v3] dts: clean cryptodev environment after a test run

This patch addresses cleanup of virtual functions created during cryptodev
testing to prevent leaving the system in a bad state.

> diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
> index ee943462c2..aa53f1e5e2 100644
> --- a/dts/framework/testbed_model/linux_session.py
> +++ b/dts/framework/testbed_model/linux_session.py
> @@ -181,6 +181,22 @@ def get_port_info(self, pci_address: str) -> PortInfo:
>  
>          return PortInfo(mac_address, logical_name, driver, is_link_up)
>  
> +    def unbind_ports(self, ports: list[Port]):
> +        """Overrides :meth:`~.os_session.OSSession.unbind_ports`.
> +
> +        The :attr:`~.devbind_script_path` property must be setup in order to call this method.
> +        """
> +        ports_pci_addrs = " ".join(port.pci for port in ports)
> +
> +        self.send_command(
> +            f"{self.devbind_script_path} -u --force {ports_pci_addrs}",
> +            privileged=True,
> +            verify=True,
> +        )
> +
> +        if self._lshw_net_info:
> +            del self._lshw_net_info

Does this code properly check whether _lshw_net_info is None? The current
check treats an empty dictionary as falsy, which may not be the intended
behavior. Should this use "if self._lshw_net_info is not None:" instead?

Also, does unbind_ports() handle the case when ports is an empty list?
The code would build an empty ports_pci_addrs string and still execute
the devbind command.

> +
>      def bind_ports_to_driver(self, ports: list[Port], driver_name: str) -> None:
>          """Overrides :meth:`~.os_session.OSSession.bind_ports_to_driver`.
>  
> @@ -289,7 +305,18 @@ def create_vfs(self, pf_port: Port) -> None:
>              self.refresh_lshw()
>  
>      def delete_crypto_vfs(self, pf_port: Port) -> None:
> -        """Overrides :meth:`~.os_session.OSSession.delete_crypto_vfs`."""
> +        """Overrides :meth:`~.os_session.OSSession.delete_crypto_vfs`.
> +
> +        The :attr:`~.devbind_script_path` property must be setup in order to call this method.
> +        """
> +        if vfs := self.get_pci_addr_of_crypto_vfs(pf_port):
> +            vf_pci_addrs = " ".join(vf for vf in vfs)
> +            self.send_command(
> +                f"{self.devbind_script_path} -u --force {vf_pci_addrs}",
> +                privileged=True,
> +                verify=True,
> +            )
> +        self.unbind_ports([pf_port])

Is there a potential issue with the ordering here? This code unbinds the
PF port after unbinding VFs, but does unbind_ports([pf_port]) succeed if
the port was already removed via the subsequent tee command? The remove
operation comes after unbind_ports, so does this introduce a race or
ordering dependency?

>          self.send_command(
>              f"echo 1 | sudo tee /sys/bus/pci/devices/{pf_port.pci}/remove".replace(":", "\\:"),
>              privileged=True,

[ ... ]