DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/5] dts: add VFs to the framework
@ 2024-08-21 19:15 jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 1/5] dts: allow binding only a single port to a different driver jspewock
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:15 UTC (permalink / raw)
  To: npratte, thomas, Luca.Vizzarro, yoan.picchi, alex.chapman,
	juraj.linkes, probb, wathsala.vithanage, paul.szczepanek,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v1 1/5] dts: allow binding only a single port to a different driver
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
@ 2024-08-21 19:15 ` jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 2/5] dts: parameterize what ports the TG sends packets to jspewock
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:15 UTC (permalink / raw)
  To: npratte, thomas, Luca.Vizzarro, yoan.picchi, alex.chapman,
	juraj.linkes, probb, wathsala.vithanage, paul.szczepanek,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v1 2/5] dts: parameterize what ports the TG sends packets to
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 1/5] dts: allow binding only a single port to a different driver jspewock
@ 2024-08-21 19:15 ` jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 3/5] dts: add class for virtual functions jspewock
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:15 UTC (permalink / raw)
  To: npratte, thomas, Luca.Vizzarro, yoan.picchi, alex.chapman,
	juraj.linkes, probb, wathsala.vithanage, paul.szczepanek,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v1 3/5] dts: add class for virtual functions
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 1/5] dts: allow binding only a single port to a different driver jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 2/5] dts: parameterize what ports the TG sends packets to jspewock
@ 2024-08-21 19:15 ` jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 4/5] dts: add OS abstractions for creating " jspewock
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:15 UTC (permalink / raw)
  To: npratte, thomas, Luca.Vizzarro, yoan.picchi, alex.chapman,
	juraj.linkes, probb, wathsala.vithanage, paul.szczepanek,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v1 4/5] dts: add OS abstractions for creating virtual functions
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (2 preceding siblings ...)
  2024-08-21 19:15 ` [RFC PATCH v1 3/5] dts: add class for virtual functions jspewock
@ 2024-08-21 19:15 ` jspewock
  2024-08-21 19:15 ` [RFC PATCH v1 5/5] dts: add functions for managing VFs to Node jspewock
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:15 UTC (permalink / raw)
  To: npratte, thomas, Luca.Vizzarro, yoan.picchi, alex.chapman,
	juraj.linkes, probb, wathsala.vithanage, paul.szczepanek,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v1 5/5] dts: add functions for managing VFs to Node
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (3 preceding siblings ...)
  2024-08-21 19:15 ` [RFC PATCH v1 4/5] dts: add OS abstractions for creating " jspewock
@ 2024-08-21 19:15 ` jspewock
  2024-08-21 19:21 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:15 UTC (permalink / raw)
  To: npratte, thomas, Luca.Vizzarro, yoan.picchi, alex.chapman,
	juraj.linkes, probb, wathsala.vithanage, paul.szczepanek,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 0/5] dts: add VFs to the framework
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (4 preceding siblings ...)
  2024-08-21 19:15 ` [RFC PATCH v1 5/5] dts: add functions for managing VFs to Node jspewock
@ 2024-08-21 19:21 ` jspewock
  2024-08-21 19:21 ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:21 UTC (permalink / raw)
  To: wathsala.vithanage, npratte, thomas, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, yoan.picchi, alex.chapman, juraj.linkes,
	paul.szczepanek
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (5 preceding siblings ...)
  2024-08-21 19:21 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
@ 2024-08-21 19:21 ` jspewock
  2024-08-21 19:21 ` [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to jspewock
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:21 UTC (permalink / raw)
  To: wathsala.vithanage, npratte, thomas, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, yoan.picchi, alex.chapman, juraj.linkes,
	paul.szczepanek
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (6 preceding siblings ...)
  2024-08-21 19:21 ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
@ 2024-08-21 19:21 ` jspewock
  2024-08-21 19:21 ` [RFC PATCH v2 3/5] dts: add class for virtual functions jspewock
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:21 UTC (permalink / raw)
  To: wathsala.vithanage, npratte, thomas, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, yoan.picchi, alex.chapman, juraj.linkes,
	paul.szczepanek
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 3/5] dts: add class for virtual functions
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (7 preceding siblings ...)
  2024-08-21 19:21 ` [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to jspewock
@ 2024-08-21 19:21 ` jspewock
  2024-08-21 19:21 ` [RFC PATCH v2 4/5] dts: add OS abstractions for creating " jspewock
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:21 UTC (permalink / raw)
  To: wathsala.vithanage, npratte, thomas, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, yoan.picchi, alex.chapman, juraj.linkes,
	paul.szczepanek
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 4/5] dts: add OS abstractions for creating virtual functions
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (8 preceding siblings ...)
  2024-08-21 19:21 ` [RFC PATCH v2 3/5] dts: add class for virtual functions jspewock
@ 2024-08-21 19:21 ` jspewock
  2024-08-21 19:21 ` [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node jspewock
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:21 UTC (permalink / raw)
  To: wathsala.vithanage, npratte, thomas, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, yoan.picchi, alex.chapman, juraj.linkes,
	paul.szczepanek
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (9 preceding siblings ...)
  2024-08-21 19:21 ` [RFC PATCH v2 4/5] dts: add OS abstractions for creating " jspewock
@ 2024-08-21 19:21 ` jspewock
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
  12 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:21 UTC (permalink / raw)
  To: wathsala.vithanage, npratte, thomas, Honnappa.Nagarahalli,
	Luca.Vizzarro, probb, yoan.picchi, alex.chapman, juraj.linkes,
	paul.szczepanek
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 0/5] dts: add VFs to the framework
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (10 preceding siblings ...)
  2024-08-21 19:21 ` [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node jspewock
@ 2024-08-21 19:38 ` jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
                     ` (5 more replies)
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
  12 siblings, 6 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:38 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
@ 2024-08-21 19:38   ` jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to jspewock
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:38 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
@ 2024-08-21 19:38   ` jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 3/5] dts: add class for virtual functions jspewock
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:38 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 3/5] dts: add class for virtual functions
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to jspewock
@ 2024-08-21 19:38   ` jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 4/5] dts: add OS abstractions for creating " jspewock
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:38 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 4/5] dts: add OS abstractions for creating virtual functions
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
                     ` (2 preceding siblings ...)
  2024-08-21 19:38   ` [RFC PATCH v2 3/5] dts: add class for virtual functions jspewock
@ 2024-08-21 19:38   ` jspewock
  2024-08-21 19:38   ` [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node jspewock
  2024-08-21 19:44   ` [RFC PATCH v2 0/5] dts: add VFs to the framework Jeremy Spewock
  5 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:38 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
                     ` (3 preceding siblings ...)
  2024-08-21 19:38   ` [RFC PATCH v2 4/5] dts: add OS abstractions for creating " jspewock
@ 2024-08-21 19:38   ` jspewock
  2024-08-21 19:44   ` [RFC PATCH v2 0/5] dts: add VFs to the framework Jeremy Spewock
  5 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 19:38 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH v2 0/5] dts: add VFs to the framework
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
                     ` (4 preceding siblings ...)
  2024-08-21 19:38   ` [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node jspewock
@ 2024-08-21 19:44   ` Jeremy Spewock
  5 siblings, 0 replies; 25+ messages in thread
From: Jeremy Spewock @ 2024-08-21 19:44 UTC (permalink / raw)
  To: npratte, paul.szczepanek, thomas, probb, yoan.picchi,
	juraj.linkes, wathsala.vithanage, Luca.Vizzarro, alex.chapman,
	Honnappa.Nagarahalli
  Cc: dev

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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v3 0/5] dts: add VFs to the framework
  2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
                   ` (11 preceding siblings ...)
  2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
@ 2024-08-21 21:30 ` jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 1/5] dts: allow binding only a single port to a different driver jspewock
                     ` (4 more replies)
  12 siblings, 5 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 21:30 UTC (permalink / raw)
  To: Luca.Vizzarro, thomas, paul.szczepanek, wathsala.vithanage,
	probb, yoan.picchi, juraj.linkes, Honnappa.Nagarahalli, npratte,
	alex.chapman
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v3 1/5] dts: allow binding only a single port to a different driver
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
@ 2024-08-21 21:30   ` jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 2/5] dts: parameterize what ports the TG sends packets to jspewock
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 21:30 UTC (permalink / raw)
  To: Luca.Vizzarro, thomas, paul.szczepanek, wathsala.vithanage,
	probb, yoan.picchi, juraj.linkes, Honnappa.Nagarahalli, npratte,
	alex.chapman
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v3 2/5] dts: parameterize what ports the TG sends packets to
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 1/5] dts: allow binding only a single port to a different driver jspewock
@ 2024-08-21 21:30   ` jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 3/5] dts: add class for virtual functions jspewock
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 21:30 UTC (permalink / raw)
  To: Luca.Vizzarro, thomas, paul.szczepanek, wathsala.vithanage,
	probb, yoan.picchi, juraj.linkes, Honnappa.Nagarahalli, npratte,
	alex.chapman
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v3 3/5] dts: add class for virtual functions
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 1/5] dts: allow binding only a single port to a different driver jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 2/5] dts: parameterize what ports the TG sends packets to jspewock
@ 2024-08-21 21:30   ` jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 4/5] dts: add OS abstractions for creating " jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 5/5] dts: add functions for managing VFs to Node jspewock
  4 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 21:30 UTC (permalink / raw)
  To: Luca.Vizzarro, thomas, paul.szczepanek, wathsala.vithanage,
	probb, yoan.picchi, juraj.linkes, Honnappa.Nagarahalli, npratte,
	alex.chapman
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v3 4/5] dts: add OS abstractions for creating virtual functions
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
                     ` (2 preceding siblings ...)
  2024-08-21 21:30   ` [RFC PATCH v3 3/5] dts: add class for virtual functions jspewock
@ 2024-08-21 21:30   ` jspewock
  2024-08-21 21:30   ` [RFC PATCH v3 5/5] dts: add functions for managing VFs to Node jspewock
  4 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 21:30 UTC (permalink / raw)
  To: Luca.Vizzarro, thomas, paul.szczepanek, wathsala.vithanage,
	probb, yoan.picchi, juraj.linkes, Honnappa.Nagarahalli, npratte,
	alex.chapman
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH v3 5/5] dts: add functions for managing VFs to Node
  2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
                     ` (3 preceding siblings ...)
  2024-08-21 21:30   ` [RFC PATCH v3 4/5] dts: add OS abstractions for creating " jspewock
@ 2024-08-21 21:30   ` jspewock
  4 siblings, 0 replies; 25+ messages in thread
From: jspewock @ 2024-08-21 21:30 UTC (permalink / raw)
  To: Luca.Vizzarro, thomas, paul.szczepanek, wathsala.vithanage,
	probb, yoan.picchi, juraj.linkes, Honnappa.Nagarahalli, npratte,
	alex.chapman
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2024-08-21 21:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-21 19:15 [RFC PATCH v1 0/5] dts: add VFs to the framework jspewock
2024-08-21 19:15 ` [RFC PATCH v1 1/5] dts: allow binding only a single port to a different driver jspewock
2024-08-21 19:15 ` [RFC PATCH v1 2/5] dts: parameterize what ports the TG sends packets to jspewock
2024-08-21 19:15 ` [RFC PATCH v1 3/5] dts: add class for virtual functions jspewock
2024-08-21 19:15 ` [RFC PATCH v1 4/5] dts: add OS abstractions for creating " jspewock
2024-08-21 19:15 ` [RFC PATCH v1 5/5] dts: add functions for managing VFs to Node jspewock
2024-08-21 19:21 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
2024-08-21 19:21 ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
2024-08-21 19:21 ` [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to jspewock
2024-08-21 19:21 ` [RFC PATCH v2 3/5] dts: add class for virtual functions jspewock
2024-08-21 19:21 ` [RFC PATCH v2 4/5] dts: add OS abstractions for creating " jspewock
2024-08-21 19:21 ` [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node jspewock
2024-08-21 19:38 ` [RFC PATCH v2 0/5] dts: add VFs to the framework jspewock
2024-08-21 19:38   ` [RFC PATCH v2 1/5] dts: allow binding only a single port to a different driver jspewock
2024-08-21 19:38   ` [RFC PATCH v2 2/5] dts: parameterize what ports the TG sends packets to jspewock
2024-08-21 19:38   ` [RFC PATCH v2 3/5] dts: add class for virtual functions jspewock
2024-08-21 19:38   ` [RFC PATCH v2 4/5] dts: add OS abstractions for creating " jspewock
2024-08-21 19:38   ` [RFC PATCH v2 5/5] dts: add functions for managing VFs to Node jspewock
2024-08-21 19:44   ` [RFC PATCH v2 0/5] dts: add VFs to the framework Jeremy Spewock
2024-08-21 21:30 ` [RFC PATCH v3 " jspewock
2024-08-21 21:30   ` [RFC PATCH v3 1/5] dts: allow binding only a single port to a different driver jspewock
2024-08-21 21:30   ` [RFC PATCH v3 2/5] dts: parameterize what ports the TG sends packets to jspewock
2024-08-21 21:30   ` [RFC PATCH v3 3/5] dts: add class for virtual functions jspewock
2024-08-21 21:30   ` [RFC PATCH v3 4/5] dts: add OS abstractions for creating " jspewock
2024-08-21 21:30   ` [RFC PATCH v3 5/5] dts: add functions for managing VFs to Node jspewock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).