[RFC PATCH v1 0/5] dts: add VFs to the framework

[RFC PATCH v1 0/5] dts: add VFs to the framework

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

There currently is no method of creating or managing virtual functions
(VFs) in the new DTS framework but there are multiple test suites in
the old DTS framework that provide testing coverage using them. This
patch adds the functionality to the framework that is needed to create
and use VFs in test suites in the future.

The series is marked as an RFC primarily because it is a new feature
that has been a recent talking point on the DTS bugzilla. The code
however is functional.

Jeremy Spewock (5):
  dts: allow binding only a single port to a different driver
  dts: parameterize what ports the TG sends packets to
  dts: add class for virtual functions
  dts: add OS abstractions for creating virtual functions
  dts: add functions for managing VFs to Node

 dts/framework/test_suite.py                  |  38 ++++--
 dts/framework/testbed_model/linux_session.py |  36 +++++-
 dts/framework/testbed_model/node.py          | 115 +++++++++++++++++--
 dts/framework/testbed_model/os_session.py    |  40 +++++++
 dts/framework/testbed_model/port.py          |  37 +++++-
 5 files changed, 247 insertions(+), 19 deletions(-)

-- 
2.46.0

[RFC PATCH v1 1/5] dts: allow binding only a single port to a different driver

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
drivers to their respective ports to solve this problem.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 8e6181e424..85d4eb1f7c 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -167,12 +167,12 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
                 the setup steps will be taken.
         """
         self._copy_dpdk_tarball()
-        self.bind_ports_to_driver()
+        self.bind_all_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
         self.__remote_dpdk_dir = None
-        self.bind_ports_to_driver(for_dpdk=False)
+        self.bind_all_ports_to_driver(for_dpdk=False)
 
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
@@ -317,7 +317,7 @@ def _copy_dpdk_tarball(self) -> None:
         # then extract to remote path
         self.main_session.extract_remote_tarball(remote_tarball_path, self._remote_dpdk_dir)
 
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+    def bind_all_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the node to a driver.
 
         Args:
@@ -325,12 +325,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
                 If :data:`False`, binds to os_driver.
         """
         for port in self.ports:
-            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
-            self.main_session.send_command(
-                f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
-                privileged=True,
-                verify=True,
-            )
+            self._bind_port_to_driver(port, for_dpdk)
+
+    def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+        driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+        self.main_session.send_command(
+            f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+            privileged=True,
+            verify=True,
+        )
 
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
-- 
2.46.0

[RFC PATCH v1 2/5] dts: parameterize what ports the TG sends packets to

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously in the DTS framework the helper methods in the TestSutie
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port since the
framework limits where the TG is allowed to send packets. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/test_suite.py | 38 ++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..d5c0021503 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,8 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +197,19 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        packet = self._adjust_addresses(packet, sut_ingress, sut_egress)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,
@@ -208,18 +218,30 @@ def send_packet_and_capture(
             duration,
         )
 
-    def get_expected_packet(self, packet: Packet) -> Packet:
+    def get_expected_packet(
+        self, packet: Packet, sut_ingress: Port | None = None, sut_egress: Port | None = None
+    ) -> Packet:
         """Inject the proper L2/L3 addresses into `packet`.
 
         Args:
             packet: The packet to modify.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`.
 
         Returns:
             `packet` with injected L2/L3 addresses.
         """
-        return self._adjust_addresses(packet, expected=True)
-
-    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        return self._adjust_addresses(packet, sut_ingress, sut_egress, expected=True)
+
+    def _adjust_addresses(
+        self, packet: Packet, sut_ingress_port: Port, sut_egress_port: Port, expected: bool = False
+    ) -> Packet:
         """L2 and L3 address additions in both directions.
 
         Assumptions:
@@ -229,11 +251,13 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             packet: The packet to modify.
             expected: If :data:`True`, the direction is SUT -> TG,
                 otherwise the direction is TG -> SUT.
+            sut_ingress_port: The port to use as the Rx port on the SUT.
+            sut_egress_port: The port to use as the Tx port on the SUT.
         """
         if expected:
             # The packet enters the TG from SUT
             # update l2 addresses
-            packet.src = self._sut_port_egress.mac_address
+            packet.src = sut_egress_port.mac_address
             packet.dst = self._tg_port_ingress.mac_address
 
             # The packet is routed from TG egress to TG ingress
@@ -244,7 +268,7 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             # The packet leaves TG towards SUT
             # update l2 addresses
             packet.src = self._tg_port_egress.mac_address
-            packet.dst = self._sut_port_ingress.mac_address
+            packet.dst = sut_ingress_port.mac_address
 
             # The packet is routed from TG egress to TG ingress
             # update l3 addresses
-- 
2.46.0

[RFC PATCH v1 3/5] dts: add class for virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
     pci: str
 
 
-@dataclass(slots=True)
+@dataclass
 class Port:
     """Physical port on a node.
 
@@ -80,6 +80,41 @@ def pci(self) -> str:
         return self.identifier.pci
 
 
+@dataclass
+class VirtualFunction(Port):
+    """Virtual Function (VF) on a port.
+
+    DPDK applications often treat VFs the same as they do the physical ports (PFs) on the host.
+    For this reason VFs are represented in the framework as a type of port with some additional
+    metadata that allows the framework to more easily identify which device the VF belongs to as
+    well as where the VF originated from.
+
+    Attributes:
+        created_by_framework: :data:`True` if this VF represents one that the DTS framework created
+            on the node, :data:`False` otherwise.
+        pf_port: The PF that this VF was created on/gathered from.
+    """
+
+    created_by_framework: bool = False
+    pf_port: Port | None = None
+
+    def __init__(
+        self, node_name: str, config: PortConfig, created_by_framework: bool, pf_port: Port
+    ) -> None:
+        """Extends :meth:`Port.__init__` with VF specific metadata.
+
+        Args:
+            node_name: The name of the node the VF resides on.
+            config: Configuration information about the VF.
+            created_by_framework: :data:`True` if DTS created this VF, otherwise :data:`False` if
+                this class represents a VF that was preexisting on the node.
+            pf_port: The PF that this VF was created on/gathered from.
+        """
+        super().__init__(node_name, config)
+        self.created_by_framework = created_by_framework
+        self.pf_port = pf_port
+
+
 @dataclass(slots=True, frozen=True)
 class PortLink:
     """The physical, cabled connection between the ports.
-- 
2.46.0

[RFC PATCH v1 4/5] dts: add OS abstractions for creating virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Virtual functions in the framework are created using SR-IOV. The process
for doing this can vary depending on the operating system, so the
commands to create VFs have to be abstracted into different classes
based on the operating system. This patch adds the stubs for methods
that create VFs and get the PCI addresses of all VFs on a port to the
abstract class as well as a linux implementation for the methods.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/linux_session.py | 36 +++++++++++++++++-
 dts/framework/testbed_model/os_session.py    | 40 ++++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 99abc21353..48bf212f6a 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -15,7 +15,7 @@
 
 from typing_extensions import NotRequired
 
-from framework.exception import ConfigurationError, RemoteCommandExecutionError
+from framework.exception import ConfigurationError, RemoteCommandExecutionError, InternalError
 from framework.utils import expand_range
 
 from .cpu import LogicalCore
@@ -210,3 +210,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
         state = 1 if enable else 0
         self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
+
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+        if num > 0 and curr_num_vfs >= num:
+            self._logger.info(
+                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}."
+            )
+            return False
+        elif num > 0 and curr_num_vfs > 0:
+            self._logger.error(
+                f"Not enough VFs configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are configured. "
+                "DTS is unable to modify number of preexisting VFs."
+            )
+            raise InternalError("Failed to create VFs on port.")
+        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, verify=True)
+        return True
+
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
+        if curr_num_vfs > 0:
+            pci_addrs = self.send_command(
+                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
+                + f"{sys_bus_path}/virtfn*/uevent",
+                privileged=True,
+            )
+            return pci_addrs.stdout.splitlines()
+        else:
+            return []
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..191fc3c0c8 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         Args:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
+
+    @abstractmethod
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Update the number of virtual functions (VFs) on a port.
+
+        It should be noted that, due to the nature of VFs, if there are already VFs that exist on
+        the physical function (PF) prior to calling this function, additional ones cannot be added.
+        The only way to add more VFs is to remove the existing and then set the desired amount. For
+        this reason, this method will handle creation in the following order:
+
+        1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to
+        `num`
+        2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set
+        on it, but the total VFs that it has are less then `num`.
+        3. Create `num` VFs on the PF if there are none on it already
+
+        Args:
+            num: The number of VFs to set on the port.
+            pf_port: The port to add the VFs to.
+
+        Raises:
+            InternalError: If `pf_port` has less than `num` VFs configured on it
+                already.
+
+        Returns:
+            :data:`True` if this method successfully created VFs, :data:`False` if existing VFs
+            were used instead.
+        """
+
+    @abstractmethod
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Find the PCI addresses of all virtual functions (VFs) on the port `pf_port`.
+
+        Args:
+            pf_port: The port to find the VFs on.
+
+        Returns:
+            A list containing all of the PCI addresses of the VFs on the port. If the port has no
+            VFs then the list will be empty.
+        """
-- 
2.46.0

[RFC PATCH v1 5/5] dts: add functions for managing VFs to Node

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In order for test suites to create virtual functions there has to be
functions in the API that developers can use. This patch adds the
ability to create virtual functions to the Node API so that they are
reachable within test suites.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 96 ++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 85d4eb1f7c..101a8edfbc 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -14,6 +14,7 @@
 """
 
 import os
+import re
 import tarfile
 from abc import ABC
 from ipaddress import IPv4Interface, IPv6Interface
@@ -24,9 +25,10 @@
     OS,
     BuildTargetConfiguration,
     NodeConfiguration,
+    PortConfig,
     TestRunConfiguration,
 )
-from framework.exception import ConfigurationError
+from framework.exception import ConfigurationError, InternalError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
 
@@ -39,7 +41,7 @@
 )
 from .linux_session import LinuxSession
 from .os_session import OSSession
-from .port import Port
+from .port import Port, VirtualFunction
 
 
 class Node(ABC):
@@ -335,6 +337,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.
+            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]
+        ]
+        # 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
+            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
+
+        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])
+            self._bind_port_to_driver(vf_port)
+            self.ports.append(vf_port)
+            ret.append(vf_port)
+        return ret
+
+    def get_vfs_on_port(self, pf_port: Port) -> list[VirtualFunction]:
+        """Get all virtual functions (VFs) that DTS is aware of on `pf_port`.
+
+        Args:
+            pf_port: The port to search for the VFs on.
+
+        Returns:
+            A list of VFs in the framework that were created/gathered from `pf_port`.
+        """
+        return [p for p in self.ports if isinstance(p, VirtualFunction) and p.pf_port == pf_port]
+
+    def remove_virtual_functions(self, pf_port: Port) -> None:
+        """Removes all virtual functions (VFs) created on `pf_port` by DTS.
+
+        Finds all the VFs that were created from `pf_port` and either removes them if they were
+        created by the DTS framework or binds them back to their os_driver if they were preexisting
+        on the node.
+
+        Args:
+            pf_port: Port to remove the VFs from.
+        """
+        vf_ports = self.get_vfs_on_port(pf_port)
+        if any(vf.created_by_framework for vf in vf_ports):
+            self.main_session.set_num_virtual_functions(0, pf_port)
+        else:
+            self._logger.info("Skipping removing VFs since they were not created by DTS.")
+            # Bind all VFs that we are no longer using back to their original driver
+            for vf in vf_ports:
+                self._bind_port_to_driver(vf, for_dpdk=False)
+        self.ports = [p for p in self.ports if p not in vf_ports]
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
-- 
2.46.0

[RFC PATCH v2 0/5] dts: add VFs to the framework

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v2:
  * Accidentally left out a formatting fix in v1.

Jeremy Spewock (5):
  dts: allow binding only a single port to a different driver
  dts: parameterize what ports the TG sends packets to
  dts: add class for virtual functions
  dts: add OS abstractions for creating virtual functions
  dts: add functions for managing VFs to Node

 dts/framework/test_suite.py                  |  38 ++++--
 dts/framework/testbed_model/linux_session.py |  40 ++++++-
 dts/framework/testbed_model/node.py          | 115 +++++++++++++++++--
 dts/framework/testbed_model/os_session.py    |  40 +++++++
 dts/framework/testbed_model/port.py          |  37 +++++-
 5 files changed, 251 insertions(+), 19 deletions(-)

-- 
2.46.0

[RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
drivers to their respective ports to solve this problem.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 8e6181e424..85d4eb1f7c 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -167,12 +167,12 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
                 the setup steps will be taken.
         """
         self._copy_dpdk_tarball()
-        self.bind_ports_to_driver()
+        self.bind_all_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
         self.__remote_dpdk_dir = None
-        self.bind_ports_to_driver(for_dpdk=False)
+        self.bind_all_ports_to_driver(for_dpdk=False)
 
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
@@ -317,7 +317,7 @@ def _copy_dpdk_tarball(self) -> None:
         # then extract to remote path
         self.main_session.extract_remote_tarball(remote_tarball_path, self._remote_dpdk_dir)
 
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+    def bind_all_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the node to a driver.
 
         Args:
@@ -325,12 +325,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
                 If :data:`False`, binds to os_driver.
         """
         for port in self.ports:
-            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
-            self.main_session.send_command(
-                f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
-                privileged=True,
-                verify=True,
-            )
+            self._bind_port_to_driver(port, for_dpdk)
+
+    def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+        driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+        self.main_session.send_command(
+            f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+            privileged=True,
+            verify=True,
+        )
 
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
-- 
2.46.0

[RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously in the DTS framework the helper methods in the TestSutie
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port since the
framework limits where the TG is allowed to send packets. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/test_suite.py | 38 ++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..d5c0021503 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,8 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +197,19 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        packet = self._adjust_addresses(packet, sut_ingress, sut_egress)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,
@@ -208,18 +218,30 @@ def send_packet_and_capture(
             duration,
         )
 
-    def get_expected_packet(self, packet: Packet) -> Packet:
+    def get_expected_packet(
+        self, packet: Packet, sut_ingress: Port | None = None, sut_egress: Port | None = None
+    ) -> Packet:
         """Inject the proper L2/L3 addresses into `packet`.
 
         Args:
             packet: The packet to modify.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`.
 
         Returns:
             `packet` with injected L2/L3 addresses.
         """
-        return self._adjust_addresses(packet, expected=True)
-
-    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        return self._adjust_addresses(packet, sut_ingress, sut_egress, expected=True)
+
+    def _adjust_addresses(
+        self, packet: Packet, sut_ingress_port: Port, sut_egress_port: Port, expected: bool = False
+    ) -> Packet:
         """L2 and L3 address additions in both directions.
 
         Assumptions:
@@ -229,11 +251,13 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             packet: The packet to modify.
             expected: If :data:`True`, the direction is SUT -> TG,
                 otherwise the direction is TG -> SUT.
+            sut_ingress_port: The port to use as the Rx port on the SUT.
+            sut_egress_port: The port to use as the Tx port on the SUT.
         """
         if expected:
             # The packet enters the TG from SUT
             # update l2 addresses
-            packet.src = self._sut_port_egress.mac_address
+            packet.src = sut_egress_port.mac_address
             packet.dst = self._tg_port_ingress.mac_address
 
             # The packet is routed from TG egress to TG ingress
@@ -244,7 +268,7 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             # The packet leaves TG towards SUT
             # update l2 addresses
             packet.src = self._tg_port_egress.mac_address
-            packet.dst = self._sut_port_ingress.mac_address
+            packet.dst = sut_ingress_port.mac_address
 
             # The packet is routed from TG egress to TG ingress
             # update l3 addresses
-- 
2.46.0

[RFC PATCH v2 3/5] dts: add class for virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
     pci: str
 
 
-@dataclass(slots=True)
+@dataclass
 class Port:
     """Physical port on a node.
 
@@ -80,6 +80,41 @@ def pci(self) -> str:
         return self.identifier.pci
 
 
+@dataclass
+class VirtualFunction(Port):
+    """Virtual Function (VF) on a port.
+
+    DPDK applications often treat VFs the same as they do the physical ports (PFs) on the host.
+    For this reason VFs are represented in the framework as a type of port with some additional
+    metadata that allows the framework to more easily identify which device the VF belongs to as
+    well as where the VF originated from.
+
+    Attributes:
+        created_by_framework: :data:`True` if this VF represents one that the DTS framework created
+            on the node, :data:`False` otherwise.
+        pf_port: The PF that this VF was created on/gathered from.
+    """
+
+    created_by_framework: bool = False
+    pf_port: Port | None = None
+
+    def __init__(
+        self, node_name: str, config: PortConfig, created_by_framework: bool, pf_port: Port
+    ) -> None:
+        """Extends :meth:`Port.__init__` with VF specific metadata.
+
+        Args:
+            node_name: The name of the node the VF resides on.
+            config: Configuration information about the VF.
+            created_by_framework: :data:`True` if DTS created this VF, otherwise :data:`False` if
+                this class represents a VF that was preexisting on the node.
+            pf_port: The PF that this VF was created on/gathered from.
+        """
+        super().__init__(node_name, config)
+        self.created_by_framework = created_by_framework
+        self.pf_port = pf_port
+
+
 @dataclass(slots=True, frozen=True)
 class PortLink:
     """The physical, cabled connection between the ports.
-- 
2.46.0

[RFC PATCH v2 4/5] dts: add OS abstractions for creating virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Virtual functions in the framework are created using SR-IOV. The process
for doing this can vary depending on the operating system, so the
commands to create VFs have to be abstracted into different classes
based on the operating system. This patch adds the stubs for methods
that create VFs and get the PCI addresses of all VFs on a port to the
abstract class as well as a linux implementation for the methods.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/linux_session.py | 40 +++++++++++++++++++-
 dts/framework/testbed_model/os_session.py    | 40 ++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 99abc21353..738ddd7600 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -15,7 +15,11 @@
 
 from typing_extensions import NotRequired
 
-from framework.exception import ConfigurationError, RemoteCommandExecutionError
+from framework.exception import (
+    ConfigurationError,
+    InternalError,
+    RemoteCommandExecutionError,
+)
 from framework.utils import expand_range
 
 from .cpu import LogicalCore
@@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
         state = 1 if enable else 0
         self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
+
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+        if num > 0 and curr_num_vfs >= num:
+            self._logger.info(
+                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}."
+            )
+            return False
+        elif num > 0 and curr_num_vfs > 0:
+            self._logger.error(
+                f"Not enough VFs configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are configured. "
+                "DTS is unable to modify number of preexisting VFs."
+            )
+            raise InternalError("Failed to create VFs on port.")
+        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, verify=True)
+        return True
+
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
+        if curr_num_vfs > 0:
+            pci_addrs = self.send_command(
+                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
+                + f"{sys_bus_path}/virtfn*/uevent",
+                privileged=True,
+            )
+            return pci_addrs.stdout.splitlines()
+        else:
+            return []
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..191fc3c0c8 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         Args:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
+
+    @abstractmethod
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Update the number of virtual functions (VFs) on a port.
+
+        It should be noted that, due to the nature of VFs, if there are already VFs that exist on
+        the physical function (PF) prior to calling this function, additional ones cannot be added.
+        The only way to add more VFs is to remove the existing and then set the desired amount. For
+        this reason, this method will handle creation in the following order:
+
+        1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to
+        `num`
+        2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set
+        on it, but the total VFs that it has are less then `num`.
+        3. Create `num` VFs on the PF if there are none on it already
+
+        Args:
+            num: The number of VFs to set on the port.
+            pf_port: The port to add the VFs to.
+
+        Raises:
+            InternalError: If `pf_port` has less than `num` VFs configured on it
+                already.
+
+        Returns:
+            :data:`True` if this method successfully created VFs, :data:`False` if existing VFs
+            were used instead.
+        """
+
+    @abstractmethod
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Find the PCI addresses of all virtual functions (VFs) on the port `pf_port`.
+
+        Args:
+            pf_port: The port to find the VFs on.
+
+        Returns:
+            A list containing all of the PCI addresses of the VFs on the port. If the port has no
+            VFs then the list will be empty.
+        """
-- 
2.46.0

[RFC PATCH v2 5/5] dts: add functions for managing VFs to Node

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In order for test suites to create virtual functions there has to be
functions in the API that developers can use. This patch adds the
ability to create virtual functions to the Node API so that they are
reachable within test suites.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 96 ++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 85d4eb1f7c..101a8edfbc 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -14,6 +14,7 @@
 """
 
 import os
+import re
 import tarfile
 from abc import ABC
 from ipaddress import IPv4Interface, IPv6Interface
@@ -24,9 +25,10 @@
     OS,
     BuildTargetConfiguration,
     NodeConfiguration,
+    PortConfig,
     TestRunConfiguration,
 )
-from framework.exception import ConfigurationError
+from framework.exception import ConfigurationError, InternalError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
 
@@ -39,7 +41,7 @@
 )
 from .linux_session import LinuxSession
 from .os_session import OSSession
-from .port import Port
+from .port import Port, VirtualFunction
 
 
 class Node(ABC):
@@ -335,6 +337,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.
+            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]
+        ]
+        # 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
+            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
+
+        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])
+            self._bind_port_to_driver(vf_port)
+            self.ports.append(vf_port)
+            ret.append(vf_port)
+        return ret
+
+    def get_vfs_on_port(self, pf_port: Port) -> list[VirtualFunction]:
+        """Get all virtual functions (VFs) that DTS is aware of on `pf_port`.
+
+        Args:
+            pf_port: The port to search for the VFs on.
+
+        Returns:
+            A list of VFs in the framework that were created/gathered from `pf_port`.
+        """
+        return [p for p in self.ports if isinstance(p, VirtualFunction) and p.pf_port == pf_port]
+
+    def remove_virtual_functions(self, pf_port: Port) -> None:
+        """Removes all virtual functions (VFs) created on `pf_port` by DTS.
+
+        Finds all the VFs that were created from `pf_port` and either removes them if they were
+        created by the DTS framework or binds them back to their os_driver if they were preexisting
+        on the node.
+
+        Args:
+            pf_port: Port to remove the VFs from.
+        """
+        vf_ports = self.get_vfs_on_port(pf_port)
+        if any(vf.created_by_framework for vf in vf_ports):
+            self.main_session.set_num_virtual_functions(0, pf_port)
+        else:
+            self._logger.info("Skipping removing VFs since they were not created by DTS.")
+            # Bind all VFs that we are no longer using back to their original driver
+            for vf in vf_ports:
+                self._bind_port_to_driver(vf, for_dpdk=False)
+        self.ports = [p for p in self.ports if p not in vf_ports]
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
-- 
2.46.0

[RFC PATCH v2 0/5] dts: add VFs to the framework

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v2:
  * Accidentally left out a formatting fix in v1.

Jeremy Spewock (5):
  dts: allow binding only a single port to a different driver
  dts: parameterize what ports the TG sends packets to
  dts: add class for virtual functions
  dts: add OS abstractions for creating virtual functions
  dts: add functions for managing VFs to Node

 dts/framework/test_suite.py                  |  38 ++++--
 dts/framework/testbed_model/linux_session.py |  40 ++++++-
 dts/framework/testbed_model/node.py          | 115 +++++++++++++++++--
 dts/framework/testbed_model/os_session.py    |  40 +++++++
 dts/framework/testbed_model/port.py          |  37 +++++-
 5 files changed, 251 insertions(+), 19 deletions(-)

-- 
2.46.0

[RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
drivers to their respective ports to solve this problem.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 8e6181e424..85d4eb1f7c 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -167,12 +167,12 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
                 the setup steps will be taken.
         """
         self._copy_dpdk_tarball()
-        self.bind_ports_to_driver()
+        self.bind_all_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
         self.__remote_dpdk_dir = None
-        self.bind_ports_to_driver(for_dpdk=False)
+        self.bind_all_ports_to_driver(for_dpdk=False)
 
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
@@ -317,7 +317,7 @@ def _copy_dpdk_tarball(self) -> None:
         # then extract to remote path
         self.main_session.extract_remote_tarball(remote_tarball_path, self._remote_dpdk_dir)
 
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+    def bind_all_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the node to a driver.
 
         Args:
@@ -325,12 +325,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
                 If :data:`False`, binds to os_driver.
         """
         for port in self.ports:
-            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
-            self.main_session.send_command(
-                f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
-                privileged=True,
-                verify=True,
-            )
+            self._bind_port_to_driver(port, for_dpdk)
+
+    def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+        driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+        self.main_session.send_command(
+            f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+            privileged=True,
+            verify=True,
+        )
 
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
-- 
2.46.0

[RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously in the DTS framework the helper methods in the TestSutie
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port since the
framework limits where the TG is allowed to send packets. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/test_suite.py | 38 ++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..d5c0021503 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,8 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +197,19 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        packet = self._adjust_addresses(packet, sut_ingress, sut_egress)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,
@@ -208,18 +218,30 @@ def send_packet_and_capture(
             duration,
         )
 
-    def get_expected_packet(self, packet: Packet) -> Packet:
+    def get_expected_packet(
+        self, packet: Packet, sut_ingress: Port | None = None, sut_egress: Port | None = None
+    ) -> Packet:
         """Inject the proper L2/L3 addresses into `packet`.
 
         Args:
             packet: The packet to modify.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`.
 
         Returns:
             `packet` with injected L2/L3 addresses.
         """
-        return self._adjust_addresses(packet, expected=True)
-
-    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        return self._adjust_addresses(packet, sut_ingress, sut_egress, expected=True)
+
+    def _adjust_addresses(
+        self, packet: Packet, sut_ingress_port: Port, sut_egress_port: Port, expected: bool = False
+    ) -> Packet:
         """L2 and L3 address additions in both directions.
 
         Assumptions:
@@ -229,11 +251,13 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             packet: The packet to modify.
             expected: If :data:`True`, the direction is SUT -> TG,
                 otherwise the direction is TG -> SUT.
+            sut_ingress_port: The port to use as the Rx port on the SUT.
+            sut_egress_port: The port to use as the Tx port on the SUT.
         """
         if expected:
             # The packet enters the TG from SUT
             # update l2 addresses
-            packet.src = self._sut_port_egress.mac_address
+            packet.src = sut_egress_port.mac_address
             packet.dst = self._tg_port_ingress.mac_address
 
             # The packet is routed from TG egress to TG ingress
@@ -244,7 +268,7 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             # The packet leaves TG towards SUT
             # update l2 addresses
             packet.src = self._tg_port_egress.mac_address
-            packet.dst = self._sut_port_ingress.mac_address
+            packet.dst = sut_ingress_port.mac_address
 
             # The packet is routed from TG egress to TG ingress
             # update l3 addresses
-- 
2.46.0

[RFC PATCH v2 3/5] dts: add class for virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
     pci: str
 
 
-@dataclass(slots=True)
+@dataclass
 class Port:
     """Physical port on a node.
 
@@ -80,6 +80,41 @@ def pci(self) -> str:
         return self.identifier.pci
 
 
+@dataclass
+class VirtualFunction(Port):
+    """Virtual Function (VF) on a port.
+
+    DPDK applications often treat VFs the same as they do the physical ports (PFs) on the host.
+    For this reason VFs are represented in the framework as a type of port with some additional
+    metadata that allows the framework to more easily identify which device the VF belongs to as
+    well as where the VF originated from.
+
+    Attributes:
+        created_by_framework: :data:`True` if this VF represents one that the DTS framework created
+            on the node, :data:`False` otherwise.
+        pf_port: The PF that this VF was created on/gathered from.
+    """
+
+    created_by_framework: bool = False
+    pf_port: Port | None = None
+
+    def __init__(
+        self, node_name: str, config: PortConfig, created_by_framework: bool, pf_port: Port
+    ) -> None:
+        """Extends :meth:`Port.__init__` with VF specific metadata.
+
+        Args:
+            node_name: The name of the node the VF resides on.
+            config: Configuration information about the VF.
+            created_by_framework: :data:`True` if DTS created this VF, otherwise :data:`False` if
+                this class represents a VF that was preexisting on the node.
+            pf_port: The PF that this VF was created on/gathered from.
+        """
+        super().__init__(node_name, config)
+        self.created_by_framework = created_by_framework
+        self.pf_port = pf_port
+
+
 @dataclass(slots=True, frozen=True)
 class PortLink:
     """The physical, cabled connection between the ports.
-- 
2.46.0

[RFC PATCH v2 4/5] dts: add OS abstractions for creating virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Virtual functions in the framework are created using SR-IOV. The process
for doing this can vary depending on the operating system, so the
commands to create VFs have to be abstracted into different classes
based on the operating system. This patch adds the stubs for methods
that create VFs and get the PCI addresses of all VFs on a port to the
abstract class as well as a linux implementation for the methods.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/linux_session.py | 40 +++++++++++++++++++-
 dts/framework/testbed_model/os_session.py    | 40 ++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 99abc21353..738ddd7600 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -15,7 +15,11 @@
 
 from typing_extensions import NotRequired
 
-from framework.exception import ConfigurationError, RemoteCommandExecutionError
+from framework.exception import (
+    ConfigurationError,
+    InternalError,
+    RemoteCommandExecutionError,
+)
 from framework.utils import expand_range
 
 from .cpu import LogicalCore
@@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
         state = 1 if enable else 0
         self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
+
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+        if num > 0 and curr_num_vfs >= num:
+            self._logger.info(
+                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}."
+            )
+            return False
+        elif num > 0 and curr_num_vfs > 0:
+            self._logger.error(
+                f"Not enough VFs configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are configured. "
+                "DTS is unable to modify number of preexisting VFs."
+            )
+            raise InternalError("Failed to create VFs on port.")
+        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, verify=True)
+        return True
+
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
+        if curr_num_vfs > 0:
+            pci_addrs = self.send_command(
+                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
+                + f"{sys_bus_path}/virtfn*/uevent",
+                privileged=True,
+            )
+            return pci_addrs.stdout.splitlines()
+        else:
+            return []
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..191fc3c0c8 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         Args:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
+
+    @abstractmethod
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Update the number of virtual functions (VFs) on a port.
+
+        It should be noted that, due to the nature of VFs, if there are already VFs that exist on
+        the physical function (PF) prior to calling this function, additional ones cannot be added.
+        The only way to add more VFs is to remove the existing and then set the desired amount. For
+        this reason, this method will handle creation in the following order:
+
+        1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to
+        `num`
+        2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set
+        on it, but the total VFs that it has are less then `num`.
+        3. Create `num` VFs on the PF if there are none on it already
+
+        Args:
+            num: The number of VFs to set on the port.
+            pf_port: The port to add the VFs to.
+
+        Raises:
+            InternalError: If `pf_port` has less than `num` VFs configured on it
+                already.
+
+        Returns:
+            :data:`True` if this method successfully created VFs, :data:`False` if existing VFs
+            were used instead.
+        """
+
+    @abstractmethod
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Find the PCI addresses of all virtual functions (VFs) on the port `pf_port`.
+
+        Args:
+            pf_port: The port to find the VFs on.
+
+        Returns:
+            A list containing all of the PCI addresses of the VFs on the port. If the port has no
+            VFs then the list will be empty.
+        """
-- 
2.46.0

[RFC PATCH v2 5/5] dts: add functions for managing VFs to Node

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In order for test suites to create virtual functions there has to be
functions in the API that developers can use. This patch adds the
ability to create virtual functions to the Node API so that they are
reachable within test suites.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 96 ++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 85d4eb1f7c..101a8edfbc 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -14,6 +14,7 @@
 """
 
 import os
+import re
 import tarfile
 from abc import ABC
 from ipaddress import IPv4Interface, IPv6Interface
@@ -24,9 +25,10 @@
     OS,
     BuildTargetConfiguration,
     NodeConfiguration,
+    PortConfig,
     TestRunConfiguration,
 )
-from framework.exception import ConfigurationError
+from framework.exception import ConfigurationError, InternalError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
 
@@ -39,7 +41,7 @@
 )
 from .linux_session import LinuxSession
 from .os_session import OSSession
-from .port import Port
+from .port import Port, VirtualFunction
 
 
 class Node(ABC):
@@ -335,6 +337,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.
+            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]
+        ]
+        # 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
+            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
+
+        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])
+            self._bind_port_to_driver(vf_port)
+            self.ports.append(vf_port)
+            ret.append(vf_port)
+        return ret
+
+    def get_vfs_on_port(self, pf_port: Port) -> list[VirtualFunction]:
+        """Get all virtual functions (VFs) that DTS is aware of on `pf_port`.
+
+        Args:
+            pf_port: The port to search for the VFs on.
+
+        Returns:
+            A list of VFs in the framework that were created/gathered from `pf_port`.
+        """
+        return [p for p in self.ports if isinstance(p, VirtualFunction) and p.pf_port == pf_port]
+
+    def remove_virtual_functions(self, pf_port: Port) -> None:
+        """Removes all virtual functions (VFs) created on `pf_port` by DTS.
+
+        Finds all the VFs that were created from `pf_port` and either removes them if they were
+        created by the DTS framework or binds them back to their os_driver if they were preexisting
+        on the node.
+
+        Args:
+            pf_port: Port to remove the VFs from.
+        """
+        vf_ports = self.get_vfs_on_port(pf_port)
+        if any(vf.created_by_framework for vf in vf_ports):
+            self.main_session.set_num_virtual_functions(0, pf_port)
+        else:
+            self._logger.info("Skipping removing VFs since they were not created by DTS.")
+            # Bind all VFs that we are no longer using back to their original driver
+            for vf in vf_ports:
+                self._bind_port_to_driver(vf, for_dpdk=False)
+        self.ports = [p for p in self.ports if p not in vf_ports]
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
-- 
2.46.0

Re: [RFC PATCH v2 0/5] dts: add VFs to the framework

[Jeremy Spewock]

Apologies for sending out the v2 twice for this series, when I
attempted the first time the reply chain broke somehow and patchwork
didn't consider all the patches as one series. This should be fixed
now.

[RFC PATCH v3 0/5] dts: add VFs to the framework

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

rfc-v3:
 * add missing depends-on tags to appropriate commits.
 * adjust some other small issues in commit bodies
 * add changes to fix name of function in os_udp

Jeremy Spewock (5):
  dts: allow binding only a single port to a different driver
  dts: parameterize what ports the TG sends packets to
  dts: add class for virtual functions
  dts: add OS abstractions for creating virtual functions
  dts: add functions for managing VFs to Node

 dts/framework/test_suite.py                  |  38 ++++--
 dts/framework/testbed_model/linux_session.py |  40 ++++++-
 dts/framework/testbed_model/node.py          | 115 +++++++++++++++++--
 dts/framework/testbed_model/os_session.py    |  40 +++++++
 dts/framework/testbed_model/port.py          |  37 +++++-
 dts/tests/TestSuite_os_udp.py                |   4 +-
 6 files changed, 253 insertions(+), 21 deletions(-)

-- 
2.46.0

[RFC PATCH v3 1/5] dts: allow binding only a single port to a different driver

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
ports to their respective drviers to solve this problem.

Depends-on: patch-143101 ("dts: add binding to different drivers to TG
node")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 21 ++++++++++++---------
 dts/tests/TestSuite_os_udp.py       |  4 ++--
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 8e6181e424..85d4eb1f7c 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -167,12 +167,12 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
                 the setup steps will be taken.
         """
         self._copy_dpdk_tarball()
-        self.bind_ports_to_driver()
+        self.bind_all_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
         self.__remote_dpdk_dir = None
-        self.bind_ports_to_driver(for_dpdk=False)
+        self.bind_all_ports_to_driver(for_dpdk=False)
 
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
@@ -317,7 +317,7 @@ def _copy_dpdk_tarball(self) -> None:
         # then extract to remote path
         self.main_session.extract_remote_tarball(remote_tarball_path, self._remote_dpdk_dir)
 
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+    def bind_all_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the node to a driver.
 
         Args:
@@ -325,12 +325,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
                 If :data:`False`, binds to os_driver.
         """
         for port in self.ports:
-            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
-            self.main_session.send_command(
-                f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
-                privileged=True,
-                verify=True,
-            )
+            self._bind_port_to_driver(port, for_dpdk)
+
+    def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+        driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+        self.main_session.send_command(
+            f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+            privileged=True,
+            verify=True,
+        )
 
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
diff --git a/dts/tests/TestSuite_os_udp.py b/dts/tests/TestSuite_os_udp.py
index a78bd74139..5e9469bbac 100644
--- a/dts/tests/TestSuite_os_udp.py
+++ b/dts/tests/TestSuite_os_udp.py
@@ -23,7 +23,7 @@ def set_up_suite(self) -> None:
             Bind the SUT ports to the OS driver, configure the ports and configure the SUT
             to route traffic from if1 to if2.
         """
-        self.sut_node.bind_ports_to_driver(for_dpdk=False)
+        self.sut_node.bind_all_ports_to_driver(for_dpdk=False)
         self.configure_testbed_ipv4()
 
     def test_os_udp(self) -> None:
@@ -50,4 +50,4 @@ def tear_down_suite(self) -> None:
         """
         self.configure_testbed_ipv4(restore=True)
         # Assume other suites will likely need dpdk driver
-        self.sut_node.bind_ports_to_driver(for_dpdk=True)
+        self.sut_node.bind_all_ports_to_driver(for_dpdk=True)
-- 
2.46.0

[RFC PATCH v3 2/5] dts: parameterize what ports the TG sends packets to

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously in the DTS framework the helper methods in the TestSuite
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/test_suite.py | 38 ++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..d5c0021503 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,8 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +197,19 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        packet = self._adjust_addresses(packet, sut_ingress, sut_egress)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,
@@ -208,18 +218,30 @@ def send_packet_and_capture(
             duration,
         )
 
-    def get_expected_packet(self, packet: Packet) -> Packet:
+    def get_expected_packet(
+        self, packet: Packet, sut_ingress: Port | None = None, sut_egress: Port | None = None
+    ) -> Packet:
         """Inject the proper L2/L3 addresses into `packet`.
 
         Args:
             packet: The packet to modify.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`.
 
         Returns:
             `packet` with injected L2/L3 addresses.
         """
-        return self._adjust_addresses(packet, expected=True)
-
-    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        return self._adjust_addresses(packet, sut_ingress, sut_egress, expected=True)
+
+    def _adjust_addresses(
+        self, packet: Packet, sut_ingress_port: Port, sut_egress_port: Port, expected: bool = False
+    ) -> Packet:
         """L2 and L3 address additions in both directions.
 
         Assumptions:
@@ -229,11 +251,13 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             packet: The packet to modify.
             expected: If :data:`True`, the direction is SUT -> TG,
                 otherwise the direction is TG -> SUT.
+            sut_ingress_port: The port to use as the Rx port on the SUT.
+            sut_egress_port: The port to use as the Tx port on the SUT.
         """
         if expected:
             # The packet enters the TG from SUT
             # update l2 addresses
-            packet.src = self._sut_port_egress.mac_address
+            packet.src = sut_egress_port.mac_address
             packet.dst = self._tg_port_ingress.mac_address
 
             # The packet is routed from TG egress to TG ingress
@@ -244,7 +268,7 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             # The packet leaves TG towards SUT
             # update l2 addresses
             packet.src = self._tg_port_egress.mac_address
-            packet.dst = self._sut_port_ingress.mac_address
+            packet.dst = sut_ingress_port.mac_address
 
             # The packet is routed from TG egress to TG ingress
             # update l3 addresses
-- 
2.46.0

[RFC PATCH v3 3/5] dts: add class for virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
     pci: str
 
 
-@dataclass(slots=True)
+@dataclass
 class Port:
     """Physical port on a node.
 
@@ -80,6 +80,41 @@ def pci(self) -> str:
         return self.identifier.pci
 
 
+@dataclass
+class VirtualFunction(Port):
+    """Virtual Function (VF) on a port.
+
+    DPDK applications often treat VFs the same as they do the physical ports (PFs) on the host.
+    For this reason VFs are represented in the framework as a type of port with some additional
+    metadata that allows the framework to more easily identify which device the VF belongs to as
+    well as where the VF originated from.
+
+    Attributes:
+        created_by_framework: :data:`True` if this VF represents one that the DTS framework created
+            on the node, :data:`False` otherwise.
+        pf_port: The PF that this VF was created on/gathered from.
+    """
+
+    created_by_framework: bool = False
+    pf_port: Port | None = None
+
+    def __init__(
+        self, node_name: str, config: PortConfig, created_by_framework: bool, pf_port: Port
+    ) -> None:
+        """Extends :meth:`Port.__init__` with VF specific metadata.
+
+        Args:
+            node_name: The name of the node the VF resides on.
+            config: Configuration information about the VF.
+            created_by_framework: :data:`True` if DTS created this VF, otherwise :data:`False` if
+                this class represents a VF that was preexisting on the node.
+            pf_port: The PF that this VF was created on/gathered from.
+        """
+        super().__init__(node_name, config)
+        self.created_by_framework = created_by_framework
+        self.pf_port = pf_port
+
+
 @dataclass(slots=True, frozen=True)
 class PortLink:
     """The physical, cabled connection between the ports.
-- 
2.46.0

[RFC PATCH v3 4/5] dts: add OS abstractions for creating virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Virtual functions in the framework are created using SR-IOV. The process
for doing this can vary depending on the operating system, so the
commands to create VFs have to be abstracted into different classes
based on the operating system. This patch adds the stubs for methods
that create VFs and get the PCI addresses of all VFs on a port to the
abstract class as well as a linux implementation for the methods.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/linux_session.py | 40 +++++++++++++++++++-
 dts/framework/testbed_model/os_session.py    | 40 ++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 99abc21353..738ddd7600 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -15,7 +15,11 @@
 
 from typing_extensions import NotRequired
 
-from framework.exception import ConfigurationError, RemoteCommandExecutionError
+from framework.exception import (
+    ConfigurationError,
+    InternalError,
+    RemoteCommandExecutionError,
+)
 from framework.utils import expand_range
 
 from .cpu import LogicalCore
@@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
         state = 1 if enable else 0
         self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
+
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+        if num > 0 and curr_num_vfs >= num:
+            self._logger.info(
+                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}."
+            )
+            return False
+        elif num > 0 and curr_num_vfs > 0:
+            self._logger.error(
+                f"Not enough VFs configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are configured. "
+                "DTS is unable to modify number of preexisting VFs."
+            )
+            raise InternalError("Failed to create VFs on port.")
+        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, verify=True)
+        return True
+
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
+        if curr_num_vfs > 0:
+            pci_addrs = self.send_command(
+                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
+                + f"{sys_bus_path}/virtfn*/uevent",
+                privileged=True,
+            )
+            return pci_addrs.stdout.splitlines()
+        else:
+            return []
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..191fc3c0c8 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         Args:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
+
+    @abstractmethod
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Update the number of virtual functions (VFs) on a port.
+
+        It should be noted that, due to the nature of VFs, if there are already VFs that exist on
+        the physical function (PF) prior to calling this function, additional ones cannot be added.
+        The only way to add more VFs is to remove the existing and then set the desired amount. For
+        this reason, this method will handle creation in the following order:
+
+        1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to
+        `num`
+        2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set
+        on it, but the total VFs that it has are less then `num`.
+        3. Create `num` VFs on the PF if there are none on it already
+
+        Args:
+            num: The number of VFs to set on the port.
+            pf_port: The port to add the VFs to.
+
+        Raises:
+            InternalError: If `pf_port` has less than `num` VFs configured on it
+                already.
+
+        Returns:
+            :data:`True` if this method successfully created VFs, :data:`False` if existing VFs
+            were used instead.
+        """
+
+    @abstractmethod
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Find the PCI addresses of all virtual functions (VFs) on the port `pf_port`.
+
+        Args:
+            pf_port: The port to find the VFs on.
+
+        Returns:
+            A list containing all of the PCI addresses of the VFs on the port. If the port has no
+            VFs then the list will be empty.
+        """
-- 
2.46.0

[RFC PATCH v3 5/5] dts: add functions for managing VFs to Node

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In order for test suites to create virtual functions there has to be
functions in the API that developers can use. This patch adds the
ability to create virtual functions to the Node API so that they are
reachable within test suites.

Bugzilla ID: 1500
Depends-on: patch-143101 ("dts: add binding to different drivers to TG
node")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 96 ++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 85d4eb1f7c..101a8edfbc 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -14,6 +14,7 @@
 """
 
 import os
+import re
 import tarfile
 from abc import ABC
 from ipaddress import IPv4Interface, IPv6Interface
@@ -24,9 +25,10 @@
     OS,
     BuildTargetConfiguration,
     NodeConfiguration,
+    PortConfig,
     TestRunConfiguration,
 )
-from framework.exception import ConfigurationError
+from framework.exception import ConfigurationError, InternalError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
 
@@ -39,7 +41,7 @@
 )
 from .linux_session import LinuxSession
 from .os_session import OSSession
-from .port import Port
+from .port import Port, VirtualFunction
 
 
 class Node(ABC):
@@ -335,6 +337,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.
+            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]
+        ]
+        # 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
+            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
+
+        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])
+            self._bind_port_to_driver(vf_port)
+            self.ports.append(vf_port)
+            ret.append(vf_port)
+        return ret
+
+    def get_vfs_on_port(self, pf_port: Port) -> list[VirtualFunction]:
+        """Get all virtual functions (VFs) that DTS is aware of on `pf_port`.
+
+        Args:
+            pf_port: The port to search for the VFs on.
+
+        Returns:
+            A list of VFs in the framework that were created/gathered from `pf_port`.
+        """
+        return [p for p in self.ports if isinstance(p, VirtualFunction) and p.pf_port == pf_port]
+
+    def remove_virtual_functions(self, pf_port: Port) -> None:
+        """Removes all virtual functions (VFs) created on `pf_port` by DTS.
+
+        Finds all the VFs that were created from `pf_port` and either removes them if they were
+        created by the DTS framework or binds them back to their os_driver if they were preexisting
+        on the node.
+
+        Args:
+            pf_port: Port to remove the VFs from.
+        """
+        vf_ports = self.get_vfs_on_port(pf_port)
+        if any(vf.created_by_framework for vf in vf_ports):
+            self.main_session.set_num_virtual_functions(0, pf_port)
+        else:
+            self._logger.info("Skipping removing VFs since they were not created by DTS.")
+            # Bind all VFs that we are no longer using back to their original driver
+            for vf in vf_ports:
+                self._bind_port_to_driver(vf, for_dpdk=False)
+        self.ports = [p for p in self.ports if p not in vf_ports]
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
-- 
2.46.0

[PATCH v4 0/5] dts: add VFs to the framework

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v4:
 * apply to next-dts

Jeremy Spewock (5):
  dts: allow binding only a single port to a different driver
  dts: parameterize what ports the TG sends packets to
  dts: add class for virtual functions
  dts: add OS abstractions for creating virtual functions
  dts: add functions for managing VFs to Node

 dts/framework/test_suite.py                  |  48 ++++++--
 dts/framework/testbed_model/linux_session.py |  40 ++++++-
 dts/framework/testbed_model/node.py          | 116 +++++++++++++++++--
 dts/framework/testbed_model/os_session.py    |  40 +++++++
 dts/framework/testbed_model/port.py          |  37 +++++-
 dts/tests/TestSuite_os_udp.py                |   4 +-
 6 files changed, 260 insertions(+), 25 deletions(-)

-- 
2.46.0

[PATCH v4 1/5] dts: allow binding only a single port to a different driver

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
ports to their respective drviers to solve this problem.

Depends-on: patch-144318 ("dts: add binding to different drivers to TG
node")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 21 ++++++++++++---------
 dts/tests/TestSuite_os_udp.py       |  4 ++--
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 6484e16a0f..106e189ce3 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -138,11 +138,11 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
                 the setup steps will be taken. This is unused in this method, but subclasses that
                 extend this method may need it.
         """
-        self.bind_ports_to_driver()
+        self.bind_all_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
         """Bind ports to their OS drivers."""
-        self.bind_ports_to_driver(for_dpdk=False)
+        self.bind_all_ports_to_driver(for_dpdk=False)
 
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
@@ -258,7 +258,7 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]:
         else:
             return func
 
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+    def bind_all_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the node to a driver.
 
         Args:
@@ -266,12 +266,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
                 If :data:`False`, binds to os_driver.
         """
         for port in self.ports:
-            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
-            self.main_session.send_command(
-                f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
-                privileged=True,
-                verify=True,
-            )
+            self._bind_port_to_driver(port, for_dpdk)
+
+    def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+        driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+        self.main_session.send_command(
+            f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+            privileged=True,
+            verify=True,
+        )
 
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
diff --git a/dts/tests/TestSuite_os_udp.py b/dts/tests/TestSuite_os_udp.py
index a78bd74139..5e9469bbac 100644
--- a/dts/tests/TestSuite_os_udp.py
+++ b/dts/tests/TestSuite_os_udp.py
@@ -23,7 +23,7 @@ def set_up_suite(self) -> None:
             Bind the SUT ports to the OS driver, configure the ports and configure the SUT
             to route traffic from if1 to if2.
         """
-        self.sut_node.bind_ports_to_driver(for_dpdk=False)
+        self.sut_node.bind_all_ports_to_driver(for_dpdk=False)
         self.configure_testbed_ipv4()
 
     def test_os_udp(self) -> None:
@@ -50,4 +50,4 @@ def tear_down_suite(self) -> None:
         """
         self.configure_testbed_ipv4(restore=True)
         # Assume other suites will likely need dpdk driver
-        self.sut_node.bind_ports_to_driver(for_dpdk=True)
+        self.sut_node.bind_all_ports_to_driver(for_dpdk=True)
-- 
2.46.0

Re: [PATCH v4 1/5] dts: allow binding only a single port to a different driver

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Previously the DTS framework only included methods that bind all ports
> that the test run was aware of to either the DPDK driver or the OS
> driver. There are however some cases, like creating virtual functions,
> where you would want some ports bound to the OS driver and others bound
> to their DPDK driver. This patch adds the ability to bind individual
> ports to their respective drviers to solve this problem.

Typo: drviers

> 
> Depends-on: patch-144318 ("dts: add binding to different drivers to TG
> node")
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>   dts/framework/testbed_model/node.py | 21 ++++++++++++---------
>   dts/tests/TestSuite_os_udp.py       |  4 ++--
>   2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py

> @@ -266,12 +266,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
>                   If :data:`False`, binds to os_driver.
>           """
>           for port in self.ports:
> -            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
> -            self.main_session.send_command(
> -                f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
> -                privileged=True,
> -                verify=True,
> -            )
> +            self._bind_port_to_driver(port, for_dpdk)
> +
> +    def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
> +        driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
> +        self.main_session.send_command(
> +            f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
> +            privileged=True,
> +            verify=True,
> +        )

I noticed we're doing this port by port, but we can bind multiple ports 
in one devbind run - just pass all ports to it. I think this improvement 
is well worth it.

Re: [PATCH v4 1/5] dts: allow binding only a single port to a different driver

[Luca Vizzarro]

Hi there,

I agree with Juraj's comments on the typo and running dpdk-devbind once.

Also noticed the commit subject is way too long, should be max 50 
characters. Is it possible to make this more concise?

Best,
Luca

[PATCH v4 2/5] dts: parameterize what ports the TG sends packets to

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously in the DTS framework the helper methods in the TestSuite
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/test_suite.py | 48 +++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 051509fb86..9b707ca46d 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -186,6 +186,8 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -196,14 +198,16 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
         return self.send_packets_and_capture(
-            [packet],
-            filter_config,
-            duration,
+            [packet], filter_config, duration, sut_ingress, sut_egress
         )
 
     def send_packets_and_capture(
@@ -211,6 +215,8 @@ def send_packets_and_capture(
         packets: list[Packet],
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packets` using the associated TG.
 
@@ -221,11 +227,19 @@ def send_packets_and_capture(
             packets: The packets to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
-        packets = [self._adjust_addresses(packet) for packet in packets]
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        packets = [self._adjust_addresses(packet, sut_ingress, sut_egress) for packet in packets]
         return self.tg_node.send_packets_and_capture(
             packets,
             self._tg_port_egress,
@@ -234,18 +248,30 @@ def send_packets_and_capture(
             duration,
         )
 
-    def get_expected_packet(self, packet: Packet) -> Packet:
+    def get_expected_packet(
+        self, packet: Packet, sut_ingress: Port | None = None, sut_egress: Port | None = None
+    ) -> Packet:
         """Inject the proper L2/L3 addresses into `packet`.
 
         Args:
             packet: The packet to modify.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`.
 
         Returns:
             `packet` with injected L2/L3 addresses.
         """
-        return self._adjust_addresses(packet, expected=True)
-
-    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        return self._adjust_addresses(packet, sut_ingress, sut_egress, expected=True)
+
+    def _adjust_addresses(
+        self, packet: Packet, sut_ingress_port: Port, sut_egress_port: Port, expected: bool = False
+    ) -> Packet:
         """L2 and L3 address additions in both directions.
 
         Assumptions:
@@ -255,11 +281,13 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             packet: The packet to modify.
             expected: If :data:`True`, the direction is SUT -> TG,
                 otherwise the direction is TG -> SUT.
+            sut_ingress_port: The port to use as the Rx port on the SUT.
+            sut_egress_port: The port to use as the Tx port on the SUT.
         """
         if expected:
             # The packet enters the TG from SUT
             # update l2 addresses
-            packet.src = self._sut_port_egress.mac_address
+            packet.src = sut_egress_port.mac_address
             packet.dst = self._tg_port_ingress.mac_address
 
             # The packet is routed from TG egress to TG ingress
@@ -270,7 +298,7 @@ def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             # The packet leaves TG towards SUT
             # update l2 addresses
             packet.src = self._tg_port_egress.mac_address
-            packet.dst = self._sut_port_ingress.mac_address
+            packet.dst = sut_ingress_port.mac_address
 
             # The packet is routed from TG egress to TG ingress
             # update l3 addresses
-- 
2.46.0

Re: [PATCH v4 2/5] dts: parameterize what ports the TG sends packets to

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Previously in the DTS framework the helper methods in the TestSuite
> class designated ports as either ingress or egress ports and would wrap
> the methods of the traffic generator to allow packets to only flow to
> those designated ingress or egress ports. This is undesirable in some
> cases, such as when you have virtual functions on top of your port,
> where the TG ports can send to more than one SUT port. This patch
> solves this problem by creating optional parameters that allow the user
> to specify which port to gather the MAC addresses from when sending and
> receiving packets.
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

I'm not a fan of exposing the functionality in this way. The developers 
needs to fiddle with ports and there are likely better ways to 
accomplish this.

Ideally, the only information the dev would provide that a test case is 
a VF test case and everything else would happen under the hood in the 
TestCase class.

Barring that, we could decorate the whole TestSuite as requiring VFs, 
which would result in automatically creating and removing the VFs in 
setup/teardown (test case marking would be similar, but possibly more 
complicated, especially if we wanted to abide only by test cases 
selected in a given test run). Then the test cases could pass a simple 
vf=True parameter to the send/receive methods.

Re: [PATCH v4 2/5] dts: parameterize what ports the TG sends packets to

[Luca Vizzarro]

Hi there,

once again I am in agreement with Juraj, this approach may risk to 
complicate things further. As he mentioned, we should allow TestSuite to 
be marked for VFs and they should be able to automate the setup behind 
the scenes. If we wanted to distinguish VF and PF test cases, I think 
that's still doable, maybe group VF and PF ones together for easier 
setup and tear down.

I re-iterate the problem with the commit subject being too long and verbose.

Luca

[PATCH v4 3/5] dts: add class for virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In DPDK applications virtual functions are treated the same as ports,
but within the framework there are benefits to differentiating the two
in order to add more metadata to VFs about where they originate from.
For this reason this patch adds a new class for handling virtual
functions that extends the Port class with some additional information
about the VF.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/port.py b/dts/framework/testbed_model/port.py
index 817405bea4..c1d85fec2b 100644
--- a/dts/framework/testbed_model/port.py
+++ b/dts/framework/testbed_model/port.py
@@ -27,7 +27,7 @@ class PortIdentifier:
     pci: str
 
 
-@dataclass(slots=True)
+@dataclass
 class Port:
     """Physical port on a node.
 
@@ -80,6 +80,41 @@ def pci(self) -> str:
         return self.identifier.pci
 
 
+@dataclass
+class VirtualFunction(Port):
+    """Virtual Function (VF) on a port.
+
+    DPDK applications often treat VFs the same as they do the physical ports (PFs) on the host.
+    For this reason VFs are represented in the framework as a type of port with some additional
+    metadata that allows the framework to more easily identify which device the VF belongs to as
+    well as where the VF originated from.
+
+    Attributes:
+        created_by_framework: :data:`True` if this VF represents one that the DTS framework created
+            on the node, :data:`False` otherwise.
+        pf_port: The PF that this VF was created on/gathered from.
+    """
+
+    created_by_framework: bool = False
+    pf_port: Port | None = None
+
+    def __init__(
+        self, node_name: str, config: PortConfig, created_by_framework: bool, pf_port: Port
+    ) -> None:
+        """Extends :meth:`Port.__init__` with VF specific metadata.
+
+        Args:
+            node_name: The name of the node the VF resides on.
+            config: Configuration information about the VF.
+            created_by_framework: :data:`True` if DTS created this VF, otherwise :data:`False` if
+                this class represents a VF that was preexisting on the node.
+            pf_port: The PF that this VF was created on/gathered from.
+        """
+        super().__init__(node_name, config)
+        self.created_by_framework = created_by_framework
+        self.pf_port = pf_port
+
+
 @dataclass(slots=True, frozen=True)
 class PortLink:
     """The physical, cabled connection between the ports.
-- 
2.46.0

Re: [PATCH v4 3/5] dts: add class for virtual functions

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> In DPDK applications virtual functions are treated the same as ports,
> but within the framework there are benefits to differentiating the two
> in order to add more metadata to VFs about where they originate from.
> For this reason this patch adds a new class for handling virtual
> functions that extends the Port class with some additional information
> about the VF.
> 
> Bugzilla ID: 1500
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>   dts/framework/testbed_model/port.py | 37 ++++++++++++++++++++++++++++-
>   1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/dts/framework/testbed_model/port.py b/dts/framework/testbed_model/port.py
> index 817405bea4..c1d85fec2b 100644
> --- a/dts/framework/testbed_model/port.py
> +++ b/dts/framework/testbed_model/port.py
> @@ -27,7 +27,7 @@ class PortIdentifier:
>       pci: str
>   
>   
> -@dataclass(slots=True)
> +@dataclass

This should be explained in the commit message.

>   class Port:
>       """Physical port on a node.
>   
> @@ -80,6 +80,41 @@ def pci(self) -> str:
>           return self.identifier.pci
>   
>   
> +@dataclass
> +class VirtualFunction(Port):
> +    """Virtual Function (VF) on a port.
> +
> +    DPDK applications often treat VFs the same as they do the physical ports (PFs) on the host.
> +    For this reason VFs are represented in the framework as a type of port with some additional
> +    metadata that allows the framework to more easily identify which device the VF belongs to as
> +    well as where the VF originated from.
> +
> +    Attributes:
> +        created_by_framework: :data:`True` if this VF represents one that the DTS framework created
> +            on the node, :data:`False` otherwise.

I had to go look in the other patches to understand why this is here. 
The patch split should be redone along logical lines (one patch should 
contain related logic, now the related logic is in basically all 
patches), not files (that doesn't help with review and it's also not 
going to result in better git history).

But I figured out this is here because of cleanup. It makes more sense 
to me for framework to remember whether it created the port or not as 
opposed to port remembering it, especially when the framework is doing 
the cleanup and not the ports.

> +        pf_port: The PF that this VF was created on/gathered from.

Maybe it would make more sense to store the VF ports in the PF port. If 
we need to use VF ports, we can just refer to the PF port which has the 
benefit making it easier to use the proper link between ports.

And the framework can store which PF ports needs VF cleanup instead of 
storing which VFs needs cleaning.

> +    """
> +
> +    created_by_framework: bool = False
> +    pf_port: Port | None = None
> +
> +    def __init__(
> +        self, node_name: str, config: PortConfig, created_by_framework: bool, pf_port: Port
> +    ) -> None:
> +        """Extends :meth:`Port.__init__` with VF specific metadata.
> +
> +        Args:
> +            node_name: The name of the node the VF resides on.
> +            config: Configuration information about the VF.
> +            created_by_framework: :data:`True` if DTS created this VF, otherwise :data:`False` if
> +                this class represents a VF that was preexisting on the node.
> +            pf_port: The PF that this VF was created on/gathered from.
> +        """
> +        super().__init__(node_name, config)
> +        self.created_by_framework = created_by_framework
> +        self.pf_port = pf_port
> +
> +
>   @dataclass(slots=True, frozen=True)
>   class PortLink:
>       """The physical, cabled connection between the ports.

Re: [PATCH v4 3/5] dts: add class for virtual functions

[Luca Vizzarro]

+1

[PATCH v4 4/5] dts: add OS abstractions for creating virtual functions

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Virtual functions in the framework are created using SR-IOV. The process
for doing this can vary depending on the operating system, so the
commands to create VFs have to be abstracted into different classes
based on the operating system. This patch adds the stubs for methods
that create VFs and get the PCI addresses of all VFs on a port to the
abstract class as well as a linux implementation for the methods.

Bugzilla ID: 1500

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/linux_session.py | 40 +++++++++++++++++++-
 dts/framework/testbed_model/os_session.py    | 40 ++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/dts/framework/testbed_model/linux_session.py b/dts/framework/testbed_model/linux_session.py
index 544a665b83..2615411470 100644
--- a/dts/framework/testbed_model/linux_session.py
+++ b/dts/framework/testbed_model/linux_session.py
@@ -15,7 +15,11 @@
 
 from typing_extensions import NotRequired
 
-from framework.exception import ConfigurationError, RemoteCommandExecutionError
+from framework.exception import (
+    ConfigurationError,
+    InternalError,
+    RemoteCommandExecutionError,
+)
 from framework.utils import expand_range
 
 from .cpu import LogicalCore
@@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
         state = 1 if enable else 0
         self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
+
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+        if num > 0 and curr_num_vfs >= num:
+            self._logger.info(
+                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}."
+            )
+            return False
+        elif num > 0 and curr_num_vfs > 0:
+            self._logger.error(
+                f"Not enough VFs configured on port {pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are configured. "
+                "DTS is unable to modify number of preexisting VFs."
+            )
+            raise InternalError("Failed to create VFs on port.")
+        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, verify=True)
+        return True
+
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
+        if curr_num_vfs > 0:
+            pci_addrs = self.send_command(
+                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
+                + f"{sys_bus_path}/virtfn*/uevent",
+                privileged=True,
+            )
+            return pci_addrs.stdout.splitlines()
+        else:
+            return []
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index 79f56b289b..191fc3c0c8 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
         Args:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
+
+    @abstractmethod
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Update the number of virtual functions (VFs) on a port.
+
+        It should be noted that, due to the nature of VFs, if there are already VFs that exist on
+        the physical function (PF) prior to calling this function, additional ones cannot be added.
+        The only way to add more VFs is to remove the existing and then set the desired amount. For
+        this reason, this method will handle creation in the following order:
+
+        1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to
+        `num`
+        2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set
+        on it, but the total VFs that it has are less then `num`.
+        3. Create `num` VFs on the PF if there are none on it already
+
+        Args:
+            num: The number of VFs to set on the port.
+            pf_port: The port to add the VFs to.
+
+        Raises:
+            InternalError: If `pf_port` has less than `num` VFs configured on it
+                already.
+
+        Returns:
+            :data:`True` if this method successfully created VFs, :data:`False` if existing VFs
+            were used instead.
+        """
+
+    @abstractmethod
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Find the PCI addresses of all virtual functions (VFs) on the port `pf_port`.
+
+        Args:
+            pf_port: The port to find the VFs on.
+
+        Returns:
+            A list containing all of the PCI addresses of the VFs on the port. If the port has no
+            VFs then the list will be empty.
+        """
-- 
2.46.0

Re: [PATCH v4 4/5] dts: add OS abstractions for creating virtual functions

[Juraj Linkeš]

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

> @@ -210,3 +214,37 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
>           """Overrides :meth:`~.os_session.OSSession.configure_ipv4_forwarding`."""
>           state = 1 if enable else 0
>           self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
> +
> +    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
> +        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
> +        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
> +        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
> +        if num > 0 and curr_num_vfs >= num:
> +            self._logger.info(
> +                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
> +                f"{pf_port.identifier.node}."
> +            )
> +            return False
> +        elif num > 0 and curr_num_vfs > 0:

These two conditions could be written as:
if num > 0:
     if curr_num_vfs >= num:
         return False
     elif curr_num_vfs > 0:
         raise InternalError()

Maybe it's not worth the extra indent, but it's easier to understand, so 
I lean towards doing it this way.


> +    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
> +        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
> +        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
> +        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
> +        if curr_num_vfs > 0:
> +            pci_addrs = self.send_command(
> +                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
> +                + f"{sys_bus_path}/virtfn*/uevent",

We could use a TextParser here. Not sure if it's a good fit though.


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

> @@ -395,3 +395,43 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:

> +    @abstractmethod
> +    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
> +        """Update the number of virtual functions (VFs) on a port.
> +
> +        It should be noted that, due to the nature of VFs, if there are already VFs that exist on
> +        the physical function (PF) prior to calling this function, additional ones cannot be added.
> +        The only way to add more VFs is to remove the existing and then set the desired amount. For
> +        this reason, this method will handle creation in the following order:
> +
> +        1. Use existing VFs on the PF if the number of existing VFs is greater than or equal to
> +        `num`
> +        2. Throw an exception noting that VFs cannot be created if the PF has some VFs already set
> +        on it, but the total VFs that it has are less then `num`.
> +        3. Create `num` VFs on the PF if there are none on it already
> +
> +        Args:
> +            num: The number of VFs to set on the port.
> +            pf_port: The port to add the VFs to.
> +
> +        Raises:
> +            InternalError: If `pf_port` has less than `num` VFs configured on it
> +                already.
> +
> +        Returns:
> +            :data:`True` if this method successfully created VFs, :data:`False` if existing VFs
> +            were used instead.
> +        """

The whole docstring talks only about the creation of VFs, but we can 
also remove VFs with this.

Re: [PATCH v4 4/5] dts: add OS abstractions for creating virtual functions

[Luca Vizzarro]

Usual problem with the commit subject.

On 23/09/2024 19:42, jspewock@iol.unh.edu wrote:
> +    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:

I'd call this set_num_of_virtual_functions, set_virtual_functions_num, 
or set_number_of_vfs. I think I prefer the last one.

> +        """Overrides :meth:`~.os_session.OSSession.set_num_virtual_functions`."""
> +        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", "\\:")
> +        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
> +        if num > 0 and curr_num_vfs >= num:
> +            self._logger.info(
> +                f"{curr_num_vfs} VFs already configured on port {pf_port.identifier.pci} on node "
> +                f"{pf_port.identifier.node}."
> +            )
> +            return False
> +        elif num > 0 and curr_num_vfs > 0:
> +            self._logger.error(
> +                f"Not enough VFs configured on port {pf_port.identifier.pci} on node "
> +                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are configured. "
> +                "DTS is unable to modify number of preexisting VFs."
> +            )
> +            raise InternalError("Failed to create VFs on port.")

This is not an InternalError. That exception represents a bug internal 
to DTS, it's not related to the node not satisfying requirements. I am 
guessing this requires a new kind of error.

> +        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, verify=True)
> +        return True
> +
> +    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
> +        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
> +        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", "\\:")
> +        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}/sriov_numvfs").stdout)
> +        if curr_num_vfs > 0:
> +            pci_addrs = self.send_command(
> +                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
> +                + f"{sys_bus_path}/virtfn*/uevent",

I like the use of awk. Using a TextWrapper may be overkill when we have 
easier options like this one.


> +    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
> +        """Update the number of virtual functions (VFs) on a port.

Like Juraj mentioned, this docstring should say that it can be used for 
creating and removing VFs.

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

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

In order for test suites to create virtual functions there has to be
functions in the API that developers can use. This patch adds the
ability to create virtual functions to the Node API so that they are
reachable within test suites.

Bugzilla ID: 1500
Depends-on: patch-144318 ("dts: add binding to different drivers to TG
node")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/testbed_model/node.py | 97 ++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 3 deletions(-)

diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 106e189ce3..d9b6ee040b 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -13,7 +13,7 @@
 The :func:`~Node.skip_setup` decorator can be used without subclassing.
 """
 
-
+import re
 from abc import ABC, abstractmethod
 from ipaddress import IPv4Interface, IPv6Interface
 from pathlib import PurePath
@@ -23,9 +23,10 @@
     OS,
     BuildTargetConfiguration,
     NodeConfiguration,
+    PortConfig,
     TestRunConfiguration,
 )
-from framework.exception import ConfigurationError
+from framework.exception import ConfigurationError, InternalError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
 
@@ -38,7 +39,7 @@
 )
 from .linux_session import LinuxSession
 from .os_session import OSSession
-from .port import Port
+from .port import Port, VirtualFunction
 
 
 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.
+            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]
+        ]
+        # 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
+            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
+
+        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])
+            self._bind_port_to_driver(vf_port)
+            self.ports.append(vf_port)
+            ret.append(vf_port)
+        return ret
+
+    def get_vfs_on_port(self, pf_port: Port) -> list[VirtualFunction]:
+        """Get all virtual functions (VFs) that DTS is aware of on `pf_port`.
+
+        Args:
+            pf_port: The port to search for the VFs on.
+
+        Returns:
+            A list of VFs in the framework that were created/gathered from `pf_port`.
+        """
+        return [p for p in self.ports if isinstance(p, VirtualFunction) and p.pf_port == pf_port]
+
+    def remove_virtual_functions(self, pf_port: Port) -> None:
+        """Removes all virtual functions (VFs) created on `pf_port` by DTS.
+
+        Finds all the VFs that were created from `pf_port` and either removes them if they were
+        created by the DTS framework or binds them back to their os_driver if they were preexisting
+        on the node.
+
+        Args:
+            pf_port: Port to remove the VFs from.
+        """
+        vf_ports = self.get_vfs_on_port(pf_port)
+        if any(vf.created_by_framework for vf in vf_ports):
+            self.main_session.set_num_virtual_functions(0, pf_port)
+        else:
+            self._logger.info("Skipping removing VFs since they were not created by DTS.")
+            # Bind all VFs that we are no longer using back to their original driver
+            for vf in vf_ports:
+                self._bind_port_to_driver(vf, for_dpdk=False)
+        self.ports = [p for p in self.ports if p not in vf_ports]
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
-- 
2.46.0

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

[Juraj Linkeš]

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

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

[Luca Vizzarro]

On 23/09/2024 19:42, jspewock@iol.unh.edu wrote:
> +
> +    def get_vfs_on_port(self, pf_port: Port) -> list[VirtualFunction]:
> +        """Get all virtual functions (VFs) that DTS is aware of on `pf_port`.
> +
> +        Args:
> +            pf_port: The port to search for the VFs on.
> +
> +        Returns:
> +            A list of VFs in the framework that were created/gathered from `pf_port`.
> +        """
> +        return [p for p in self.ports if isinstance(p, VirtualFunction) and p.pf_port == pf_port]

If the change proposed by Juraj to store VFs under the PF goes through, 
this could end up being simpler.

> +
> +    def remove_virtual_functions(self, pf_port: Port) -> None:
> +        """Removes all virtual functions (VFs) created on `pf_port` by DTS.
> +
> +        Finds all the VFs that were created from `pf_port` and either removes them if they were
> +        created by the DTS framework or binds them back to their os_driver if they were preexisting
> +        on the node.
> +
> +        Args:
> +            pf_port: Port to remove the VFs from.
> +        """
> +        vf_ports = self.get_vfs_on_port(pf_port)
> +        if any(vf.created_by_framework for vf in vf_ports):
> +            self.main_session.set_num_virtual_functions(0, pf_port)
> +        else:
> +            self._logger.info("Skipping removing VFs since they were not created by DTS.")

This will cause VFs that were created by us to not be removed. We should 
ensure at least these ones are cleaned up.

Re: [PATCH v4 0/5] dts: add VFs to the framework

[Juraj Linkeš]

On 23. 9. 2024 20:42, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 

This is no longer an RFC, the body should be there so that we know what 
the state after it stopped being an RFC is.

> v4:
>   * apply to next-dts
> 
> Jeremy Spewock (5):
>    dts: allow binding only a single port to a different driver
>    dts: parameterize what ports the TG sends packets to
>    dts: add class for virtual functions
>    dts: add OS abstractions for creating virtual functions
>    dts: add functions for managing VFs to Node
> 
>   dts/framework/test_suite.py                  |  48 ++++++--
>   dts/framework/testbed_model/linux_session.py |  40 ++++++-
>   dts/framework/testbed_model/node.py          | 116 +++++++++++++++++--
>   dts/framework/testbed_model/os_session.py    |  40 +++++++
>   dts/framework/testbed_model/port.py          |  37 +++++-
>   dts/tests/TestSuite_os_udp.py                |   4 +-
>   6 files changed, 260 insertions(+), 25 deletions(-)
>