diff --git a/dts/framework/remote_session/dpdk_shell.py
b/dts/framework/remote_session/dpdk_shell.py
new file mode 100644
index 0000000000..25e3df4eaa
--- /dev/null
+++ b/dts/framework/remote_session/dpdk_shell.py
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 Arm Limited
+
+"""DPDK-based interactive shell.
I think this means the shell uses DPDK libraries. This would be better
worded as "Base interactive shell for DPDK applications."
+
+Provides a base class to create interactive shells based on DPDK.
+"""
+
+
+from abc import ABC
+
+from framework.params.eal import EalParams
+from framework.remote_session.interactive_shell import InteractiveShell
+from framework.settings import SETTINGS
+from framework.testbed_model.cpu import LogicalCoreCount, LogicalCoreList
+from framework.testbed_model.sut_node import SutNode
+
+
+def compute_eal_params(
+ node: SutNode,
Let's rename this sut_node. I got confused a bit when reading the code.
+ params: EalParams | None = None,
+ lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =
LogicalCoreCount(),
+ ascending_cores: bool = True,
+ append_prefix_timestamp: bool = True,
+) -> EalParams:
+ """Compute EAL parameters based on the node's specifications.
+
+ Args:
+ node: The SUT node to compute the values for.
+ params: The EalParams object to amend, if set to None a new object is
created and returned.
This could use some additional explanation about how it's amended -
what's replaced, what isn't and in general what happens.
+ lcore_filter_specifier: A number of lcores/cores/sockets to use
+ or a list of lcore ids to use.
+ The default will select one lcore for each of two cores
+ on one socket, in ascending order of core ids.
+ ascending_cores: Sort cores in ascending order (lowest to highest IDs).
+ If :data:`False`, sort in descending order.
+ append_prefix_timestamp: If :data:`True`, will append a timestamp to
DPDK file prefix.
+ """
+ if params is None:
+ params = EalParams()
+
+ if params.lcore_list is None:
+ params.lcore_list = LogicalCoreList(
+ node.filter_lcores(lcore_filter_specifier, ascending_cores)
+ )
+
+ prefix = params.prefix
+ if append_prefix_timestamp:
+ prefix = f"{prefix}_{node._dpdk_timestamp}"
+ prefix = node.main_session.get_dpdk_file_prefix(prefix)
+ if prefix:
+ node._dpdk_prefix_list.append(prefix)
We should make _dpdk_prefix_list public. Also _dpdk_timestamp.
+ params.prefix = prefix
+
+ if params.ports is None:
+ params.ports = node.ports
+
+ return params
+
+
+class DPDKShell(InteractiveShell, ABC):
+ """The base class for managing DPDK-based interactive shells.
+
+ This class shouldn't be instantiated directly, but instead be extended.
+ It automatically injects computed EAL parameters based on the node in the
+ supplied app parameters.
+ """
+
+ _node: SutNode
Same here, better to be explicit with _sut_node.
+ _app_params: EalParams
+
+ _lcore_filter_specifier: LogicalCoreCount | LogicalCoreList
+ _ascending_cores: bool
+ _append_prefix_timestamp: bool
+
+ def __init__(
+ self,
+ node: SutNode,
+ app_params: EalParams,
+ privileged: bool = True,
+ timeout: float = SETTINGS.timeout,
+ lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =
LogicalCoreCount(),
+ ascending_cores: bool = True,
+ append_prefix_timestamp: bool = True,
+ start_on_init: bool = True,
+ ) -> None:
+ """Overrides :meth:`~.interactive_shell.InteractiveShell.__init__`."""
+ self._lcore_filter_specifier = lcore_filter_specifier
+ self._ascending_cores = ascending_cores
+ self._append_prefix_timestamp = append_prefix_timestamp
+
+ super().__init__(node, app_params, privileged, timeout, start_on_init)
+
+ def _post_init(self):
+ """Computes EAL params based on the node capabilities before start."""
We could just put this before calling super().__init__() in this class
if we update path some other way, right? It's probably better to
override the class method (_update_path()) in subclasses than having
this _post_init() method.
+ self._app_params = compute_eal_params(
+ self._node,
+ self._app_params,
+ self._lcore_filter_specifier,
+ self._ascending_cores,
+ self._append_prefix_timestamp,
+ )
+
+ self._update_path(self._node.remote_dpdk_build_dir.joinpath(self.path))
diff --git a/dts/framework/remote_session/interactive_shell.py
b/dts/framework/remote_session/interactive_shell.py
index 9da66d1c7e..4be7966672 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -56,55 +58,63 @@ class InteractiveShell(ABC):
<snip>
+ def start_application(self) -> None:
"""Starts a new interactive application based on the path to the app.
This method is often overridden by subclasses as their process for
starting may look different.
-
- Args:
- get_privileged_command: A function (but could be any callable)
that produces
- the version of the command with elevated privileges.
"""
- start_command = f"{self.path} {self._app_params}"
- if get_privileged_command is not None:
- start_command = get_privileged_command(start_command)
+ self._setup_ssh_channel()
+
+ start_command = self._make_start_command()
+ if self._privileged:
+ start_command =
self._node.main_session._get_privileged_command(start_command)
This update of the command should be in _make_start_command().
self.send_command(start_command)
def send_command(self, command: str, prompt: str | None = None) -> str:
@@ -49,52 +48,48 @@ def __str__(self) -> str:
<snip>
+ def __init__(
+ self,
+ node: SutNode,
+ privileged: bool = True,
+ timeout: float = SETTINGS.timeout,
+ lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =
LogicalCoreCount(),
+ ascending_cores: bool = True,
+ append_prefix_timestamp: bool = True,
+ start_on_init: bool = True,
+ **app_params,
+ ) -> None:
+ """Overrides :meth:`~.dpdk_shell.DPDKShell.__init__`. Changes app_params to
kwargs."""
+ super().__init__(
+ node,
+ TestPmdParams(**app_params),
+ privileged,
+ timeout,
+ lcore_filter_specifier,
+ ascending_cores,
+ append_prefix_timestamp,
+ start_on_init,
Just a note on the differences in signatures. TestPmdShell has the
parameters at the end while DPDKShell and InteractiveShell have them
second. I think we could make app_params the last parameter in all of
these classes - that works for both kwargs and just singular Params.
)
- super()._start_application(get_privileged_command)
-
def start(self, verify: bool = True) -> None:
"""Start packet forwarding with the current configuration.