DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] dts: enable port binding on the TG
@ 2025-08-14 20:38 Andrew Bailey
  2025-08-15  8:50 ` Luca Vizzarro
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Bailey @ 2025-08-14 20:38 UTC (permalink / raw)
  To: luca.vizzarro; +Cc: abailey, dev, dmarx, probb

Currently, driver binding for all nodes relies on a devbind_script_path
attribute in the node main sessions. This attribute is being set on the
SUT node, but not the TG node. Consequently, if the TG node ports are
not bound to the correct driver before DTS execution, the TG ports will
fail to bind and DTS will error.

relocate prepare devbind script method to topology class to be used by both
TG and SUT nodes.

Signed-off-by: Andrew Bailey <abailey@iol.unh.edu>

done
---
 .mailmap                                |  1 +
 dts/framework/remote_session/dpdk.py    | 36 +----------------------
 dts/framework/testbed_model/topology.py | 39 +++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/.mailmap b/.mailmap
index 34a99f93a1..df06862d8b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -104,6 +104,7 @@ Andre Muezerie <andremue@linux.microsoft.com> <andremue@microsoft.com>
 Andrea Arcangeli <aarcange@redhat.com>
 Andrea Grandi <andrea.grandi@intel.com>
 Andre Richter <andre.o.richter@gmail.com>
+Andrew Bailey <abailey@iol.unh.edu>
 Andrew Boyer <andrew.boyer@amd.com> <aboyer@pensando.io>
 Andrew Harvey <agh@cisco.com>
 Andrew Jackson <ajackson@solarflare.com>
diff --git a/dts/framework/remote_session/dpdk.py b/dts/framework/remote_session/dpdk.py
index 606d6e22fe..d1ab3ae50d 100644
--- a/dts/framework/remote_session/dpdk.py
+++ b/dts/framework/remote_session/dpdk.py
@@ -24,12 +24,11 @@
     RemoteDPDKTarballLocation,
     RemoteDPDKTreeLocation,
 )
-from framework.exception import ConfigurationError, InternalError, RemoteFileNotFoundError
+from framework.exception import ConfigurationError, RemoteFileNotFoundError
 from framework.logger import DTSLogger, get_dts_logger
 from framework.params.eal import EalParams
 from framework.remote_session.remote_session import CommandResult
 from framework.testbed_model.cpu import LogicalCore, LogicalCoreCount, LogicalCoreList, lcore_filter
-from framework.testbed_model.linux_session import LinuxSession
 from framework.testbed_model.node import Node
 from framework.testbed_model.os_session import OSSession
 from framework.testbed_model.virtual_device import VirtualDevice
@@ -358,7 +357,6 @@ def setup(self):
         """Set up the DPDK runtime on the target node."""
         if self.build:
             self.build.setup()
-        self._prepare_devbind_script()
 
     def teardown(self) -> None:
         """Reset DPDK variables and bind port driver to the OS driver."""
@@ -385,38 +383,6 @@ def run_dpdk_app(
             f"{app_path} {eal_params}", timeout, privileged=True, verify=True
         )
 
-    def _prepare_devbind_script(self) -> None:
-        """Prepare the devbind script.
-
-        If the environment has a build associated with it, then use the script within that build's
-        tree. Otherwise, copy the script from the local repository.
-
-        This script is only available for Linux, if the detected session is not Linux then do
-        nothing.
-
-        Raises:
-            InternalError: If dpdk-devbind.py could not be found.
-        """
-        if not isinstance(self._node.main_session, LinuxSession):
-            return
-
-        if self.build:
-            devbind_script_path = self._node.main_session.join_remote_path(
-                self.build.remote_dpdk_tree_path, "usertools", "dpdk-devbind.py"
-            )
-        else:
-            local_script_path = Path("..", "usertools", "dpdk-devbind.py").resolve()
-            if not local_script_path.exists():
-                raise InternalError("Could not find dpdk-devbind.py locally.")
-
-            devbind_script_path = self._node.main_session.join_remote_path(
-                self._node.tmp_dir, local_script_path.name
-            )
-
-            self._node.main_session.copy_to(local_script_path, devbind_script_path)
-
-        self._node.main_session.devbind_script_path = devbind_script_path
-
     def filter_lcores(
         self,
         filter_specifier: LogicalCoreCount | LogicalCoreList,
diff --git a/dts/framework/testbed_model/topology.py b/dts/framework/testbed_model/topology.py
index 899ea0ad3a..db96aaf170 100644
--- a/dts/framework/testbed_model/topology.py
+++ b/dts/framework/testbed_model/topology.py
@@ -12,11 +12,13 @@
 from collections.abc import Iterator
 from dataclasses import dataclass
 from enum import Enum
+from pathlib import Path
 from typing import Literal, NamedTuple
 
 from typing_extensions import Self
 
 from framework.exception import ConfigurationError, InternalError
+from framework.testbed_model.linux_session import LinuxSession
 from framework.testbed_model.node import Node
 
 from .port import DriverKind, Port, PortConfig
@@ -128,6 +130,7 @@ def setup(self) -> None:
 
         Binds all the ports to the right kernel driver to retrieve MAC addresses and logical names.
         """
+        self._prepare_devbind_script()
         self._setup_ports("sut")
         self._setup_ports("tg")
 
@@ -250,6 +253,42 @@ def _bind_ports_to_drivers(
         for driver_name, ports in driver_to_ports.items():
             node.main_session.bind_ports_to_driver(ports, driver_name)
 
+    def _prepare_devbind_script(self) -> None:
+        """Prepare the devbind script.
+
+        If the environment has a build associated with it, then use the script within that build's
+        tree. Otherwise, copy the script from the local repository.
+
+        This script is only available for Linux, if the detected session is not Linux then do
+        nothing.
+
+        Raises:
+            InternalError: If dpdk-devbind.py could not be found.
+        """
+        from framework.context import get_ctx
+
+        ctx = get_ctx()
+        tg = ctx.tg_node
+        sut = ctx.sut_node
+
+        def prepare_node(node: Node) -> None:
+            if not isinstance(node.main_session, LinuxSession):
+                return
+
+            local_script_path = Path("..", "usertools", "dpdk-devbind.py").resolve()
+            if not local_script_path.exists():
+                raise InternalError("Could not find dpdk-devbind.py locally.")
+
+            devbind_script_path = node.main_session.join_remote_path(
+                node.tmp_dir, local_script_path.name
+            )
+
+            node.main_session.copy_to(local_script_path, devbind_script_path)
+            node.main_session.devbind_script_path = devbind_script_path
+
+        prepare_node(tg)
+        prepare_node(sut)
+
     @property
     def sut_dpdk_ports(self) -> list[Port]:
         """The DPDK ports for the SUT node."""
-- 
2.50.1


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

* Re: [PATCH] dts: enable port binding on the TG
  2025-08-14 20:38 [PATCH] dts: enable port binding on the TG Andrew Bailey
@ 2025-08-15  8:50 ` Luca Vizzarro
  0 siblings, 0 replies; 2+ messages in thread
From: Luca Vizzarro @ 2025-08-15  8:50 UTC (permalink / raw)
  To: Andrew Bailey; +Cc: dev, dmarx, probb

Hi Andrew,

thank you for the very quick turnaround! I've just noticed that this is 
not precisely what I had in mind, but in fairness this approach is 
perfectly good.

On 14/08/2025 21:38, Andrew Bailey wrote:
> Currently, driver binding for all nodes relies on a devbind_script_path
> attribute in the node main sessions. This attribute is being set on the
> SUT node, but not the TG node. Consequently, if the TG node ports are
> not bound to the correct driver before DTS execution, the TG ports will
> fail to bind and DTS will error.
> 
> relocate prepare devbind script method to topology class to be used by both
> TG and SUT nodes.
nit: missed capitalisation.>
> Signed-off-by: Andrew Bailey <abailey@iol.unh.edu>
> 
> done
> ---
>   .mailmap                                |  1 +
>   dts/framework/remote_session/dpdk.py    | 36 +----------------------
>   dts/framework/testbed_model/topology.py | 39 +++++++++++++++++++++++++
>   3 files changed, 41 insertions(+), 35 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 34a99f93a1..df06862d8b 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -104,6 +104,7 @@ Andre Muezerie <andremue@linux.microsoft.com> <andremue@microsoft.com>
>   Andrea Arcangeli <aarcange@redhat.com>
>   Andrea Grandi <andrea.grandi@intel.com>
>   Andre Richter <andre.o.richter@gmail.com>
> +Andrew Bailey <abailey@iol.unh.edu>
>   Andrew Boyer <andrew.boyer@amd.com> <aboyer@pensando.io>
>   Andrew Harvey <agh@cisco.com>
>   Andrew Jackson <ajackson@solarflare.com>
> diff --git a/dts/framework/remote_session/dpdk.py b/dts/framework/remote_session/dpdk.py
> index 606d6e22fe..d1ab3ae50d 100644
> --- a/dts/framework/remote_session/dpdk.py
> +++ b/dts/framework/remote_session/dpdk.py
> @@ -24,12 +24,11 @@
>       RemoteDPDKTarballLocation,
>       RemoteDPDKTreeLocation,
>   )
> -from framework.exception import ConfigurationError, InternalError, RemoteFileNotFoundError
> +from framework.exception import ConfigurationError, RemoteFileNotFoundError
>   from framework.logger import DTSLogger, get_dts_logger
>   from framework.params.eal import EalParams
>   from framework.remote_session.remote_session import CommandResult
>   from framework.testbed_model.cpu import LogicalCore, LogicalCoreCount, LogicalCoreList, lcore_filter
> -from framework.testbed_model.linux_session import LinuxSession
>   from framework.testbed_model.node import Node
>   from framework.testbed_model.os_session import OSSession
>   from framework.testbed_model.virtual_device import VirtualDevice
> @@ -358,7 +357,6 @@ def setup(self):
>           """Set up the DPDK runtime on the target node."""
>           if self.build:
>               self.build.setup()
> -        self._prepare_devbind_script()
>   
>       def teardown(self) -> None:
>           """Reset DPDK variables and bind port driver to the OS driver."""
> @@ -385,38 +383,6 @@ def run_dpdk_app(
>               f"{app_path} {eal_params}", timeout, privileged=True, verify=True
>           )
>   
> -    def _prepare_devbind_script(self) -> None:
> -        """Prepare the devbind script.
> -
> -        If the environment has a build associated with it, then use the script within that build's
> -        tree. Otherwise, copy the script from the local repository.
> -
> -        This script is only available for Linux, if the detected session is not Linux then do
> -        nothing.
> -
> -        Raises:
> -            InternalError: If dpdk-devbind.py could not be found.
> -        """
> -        if not isinstance(self._node.main_session, LinuxSession):
> -            return
> -
> -        if self.build:
> -            devbind_script_path = self._node.main_session.join_remote_path(
> -                self.build.remote_dpdk_tree_path, "usertools", "dpdk-devbind.py"
> -            )
> -        else:
> -            local_script_path = Path("..", "usertools", "dpdk-devbind.py").resolve()
> -            if not local_script_path.exists():
> -                raise InternalError("Could not find dpdk-devbind.py locally.")
> -
> -            devbind_script_path = self._node.main_session.join_remote_path(
> -                self._node.tmp_dir, local_script_path.name
> -            )
> -
> -            self._node.main_session.copy_to(local_script_path, devbind_script_path)
> -
> -        self._node.main_session.devbind_script_path = devbind_script_path
> -
>       def filter_lcores(
>           self,
>           filter_specifier: LogicalCoreCount | LogicalCoreList,
> diff --git a/dts/framework/testbed_model/topology.py b/dts/framework/testbed_model/topology.py
> index 899ea0ad3a..db96aaf170 100644
> --- a/dts/framework/testbed_model/topology.py
> +++ b/dts/framework/testbed_model/topology.py
> @@ -12,11 +12,13 @@
>   from collections.abc import Iterator
>   from dataclasses import dataclass
>   from enum import Enum
> +from pathlib import Path
>   from typing import Literal, NamedTuple
>   
>   from typing_extensions import Self
>   
>   from framework.exception import ConfigurationError, InternalError
> +from framework.testbed_model.linux_session import LinuxSession
>   from framework.testbed_model.node import Node
>   
>   from .port import DriverKind, Port, PortConfig
> @@ -128,6 +130,7 @@ def setup(self) -> None:
>   
>           Binds all the ports to the right kernel driver to retrieve MAC addresses and logical names.
>           """
> +        self._prepare_devbind_script()
>           self._setup_ports("sut")
>           self._setup_ports("tg")
>   
> @@ -250,6 +253,42 @@ def _bind_ports_to_drivers(
>           for driver_name, ports in driver_to_ports.items():
>               node.main_session.bind_ports_to_driver(ports, driver_name)
>   
> +    def _prepare_devbind_script(self) -> None:
> +        """Prepare the devbind script.
> +
> +        If the environment has a build associated with it, then use the script within that build's
> +        tree. Otherwise, copy the script from the local repository.
This is no longer true as you are now only copying the script.> +
> +        This script is only available for Linux, if the detected session is not Linux then do
> +        nothing.
> +
> +        Raises:
> +            InternalError: If dpdk-devbind.py could not be found.
> +        """
> +        from framework.context import get_ctx
> +
> +        ctx = get_ctx()
> +        tg = ctx.tg_node
> +        sut = ctx.sut_node
Since we are only calling these once there is no need in placing them in 
their own variables, see below.> +
> +        def prepare_node(node: Node) -> None:
> +            if not isinstance(node.main_session, LinuxSession):
> +                return
> +
> +            local_script_path = Path("..", "usertools", "dpdk-devbind.py").resolve()
> +            if not local_script_path.exists():
> +                raise InternalError("Could not find dpdk-devbind.py locally.")I guess local_script_path can go outside the inner function. Similarly 
we can store the result of `not local_script_path.exists()` in a 
variable in the outer function, but keep the check here as we only want 
to check if session is LinuxSession of course.> +
> +            devbind_script_path = node.main_session.join_remote_path(
> +                node.tmp_dir, local_script_path.name
> +            )
> +
> +            node.main_session.copy_to(local_script_path, devbind_script_path)
> +            node.main_session.devbind_script_path = devbind_script_path
> +
> +        prepare_node(tg)
> +        prepare_node(sut)
can just do:

   prepare_node(ctx.tg_node)
   ...

and spare 2 redundant lines.> +
>       @property
>       def sut_dpdk_ports(self) -> list[Port]:
>           """The DPDK ports for the SUT node."""
Thank you!
Best,
Luca

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

end of thread, other threads:[~2025-08-15  8:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-14 20:38 [PATCH] dts: enable port binding on the TG Andrew Bailey
2025-08-15  8:50 ` Luca Vizzarro

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