[PATCH 0/1] dts: add driver binding on TG

[PATCH 0/1] dts: add driver binding on TG

[jspewock]

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

Previously in DTS there was support for binding ports a node to
different drivers on a SUT, but there was no option on the TG. Since
there are likely to be some traffic generators in the future that would
require different drivers to operate properly, this support is something
that would likely be useful to have, and it very simple to add. All that
is done in this patch is moving functionality for copying the DPDK
tarball onto a host out of the SUT node and into the generic Node class
so that both the TG and the SUT can take advantage of DPDK tools. It
should be noted however that the TG node still does not build DPDK as it
likely wouldn't need the compiled binaries.

Jeremy Spewock (1):
  dts: add binding to different drivers to TG node

 dts/framework/runner.py                 |   2 +
 dts/framework/testbed_model/node.py     | 106 +++++++++++++++++++++++-
 dts/framework/testbed_model/sut_node.py |  86 +------------------
 3 files changed, 109 insertions(+), 85 deletions(-)

-- 
2.45.2

[PATCH 1/1] dts: add binding to different drivers to TG node

[jspewock]

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

The DTS framework in its current state supports binding ports to
different drivers on the SUT node but not the TG node. The TG node
already has the information that it needs about the different drivers
that it has available in the configuration file, but it did not
previously have access to the devbind script, so it did not use that
information for anything.

This patch moves the steps to copy the DPDK tarball into the node class
rather than the SUT node class, and calls this function on the TG node
as well as the SUT. It also moves the driver binding step into the Node
class and triggers the same pattern of binding to ports that existed on
the SUT on the TG.

Bugzilla ID: 1420

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/runner.py                 |   2 +
 dts/framework/testbed_model/node.py     | 106 +++++++++++++++++++++++-
 dts/framework/testbed_model/sut_node.py |  86 +------------------
 3 files changed, 109 insertions(+), 85 deletions(-)

diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..ed9e58b172 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -484,6 +484,7 @@ def _run_build_target(
 
         try:
             sut_node.set_up_build_target(build_target_config)
+            tg_node.set_up_build_target(build_target_config)
             self._result.dpdk_version = sut_node.dpdk_version
             build_target_result.add_build_target_info(sut_node.get_build_target_info())
             build_target_result.update_setup(Result.PASS)
@@ -498,6 +499,7 @@ def _run_build_target(
             try:
                 self._logger.set_stage(DtsStage.build_target_teardown)
                 sut_node.tear_down_build_target()
+                tg_node.tear_down_build_target()
                 build_target_result.update_teardown(Result.PASS)
             except Exception as e:
                 self._logger.exception("Build target teardown failed.")
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 12a40170ac..8e6181e424 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -13,11 +13,19 @@
 The :func:`~Node.skip_setup` decorator can be used without subclassing.
 """
 
+import os
+import tarfile
 from abc import ABC
 from ipaddress import IPv4Interface, IPv6Interface
+from pathlib import PurePath
 from typing import Any, Callable, Union
 
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import (
+    OS,
+    BuildTargetConfiguration,
+    NodeConfiguration,
+    TestRunConfiguration,
+)
 from framework.exception import ConfigurationError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
@@ -58,8 +66,11 @@ class Node(ABC):
     lcores: list[LogicalCore]
     ports: list[Port]
     _logger: DTSLogger
+    _remote_tmp_dir: PurePath
+    __remote_dpdk_dir: PurePath | None
     _other_sessions: list[OSSession]
     _test_run_config: TestRunConfiguration
+    _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: NodeConfiguration):
         """Connect to the node and gather info during initialization.
@@ -88,6 +99,9 @@ def __init__(self, node_config: NodeConfiguration):
 
         self._other_sessions = []
         self._init_ports()
+        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
+        self.__remote_dpdk_dir = None
+        self._path_to_devbind_script = None
 
     def _init_ports(self) -> None:
         self.ports = [Port(self.name, port_config) for port_config in self.config.ports]
@@ -95,6 +109,34 @@ def _init_ports(self) -> None:
         for port in self.ports:
             self.configure_port_state(port)
 
+    def _guess_dpdk_remote_dir(self) -> PurePath:
+        return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
+
+    @property
+    def _remote_dpdk_dir(self) -> PurePath:
+        """The remote DPDK dir.
+
+        This internal property should be set after extracting the DPDK tarball. If it's not set,
+        that implies the DPDK setup step has been skipped, in which case we can guess where
+        a previous build was located.
+        """
+        if self.__remote_dpdk_dir is None:
+            self.__remote_dpdk_dir = self._guess_dpdk_remote_dir()
+        return self.__remote_dpdk_dir
+
+    @_remote_dpdk_dir.setter
+    def _remote_dpdk_dir(self, value: PurePath) -> None:
+        self.__remote_dpdk_dir = value
+
+    @property
+    def path_to_devbind_script(self) -> PurePath:
+        """The path to the dpdk-devbind.py script on the node."""
+        if self._path_to_devbind_script is None:
+            self._path_to_devbind_script = self.main_session.join_remote_path(
+                self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
+            )
+        return self._path_to_devbind_script
+
     def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
         """Test run setup steps.
 
@@ -114,6 +156,24 @@ def tear_down_test_run(self) -> None:
         Additional steps can be added by extending the method in subclasses with the use of super().
         """
 
+    def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
+        """Set up DPDK the node and bind ports.
+
+        DPDK setup includes setting all internals needed for the build, the copying of DPDK tarball
+        and then building DPDK. The drivers are bound to those that DPDK needs.
+
+        Args:
+            build_target_config: The build target test run configuration according to which
+                the setup steps will be taken.
+        """
+        self._copy_dpdk_tarball()
+        self.bind_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)
+
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
 
@@ -228,6 +288,50 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]:
         else:
             return func
 
+    @skip_setup
+    def _copy_dpdk_tarball(self) -> None:
+        """Copy to and extract DPDK tarball on the node."""
+        self._logger.info(f"Copying DPDK tarball to {self.name}.")
+        self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remote_tmp_dir)
+
+        # construct remote tarball path
+        # the basename is the same on local host and on remote Node
+        remote_tarball_path = self.main_session.join_remote_path(
+            self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball_path)
+        )
+
+        # construct remote path after extracting
+        with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar:
+            dpdk_top_dir = dpdk_tar.getnames()[0]
+        self._remote_dpdk_dir = self.main_session.join_remote_path(
+            self._remote_tmp_dir, dpdk_top_dir
+        )
+
+        self._logger.info(
+            f"Extracting DPDK tarball on {self.name}: "
+            f"'{remote_tarball_path}' into '{self._remote_dpdk_dir}'."
+        )
+        # clean remote path where we're extracting
+        self.main_session.remove_remote_dir(self._remote_dpdk_dir)
+
+        # 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:
+        """Bind all ports on the node to a driver.
+
+        Args:
+            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
+                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,
+            )
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 2855fe0276..f3fd4e2304 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -13,7 +13,6 @@
 
 
 import os
-import tarfile
 import time
 from pathlib import PurePath
 
@@ -26,7 +25,6 @@
 )
 from framework.params.eal import EalParams
 from framework.remote_session.remote_session import CommandResult
-from framework.settings import SETTINGS
 from framework.utils import MesonArgs
 
 from .node import Node
@@ -59,14 +57,11 @@ class SutNode(Node):
     dpdk_timestamp: str
     _build_target_config: BuildTargetConfiguration | None
     _env_vars: dict
-    _remote_tmp_dir: PurePath
-    __remote_dpdk_dir: PurePath | None
     _app_compile_timeout: float
     _dpdk_kill_session: OSSession | None
     _dpdk_version: str | None
     _node_info: NodeInfo | None
     _compiler_version: str | None
-    _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: SutNodeConfiguration):
         """Extend the constructor with SUT node specifics.
@@ -79,8 +74,6 @@ def __init__(self, node_config: SutNodeConfiguration):
         self.dpdk_prefix_list = []
         self._build_target_config = None
         self._env_vars = {}
-        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
-        self.__remote_dpdk_dir = None
         self._app_compile_timeout = 90
         self._dpdk_kill_session = None
         self.dpdk_timestamp = (
@@ -89,25 +82,8 @@ def __init__(self, node_config: SutNodeConfiguration):
         self._dpdk_version = None
         self._node_info = None
         self._compiler_version = None
-        self._path_to_devbind_script = None
         self._logger.info(f"Created node: {self.name}")
 
-    @property
-    def _remote_dpdk_dir(self) -> PurePath:
-        """The remote DPDK dir.
-
-        This internal property should be set after extracting the DPDK tarball. If it's not set,
-        that implies the DPDK setup step has been skipped, in which case we can guess where
-        a previous build was located.
-        """
-        if self.__remote_dpdk_dir is None:
-            self.__remote_dpdk_dir = self._guess_dpdk_remote_dir()
-        return self.__remote_dpdk_dir
-
-    @_remote_dpdk_dir.setter
-    def _remote_dpdk_dir(self, value: PurePath) -> None:
-        self.__remote_dpdk_dir = value
-
     @property
     def remote_dpdk_build_dir(self) -> PurePath:
         """The remote DPDK build directory.
@@ -151,15 +127,6 @@ def compiler_version(self) -> str:
                 return ""
         return self._compiler_version
 
-    @property
-    def path_to_devbind_script(self) -> PurePath:
-        """The path to the dpdk-devbind.py script on the node."""
-        if self._path_to_devbind_script is None:
-            self._path_to_devbind_script = self.main_session.join_remote_path(
-                self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
-            )
-        return self._path_to_devbind_script
-
     def get_build_target_info(self) -> BuildTargetInfo:
         """Get additional build target information.
 
@@ -170,9 +137,6 @@ def get_build_target_info(self) -> BuildTargetInfo:
             dpdk_version=self.dpdk_version, compiler_version=self.compiler_version
         )
 
-    def _guess_dpdk_remote_dir(self) -> PurePath:
-        return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
-
     def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
         """Extend the test run setup with vdev config.
 
@@ -199,19 +163,17 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
             build_target_config: The build target test run configuration according to which
                 the setup steps will be taken.
         """
+        super().set_up_build_target(build_target_config)
         self._configure_build_target(build_target_config)
-        self._copy_dpdk_tarball()
         self._build_dpdk()
-        self.bind_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
+        super().tear_down_build_target()
         self._env_vars = {}
         self._build_target_config = None
-        self.__remote_dpdk_dir = None
         self._dpdk_version = None
         self._compiler_version = None
-        self.bind_ports_to_driver(for_dpdk=False)
 
     def _configure_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
         """Populate common environment variables and set build target config."""
@@ -224,35 +186,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
                 f"'{build_target_config.compiler_wrapper} {build_target_config.compiler.name}'"
             )  # fmt: skip
 
-    @Node.skip_setup
-    def _copy_dpdk_tarball(self) -> None:
-        """Copy to and extract DPDK tarball on the SUT node."""
-        self._logger.info("Copying DPDK tarball to SUT.")
-        self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remote_tmp_dir)
-
-        # construct remote tarball path
-        # the basename is the same on local host and on remote Node
-        remote_tarball_path = self.main_session.join_remote_path(
-            self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball_path)
-        )
-
-        # construct remote path after extracting
-        with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar:
-            dpdk_top_dir = dpdk_tar.getnames()[0]
-        self._remote_dpdk_dir = self.main_session.join_remote_path(
-            self._remote_tmp_dir, dpdk_top_dir
-        )
-
-        self._logger.info(
-            f"Extracting DPDK tarball on SUT: "
-            f"'{remote_tarball_path}' into '{self._remote_dpdk_dir}'."
-        )
-        # clean remote path where we're extracting
-        self.main_session.remove_remote_dir(self._remote_dpdk_dir)
-
-        # then extract to remote path
-        self.main_session.extract_remote_tarball(remote_tarball_path, self._remote_dpdk_dir)
-
     @Node.skip_setup
     def _build_dpdk(self) -> None:
         """Build DPDK.
@@ -335,18 +268,3 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
         self.main_session.configure_ipv4_forwarding(enable)
-
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
-        """Bind all ports on the SUT to a driver.
-
-        Args:
-            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
-                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,
-            )
-- 
2.45.2

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Nicholas Pratte]

Makes sense to me!

Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>

On Mon, Aug 12, 2024 at 1:23 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> The DTS framework in its current state supports binding ports to
> different drivers on the SUT node but not the TG node. The TG node
> already has the information that it needs about the different drivers
> that it has available in the configuration file, but it did not
> previously have access to the devbind script, so it did not use that
> information for anything.
>
> This patch moves the steps to copy the DPDK tarball into the node class
> rather than the SUT node class, and calls this function on the TG node
> as well as the SUT. It also moves the driver binding step into the Node
> class and triggers the same pattern of binding to ports that existed on
> the SUT on the TG.
>
> Bugzilla ID: 1420
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/framework/runner.py                 |   2 +
>  dts/framework/testbed_model/node.py     | 106 +++++++++++++++++++++++-
>  dts/framework/testbed_model/sut_node.py |  86 +------------------
>  3 files changed, 109 insertions(+), 85 deletions(-)
>
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 6b6f6a05f5..ed9e58b172 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -484,6 +484,7 @@ def _run_build_target(
>
>          try:
>              sut_node.set_up_build_target(build_target_config)
> +            tg_node.set_up_build_target(build_target_config)
>              self._result.dpdk_version = sut_node.dpdk_version
>              build_target_result.add_build_target_info(sut_node.get_build_target_info())
>              build_target_result.update_setup(Result.PASS)
> @@ -498,6 +499,7 @@ def _run_build_target(
>              try:
>                  self._logger.set_stage(DtsStage.build_target_teardown)
>                  sut_node.tear_down_build_target()
> +                tg_node.tear_down_build_target()
>                  build_target_result.update_teardown(Result.PASS)
>              except Exception as e:
>                  self._logger.exception("Build target teardown failed.")
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> index 12a40170ac..8e6181e424 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -13,11 +13,19 @@
>  The :func:`~Node.skip_setup` decorator can be used without subclassing.
>  """
>
> +import os
> +import tarfile
>  from abc import ABC
>  from ipaddress import IPv4Interface, IPv6Interface
> +from pathlib import PurePath
>  from typing import Any, Callable, Union
>
> -from framework.config import OS, NodeConfiguration, TestRunConfiguration
> +from framework.config import (
> +    OS,
> +    BuildTargetConfiguration,
> +    NodeConfiguration,
> +    TestRunConfiguration,
> +)
>  from framework.exception import ConfigurationError
>  from framework.logger import DTSLogger, get_dts_logger
>  from framework.settings import SETTINGS
> @@ -58,8 +66,11 @@ class Node(ABC):
>      lcores: list[LogicalCore]
>      ports: list[Port]
>      _logger: DTSLogger
> +    _remote_tmp_dir: PurePath
> +    __remote_dpdk_dir: PurePath | None
>      _other_sessions: list[OSSession]
>      _test_run_config: TestRunConfiguration
> +    _path_to_devbind_script: PurePath | None
>
>      def __init__(self, node_config: NodeConfiguration):
>          """Connect to the node and gather info during initialization.
> @@ -88,6 +99,9 @@ def __init__(self, node_config: NodeConfiguration):
>
>          self._other_sessions = []
>          self._init_ports()
> +        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
> +        self.__remote_dpdk_dir = None
> +        self._path_to_devbind_script = None
>
>      def _init_ports(self) -> None:
>          self.ports = [Port(self.name, port_config) for port_config in self.config.ports]
> @@ -95,6 +109,34 @@ def _init_ports(self) -> None:
>          for port in self.ports:
>              self.configure_port_state(port)
>
> +    def _guess_dpdk_remote_dir(self) -> PurePath:
> +        return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
> +
> +    @property
> +    def _remote_dpdk_dir(self) -> PurePath:
> +        """The remote DPDK dir.
> +
> +        This internal property should be set after extracting the DPDK tarball. If it's not set,
> +        that implies the DPDK setup step has been skipped, in which case we can guess where
> +        a previous build was located.
> +        """
> +        if self.__remote_dpdk_dir is None:
> +            self.__remote_dpdk_dir = self._guess_dpdk_remote_dir()
> +        return self.__remote_dpdk_dir
> +
> +    @_remote_dpdk_dir.setter
> +    def _remote_dpdk_dir(self, value: PurePath) -> None:
> +        self.__remote_dpdk_dir = value
> +
> +    @property
> +    def path_to_devbind_script(self) -> PurePath:
> +        """The path to the dpdk-devbind.py script on the node."""
> +        if self._path_to_devbind_script is None:
> +            self._path_to_devbind_script = self.main_session.join_remote_path(
> +                self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
> +            )
> +        return self._path_to_devbind_script
> +
>      def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
>          """Test run setup steps.
>
> @@ -114,6 +156,24 @@ def tear_down_test_run(self) -> None:
>          Additional steps can be added by extending the method in subclasses with the use of super().
>          """
>
> +    def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
> +        """Set up DPDK the node and bind ports.
> +
> +        DPDK setup includes setting all internals needed for the build, the copying of DPDK tarball
> +        and then building DPDK. The drivers are bound to those that DPDK needs.
> +
> +        Args:
> +            build_target_config: The build target test run configuration according to which
> +                the setup steps will be taken.
> +        """
> +        self._copy_dpdk_tarball()
> +        self.bind_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)
> +
>      def create_session(self, name: str) -> OSSession:
>          """Create and return a new OS-aware remote session.
>
> @@ -228,6 +288,50 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]:
>          else:
>              return func
>
> +    @skip_setup
> +    def _copy_dpdk_tarball(self) -> None:
> +        """Copy to and extract DPDK tarball on the node."""
> +        self._logger.info(f"Copying DPDK tarball to {self.name}.")
> +        self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remote_tmp_dir)
> +
> +        # construct remote tarball path
> +        # the basename is the same on local host and on remote Node
> +        remote_tarball_path = self.main_session.join_remote_path(
> +            self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball_path)
> +        )
> +
> +        # construct remote path after extracting
> +        with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar:
> +            dpdk_top_dir = dpdk_tar.getnames()[0]
> +        self._remote_dpdk_dir = self.main_session.join_remote_path(
> +            self._remote_tmp_dir, dpdk_top_dir
> +        )
> +
> +        self._logger.info(
> +            f"Extracting DPDK tarball on {self.name}: "
> +            f"'{remote_tarball_path}' into '{self._remote_dpdk_dir}'."
> +        )
> +        # clean remote path where we're extracting
> +        self.main_session.remove_remote_dir(self._remote_dpdk_dir)
> +
> +        # 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:
> +        """Bind all ports on the node to a driver.
> +
> +        Args:
> +            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
> +                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,
> +            )
> +
>
>  def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
>      """Factory for OS-aware sessions.
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index 2855fe0276..f3fd4e2304 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -13,7 +13,6 @@
>
>
>  import os
> -import tarfile
>  import time
>  from pathlib import PurePath
>
> @@ -26,7 +25,6 @@
>  )
>  from framework.params.eal import EalParams
>  from framework.remote_session.remote_session import CommandResult
> -from framework.settings import SETTINGS
>  from framework.utils import MesonArgs
>
>  from .node import Node
> @@ -59,14 +57,11 @@ class SutNode(Node):
>      dpdk_timestamp: str
>      _build_target_config: BuildTargetConfiguration | None
>      _env_vars: dict
> -    _remote_tmp_dir: PurePath
> -    __remote_dpdk_dir: PurePath | None
>      _app_compile_timeout: float
>      _dpdk_kill_session: OSSession | None
>      _dpdk_version: str | None
>      _node_info: NodeInfo | None
>      _compiler_version: str | None
> -    _path_to_devbind_script: PurePath | None
>
>      def __init__(self, node_config: SutNodeConfiguration):
>          """Extend the constructor with SUT node specifics.
> @@ -79,8 +74,6 @@ def __init__(self, node_config: SutNodeConfiguration):
>          self.dpdk_prefix_list = []
>          self._build_target_config = None
>          self._env_vars = {}
> -        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
> -        self.__remote_dpdk_dir = None
>          self._app_compile_timeout = 90
>          self._dpdk_kill_session = None
>          self.dpdk_timestamp = (
> @@ -89,25 +82,8 @@ def __init__(self, node_config: SutNodeConfiguration):
>          self._dpdk_version = None
>          self._node_info = None
>          self._compiler_version = None
> -        self._path_to_devbind_script = None
>          self._logger.info(f"Created node: {self.name}")
>
> -    @property
> -    def _remote_dpdk_dir(self) -> PurePath:
> -        """The remote DPDK dir.
> -
> -        This internal property should be set after extracting the DPDK tarball. If it's not set,
> -        that implies the DPDK setup step has been skipped, in which case we can guess where
> -        a previous build was located.
> -        """
> -        if self.__remote_dpdk_dir is None:
> -            self.__remote_dpdk_dir = self._guess_dpdk_remote_dir()
> -        return self.__remote_dpdk_dir
> -
> -    @_remote_dpdk_dir.setter
> -    def _remote_dpdk_dir(self, value: PurePath) -> None:
> -        self.__remote_dpdk_dir = value
> -
>      @property
>      def remote_dpdk_build_dir(self) -> PurePath:
>          """The remote DPDK build directory.
> @@ -151,15 +127,6 @@ def compiler_version(self) -> str:
>                  return ""
>          return self._compiler_version
>
> -    @property
> -    def path_to_devbind_script(self) -> PurePath:
> -        """The path to the dpdk-devbind.py script on the node."""
> -        if self._path_to_devbind_script is None:
> -            self._path_to_devbind_script = self.main_session.join_remote_path(
> -                self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
> -            )
> -        return self._path_to_devbind_script
> -
>      def get_build_target_info(self) -> BuildTargetInfo:
>          """Get additional build target information.
>
> @@ -170,9 +137,6 @@ def get_build_target_info(self) -> BuildTargetInfo:
>              dpdk_version=self.dpdk_version, compiler_version=self.compiler_version
>          )
>
> -    def _guess_dpdk_remote_dir(self) -> PurePath:
> -        return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
> -
>      def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
>          """Extend the test run setup with vdev config.
>
> @@ -199,19 +163,17 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
>              build_target_config: The build target test run configuration according to which
>                  the setup steps will be taken.
>          """
> +        super().set_up_build_target(build_target_config)
>          self._configure_build_target(build_target_config)
> -        self._copy_dpdk_tarball()
>          self._build_dpdk()
> -        self.bind_ports_to_driver()
>
>      def tear_down_build_target(self) -> None:
>          """Reset DPDK variables and bind port driver to the OS driver."""
> +        super().tear_down_build_target()
>          self._env_vars = {}
>          self._build_target_config = None
> -        self.__remote_dpdk_dir = None
>          self._dpdk_version = None
>          self._compiler_version = None
> -        self.bind_ports_to_driver(for_dpdk=False)
>
>      def _configure_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
>          """Populate common environment variables and set build target config."""
> @@ -224,35 +186,6 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
>                  f"'{build_target_config.compiler_wrapper} {build_target_config.compiler.name}'"
>              )  # fmt: skip
>
> -    @Node.skip_setup
> -    def _copy_dpdk_tarball(self) -> None:
> -        """Copy to and extract DPDK tarball on the SUT node."""
> -        self._logger.info("Copying DPDK tarball to SUT.")
> -        self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remote_tmp_dir)
> -
> -        # construct remote tarball path
> -        # the basename is the same on local host and on remote Node
> -        remote_tarball_path = self.main_session.join_remote_path(
> -            self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball_path)
> -        )
> -
> -        # construct remote path after extracting
> -        with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar:
> -            dpdk_top_dir = dpdk_tar.getnames()[0]
> -        self._remote_dpdk_dir = self.main_session.join_remote_path(
> -            self._remote_tmp_dir, dpdk_top_dir
> -        )
> -
> -        self._logger.info(
> -            f"Extracting DPDK tarball on SUT: "
> -            f"'{remote_tarball_path}' into '{self._remote_dpdk_dir}'."
> -        )
> -        # clean remote path where we're extracting
> -        self.main_session.remove_remote_dir(self._remote_dpdk_dir)
> -
> -        # then extract to remote path
> -        self.main_session.extract_remote_tarball(remote_tarball_path, self._remote_dpdk_dir)
> -
>      @Node.skip_setup
>      def _build_dpdk(self) -> None:
>          """Build DPDK.
> @@ -335,18 +268,3 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
>              enable: If :data:`True`, enable the forwarding, otherwise disable it.
>          """
>          self.main_session.configure_ipv4_forwarding(enable)
> -
> -    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> -        """Bind all ports on the SUT to a driver.
> -
> -        Args:
> -            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
> -                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,
> -            )
> --
> 2.45.2
>

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Juraj Linkeš]

On 12. 8. 2024 19:22, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> The DTS framework in its current state supports binding ports to
> different drivers on the SUT node but not the TG node. The TG node
> already has the information that it needs about the different drivers
> that it has available in the configuration file, but it did not
> previously have access to the devbind script, so it did not use that
> information for anything.
> 
> This patch moves the steps to copy the DPDK tarball into the node class
> rather than the SUT node class, and calls this function on the TG node
> as well as the SUT. It also moves the driver binding step into the Node
> class and triggers the same pattern of binding to ports that existed on
> the SUT on the TG.
> 

This is a very inefficient way to do this. We'll have to build DPDK 
twice and that's very time consuming. I was thinking in terms of just 
copying the script to the TG node and storing its location on the TG 
node. We should have access to the script whether DTS is run from the 
repository or a tarball.

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Jeremy Spewock]

On Mon, Sep 9, 2024 at 8:16 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> On 12. 8. 2024 19:22, jspewock@iol.unh.edu wrote:
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > The DTS framework in its current state supports binding ports to
> > different drivers on the SUT node but not the TG node. The TG node
> > already has the information that it needs about the different drivers
> > that it has available in the configuration file, but it did not
> > previously have access to the devbind script, so it did not use that
> > information for anything.
> >
> > This patch moves the steps to copy the DPDK tarball into the node class
> > rather than the SUT node class, and calls this function on the TG node
> > as well as the SUT. It also moves the driver binding step into the Node
> > class and triggers the same pattern of binding to ports that existed on
> > the SUT on the TG.
> >
>
> This is a very inefficient way to do this. We'll have to build DPDK
> twice and that's very time consuming. I was thinking in terms of just

This patch shouldn't be compiling DPDK twice, are you referring to the
process of copying the tarball over and extracting it taking too long?
If so, that makes sense that it takes longer than we need for this one
task. I figured it wouldn't hurt to have the whole DPDK directory
there, and that it could even be potentially useful to have it if the
TG ever needed it. That and it seemed like the most straightforward
way that kept these two set up in a similar way. Extracting the
tarball is obviously pretty quick, so I guess the real question here
is whether it is fine to add the time of one extra SCP of the DPDK
tarball around.

> copying the script to the TG node and storing its location on the TG
> node. We should have access to the script whether DTS is run from the
> repository or a tarball.

We should have access to it regardless, but extracting only that one
script would be different based on if it was a tarball or a repository
since, I believe at least, I would have to use the tarfile library to
read and extract only this one file to copy over if it was a tarball.
It would be faster I assume, so if you think it is worth it I could
make the change. Unless you are saying that we wouldn't need to take
the devbind script from the tarball that is passed into the DTS run
and instead assume that we can just go one directory up from `dts/` on
the runner host. That could be an interesting idea which would be
faster, but I wasn't sure if that was something that was fine to do
since (I don't think at least) there is anything that technically ties
you to running from in a DPDK directory other than the docker
container.

>

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Juraj Linkeš]

On 9. 9. 2024 17:55, Jeremy Spewock wrote:
> On Mon, Sep 9, 2024 at 8:16 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>>
>>
>> On 12. 8. 2024 19:22, jspewock@iol.unh.edu wrote:
>>> From: Jeremy Spewock <jspewock@iol.unh.edu>
>>>
>>> The DTS framework in its current state supports binding ports to
>>> different drivers on the SUT node but not the TG node. The TG node
>>> already has the information that it needs about the different drivers
>>> that it has available in the configuration file, but it did not
>>> previously have access to the devbind script, so it did not use that
>>> information for anything.
>>>
>>> This patch moves the steps to copy the DPDK tarball into the node class
>>> rather than the SUT node class, and calls this function on the TG node
>>> as well as the SUT. It also moves the driver binding step into the Node
>>> class and triggers the same pattern of binding to ports that existed on
>>> the SUT on the TG.
>>>
>>
>> This is a very inefficient way to do this. We'll have to build DPDK
>> twice and that's very time consuming. I was thinking in terms of just
> 
> This patch shouldn't be compiling DPDK twice, are you referring to the
> process of copying the tarball over and extracting it taking too long?
> If so, that makes sense that it takes longer than we need for this one
> task. I figured it wouldn't hurt to have the whole DPDK directory
> there, and that it could even be potentially useful to have it if the
> TG ever needed it. That and it seemed like the most straightforward
> way that kept these two set up in a similar way. Extracting the
> tarball is obviously pretty quick, so I guess the real question here
> is whether it is fine to add the time of one extra SCP of the DPDK
> tarball around.
> 

Ah, I didn't look carefully at the split. This is fine, but there some 
things I noticed.

As Patrick mentioned, the docstrings in Node.set_up_build_target() and 
SutNode.set_up_build_target() would need to be updated.
Why are we binding ports on the TG node?
This shouldn't really be part of set_up_build_target; set_up_test_run is 
a better place to put this, as we don't need to copy it for each build 
target. And, as I realized then thinking about the property (down 
below), we don't need to do that even per test_run; once per TG node's 
lifetime is enough.

>> copying the script to the TG node and storing its location on the TG
>> node. We should have access to the script whether DTS is run from the
>> repository or a tarball.
> 
> We should have access to it regardless, but extracting only that one
> script would be different based on if it was a tarball or a repository
> since, I believe at least, I would have to use the tarfile library to
> read and extract only this one file to copy over if it was a tarball.
> It would be faster I assume, so if you think it is worth it I could
> make the change. Unless you are saying that we wouldn't need to take
> the devbind script from the tarball that is passed into the DTS run
> and instead assume that we can just go one directory up from `dts/` on
> the runner host. That could be an interesting idea which would be
> faster, but I wasn't sure if that was something that was fine to do
> since (I don't think at least) there is anything that technically ties
> you to running from in a DPDK directory other than the docker
> container.

You can run DTS from any directory, but currently DTS it's always going 
to be in a DPDK tree (there's no other way to get DTS), so I think it's 
safe to assume the script is there. We can put a variable pointing to 
dpdk_root into utils.py and use that.

My idea was copying that one file, nothing else (no tarball or anything 
would be needed).
I think we'd only need to move _remote_tmp_dir and 
_path_to_devbind_script to Node and then implement set_up_test_run() on 
the TG node to copy just the script (with self.main_session.copy_to()) 
and set _path_to_devbind_script. And I guess set _path_to_devbind_script 
in SutNode.tear_down_build_target() and TGNode.tear_down_test_run() 
since those seems to be missing.

But there's actually one more thing we could improve on top of that. 
_path_to_devbind_script could be a property which would be used the same 
way in SutNode, but in TGNode, we could copy the script only if it's 
None and set it back to None only when closing the Node (as we need to 
copy the script only once per TG node lifespan).

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Jeremy Spewock]

On Mon, Sep 16, 2024 at 6:04 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> On 9. 9. 2024 17:55, Jeremy Spewock wrote:
> > On Mon, Sep 9, 2024 at 8:16 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >>
> >>
> >>
> >> On 12. 8. 2024 19:22, jspewock@iol.unh.edu wrote:
> >>> From: Jeremy Spewock <jspewock@iol.unh.edu>
> >>>
> >>> The DTS framework in its current state supports binding ports to
> >>> different drivers on the SUT node but not the TG node. The TG node
> >>> already has the information that it needs about the different drivers
> >>> that it has available in the configuration file, but it did not
> >>> previously have access to the devbind script, so it did not use that
> >>> information for anything.
> >>>
> >>> This patch moves the steps to copy the DPDK tarball into the node class
> >>> rather than the SUT node class, and calls this function on the TG node
> >>> as well as the SUT. It also moves the driver binding step into the Node
> >>> class and triggers the same pattern of binding to ports that existed on
> >>> the SUT on the TG.
> >>>
> >>
> >> This is a very inefficient way to do this. We'll have to build DPDK
> >> twice and that's very time consuming. I was thinking in terms of just
> >
> > This patch shouldn't be compiling DPDK twice, are you referring to the
> > process of copying the tarball over and extracting it taking too long?
> > If so, that makes sense that it takes longer than we need for this one
> > task. I figured it wouldn't hurt to have the whole DPDK directory
> > there, and that it could even be potentially useful to have it if the
> > TG ever needed it. That and it seemed like the most straightforward
> > way that kept these two set up in a similar way. Extracting the
> > tarball is obviously pretty quick, so I guess the real question here
> > is whether it is fine to add the time of one extra SCP of the DPDK
> > tarball around.
> >
>
> Ah, I didn't look carefully at the split. This is fine, but there some
> things I noticed.
>
> As Patrick mentioned, the docstrings in Node.set_up_build_target() and
> SutNode.set_up_build_target() would need to be updated.
> Why are we binding ports on the TG node?

I figured that the assumption would be that whatever is in the config
file is what the TG needs to be bound to in order to run the testing,
similarly to how we always bind on the SUT assuming that we need to be
using the DPDK driver to test DPDK.

> This shouldn't really be part of set_up_build_target; set_up_test_run is
> a better place to put this, as we don't need to copy it for each build
> target. And, as I realized then thinking about the property (down
> below), we don't need to do that even per test_run; once per TG node's
> lifetime is enough.

That makes sense to me actually considering the traffic generator
being used cannot change throughout the node's lifetime. I will make
that change.

>
> >> copying the script to the TG node and storing its location on the TG
> >> node. We should have access to the script whether DTS is run from the
> >> repository or a tarball.
> >
> > We should have access to it regardless, but extracting only that one
> > script would be different based on if it was a tarball or a repository
> > since, I believe at least, I would have to use the tarfile library to
> > read and extract only this one file to copy over if it was a tarball.
> > It would be faster I assume, so if you think it is worth it I could
> > make the change. Unless you are saying that we wouldn't need to take
> > the devbind script from the tarball that is passed into the DTS run
> > and instead assume that we can just go one directory up from `dts/` on
> > the runner host. That could be an interesting idea which would be
> > faster, but I wasn't sure if that was something that was fine to do
> > since (I don't think at least) there is anything that technically ties
> > you to running from in a DPDK directory other than the docker
> > container.
>
> You can run DTS from any directory, but currently DTS it's always going
> to be in a DPDK tree (there's no other way to get DTS), so I think it's
> safe to assume the script is there. We can put a variable pointing to
> dpdk_root into utils.py and use that.
>

Fair enough, I don't see why it would be run outside of the DPDK
directory in most cases. There is one situation where it could happen,
which is the runner target for the Docker container copies only the
DTS directory into the container when it "bakes DTS in." It does so
because it really only needs the .toml file for the poetry install
and, realistically, it seems like you should normally be mounting your
local DPDK directory over the DTS directory, but maybe that isn't
super clear. Out of scope for this patch, but just something to note.

> My idea was copying that one file, nothing else (no tarball or anything
> would be needed).
> I think we'd only need to move _remote_tmp_dir and
> _path_to_devbind_script to Node and then implement set_up_test_run() on
> the TG node to copy just the script (with self.main_session.copy_to())
> and set _path_to_devbind_script. And I guess set _path_to_devbind_script
> in SutNode.tear_down_build_target() and TGNode.tear_down_test_run()
> since those seems to be missing.
>
> But there's actually one more thing we could improve on top of that.
> _path_to_devbind_script could be a property which would be used the same
> way in SutNode, but in TGNode, we could copy the script only if it's
> None and set it back to None only when closing the Node (as we need to
> copy the script only once per TG node lifespan).

Sure, that makes sense to me. I'll make that change.

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Juraj Linkeš]

On 18. 9. 2024 20:50, Jeremy Spewock wrote:
> On Mon, Sep 16, 2024 at 6:04 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>>
>>
>> On 9. 9. 2024 17:55, Jeremy Spewock wrote:
>>> On Mon, Sep 9, 2024 at 8:16 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>>>
>>>>
>>>>
>>>> On 12. 8. 2024 19:22, jspewock@iol.unh.edu wrote:
>>>>> From: Jeremy Spewock <jspewock@iol.unh.edu>
>>>>>
>>>>> The DTS framework in its current state supports binding ports to
>>>>> different drivers on the SUT node but not the TG node. The TG node
>>>>> already has the information that it needs about the different drivers
>>>>> that it has available in the configuration file, but it did not
>>>>> previously have access to the devbind script, so it did not use that
>>>>> information for anything.
>>>>>
>>>>> This patch moves the steps to copy the DPDK tarball into the node class
>>>>> rather than the SUT node class, and calls this function on the TG node
>>>>> as well as the SUT. It also moves the driver binding step into the Node
>>>>> class and triggers the same pattern of binding to ports that existed on
>>>>> the SUT on the TG.
>>>>>
>>>>
>>>> This is a very inefficient way to do this. We'll have to build DPDK
>>>> twice and that's very time consuming. I was thinking in terms of just
>>>
>>> This patch shouldn't be compiling DPDK twice, are you referring to the
>>> process of copying the tarball over and extracting it taking too long?
>>> If so, that makes sense that it takes longer than we need for this one
>>> task. I figured it wouldn't hurt to have the whole DPDK directory
>>> there, and that it could even be potentially useful to have it if the
>>> TG ever needed it. That and it seemed like the most straightforward
>>> way that kept these two set up in a similar way. Extracting the
>>> tarball is obviously pretty quick, so I guess the real question here
>>> is whether it is fine to add the time of one extra SCP of the DPDK
>>> tarball around.
>>>
>>
>> Ah, I didn't look carefully at the split. This is fine, but there some
>> things I noticed.
>>
>> As Patrick mentioned, the docstrings in Node.set_up_build_target() and
>> SutNode.set_up_build_target() would need to be updated.
>> Why are we binding ports on the TG node?
> 
> I figured that the assumption would be that whatever is in the config
> file is what the TG needs to be bound to in order to run the testing,
> similarly to how we always bind on the SUT assuming that we need to be
> using the DPDK driver to test DPDK.
> 

Ah, I see. That makes sense now and we should do that. I was thinking a 
bit ahead. If we have two traffic generators, one for performance, one 
for functional testing, each using a different driver, we'd run into 
problems there. We're not there yet, so that's a problem that will need 
solving in a future patchset.

Re: [PATCH 1/1] dts: add binding to different drivers to TG node

[Patrick Robb]

Looks good to me, and I don't understand it as triggering a second
compile on the TG node.

I wonder whether the docstring for set_up_build_target() makes it
sound like it does though?

Reviewed-by: Patrick Robb <probb@iol.unh.edu>

[PATCH v2 0/1] dts: add driver binding on TG

[jspewock]

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

v2:
  * only copy the devbind script onto the TG node rather than the
    entire DPDK directory

Jeremy Spewock (1):
  dts: add binding to different drivers to TG node

 dts/framework/runner.py                 |  2 +
 dts/framework/testbed_model/node.py     | 49 ++++++++++++++++++++++++-
 dts/framework/testbed_model/sut_node.py | 39 ++++++--------------
 dts/framework/testbed_model/tg_node.py  | 21 +++++++++++
 dts/framework/utils.py                  |  4 +-
 5 files changed, 85 insertions(+), 30 deletions(-)

-- 
2.46.0


-- 



*Let's Connect!*

...  *October Webinars*

Ask Us Anything: IOL Services 
Open Q&A 
<https://unh.zoom.us/webinar/register/9017265932716/WN_OUo5S7iQRLmKKY7CsmwZhw#/registration>Your 
questions. Our answers. Let's get started.


Oct 3rd


Live Tour of INTACT(R) 
for IPv6 Testing and Validation 
<https://unh.zoom.us/webinar/register/7117231236474/WN_I2zfyi_2S2yEiXkxBRi8sA#/registration>
Open tour. Open Q&A. See why we think you'll love INTACT.

Oct 9th


How to 
Prep for Our NVMe(R) Plugfest #21 
<https://unh.zoom.us/webinar/register/4017266809553/WN_X1iA2SZ8QhmcGboF2DImNg#/registration>
Checklists. Conversation. Let's get ready to plugin! 
Oct 15th


... * 
Newsletter*

*
*
Get the IOL Connector 
<https://www.iol.unh.edu/news/email-newsletters> for our latest news and 
event info.



.

[PATCH v2 1/1] dts: add binding to different drivers to TG node

[jspewock]

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

The DTS framework in its current state supports binding ports to
different drivers on the SUT node but not the TG node. The TG node
already has the information that it needs about the different drivers
that it has available in the configuration file, but it did not
previously have access to the devbind script, so it did not use that
information for anything.

This patch moves the location of the tmp directory as well as the method
for binding ports into the node class rather than the SUT node class and
adds an abstract method for getting the path to the devbind script into
the node class. Then, binding ports to the correct drivers is moved into
the build target setup and run on both nodes.

Bugzilla ID: 1420

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/runner.py                 |  2 +
 dts/framework/testbed_model/node.py     | 49 ++++++++++++++++++++++++-
 dts/framework/testbed_model/sut_node.py | 39 ++++++--------------
 dts/framework/testbed_model/tg_node.py  | 21 +++++++++++
 dts/framework/utils.py                  |  4 +-
 5 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index ab98de8353..4c884fbcd4 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -486,6 +486,7 @@ def _run_build_target(
 
         try:
             sut_node.set_up_build_target(build_target_config)
+            tg_node.set_up_build_target(build_target_config)
             self._result.dpdk_version = sut_node.dpdk_version
             build_target_result.add_build_target_info(sut_node.get_build_target_info())
             build_target_result.update_setup(Result.PASS)
@@ -500,6 +501,7 @@ def _run_build_target(
             try:
                 self._logger.set_stage(DtsStage.build_target_teardown)
                 sut_node.tear_down_build_target()
+                tg_node.tear_down_build_target()
                 build_target_result.update_teardown(Result.PASS)
             except Exception as e:
                 self._logger.exception("Build target teardown failed.")
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 12a40170ac..6484e16a0f 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -13,11 +13,18 @@
 The :func:`~Node.skip_setup` decorator can be used without subclassing.
 """
 
-from abc import ABC
+
+from abc import ABC, abstractmethod
 from ipaddress import IPv4Interface, IPv6Interface
+from pathlib import PurePath
 from typing import Any, Callable, Union
 
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import (
+    OS,
+    BuildTargetConfiguration,
+    NodeConfiguration,
+    TestRunConfiguration,
+)
 from framework.exception import ConfigurationError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
@@ -58,8 +65,10 @@ class Node(ABC):
     lcores: list[LogicalCore]
     ports: list[Port]
     _logger: DTSLogger
+    _remote_tmp_dir: PurePath
     _other_sessions: list[OSSession]
     _test_run_config: TestRunConfiguration
+    _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: NodeConfiguration):
         """Connect to the node and gather info during initialization.
@@ -88,6 +97,8 @@ def __init__(self, node_config: NodeConfiguration):
 
         self._other_sessions = []
         self._init_ports()
+        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
+        self._path_to_devbind_script = None
 
     def _init_ports(self) -> None:
         self.ports = [Port(self.name, port_config) for port_config in self.config.ports]
@@ -95,6 +106,11 @@ def _init_ports(self) -> None:
         for port in self.ports:
             self.configure_port_state(port)
 
+    @property
+    @abstractmethod
+    def path_to_devbind_script(self) -> PurePath:
+        """The path to the dpdk-devbind.py script on the node."""
+
     def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
         """Test run setup steps.
 
@@ -114,6 +130,20 @@ def tear_down_test_run(self) -> None:
         Additional steps can be added by extending the method in subclasses with the use of super().
         """
 
+    def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
+        """Bind ports to their DPDK drivers.
+
+        Args:
+            build_target_config: The build target test run configuration according to which
+                the setup steps will be taken. This is unused in this method, but subclasses that
+                extend this method may need it.
+        """
+        self.bind_ports_to_driver()
+
+    def tear_down_build_target(self) -> None:
+        """Bind ports to their OS drivers."""
+        self.bind_ports_to_driver(for_dpdk=False)
+
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
 
@@ -228,6 +258,21 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]:
         else:
             return func
 
+    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+        """Bind all ports on the node to a driver.
+
+        Args:
+            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
+                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,
+            )
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 2855fe0276..d68270eee1 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -59,14 +59,12 @@ class SutNode(Node):
     dpdk_timestamp: str
     _build_target_config: BuildTargetConfiguration | None
     _env_vars: dict
-    _remote_tmp_dir: PurePath
     __remote_dpdk_dir: PurePath | None
     _app_compile_timeout: float
     _dpdk_kill_session: OSSession | None
     _dpdk_version: str | None
     _node_info: NodeInfo | None
     _compiler_version: str | None
-    _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: SutNodeConfiguration):
         """Extend the constructor with SUT node specifics.
@@ -79,7 +77,6 @@ def __init__(self, node_config: SutNodeConfiguration):
         self.dpdk_prefix_list = []
         self._build_target_config = None
         self._env_vars = {}
-        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
         self.__remote_dpdk_dir = None
         self._app_compile_timeout = 90
         self._dpdk_kill_session = None
@@ -89,7 +86,6 @@ def __init__(self, node_config: SutNodeConfiguration):
         self._dpdk_version = None
         self._node_info = None
         self._compiler_version = None
-        self._path_to_devbind_script = None
         self._logger.info(f"Created node: {self.name}")
 
     @property
@@ -153,7 +149,11 @@ def compiler_version(self) -> str:
 
     @property
     def path_to_devbind_script(self) -> PurePath:
-        """The path to the dpdk-devbind.py script on the node."""
+        """Implements :meth:`Node.path_to_devbind_script` on the SUT node.
+
+        It is expected that the DPDK directory will be available on this host before this property
+        is accessed.
+        """
         if self._path_to_devbind_script is None:
             self._path_to_devbind_script = self.main_session.join_remote_path(
                 self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
@@ -190,28 +190,28 @@ def tear_down_test_run(self) -> None:
         self.virtual_devices = []
 
     def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """Set up DPDK the SUT node and bind ports.
+        """Extend :meth:`Node.set_up_build_target` with DPDK setup steps.
 
         DPDK setup includes setting all internals needed for the build, the copying of DPDK tarball
-        and then building DPDK. The drivers are bound to those that DPDK needs.
+        and then building DPDK.
 
         Args:
             build_target_config: The build target test run configuration according to which
                 the setup steps will be taken.
         """
-        self._configure_build_target(build_target_config)
         self._copy_dpdk_tarball()
+        super().set_up_build_target(build_target_config)
+        self._configure_build_target(build_target_config)
         self._build_dpdk()
-        self.bind_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
-        """Reset DPDK variables and bind port driver to the OS driver."""
+        """Extend :meth:`Node.tear_down_build_target` with the resetting of DPDK variables."""
+        super().tear_down_build_target()
         self._env_vars = {}
-        self._build_target_config = None
         self.__remote_dpdk_dir = None
+        self._build_target_config = None
         self._dpdk_version = None
         self._compiler_version = None
-        self.bind_ports_to_driver(for_dpdk=False)
 
     def _configure_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
         """Populate common environment variables and set build target config."""
@@ -335,18 +335,3 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
         self.main_session.configure_ipv4_forwarding(enable)
-
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
-        """Bind all ports on the SUT to a driver.
-
-        Args:
-            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
-                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,
-            )
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 19b5b6e74c..25facb5e67 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -9,12 +9,15 @@
 A TG node is where the TG runs.
 """
 
+from pathlib import PurePath
+
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import TGNodeConfiguration
 from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
 )
+from framework.utils import LOCAL_DPDK_DIR
 
 from .node import Node
 from .port import Port
@@ -51,6 +54,24 @@ def __init__(self, node_config: TGNodeConfiguration):
         self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
         self._logger.info(f"Created node: {self.name}")
 
+    @property
+    def path_to_devbind_script(self) -> PurePath:
+        """Implements :meth:`Node.path_to_devbind_script` on the TG node.
+
+        For traffic generators this script is only copied onto the host when needed by the
+        framework.
+        """
+        if self._path_to_devbind_script is None:
+            self._logger.info(f"Copying dpdk-devbind script into {self._remote_tmp_dir}")
+            self.main_session.copy_to(
+                PurePath(LOCAL_DPDK_DIR, "usertools", "dpdk-devbind.py"),
+                PurePath(self._remote_tmp_dir, "dpdk-devbind.py"),
+            )
+            self._path_to_devbind_script = self.main_session.join_remote_path(
+                self._remote_tmp_dir, "dpdk-devbind.py"
+            )
+        return self._path_to_devbind_script
+
     def send_packets_and_capture(
         self,
         packets: list[Packet],
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index c768dd0c99..fcc178e600 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -20,7 +20,7 @@
 import random
 import subprocess
 from enum import Enum, Flag
-from pathlib import Path
+from pathlib import Path, PurePath
 from subprocess import SubprocessError
 
 from scapy.layers.inet import IP, TCP, UDP, Ether  # type: ignore[import-untyped]
@@ -29,6 +29,8 @@
 from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+#: Path to DPDK directory on the host where DTS is being run.
+LOCAL_DPDK_DIR: PurePath = PurePath(__file__).parents[2]
 
 
 def expand_range(range_str: str) -> list[int]:
-- 
2.46.0


-- 



*Let's Connect!*

...  *October Webinars*

Ask Us Anything: IOL Services 
Open Q&A 
<https://unh.zoom.us/webinar/register/9017265932716/WN_OUo5S7iQRLmKKY7CsmwZhw#/registration>Your 
questions. Our answers. Let's get started.


Oct 3rd


Live Tour of INTACT(R) 
for IPv6 Testing and Validation 
<https://unh.zoom.us/webinar/register/7117231236474/WN_I2zfyi_2S2yEiXkxBRi8sA#/registration>
Open tour. Open Q&A. See why we think you'll love INTACT.

Oct 9th


How to 
Prep for Our NVMe(R) Plugfest #21 
<https://unh.zoom.us/webinar/register/4017266809553/WN_X1iA2SZ8QhmcGboF2DImNg#/registration>
Checklists. Conversation. Let's get ready to plugin! 
Oct 15th


... * 
Newsletter*

*
*
Get the IOL Connector 
<https://www.iol.unh.edu/news/email-newsletters> for our latest news and 
event info.



.

Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node

[Juraj Linkeš]

I have some thoughts for the future:
1a. The traffic generator is specified per-node, so maybe we could also 
change the binding to be for the whole lifetime of the TG node,
1b. But the same is true for the SUT node as well, right? After we do 
the port update (with kernel driver), we can just bind to DPDK driver. 
With SUT in the mix, this looks like a change for a different patch,
2. We could add a symlink to the devbind script with the target being in 
the dts directory. This way we don't have to go outside the dts 
directory and if DTS ever become a python package, we could just copy 
the script to the appropriate place. This is also something we don't 
really need to do.

And also two minor comments. A lot of suggestions for separate patches 
overall. :-)

On 19. 9. 2024 20:16, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> The DTS framework in its current state supports binding ports to
> different drivers on the SUT node but not the TG node. The TG node
> already has the information that it needs about the different drivers
> that it has available in the configuration file, but it did not
> previously have access to the devbind script, so it did not use that
> information for anything.
> 
> This patch moves the location of the tmp directory as well as the method
> for binding ports into the node class rather than the SUT node class and
> adds an abstract method for getting the path to the devbind script into
> the node class. Then, binding ports to the correct drivers is moved into
> the build target setup and run on both nodes.
> 
> Bugzilla ID: 1420
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

With the two minor comments,
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

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

> @@ -58,8 +65,10 @@ class Node(ABC):
>       lcores: list[LogicalCore]
>       ports: list[Port]
>       _logger: DTSLogger
> +    _remote_tmp_dir: PurePath
>       _other_sessions: list[OSSession]
>       _test_run_config: TestRunConfiguration
> +    _path_to_devbind_script: PurePath | None

A note on the naming. We have _remote_tmp_dir and 
_path_to_devbind_script. Both are pointing to a remote file/dir, but 
only one has the _remote prefix. They should probably be unified.

I've thought a bit about what the right name is. Dropping the prefix 
makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node 
(which would make it remote from the execution host's point of view, but 
not the node's view; the file is local to SUT node). This could be a 
good separate patch (improving the remote/local naming scheme to make it 
consistent and as sensible as possible).


> diff --git a/dts/framework/utils.py b/dts/framework/utils.py

> @@ -29,6 +29,8 @@
>   from .exception import ConfigurationError, InternalError
>   
>   REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
> +#: Path to DPDK directory on the host where DTS is being run.
> +LOCAL_DPDK_DIR: PurePath = PurePath(__file__).parents[2]

Local paths can be just pathlib.Path. PurePaths are for path 
manipulations only (useful for remote paths in RemoteSessions, 
OSSessions and Nodes), but for local existing paths we should use Path.

The OSSession and subclasses need a bit of an update in this regard - 
use Path for local paths and PurePaths for remote ones. We added this 
into our pre-built DPDK patch.

>   
>   
>   def expand_range(range_str: str) -> list[int]:

Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node

[Jeremy Spewock]

On Tue, Sep 24, 2024 at 5:12 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> I have some thoughts for the future:
> 1a. The traffic generator is specified per-node, so maybe we could also
> change the binding to be for the whole lifetime of the TG node,
> 1b. But the same is true for the SUT node as well, right? After we do
> the port update (with kernel driver), we can just bind to DPDK driver.
> With SUT in the mix, this looks like a change for a different patch,

Right, these are good points. A good observation too that we only
really need the kernel driver at the start in both cases. You had
mentioned in your previous comments as well that we should only be
binding on the TG once per lifetime, but I ended up not adding it for
that very reason of I still wanted the binding to be in Node, but I
didn't want to change the process for the SUT.

> 2. We could add a symlink to the devbind script with the target being in
> the dts directory. This way we don't have to go outside the dts
> directory and if DTS ever become a python package, we could just copy
> the script to the appropriate place. This is also something we don't
> really need to do.

I like this idea a lot actually. It feels very weird to me having to
step out of the DTS directory and I like the idea of keeping it
together like it were a package (even if it isn't yet).

>
> And also two minor comments. A lot of suggestions for separate patches
> overall. :-)
>
> On 19. 9. 2024 20:16, jspewock@iol.unh.edu wrote:
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > The DTS framework in its current state supports binding ports to
> > different drivers on the SUT node but not the TG node. The TG node
> > already has the information that it needs about the different drivers
> > that it has available in the configuration file, but it did not
> > previously have access to the devbind script, so it did not use that
> > information for anything.
> >
> > This patch moves the location of the tmp directory as well as the method
> > for binding ports into the node class rather than the SUT node class and
> > adds an abstract method for getting the path to the devbind script into
> > the node class. Then, binding ports to the correct drivers is moved into
> > the build target setup and run on both nodes.
> >
> > Bugzilla ID: 1420
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
>
> With the two minor comments,
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>
> > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
>
> > @@ -58,8 +65,10 @@ class Node(ABC):
> >       lcores: list[LogicalCore]
> >       ports: list[Port]
> >       _logger: DTSLogger
> > +    _remote_tmp_dir: PurePath
> >       _other_sessions: list[OSSession]
> >       _test_run_config: TestRunConfiguration
> > +    _path_to_devbind_script: PurePath | None
>
> A note on the naming. We have _remote_tmp_dir and
> _path_to_devbind_script. Both are pointing to a remote file/dir, but
> only one has the _remote prefix. They should probably be unified.

I didn't think of this but you're right, the two are very similar but
named differently.

>
> I've thought a bit about what the right name is. Dropping the prefix
> makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node
> (which would make it remote from the execution host's point of view, but
> not the node's view; the file is local to SUT node). This could be a
> good separate patch (improving the remote/local naming scheme to make it
> consistent and as sensible as possible).

I also like the sound of it without the prefix and how it actually has
a more fitting meaning from the two perspectives. I agree that there
is probably some other work to be done on this in another patch, but
since I am moving the _remote_tmp_dir variable around anyway I think
it wouldn't hurt for me to rename it.

>
>
> > diff --git a/dts/framework/utils.py b/dts/framework/utils.py
>
> > @@ -29,6 +29,8 @@
> >   from .exception import ConfigurationError, InternalError
> >
> >   REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
> > +#: Path to DPDK directory on the host where DTS is being run.
> > +LOCAL_DPDK_DIR: PurePath = PurePath(__file__).parents[2]
>
> Local paths can be just pathlib.Path. PurePaths are for path
> manipulations only (useful for remote paths in RemoteSessions,
> OSSessions and Nodes), but for local existing paths we should use Path.
>
> The OSSession and subclasses need a bit of an update in this regard -
> use Path for local paths and PurePaths for remote ones. We added this
> into our pre-built DPDK patch.

Ack. I tried to search for the differences between the two actually
and saw a similar answer that PurePaths are used for manipulation, but
I didn't fully understand as I am still manipulated this path and it
isn't referencing a real file on host where I am defining the variable
(what ever host is running DTS that is). This is a good distinction
though to use Paths when it is used as a local path somewhere.

>
> >
> >
> >   def expand_range(range_str: str) -> list[int]:
>

Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node

[Juraj Linkeš]

On 24. 9. 2024 15:57, Jeremy Spewock wrote:
> On Tue, Sep 24, 2024 at 5:12 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> I have some thoughts for the future:
>> 1a. The traffic generator is specified per-node, so maybe we could also
>> change the binding to be for the whole lifetime of the TG node,
>> 1b. But the same is true for the SUT node as well, right? After we do
>> the port update (with kernel driver), we can just bind to DPDK driver.
>> With SUT in the mix, this looks like a change for a different patch,
> 
> Right, these are good points. A good observation too that we only
> really need the kernel driver at the start in both cases. You had
> mentioned in your previous comments as well that we should only be
> binding on the TG once per lifetime, but I ended up not adding it for
> that very reason of I still wanted the binding to be in Node, but I
> didn't want to change the process for the SUT.
> 
>> 2. We could add a symlink to the devbind script with the target being in
>> the dts directory. This way we don't have to go outside the dts
>> directory and if DTS ever become a python package, we could just copy
>> the script to the appropriate place. This is also something we don't
>> really need to do.
> 
> I like this idea a lot actually. It feels very weird to me having to
> step out of the DTS directory and I like the idea of keeping it
> together like it were a package (even if it isn't yet).
> 

Ok, you can add that to the next version.

>>> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
>>
>>> @@ -58,8 +65,10 @@ class Node(ABC):
>>>        lcores: list[LogicalCore]
>>>        ports: list[Port]
>>>        _logger: DTSLogger
>>> +    _remote_tmp_dir: PurePath
>>>        _other_sessions: list[OSSession]
>>>        _test_run_config: TestRunConfiguration
>>> +    _path_to_devbind_script: PurePath | None
>>
>> A note on the naming. We have _remote_tmp_dir and
>> _path_to_devbind_script. Both are pointing to a remote file/dir, but
>> only one has the _remote prefix. They should probably be unified.
> 
> I didn't think of this but you're right, the two are very similar but
> named differently.
> 
>>
>> I've thought a bit about what the right name is. Dropping the prefix
>> makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node
>> (which would make it remote from the execution host's point of view, but
>> not the node's view; the file is local to SUT node). This could be a
>> good separate patch (improving the remote/local naming scheme to make it
>> consistent and as sensible as possible).
> 
> I also like the sound of it without the prefix and how it actually has
> a more fitting meaning from the two perspectives. I agree that there
> is probably some other work to be done on this in another patch, but
> since I am moving the _remote_tmp_dir variable around anyway I think
> it wouldn't hurt for me to rename it.
> 

Yes, we should pick one naming convention to be consistent in this patch 
and we can have a broader (framework-wide) look at this in a separate patch.

[PATCH v3 0/2] dts: add driver binding on TG

[jspewock]

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

v3:
 * removed _remote from _remote_tmp_dir in node
 * switched to using Path where appropriate
 * added symlink to dpdk-devbind to dts/ and excluded from the
   the formatting script

Jeremy Spewock (2):
  dts: add synbolic link to dpdk-devbind script
  dts: add binding to different drivers to TG node

 devtools/dts-check-format.sh            |  9 +++--
 dts/dpdk-devbind.py                     |  1 +
 dts/framework/runner.py                 |  2 +
 dts/framework/testbed_model/node.py     | 49 ++++++++++++++++++++++++-
 dts/framework/testbed_model/sut_node.py | 49 ++++++++-----------------
 dts/framework/testbed_model/tg_node.py  | 21 +++++++++++
 dts/framework/utils.py                  |  2 +
 7 files changed, 94 insertions(+), 39 deletions(-)
 create mode 120000 dts/dpdk-devbind.py

-- 
2.46.0

[PATCH v3 1/2] dts: add symbolic link to dpdk-devbind script

[jspewock]

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

The devbind script is used throughout DTS to manage drivers on the
remote hosts. Currently, the only way to copy this script onto a host
is to either copy the entire DPDK directory onto a host, or reach out
of the dts directory into its parent DPDK directory to access the
script. The first is undesirable if the host doesn't require any other
DPDK tools since you would be copying extra unneeded information and
the second is undesirable since it enforces the assumption that DTS is
being run from within the DPDK directory. To solve this issue a symbolic
link is added which links the devbind script from the parent into the
DTS directory.

Since this file is not part of DTS and therefore is not expected to
follow DTS formatting rules, it is excluded from the DTS formatting
script.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 devtools/dts-check-format.sh | 9 +++++----
 dts/dpdk-devbind.py          | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)
 create mode 120000 dts/dpdk-devbind.py

diff --git a/devtools/dts-check-format.sh b/devtools/dts-check-format.sh
index 3f43e17e88..adc199d34e 100755
--- a/devtools/dts-check-format.sh
+++ b/devtools/dts-check-format.sh
@@ -13,6 +13,7 @@ usage() {
 format=true
 lint=true
 typecheck=true
+ignore="dpdk-devbind.py"
 
 # Comments after args serve as documentation; must be present
 while getopts "hflt" arg; do
@@ -54,14 +55,14 @@ if $format; then
 			heading "Formatting in $directory/"
 			if command -v black > /dev/null; then
 				echo "Formatting code with black:"
-				black .
+				black --exclude "$ignore" .
 			else
 				echo "black is not installed, not formatting"
 				errors=$((errors + 1))
 			fi
 			if command -v isort > /dev/null; then
 				echo "Sorting imports with isort:"
-				isort .
+				isort --skip "$ignore" .
 			else
 				echo "isort is not installed, not sorting imports"
 				errors=$((errors + 1))
@@ -90,7 +91,7 @@ if $lint; then
 	fi
 	heading "Linting in $directory/"
 	if command -v pylama > /dev/null; then
-		pylama .
+		pylama --skip "$ignore" .
 		errors=$((errors + $?))
 	else
 		echo "pylama not found, unable to run linter"
@@ -104,7 +105,7 @@ if $typecheck; then
 	fi
 	heading "Checking types in $directory/"
 	if command -v mypy > /dev/null; then
-		mypy .
+		mypy --exclude "$ignore" .
 		errors=$((errors + $?))
 	else
 		echo "mypy not found, unable to check types"
diff --git a/dts/dpdk-devbind.py b/dts/dpdk-devbind.py
new file mode 120000
index 0000000000..9d042fad14
--- /dev/null
+++ b/dts/dpdk-devbind.py
@@ -0,0 +1 @@
+../usertools/dpdk-devbind.py
\ No newline at end of file
-- 
2.46.0

Re: [PATCH v3 1/2] dts: add symbolic link to dpdk-devbind script

[Juraj Linkeš]

On 24. 9. 2024 18:28, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> The devbind script is used throughout DTS to manage drivers on the
> remote hosts. Currently, the only way to copy this script onto a host
> is to either copy the entire DPDK directory onto a host, or reach out
> of the dts directory into its parent DPDK directory to access the
> script. The first is undesirable if the host doesn't require any other
> DPDK tools since you would be copying extra unneeded information and
> the second is undesirable since it enforces the assumption that DTS is
> being run from within the DPDK directory. To solve this issue a symbolic
> link is added which links the devbind script from the parent into the
> DTS directory.
> 
> Since this file is not part of DTS and therefore is not expected to
> follow DTS formatting rules, it is excluded from the DTS formatting
> script.
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Re: [PATCH v3 1/2] dts: add symbolic link to dpdk-devbind script

[Luca Vizzarro]

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

[PATCH v3 2/2] dts: add binding to different drivers to TG node

[jspewock]

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

The DTS framework in its current state supports binding ports to
different drivers on the SUT node but not the TG node. The TG node
already has the information that it needs about the different drivers
that it has available in the configuration file, but it did not
previously have access to the devbind script, so it did not use that
information for anything.

This patch moves the location of the tmp directory as well as the method
for binding ports into the node class rather than the SUT node class and
adds an abstract method for getting the path to the devbind script into
the node class. Then, binding ports to the correct drivers is moved into
the build target setup and run on both nodes.

Bugzilla ID: 1420

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/runner.py                 |  2 +
 dts/framework/testbed_model/node.py     | 49 ++++++++++++++++++++++++-
 dts/framework/testbed_model/sut_node.py | 49 ++++++++-----------------
 dts/framework/testbed_model/tg_node.py  | 21 +++++++++++
 dts/framework/utils.py                  |  2 +
 5 files changed, 88 insertions(+), 35 deletions(-)

diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index ab98de8353..4c884fbcd4 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -486,6 +486,7 @@ def _run_build_target(
 
         try:
             sut_node.set_up_build_target(build_target_config)
+            tg_node.set_up_build_target(build_target_config)
             self._result.dpdk_version = sut_node.dpdk_version
             build_target_result.add_build_target_info(sut_node.get_build_target_info())
             build_target_result.update_setup(Result.PASS)
@@ -500,6 +501,7 @@ def _run_build_target(
             try:
                 self._logger.set_stage(DtsStage.build_target_teardown)
                 sut_node.tear_down_build_target()
+                tg_node.tear_down_build_target()
                 build_target_result.update_teardown(Result.PASS)
             except Exception as e:
                 self._logger.exception("Build target teardown failed.")
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 12a40170ac..aa26f2a2a7 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -13,11 +13,18 @@
 The :func:`~Node.skip_setup` decorator can be used without subclassing.
 """
 
-from abc import ABC
+
+from abc import ABC, abstractmethod
 from ipaddress import IPv4Interface, IPv6Interface
+from pathlib import PurePath
 from typing import Any, Callable, Union
 
-from framework.config import OS, NodeConfiguration, TestRunConfiguration
+from framework.config import (
+    OS,
+    BuildTargetConfiguration,
+    NodeConfiguration,
+    TestRunConfiguration,
+)
 from framework.exception import ConfigurationError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.settings import SETTINGS
@@ -58,8 +65,10 @@ class Node(ABC):
     lcores: list[LogicalCore]
     ports: list[Port]
     _logger: DTSLogger
+    _tmp_dir: PurePath
     _other_sessions: list[OSSession]
     _test_run_config: TestRunConfiguration
+    _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: NodeConfiguration):
         """Connect to the node and gather info during initialization.
@@ -88,6 +97,8 @@ def __init__(self, node_config: NodeConfiguration):
 
         self._other_sessions = []
         self._init_ports()
+        self._tmp_dir = self.main_session.get_remote_tmp_dir()
+        self._path_to_devbind_script = None
 
     def _init_ports(self) -> None:
         self.ports = [Port(self.name, port_config) for port_config in self.config.ports]
@@ -95,6 +106,11 @@ def _init_ports(self) -> None:
         for port in self.ports:
             self.configure_port_state(port)
 
+    @property
+    @abstractmethod
+    def path_to_devbind_script(self) -> PurePath:
+        """The path to the dpdk-devbind.py script on the node."""
+
     def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
         """Test run setup steps.
 
@@ -114,6 +130,20 @@ def tear_down_test_run(self) -> None:
         Additional steps can be added by extending the method in subclasses with the use of super().
         """
 
+    def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
+        """Bind ports to their DPDK drivers.
+
+        Args:
+            build_target_config: The build target test run configuration according to which
+                the setup steps will be taken. This is unused in this method, but subclasses that
+                extend this method may need it.
+        """
+        self.bind_ports_to_driver()
+
+    def tear_down_build_target(self) -> None:
+        """Bind ports to their OS drivers."""
+        self.bind_ports_to_driver(for_dpdk=False)
+
     def create_session(self, name: str) -> OSSession:
         """Create and return a new OS-aware remote session.
 
@@ -228,6 +258,21 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]:
         else:
             return func
 
+    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+        """Bind all ports on the node to a driver.
+
+        Args:
+            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
+                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,
+            )
+
 
 def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
     """Factory for OS-aware sessions.
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 2855fe0276..8bd5c03f12 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -59,14 +59,12 @@ class SutNode(Node):
     dpdk_timestamp: str
     _build_target_config: BuildTargetConfiguration | None
     _env_vars: dict
-    _remote_tmp_dir: PurePath
     __remote_dpdk_dir: PurePath | None
     _app_compile_timeout: float
     _dpdk_kill_session: OSSession | None
     _dpdk_version: str | None
     _node_info: NodeInfo | None
     _compiler_version: str | None
-    _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: SutNodeConfiguration):
         """Extend the constructor with SUT node specifics.
@@ -79,7 +77,6 @@ def __init__(self, node_config: SutNodeConfiguration):
         self.dpdk_prefix_list = []
         self._build_target_config = None
         self._env_vars = {}
-        self._remote_tmp_dir = self.main_session.get_remote_tmp_dir()
         self.__remote_dpdk_dir = None
         self._app_compile_timeout = 90
         self._dpdk_kill_session = None
@@ -89,7 +86,6 @@ def __init__(self, node_config: SutNodeConfiguration):
         self._dpdk_version = None
         self._node_info = None
         self._compiler_version = None
-        self._path_to_devbind_script = None
         self._logger.info(f"Created node: {self.name}")
 
     @property
@@ -153,7 +149,11 @@ def compiler_version(self) -> str:
 
     @property
     def path_to_devbind_script(self) -> PurePath:
-        """The path to the dpdk-devbind.py script on the node."""
+        """Implements :meth:`Node.path_to_devbind_script` on the SUT node.
+
+        It is expected that the DPDK directory will be available on this host before this property
+        is accessed.
+        """
         if self._path_to_devbind_script is None:
             self._path_to_devbind_script = self.main_session.join_remote_path(
                 self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
@@ -171,7 +171,7 @@ def get_build_target_info(self) -> BuildTargetInfo:
         )
 
     def _guess_dpdk_remote_dir(self) -> PurePath:
-        return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
+        return self.main_session.guess_dpdk_remote_dir(self._tmp_dir)
 
     def set_up_test_run(self, test_run_config: TestRunConfiguration) -> None:
         """Extend the test run setup with vdev config.
@@ -190,28 +190,28 @@ def tear_down_test_run(self) -> None:
         self.virtual_devices = []
 
     def set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """Set up DPDK the SUT node and bind ports.
+        """Extend :meth:`Node.set_up_build_target` with DPDK setup steps.
 
         DPDK setup includes setting all internals needed for the build, the copying of DPDK tarball
-        and then building DPDK. The drivers are bound to those that DPDK needs.
+        and then building DPDK.
 
         Args:
             build_target_config: The build target test run configuration according to which
                 the setup steps will be taken.
         """
-        self._configure_build_target(build_target_config)
         self._copy_dpdk_tarball()
+        super().set_up_build_target(build_target_config)
+        self._configure_build_target(build_target_config)
         self._build_dpdk()
-        self.bind_ports_to_driver()
 
     def tear_down_build_target(self) -> None:
-        """Reset DPDK variables and bind port driver to the OS driver."""
+        """Extend :meth:`Node.tear_down_build_target` with the resetting of DPDK variables."""
+        super().tear_down_build_target()
         self._env_vars = {}
-        self._build_target_config = None
         self.__remote_dpdk_dir = None
+        self._build_target_config = None
         self._dpdk_version = None
         self._compiler_version = None
-        self.bind_ports_to_driver(for_dpdk=False)
 
     def _configure_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
         """Populate common environment variables and set build target config."""
@@ -228,20 +228,18 @@ def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
     def _copy_dpdk_tarball(self) -> None:
         """Copy to and extract DPDK tarball on the SUT node."""
         self._logger.info("Copying DPDK tarball to SUT.")
-        self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remote_tmp_dir)
+        self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._tmp_dir)
 
         # construct remote tarball path
         # the basename is the same on local host and on remote Node
         remote_tarball_path = self.main_session.join_remote_path(
-            self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball_path)
+            self._tmp_dir, os.path.basename(SETTINGS.dpdk_tarball_path)
         )
 
         # construct remote path after extracting
         with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar:
             dpdk_top_dir = dpdk_tar.getnames()[0]
-        self._remote_dpdk_dir = self.main_session.join_remote_path(
-            self._remote_tmp_dir, dpdk_top_dir
-        )
+        self._remote_dpdk_dir = self.main_session.join_remote_path(self._tmp_dir, dpdk_top_dir)
 
         self._logger.info(
             f"Extracting DPDK tarball on SUT: "
@@ -335,18 +333,3 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
             enable: If :data:`True`, enable the forwarding, otherwise disable it.
         """
         self.main_session.configure_ipv4_forwarding(enable)
-
-    def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
-        """Bind all ports on the SUT to a driver.
-
-        Args:
-            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
-                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,
-            )
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 19b5b6e74c..45873b359f 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -9,12 +9,15 @@
 A TG node is where the TG runs.
 """
 
+from pathlib import Path, PurePath
+
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import TGNodeConfiguration
 from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
 )
+from framework.utils import LOCAL_DTS_DIR
 
 from .node import Node
 from .port import Port
@@ -51,6 +54,24 @@ def __init__(self, node_config: TGNodeConfiguration):
         self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
         self._logger.info(f"Created node: {self.name}")
 
+    @property
+    def path_to_devbind_script(self) -> PurePath:
+        """Implements :meth:`Node.path_to_devbind_script` on the TG node.
+
+        For traffic generators this script is only copied onto the host when needed by the
+        framework.
+        """
+        if self._path_to_devbind_script is None:
+            self._logger.info(f"Copying dpdk-devbind script into {self._tmp_dir}")
+            self.main_session.copy_to(
+                Path(LOCAL_DTS_DIR, "dpdk-devbind.py"),
+                PurePath(self._tmp_dir, "dpdk-devbind.py"),
+            )
+            self._path_to_devbind_script = self.main_session.join_remote_path(
+                self._tmp_dir, "dpdk-devbind.py"
+            )
+        return self._path_to_devbind_script
+
     def send_packets_and_capture(
         self,
         packets: list[Packet],
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index c768dd0c99..9056b2245f 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -29,6 +29,8 @@
 from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+#: Path to the DTS directory on the host where DTS is being run.
+LOCAL_DTS_DIR: Path = Path(__file__).parents[1]
 
 
 def expand_range(range_str: str) -> list[int]:
-- 
2.46.0

Re: [PATCH v3 2/2] dts: add binding to different drivers to TG node

[Juraj Linkeš]

On 24. 9. 2024 18:28, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> The DTS framework in its current state supports binding ports to
> different drivers on the SUT node but not the TG node. The TG node
> already has the information that it needs about the different drivers
> that it has available in the configuration file, but it did not
> previously have access to the devbind script, so it did not use that
> information for anything.
> 
> This patch moves the location of the tmp directory as well as the method
> for binding ports into the node class rather than the SUT node class and
> adds an abstract method for getting the path to the devbind script into
> the node class. Then, binding ports to the correct drivers is moved into
> the build target setup and run on both nodes.
> 
> Bugzilla ID: 1420
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Re: [PATCH v3 2/2] dts: add binding to different drivers to TG node

[Luca Vizzarro]

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

Re: [PATCH v3 0/2] dts: add driver binding on TG

[Juraj Linkeš]

On 24. 9. 2024 18:28, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> v3:
>   * removed _remote from _remote_tmp_dir in node
>   * switched to using Path where appropriate
>   * added symlink to dpdk-devbind to dts/ and excluded from the
>     the formatting script
> 
> Jeremy Spewock (2):
>    dts: add synbolic link to dpdk-devbind script
>    dts: add binding to different drivers to TG node
> 
>   devtools/dts-check-format.sh            |  9 +++--
>   dts/dpdk-devbind.py                     |  1 +
>   dts/framework/runner.py                 |  2 +
>   dts/framework/testbed_model/node.py     | 49 ++++++++++++++++++++++++-
>   dts/framework/testbed_model/sut_node.py | 49 ++++++++-----------------
>   dts/framework/testbed_model/tg_node.py  | 21 +++++++++++
>   dts/framework/utils.py                  |  2 +
>   7 files changed, 94 insertions(+), 39 deletions(-)
>   create mode 120000 dts/dpdk-devbind.py
> 

Applied to next-dts, thanks.