[PATCH] dts: add dpdk shell warm up period

[PATCH] dts: add dpdk shell warm up period

[Patrick Robb]

When running our existing DTS testsuites on a new
NIC we observed packets would not transmit from
the traffic generator to the system under test
even after DPDK testpmd and the NIC under test
had indicated readiness through the existing
readiness checks in DTS. After adding in a warm up
sleep to DPDK shells, this issue was resolved.
Correctness is more important than execution
speed in DTS, so it seems justified to slow down
the execution a little in order to make the
testing framework less fragile to such device
specific bringup behaviors.

Signed-off-by: Patrick Robb <probb@iol.unh.edu>
---
 dts/framework/remote_session/dpdk_shell.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/dts/framework/remote_session/dpdk_shell.py b/dts/framework/remote_session/dpdk_shell.py
index d4aa02f39b..b0868d32fb 100644
--- a/dts/framework/remote_session/dpdk_shell.py
+++ b/dts/framework/remote_session/dpdk_shell.py
@@ -6,9 +6,12 @@
 Provides a base class to create interactive shells based on DPDK.
 """
 
+import time
 from abc import ABC, abstractmethod
 from pathlib import PurePath
 
+from typing_extensions import Self
+
 from framework.context import get_ctx
 from framework.params.eal import EalParams
 from framework.remote_session.interactive_shell import (
@@ -84,3 +87,9 @@ def _make_real_path(self):
         Adds the remote DPDK build directory to the path.
         """
         return get_ctx().dpdk_build.remote_dpdk_build_dir.joinpath(self.path)
+
+    def __enter__(self) -> Self:
+        """Overrides :meth:`~.interactive_shell.InteractiveShell.__enter__`."""
+        super().__enter__()
+        time.sleep(5)
+        return self
-- 
2.49.0

Re: [PATCH] dts: add dpdk shell warm up period

[Patrick Robb]

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

To provide a little additional context, specifically what was happening was
we paired up an Intel E810 (traffic generator) against a broadcom p2100G
(SUT) using a generic DAC cable, and then like I said in the original
commit, packets would not transmit even after the regular readiness checks
in DTS had passed (ports are reporting they are started and we have run
testpmd start and the testpmd prompt "testpmd>" had entered the output).
So, this outcome was a little confusing. I tried to throw in some
additional steps (like throw in a bring link down and up via kernel on TG
side and via testpmd on SUT side) but that didn't make a difference. I also
looked through the DTS verbose logs after having done that and saw that
testpmd was reporting link was up on the Broadcom NIC. I briefly thought
about swapping out the E810 for a second P2100G so it would be a broadcom
to broadcom connection, but I didn't have an extra on hand and if we are
going to spend money getting new hardware it should be on the newer devices
Broadcom has put out vs this model which has been out for a few years.

Anyhow, without getting into why exactly this is happening with this
specific mix of hardware, I think the important thing from my perspective
is that DTS should not be fragile to device specific behavior like this
since correctness is more important than speed. And, I don't know when this
will happen in the future with a different device. A (sort of) arbitrary
sleep isn't an ideal addition to DTS in my mind, but given that our
existing readiness checks seem correct (but may not be sufficient) this
seems like the correct action to me.

On Sun, Sep 7, 2025 at 9:48 PM Patrick Robb <probb@iol.unh.edu> wrote:

> When running our existing DTS testsuites on a new
> NIC we observed packets would not transmit from
> the traffic generator to the system under test
> even after DPDK testpmd and the NIC under test
> had indicated readiness through the existing
> readiness checks in DTS. After adding in a warm up
> sleep to DPDK shells, this issue was resolved.
> Correctness is more important than execution
> speed in DTS, so it seems justified to slow down
> the execution a little in order to make the
> testing framework less fragile to such device
> specific bringup behaviors.
>
> Signed-off-by: Patrick Robb <probb@iol.unh.edu>
> ---
>  dts/framework/remote_session/dpdk_shell.py | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/dts/framework/remote_session/dpdk_shell.py
> b/dts/framework/remote_session/dpdk_shell.py
> index d4aa02f39b..b0868d32fb 100644
> --- a/dts/framework/remote_session/dpdk_shell.py
> +++ b/dts/framework/remote_session/dpdk_shell.py
> @@ -6,9 +6,12 @@
>  Provides a base class to create interactive shells based on DPDK.
>  """
>
> +import time
>  from abc import ABC, abstractmethod
>  from pathlib import PurePath
>
> +from typing_extensions import Self
> +
>  from framework.context import get_ctx
>  from framework.params.eal import EalParams
>  from framework.remote_session.interactive_shell import (
> @@ -84,3 +87,9 @@ def _make_real_path(self):
>          Adds the remote DPDK build directory to the path.
>          """
>          return
> get_ctx().dpdk_build.remote_dpdk_build_dir.joinpath(self.path)
> +
> +    def __enter__(self) -> Self:
> +        """Overrides
> :meth:`~.interactive_shell.InteractiveShell.__enter__`."""
> +        super().__enter__()
> +        time.sleep(5)
> +        return self
> --
> 2.49.0
>
>

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

Re: [PATCH] dts: add dpdk shell warm up period

[Luca Vizzarro]

Hi Patrick,

Perfectly understand the situation, I am just afraid that adding a whole 
5 seconds every time an InteractiveShell is spawned may slow down 
everything by a lot. These are spawned a lot of times, so it will stack 
up. Is there no other way to test for readiness?

Also I think we are providing the ability to run shells outside the 
context manager as well for some scenarios where it's needed, so this 
approach won't cover that case unfortunately.

Best,
Luca

Re: [PATCH] dts: add dpdk shell warm up period

[Patrick Robb]

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

On Mon, Sep 8, 2025 at 6:03 AM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:

> Hi Patrick,
>
> Perfectly understand the situation, I am just afraid that adding a whole
> 5 seconds every time an InteractiveShell is spawned may slow down
> everything by a lot. These are spawned a lot of times, so it will stack
> up. Is there no other way to test for readiness?
>

Yeah. I was guessing at the math before and thought it was like +25%
execution time, but now I see it's more like +90%. Definitely not ideal.

I wasn't able to come up with any other readiness checks but maybe there
is. Anyhow let's discuss all this at the Thursday meeting as it's not
urgent.


>
> Also I think we are providing the ability to run shells outside the
> context manager as well for some scenarios where it's needed, so this
> approach won't cover that case unfortunately.
>

Good point I'll take another look.


>
> Best,
> Luca
>

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

[PATCH v2] dts: testpmd link check on port start

[Patrick Robb]

When running our existing DTS testsuites on a new
NIC we observed packets would not transmit from
the traffic generator to the system under test
even after DPDK testpmd and the NIC under test
had indicated readiness. After investigation, we
determined that the existing readiness check in DTS
for testpmd start (checking that port is started)
is insufficient, because on some systems the link
will remain down for some measurable time, creating
a race condition between the testpmd port's link
coming up and the DTS execution reaching the packet
transmission step. This change will ensure that
testpmd start will block until the port is reporting
that its link is up. In addition, the interval in
between checking the link state has been reduced in
order to speed up the execution.

Signed-off-by: Patrick Robb <probb@iol.unh.edu>
Tested-by: Patrick Robb <probb@iol.unh.edu>
---
 dts/api/testpmd/__init__.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/dts/api/testpmd/__init__.py b/dts/api/testpmd/__init__.py
index aadb7f4e70..49a847494a 100644
--- a/dts/api/testpmd/__init__.py
+++ b/dts/api/testpmd/__init__.py
@@ -93,6 +93,9 @@ def _requires_started_ports(func: TestPmdMethod) -> TestPmdMethod:
 
     Args:
         func: The :class:`TestPmd` method to decorate.
+
+    Raises:
+        InteractiveCommandExecutionError: If the ports has been started but a port link will not come up.
     """
 
     @functools.wraps(func)
@@ -100,6 +103,10 @@ def _wrapper(self: "TestPmd", *args: P.args, **kwargs: P.kwargs) -> Any:
         if not self.ports_started:
             self._logger.debug("Ports need to be started to continue.")
             self.start_all_ports()
+        if get_ctx().topology.type is not LinkTopology.NO_LINK:
+            for port in self.ports:
+                if not self.wait_link_status_up(port.id):
+                    raise InteractiveCommandExecutionError(f"Port {port.id} link failed to come up.")
 
         return func(self, *args, **kwargs)
 
@@ -265,7 +272,7 @@ def wait_link_status_up(self, port_id: int, timeout=SETTINGS.timeout) -> bool:
             port_info = self.send_command(f"show port info {port_id}")
             if "Link status: up" in port_info:
                 break
-            time.sleep(0.5)
+            time.sleep(0.25)
         else:
             self._logger.error(f"The link for port {port_id} did not come up in the given timeout.")
         return "Link status: up" in port_info
-- 
2.49.0