DPDK patches and discussions
 help / color / mirror / Atom feed
From: jspewock@iol.unh.edu
To: paul.szczepanek@arm.com, probb@iol.unh.edu, thomas@monjalon.net,
	alex.chapman@arm.com, Honnappa.Nagarahalli@arm.com,
	wathsala.vithanage@arm.com, yoan.picchi@foss.arm.com,
	Luca.Vizzarro@arm.com, juraj.linkes@pantheon.tech,
	npratte@iol.unh.edu
Cc: dev@dpdk.org, Jeremy Spewock <jspewock@iol.unh.edu>
Subject: [PATCH v3 2/2] dts: add binding to different drivers to TG node
Date: Tue, 24 Sep 2024 12:28:42 -0400	[thread overview]
Message-ID: <20240924162842.11714-3-jspewock@iol.unh.edu> (raw)
In-Reply-To: <20240924162842.11714-1-jspewock@iol.unh.edu>

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


  parent reply	other threads:[~2024-09-24 16:29 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-12 17:22 [PATCH 0/1] dts: add driver binding on TG jspewock
2024-08-12 17:22 ` [PATCH 1/1] dts: add binding to different drivers to TG node jspewock
2024-08-12 17:49   ` Nicholas Pratte
2024-09-09 12:16   ` Juraj Linkeš
2024-09-09 15:55     ` Jeremy Spewock
2024-09-16 10:04       ` Juraj Linkeš
2024-09-18 18:50         ` Jeremy Spewock
2024-09-19  9:10           ` Juraj Linkeš
2024-09-12 13:00   ` Patrick Robb
2024-09-19 18:16 ` [PATCH v2 0/1] dts: add driver binding on TG jspewock
2024-09-19 18:16   ` [PATCH v2 1/1] dts: add binding to different drivers to TG node jspewock
2024-09-24  9:12     ` Juraj Linkeš
2024-09-24 13:57       ` Jeremy Spewock
2024-09-24 14:03         ` Juraj Linkeš
2024-09-24 16:28 ` [PATCH v3 0/2] dts: add driver binding on TG jspewock
2024-09-24 16:28   ` [PATCH v3 1/2] dts: add symbolic link to dpdk-devbind script jspewock
2024-09-25  5:48     ` Juraj Linkeš
2024-09-27 11:49     ` Luca Vizzarro
2024-09-24 16:28   ` jspewock [this message]
2024-09-25  6:01     ` [PATCH v3 2/2] dts: add binding to different drivers to TG node Juraj Linkeš
2024-09-27 11:50     ` Luca Vizzarro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240924162842.11714-3-jspewock@iol.unh.edu \
    --to=jspewock@iol.unh.edu \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Luca.Vizzarro@arm.com \
    --cc=alex.chapman@arm.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=npratte@iol.unh.edu \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    --cc=thomas@monjalon.net \
    --cc=wathsala.vithanage@arm.com \
    --cc=yoan.picchi@foss.arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).