Re: [PATCH v4 5/5] dts: add functions for managing VFs to Node

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

I'm wondering whether we should move some of the functionality to the 
Port class, such as creating VFs and related logic. I wanted to move 
update_port and such there, but I ran into problems with imports. Maybe 
if we utilize the if TYPE_CHECKING: guard the imports would work.

Seems like a lot would be simplified if we moved the VFs ports inside 
the Port class.

> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py

>   class Node(ABC):
> @@ -276,6 +277,96 @@ def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
>               verify=True,
>           )
>   
> +    def create_virtual_functions(
> +        self, num: int, pf_port: Port, dpdk_driver: str | None = None
> +    ) -> list[VirtualFunction]:
> +        """Create virtual functions (VFs) from a given physical function (PF) on the node.
> +
> +        Virtual functions will be created if there are not any currently configured on `pf_port`.
> +        If there are greater than or equal to `num` VFs already configured on `pf_port`, those will
> +        be used instead of creating more. In order to create VFs, the PF must be bound to its
> +        kernel driver. This method will handle binding `pf_port` and any other ports in the test
> +        run that reside on the same device back to their OS drivers if this was not done already.
> +        VFs gathered in this method will be bound to `driver` if one is provided, or the DPDK
> +        driver for `pf_port` and then added to `self.ports`.
> +
> +        Args:
> +            num: The number of VFs to create. Must be greater than 0.
> +            pf_port: The PF to create the VFs on.

We should check that the passed port actually resides on this node.

> +            dpdk_driver: Optional driver to bind the VFs to after they are created. Defaults to the
> +                DPDK driver of `pf_port`.
> +
> +        Raises:
> +            InternalError: If `num` is less than or equal to 0.
> +        """
> +        if num <= 0:
> +            raise InternalError(
> +                "Method for creating virtual functions received a non-positive value."
> +            )
> +        if not dpdk_driver:
> +            dpdk_driver = pf_port.os_driver_for_dpdk
> +        # Get any other port that is on the same device which DTS is aware of
> +        all_device_ports = [
> +            p for p in self.ports if p.pci.split(".")[0] == pf_port.pci.split(".")[0]
> +        ]

Maybe we should create a PciAddress class that would process the address 
and provide useful methods, one of which we'd use here.

> +        # Ports must be bound to the kernel driver in order to create VFs from them
> +        for port in all_device_ports:
> +            self._bind_port_to_driver(port, False)
> +            # Some PMDs require the interface being up in order to make VFs

These are OS drivers, not PMDs.

> +            self.configure_port_state(port)
> +        created_vfs = self.main_session.set_num_virtual_functions(num, pf_port)
> +        # We don't need more then `num` VFs from the list
> +        vf_pcis = self.main_session.get_pci_addr_of_vfs(pf_port)[:num]
> +        devbind_info = self.main_session.send_command(
> +            f"{self.path_to_devbind_script} -s", privileged=True
> +        ).stdout

This looks like a good candidate for TextParser.

> +
> +        ret = []
> +
> +        for pci in vf_pcis:
> +            original_driver = re.search(f"{pci}.*drv=([\\d\\w-]*)", devbind_info)
> +            os_driver = original_driver[1] if original_driver else pf_port.os_driver
> +            vf_config = PortConfig(
> +                self.name, pci, dpdk_driver, os_driver, pf_port.peer.node, pf_port.peer.pci
> +            )
> +            vf_port = VirtualFunction(self.name, vf_config, created_vfs, pf_port)
> +            self.main_session.update_ports([vf_port])

This should be called after the for cycle so we only call it once. We 
can bind all VF ports after (again, preferably with just one call).