[PATCH v1] dts: fix devbind initialization bug

[PATCH v1] dts: fix devbind initialization bug

[Dean Marx]

Rearrange the topology and DPDK setup/teardown calls during
test runs to ensure the devbind script is not called
while the DPDK tmp directory doesn't exist.

Fixes: 4cef16f1f0a4 ("dts: improve port handling")

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/test_run.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dts/framework/test_run.py b/dts/framework/test_run.py
index cff0085317..4287028b4b 100644
--- a/dts/framework/test_run.py
+++ b/dts/framework/test_run.py
@@ -344,8 +344,8 @@ def next(self) -> State | None:
 
         test_run.ctx.sut_node.setup()
         test_run.ctx.tg_node.setup()
-        test_run.ctx.topology.setup()
         test_run.ctx.dpdk.setup()
+        test_run.ctx.topology.setup()
         test_run.ctx.tg.setup(test_run.ctx.topology)
 
         self.result.ports = test_run.ctx.topology.sut_ports + test_run.ctx.topology.tg_ports
@@ -433,8 +433,8 @@ def next(self) -> State | None:
         """Next state."""
         self.test_run.ctx.shell_pool.terminate_current_pool()
         self.test_run.ctx.tg.teardown()
-        self.test_run.ctx.dpdk.teardown()
         self.test_run.ctx.topology.teardown()
+        self.test_run.ctx.dpdk.teardown()
         self.test_run.ctx.tg_node.teardown()
         self.test_run.ctx.sut_node.teardown()
         self.result.update_teardown(Result.PASS)
-- 
2.49.0

Re: [PATCH v1] dts: fix devbind initialization bug

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 4811 bytes --]

On Thu, Jun 12, 2025 at 4:12 PM Dean Marx <dmarx@iol.unh.edu> wrote:

> Rearrange the topology and DPDK setup/teardown calls during
> test runs to ensure the devbind script is not called
> while the DPDK tmp directory doesn't exist.
>
> Fixes: 4cef16f1f0a4 ("dts: improve port handling")
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/test_run.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/dts/framework/test_run.py b/dts/framework/test_run.py
> index cff0085317..4287028b4b 100644
> --- a/dts/framework/test_run.py
> +++ b/dts/framework/test_run.py
> @@ -344,8 +344,8 @@ def next(self) -> State | None:
>
>          test_run.ctx.sut_node.setup()
>          test_run.ctx.tg_node.setup()
> -        test_run.ctx.topology.setup()
>          test_run.ctx.dpdk.setup()
> +        test_run.ctx.topology.setup()
>          test_run.ctx.tg.setup(test_run.ctx.topology)
>
>          self.result.ports = test_run.ctx.topology.sut_ports +
> test_run.ctx.topology.tg_ports
> @@ -433,8 +433,8 @@ def next(self) -> State | None:
>          """Next state."""
>          self.test_run.ctx.shell_pool.terminate_current_pool()
>          self.test_run.ctx.tg.teardown()
> -        self.test_run.ctx.dpdk.teardown()
>          self.test_run.ctx.topology.teardown()
> +        self.test_run.ctx.dpdk.teardown()
>          self.test_run.ctx.tg_node.teardown()
>          self.test_run.ctx.sut_node.teardown()
>          self.result.update_teardown(Result.PASS)
> --
> 2.49.0
>
>
Thanks this makes sense in principle but I think there is an additional
issue, which I saw when I just tested your patch on 2 XL710 NICs connected
both on the Braum server.

Here is the execution order from the testrun

1. testrun init creates DPDKRuntimeEnvironment with _node=sutnote like:

dpdk_runtime_env = DPDKRuntimeEnvironment(config.dpdk, sut_node,
dpdk_build_env)

which is then added to ctx.

2. TestRunSetup calls test_run.ctx.dpdk.setup() which
calls _prepare_devbind_script() which adds the devbind_script_path to
_node.main_session, but _node in this case is the sut node. I don't believe
there is the code path for adding devbind_script_path to the tg node
main_session, so later at topology.setup() we call self._setup_ports("sut")
which runs fine, and then error on self._setup_ports("tg") because again
the tg node main session is lacking the devbind_script_path attribute. I
think you will need to address this second part for your patch to be
complete. In any case, let's chat in the morning and figure out what a v2
of this bugfix can be.

See this stacktrace:

2025/06/16 02:59:03 - test_run_setup - dts.braum-intel-40g-SUT - INFO -
Sending: 'lshw -quiet -json -C network'
2025/06/16 02:59:04 - test_run_setup - dts.braum-intel-40g-TG - INFO -
Sending: 'lshw -quiet -json -C network'
2025/06/16 02:59:05 - test_run_setup - dts - ERROR - A CRITICAL ERROR has
occurred during test run setup. Unrecoverable state reached, shutting down.
2025/06/16 02:59:05 - test_run_setup - dts - ERROR - An unexpected error
has occurred.
Traceback (most recent call last):
  File "/dpdk/dts/framework/runner.py", line 81, in run
    test_run.spin()
  File "/dpdk/dts/framework/test_run.py", line 242, in spin
    next_state = self.state.handle_exception(e)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dpdk/dts/framework/test_run.py", line 240, in spin
    next_state = self.state.next()
                 ^^^^^^^^^^^^^^^^^
  File "/dpdk/dts/framework/test_run.py", line 348, in next
    test_run.ctx.topology.setup()
  File "/dpdk/dts/framework/testbed_model/topology.py", line 130, in setup
    self._setup_ports("tg")
  File "/dpdk/dts/framework/testbed_model/topology.py", line 154, in
_setup_ports
    self._bind_ports_to_drivers(node, ports, "kernel")
  File "/dpdk/dts/framework/testbed_model/topology.py", line 205, in
_bind_ports_to_drivers
    node.main_session.bind_ports_to_driver(ports, driver_name)
  File "/dpdk/dts/framework/testbed_model/linux_session.py", line 187, in
bind_ports_to_driver
    f"{self.devbind_script_path} -b {driver_name} --force
{ports_pci_addrs}",
       ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/functools.py", line 995, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File "/dpdk/dts/framework/testbed_model/linux_session.py", line 212, in
devbind_script_path
    raise InternalError("Accessed devbind script path before setup.")
framework.exception.InternalError: Accessed devbind script path before
setup.
2025/06/16 02:59:05 - test_run_setup - dts - INFO - Moving from stage
'test_run_setup' to stage 'post_run'.
2025/06/16 02:59:05 - post_run - dts - INFO - DTS execution has ended.

[-- Attachment #2: Type: text/html, Size: 5897 bytes --]

[PATCH v2] dts: fix devbind initialization bug

[Dean Marx]

Rearrange the topology and DPDK setup/teardown calls during
test runs to ensure the devbind script is not called
while the DPDK tmp directory doesn't exist.

Fixes: 4cef16f1f0a4 ("dts: improve port handling")

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/test_run.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dts/framework/test_run.py b/dts/framework/test_run.py
index cff0085317..60a9ec8148 100644
--- a/dts/framework/test_run.py
+++ b/dts/framework/test_run.py
@@ -344,8 +344,9 @@ def next(self) -> State | None:
 
         test_run.ctx.sut_node.setup()
         test_run.ctx.tg_node.setup()
-        test_run.ctx.topology.setup()
         test_run.ctx.dpdk.setup()
+        test_run.ctx.topology.setup()
+        self.test_run.ctx.topology.configure_ports("sut", "dpdk")
         test_run.ctx.tg.setup(test_run.ctx.topology)
 
         self.result.ports = test_run.ctx.topology.sut_ports + test_run.ctx.topology.tg_ports
@@ -433,8 +434,8 @@ def next(self) -> State | None:
         """Next state."""
         self.test_run.ctx.shell_pool.terminate_current_pool()
         self.test_run.ctx.tg.teardown()
-        self.test_run.ctx.dpdk.teardown()
         self.test_run.ctx.topology.teardown()
+        self.test_run.ctx.dpdk.teardown()
         self.test_run.ctx.tg_node.teardown()
         self.test_run.ctx.sut_node.teardown()
         self.result.update_teardown(Result.PASS)
-- 
2.49.0

[PATCH v2] dts: fix devbind initialization bug

[Dean Marx]

Rearrange the topology and DPDK setup/teardown calls during
test runs to ensure the devbind script is not called
while the DPDK tmp directory doesn't exist.

Fixes: 4cef16f1f0a4 ("dts: improve port handling")

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/test_run.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dts/framework/test_run.py b/dts/framework/test_run.py
index cff0085317..60a9ec8148 100644
--- a/dts/framework/test_run.py
+++ b/dts/framework/test_run.py
@@ -344,8 +344,9 @@ def next(self) -> State | None:
 
         test_run.ctx.sut_node.setup()
         test_run.ctx.tg_node.setup()
-        test_run.ctx.topology.setup()
         test_run.ctx.dpdk.setup()
+        test_run.ctx.topology.setup()
+        self.test_run.ctx.topology.configure_ports("sut", "dpdk")
         test_run.ctx.tg.setup(test_run.ctx.topology)
 
         self.result.ports = test_run.ctx.topology.sut_ports + test_run.ctx.topology.tg_ports
@@ -433,8 +434,8 @@ def next(self) -> State | None:
         """Next state."""
         self.test_run.ctx.shell_pool.terminate_current_pool()
         self.test_run.ctx.tg.teardown()
-        self.test_run.ctx.dpdk.teardown()
         self.test_run.ctx.topology.teardown()
+        self.test_run.ctx.dpdk.teardown()
         self.test_run.ctx.tg_node.teardown()
         self.test_run.ctx.sut_node.teardown()
         self.result.update_teardown(Result.PASS)
-- 
2.49.0

Re: [PATCH v2] dts: fix devbind initialization bug

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 38 bytes --]

Recheck-request: iol-intel-Functional

[-- Attachment #2: Type: text/html, Size: 59 bytes --]

Re: [PATCH v2] dts: fix devbind initialization bug

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Thanks, looks good and I gave it a testrun on some XL710.

I'm going to include the Slack summary of the bugs for clarity on the v2:

Bugs:
1. topology setup is called before DPDKRuntimeEnvironment setup, but
DPDKRuntimeEnvironment setup is required for topology setup, so the
ordering needs to be reversed, which is done in Dean's V1 patch.

2. The capability check in TestRunSetup requires that the SUT be bound to
the DPDK driver, but this cannot work (except on mellanox NICs) because the
SUT NIC is bound to the kernel driver just before, for the topology setup,
and not touched again before the capability check.

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

On Mon, Jun 16, 2025 at 8:17 PM Patrick Robb <probb@iol.unh.edu> wrote:

> Recheck-request: iol-intel-Functional
>

[-- Attachment #2: Type: text/html, Size: 1257 bytes --]