* [PATCH] dts: add dpdk shell warm up period @ 2025-09-08 1:41 Patrick Robb 2025-09-08 1:57 ` Patrick Robb 2025-09-08 10:03 ` Luca Vizzarro 0 siblings, 2 replies; 4+ messages in thread From: Patrick Robb @ 2025-09-08 1:41 UTC (permalink / raw) To: luca.vizzarro; +Cc: dev, dmarx, abailey, 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dts: add dpdk shell warm up period 2025-09-08 1:41 [PATCH] dts: add dpdk shell warm up period Patrick Robb @ 2025-09-08 1:57 ` Patrick Robb 2025-09-08 10:03 ` Luca Vizzarro 1 sibling, 0 replies; 4+ messages in thread From: Patrick Robb @ 2025-09-08 1:57 UTC (permalink / raw) To: luca.vizzarro; +Cc: dev, dmarx, abailey, Ajit Khaparde, Bruce Richardson [-- 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 --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dts: add dpdk shell warm up period 2025-09-08 1:41 [PATCH] dts: add dpdk shell warm up period Patrick Robb 2025-09-08 1:57 ` Patrick Robb @ 2025-09-08 10:03 ` Luca Vizzarro 2025-09-08 13:21 ` Patrick Robb 1 sibling, 1 reply; 4+ messages in thread From: Luca Vizzarro @ 2025-09-08 10:03 UTC (permalink / raw) To: Patrick Robb; +Cc: dev, dmarx, abailey 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dts: add dpdk shell warm up period 2025-09-08 10:03 ` Luca Vizzarro @ 2025-09-08 13:21 ` Patrick Robb 0 siblings, 0 replies; 4+ messages in thread From: Patrick Robb @ 2025-09-08 13:21 UTC (permalink / raw) To: Luca Vizzarro; +Cc: dev, dmarx, abailey [-- 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 --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-08 13:28 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-09-08 1:41 [PATCH] dts: add dpdk shell warm up period Patrick Robb 2025-09-08 1:57 ` Patrick Robb 2025-09-08 10:03 ` Luca Vizzarro 2025-09-08 13:21 ` Patrick Robb
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).