From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 86E2745482;
Mon, 17 Jun 2024 17:25:21 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 7741740BA3;
Mon, 17 Jun 2024 17:25:21 +0200 (CEST)
Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com
[209.85.208.182])
by mails.dpdk.org (Postfix) with ESMTP id 4BAA440A77
for <dev@dpdk.org>; Mon, 17 Jun 2024 17:25:19 +0200 (CEST)
Received: by mail-lj1-f182.google.com with SMTP id
38308e7fff4ca-2ebdd684ef6so5326491fa.1
for <dev@dpdk.org>; Mon, 17 Jun 2024 08:25:19 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=iol.unh.edu; s=unh-iol; t=1718637919; x=1719242719; darn=dpdk.org;
h=content-transfer-encoding:cc:to:subject:message-id:date:from
:in-reply-to:references:mime-version:from:to:cc:subject:date
:message-id:reply-to;
bh=HxekJ9J2itvBoTTupesIVTPFTBMa0a6V2wQUzXUdKpY=;
b=Cbb6zyfxxM0vGq18/8+n3arMroA31M58MnEuZy6hPYjUTS9Tl/JdpFEXXduKUaHFg4
OGTf2BwjgbF88D/OxGOztBUnQtqvwXKf2oqES+1wYMeQfJy5jT5jP+YX20Qd3IqSRB4O
2iwZtsbVcluIalC6xEbhRQuEVDUJadq2NlbCc=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1718637919; x=1719242719;
h=content-transfer-encoding:cc:to:subject:message-id:date:from
:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
:subject:date:message-id:reply-to;
bh=HxekJ9J2itvBoTTupesIVTPFTBMa0a6V2wQUzXUdKpY=;
b=lCP/zhVy/ODUle2N/cmMVL75EPeqqSs5GKr7fpsOoc68Od/8p896jRz9hhw5z5qu3S
0pM42ZAXYpRMoWm6no6RSYR5A/kaddb3UrWHg7msZ0v0vWqmGJOFv+kRFc6qg99PkRoW
7pwRS2muiKYjMH8we2e5d/Y+7OYS3hA2BEU+8wauz3FYS0t9erWcM2dyG4IvnB7FQ4JU
R/emGHeyHiCqqcukaeQWJo1lquKeBgXard4SA9Wrolkf19xEMDjc72fTe0PwDfsTNycl
aB2aUaTnSMryUiQTBj/Hd2RHJiWUdzxqAzWot0ZBY1Qzv0fXMfW/dX+jHW4sbQWayvtP
xp3w==
X-Gm-Message-State: AOJu0Yz0EJEQjocwOoyZQ9KLOQX+6umc66DMupKLk3VNn3A3go1QQwLX
jDGrwUTQ2LNUgf1dcKsYGS6pWC0LVuA5EuGdA2ciTsrJec0iXCxmqCbnIUu8KpGfmzczQQZZrve
4YlYQepFgiH/BufmLzgA3fVWGfkZiCm5D2netpw==
X-Google-Smtp-Source: AGHT+IHtWtVrI0ao0SkbUE+2Xn4Oil5ObGEtVW2RfhW8do1tiKoLq9RHP6lIIDY3k6ByTEhU0oO/CHecTdQA3V4zuGM=
X-Received: by 2002:a05:651c:1a1e:b0:2ea:eac6:6872 with SMTP id
38308e7fff4ca-2ec0e3e2157mr71871921fa.0.1718637917669; Mon, 17 Jun 2024
08:25:17 -0700 (PDT)
MIME-Version: 1.0
References: <20240326190422.577028-1-luca.vizzarro@arm.com>
<20240617145409.67407-1-luca.vizzarro@arm.com>
<20240617145409.67407-8-luca.vizzarro@arm.com>
In-Reply-To: <20240617145409.67407-8-luca.vizzarro@arm.com>
From: Nicholas Pratte <npratte@iol.unh.edu>
Date: Mon, 17 Jun 2024 11:25:06 -0400
Message-ID: <CAKXZ7ejp+QxWWVGbHUYkTk8XkkUaD003hSSPoRYkoWp5F7T5wA@mail.gmail.com>
Subject: Re: [PATCH v5 7/8] dts: rework interactive shells
To: Luca Vizzarro <luca.vizzarro@arm.com>
Cc: dev@dpdk.org, Jeremy Spewock <jspewock@iol.unh.edu>,
=?UTF-8?Q?Juraj_Linke=C5=A1?= <juraj.linkes@pantheon.tech>,
Paul Szczepanek <paul.szczepanek@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Tested-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
On Mon, Jun 17, 2024 at 10:54=E2=80=AFAM Luca Vizzarro <luca.vizzarro@arm.c=
om> wrote:
>
> The way nodes and interactive shells interact makes it difficult to
> develop for static type checking and hinting. The current system relies
> on a top-down approach, attempting to give a generic interface to the
> test developer, hiding the interaction of concrete shell classes as much
> as possible. When working with strong typing this approach is not ideal,
> as Python's implementation of generics is still rudimentary.
>
> This rework reverses the tests interaction to a bottom-up approach,
> allowing the test developer to call concrete shell classes directly,
> and let them ingest nodes independently. While also re-enforcing type
> checking and making the code easier to read.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
> dts/framework/params/eal.py | 6 +-
> dts/framework/remote_session/dpdk_shell.py | 106 +++++++++++++++
> .../remote_session/interactive_shell.py | 79 ++++++-----
> dts/framework/remote_session/python_shell.py | 4 +-
> dts/framework/remote_session/testpmd_shell.py | 64 +++++----
> dts/framework/testbed_model/node.py | 36 +----
> dts/framework/testbed_model/os_session.py | 36 +----
> dts/framework/testbed_model/sut_node.py | 124 +-----------------
> .../testbed_model/traffic_generator/scapy.py | 4 +-
> dts/tests/TestSuite_hello_world.py | 7 +-
> dts/tests/TestSuite_pmd_buffer_scatter.py | 21 ++-
> dts/tests/TestSuite_smoke_tests.py | 2 +-
> 12 files changed, 210 insertions(+), 279 deletions(-)
> create mode 100644 dts/framework/remote_session/dpdk_shell.py
>
> diff --git a/dts/framework/params/eal.py b/dts/framework/params/eal.py
> index bbdbc8f334..8d7766fefc 100644
> --- a/dts/framework/params/eal.py
> +++ b/dts/framework/params/eal.py
> @@ -35,9 +35,9 @@ class EalParams(Params):
> ``other_eal_param=3D'--single-file-segments'``
> """
>
> - lcore_list: LogicalCoreList =3D field(metadata=3DParams.short("l"))
> - memory_channels: int =3D field(metadata=3DParams.short("n"))
> - prefix: str =3D field(metadata=3DParams.long("file-prefix"))
> + lcore_list: LogicalCoreList | None =3D field(default=3DNone, metadat=
a=3DParams.short("l"))
> + memory_channels: int | None =3D field(default=3DNone, metadata=3DPar=
ams.short("n"))
> + prefix: str =3D field(default=3D"dpdk", metadata=3DParams.long("file=
-prefix"))
> no_pci: Switch =3D None
> vdevs: list[VirtualDevice] | None =3D field(
> default=3DNone, metadata=3DParams.multiple() | Params.long("vdev=
")
> diff --git a/dts/framework/remote_session/dpdk_shell.py b/dts/framework/r=
emote_session/dpdk_shell.py
> new file mode 100644
> index 0000000000..2cbf69ae9a
> --- /dev/null
> +++ b/dts/framework/remote_session/dpdk_shell.py
> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2024 Arm Limited
> +
> +"""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, LogicalCoreLis=
t
> +from framework.testbed_model.sut_node import SutNode
> +
> +
> +def compute_eal_params(
> + sut_node: SutNode,
> + params: EalParams | None =3D None,
> + lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =3D Logic=
alCoreCount(),
> + ascending_cores: bool =3D True,
> + append_prefix_timestamp: bool =3D True,
> +) -> EalParams:
> + """Compute EAL parameters based on the node's specifications.
> +
> + Args:
> + sut_node: The SUT node to compute the values for.
> + params: If set to None a new object is created and returned. Oth=
erwise the given
> + :class:`EalParams`'s lcore_list is modified according to the=
given filter specifier.
> + A DPDK prefix is added. If ports is set to None, all the SUT=
node's ports are
> + automatically assigned.
> + 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 highes=
t IDs). If :data:`False`,
> + sort in descending order.
> + append_prefix_timestamp: If :data:`True`, will append a timestam=
p to DPDK file prefix.
> + """
> + if params is None:
> + params =3D EalParams()
> +
> + if params.lcore_list is None:
> + params.lcore_list =3D LogicalCoreList(
> + sut_node.filter_lcores(lcore_filter_specifier, ascending_cor=
es)
> + )
> +
> + prefix =3D params.prefix
> + if append_prefix_timestamp:
> + prefix =3D f"{prefix}_{sut_node.dpdk_timestamp}"
> + prefix =3D sut_node.main_session.get_dpdk_file_prefix(prefix)
> + if prefix:
> + sut_node.dpdk_prefix_list.append(prefix)
> + params.prefix =3D prefix
> +
> + if params.ports is None:
> + params.ports =3D sut_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 extend=
ed.
> + It automatically injects computed EAL parameters based on the node i=
n the
> + supplied app parameters.
> + """
> +
> + _node: SutNode
> + _app_params: EalParams
> +
> + _lcore_filter_specifier: LogicalCoreCount | LogicalCoreList
> + _ascending_cores: bool
> + _append_prefix_timestamp: bool
> +
> + def __init__(
> + self,
> + node: SutNode,
> + privileged: bool =3D True,
> + timeout: float =3D SETTINGS.timeout,
> + lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =3D L=
ogicalCoreCount(),
> + ascending_cores: bool =3D True,
> + append_prefix_timestamp: bool =3D True,
> + start_on_init: bool =3D True,
> + app_params: EalParams =3D EalParams(),
> + ) -> None:
> + """Overrides :meth:`~.interactive_shell.InteractiveShell.__init_=
_`."""
> + self._lcore_filter_specifier =3D lcore_filter_specifier
> + self._ascending_cores =3D ascending_cores
> + self._append_prefix_timestamp =3D append_prefix_timestamp
> +
> + super().__init__(node, privileged, timeout, start_on_init, app_p=
arams)
> +
> + def _post_init(self):
> + """Computes EAL params based on the node capabilities before sta=
rt."""
> + self._app_params =3D 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/fram=
ework/remote_session/interactive_shell.py
> index 8191b36630..5a8a6d6d15 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -17,13 +17,14 @@
>
> from abc import ABC
> from pathlib import PurePath
> -from typing import Callable, ClassVar
> +from typing import ClassVar
>
> -from paramiko import Channel, SSHClient, channel # type: ignore[import-=
untyped]
> +from paramiko import Channel, channel # type: ignore[import-untyped]
>
> from framework.logger import DTSLogger
> from framework.params import Params
> from framework.settings import SETTINGS
> +from framework.testbed_model.node import Node
>
>
> class InteractiveShell(ABC):
> @@ -36,13 +37,14 @@ class InteractiveShell(ABC):
> session.
> """
>
> - _interactive_session: SSHClient
> + _node: Node
> _stdin: channel.ChannelStdinFile
> _stdout: channel.ChannelFile
> _ssh_channel: Channel
> _logger: DTSLogger
> _timeout: float
> _app_params: Params
> + _privileged: bool
>
> #: Prompt to expect at the end of output when sending a command.
> #: This is often overridden by subclasses.
> @@ -56,56 +58,63 @@ class InteractiveShell(ABC):
> #: Path to the executable to start the interactive application.
> path: ClassVar[PurePath]
>
> - #: Whether this application is a DPDK app. If it is, the build direc=
tory
> - #: for DPDK on the node will be prepended to the path to the executa=
ble.
> - dpdk_app: ClassVar[bool] =3D False
> -
> def __init__(
> self,
> - interactive_session: SSHClient,
> - logger: DTSLogger,
> - get_privileged_command: Callable[[str], str] | None,
> - app_params: Params =3D Params(),
> + node: Node,
> + privileged: bool =3D False,
> timeout: float =3D SETTINGS.timeout,
> + start_on_init: bool =3D True,
> + app_params: Params =3D Params(),
> ) -> None:
> """Create an SSH channel during initialization.
>
> Args:
> - interactive_session: The SSH session dedicated to interactiv=
e shells.
> - logger: The logger instance this session will use.
> - get_privileged_command: A method for modifying a command to =
allow it to use
> - elevated privileges. If :data:`None`, the application wi=
ll not be started
> - with elevated privileges.
> - app_params: The command line parameters to be passed to the =
application on startup.
> + node: The node on which to run start the interactive shell.
> + privileged: Enables the shell to run as superuser.
> timeout: The timeout used for the SSH channel that is dedica=
ted to this interactive
> shell. This timeout is for collecting output, so if read=
ing from the buffer
> and no output is gathered within the timeout, an excepti=
on is thrown.
> + start_on_init: Start interactive shell automatically after o=
bject initialisation.
> + app_params: The command line parameters to be passed to the =
application on startup.
> """
> - self._interactive_session =3D interactive_session
> - self._ssh_channel =3D self._interactive_session.invoke_shell()
> + self._node =3D node
> + self._logger =3D node._logger
> + self._app_params =3D app_params
> + self._privileged =3D privileged
> + self._timeout =3D timeout
> + # Ensure path is properly formatted for the host
> + self._update_path(self._node.main_session.join_remote_path(self.=
path))
> +
> + self._post_init()
> +
> + if start_on_init:
> + self.start_application()
> +
> + def _post_init(self):
> + """Overridable. Method called after the object init and before a=
pplication start."""
> +
> + def _setup_ssh_channel(self):
> + self._ssh_channel =3D self._node.main_session.interactive_sessio=
n.session.invoke_shell()
> self._stdin =3D self._ssh_channel.makefile_stdin("w")
> self._stdout =3D self._ssh_channel.makefile("r")
> - self._ssh_channel.settimeout(timeout)
> + self._ssh_channel.settimeout(self._timeout)
> self._ssh_channel.set_combine_stderr(True) # combines stdout an=
d stderr streams
> - self._logger =3D logger
> - self._timeout =3D timeout
> - self._app_params =3D app_params
> - self._start_application(get_privileged_command)
>
> - def _start_application(self, get_privileged_command: Callable[[str],=
str] | None) -> None:
> + def _make_start_command(self) -> str:
> + """Makes the command that starts the interactive shell."""
> + start_command =3D f"{self.path} {self._app_params or ''}"
> + if self._privileged:
> + start_command =3D self._node.main_session._get_privileged_co=
mmand(start_command)
> + return start_command
> +
> + 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 f=
or
> starting may look different.
> -
> - Args:
> - get_privileged_command: A function (but could be any callabl=
e) that produces
> - the version of the command with elevated privileges.
> """
> - start_command =3D f"{self.path} {self._app_params}"
> - if get_privileged_command is not None:
> - start_command =3D get_privileged_command(start_command)
> - self.send_command(start_command)
> + self._setup_ssh_channel()
> + self.send_command(self._make_start_command())
>
> def send_command(
> self, command: str, prompt: str | None =3D None, skip_first_line=
: bool =3D False
> @@ -156,3 +165,7 @@ def close(self) -> None:
> def __del__(self) -> None:
> """Make sure the session is properly closed before deleting the =
object."""
> self.close()
> +
> + @classmethod
> + def _update_path(cls, path: PurePath) -> None:
> + cls.path =3D path
> diff --git a/dts/framework/remote_session/python_shell.py b/dts/framework=
/remote_session/python_shell.py
> index ccfd3783e8..953ed100df 100644
> --- a/dts/framework/remote_session/python_shell.py
> +++ b/dts/framework/remote_session/python_shell.py
> @@ -6,9 +6,7 @@
> Typical usage example in a TestSuite::
>
> from framework.remote_session import PythonShell
> - python_shell =3D self.tg_node.create_interactive_shell(
> - PythonShell, timeout=3D5, privileged=3DTrue
> - )
> + python_shell =3D PythonShell(self.tg_node, timeout=3D5, privileged=
=3DTrue)
> python_shell.send_command("print('Hello World')")
> python_shell.close()
> """
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor=
k/remote_session/testpmd_shell.py
> index 82701a9839..8ee6829067 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -7,9 +7,7 @@
>
> Typical usage example in a TestSuite::
>
> - testpmd_shell =3D self.sut_node.create_interactive_shell(
> - TestPmdShell, privileged=3DTrue
> - )
> + testpmd_shell =3D TestPmdShell(self.sut_node)
> devices =3D testpmd_shell.get_devices()
> for device in devices:
> print(device)
> @@ -21,18 +19,19 @@
> from dataclasses import dataclass, field
> from enum import Flag, auto
> from pathlib import PurePath
> -from typing import Callable, ClassVar
> +from typing import ClassVar
>
> from typing_extensions import Self
>
> from framework.exception import InteractiveCommandExecutionError
> from framework.params.testpmd import SimpleForwardingModes, TestPmdParam=
s
> from framework.parser import ParserFn, TextParser
> +from framework.remote_session.dpdk_shell import DPDKShell
> from framework.settings import SETTINGS
> +from framework.testbed_model.cpu import LogicalCoreCount, LogicalCoreLis=
t
> +from framework.testbed_model.sut_node import SutNode
> from framework.utils import StrEnum
>
> -from .interactive_shell import InteractiveShell
> -
>
> class TestPmdDevice(object):
> """The data of a device that testpmd can recognize.
> @@ -577,52 +576,48 @@ class TestPmdPortStats(TextParser):
> tx_bps: int =3D field(metadata=3DTextParser.find_int(r"Tx-bps:\s+(\d=
+)"))
>
>
> -class TestPmdShell(InteractiveShell):
> +class TestPmdShell(DPDKShell):
> """Testpmd interactive shell.
>
> The testpmd shell users should never use
> the :meth:`~.interactive_shell.InteractiveShell.send_command` method=
directly, but rather
> call specialized methods. If there isn't one that satisfies a need, =
it should be added.
> -
> - Attributes:
> - number_of_ports: The number of ports which were allowed on the c=
ommand-line when testpmd
> - was started.
> """
>
> - number_of_ports: int
> + _app_params: TestPmdParams
>
> #: The path to the testpmd executable.
> path: ClassVar[PurePath] =3D PurePath("app", "dpdk-testpmd")
>
> - #: Flag this as a DPDK app so that it's clear this is not a system a=
pp and
> - #: needs to be looked in a specific path.
> - dpdk_app: ClassVar[bool] =3D True
> -
> #: The testpmd's prompt.
> _default_prompt: ClassVar[str] =3D "testpmd>"
>
> #: This forces the prompt to appear after sending a command.
> _command_extra_chars: ClassVar[str] =3D "\n"
>
> - def _start_application(self, get_privileged_command: Callable[[str],=
str] | None) -> None:
> - """Overrides :meth:`~.interactive_shell._start_application`.
> -
> - Add flags for starting testpmd in interactive mode and disabling=
messages for link state
> - change events before starting the application. Link state is ver=
ified before starting
> - packet forwarding and the messages create unexpected newlines in=
the terminal which
> - complicates output collection.
> -
> - Also find the number of pci addresses which were allowed on the =
command line when the app
> - was started.
> - """
> - assert isinstance(self._app_params, TestPmdParams)
> -
> - self.number_of_ports =3D (
> - len(self._app_params.ports) if self._app_params.ports is not=
None else 0
> + def __init__(
> + self,
> + node: SutNode,
> + privileged: bool =3D True,
> + timeout: float =3D SETTINGS.timeout,
> + lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =3D L=
ogicalCoreCount(),
> + ascending_cores: bool =3D True,
> + append_prefix_timestamp: bool =3D True,
> + start_on_init: bool =3D True,
> + **app_params,
> + ) -> None:
> + """Overrides :meth:`~.dpdk_shell.DPDKShell.__init__`. Changes ap=
p_params to kwargs."""
> + super().__init__(
> + node,
> + privileged,
> + timeout,
> + lcore_filter_specifier,
> + ascending_cores,
> + append_prefix_timestamp,
> + start_on_init,
> + TestPmdParams(**app_params),
> )
>
> - super()._start_application(get_privileged_command)
> -
> def start(self, verify: bool =3D True) -> None:
> """Start packet forwarding with the current configuration.
>
> @@ -642,7 +637,8 @@ def start(self, verify: bool =3D True) -> None:
> self._logger.debug(f"Failed to start packet forwarding: =
\n{start_cmd_output}")
> raise InteractiveCommandExecutionError("Testpmd failed t=
o start packet forwarding.")
>
> - for port_id in range(self.number_of_ports):
> + number_of_ports =3D len(self._app_params.ports or [])
> + for port_id in range(number_of_ports):
> if not self.wait_link_status_up(port_id):
> raise InteractiveCommandExecutionError(
> "Not all ports came up after starting packet for=
warding in testpmd."
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_=
model/node.py
> index 6af4f25a3c..88395faabe 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -15,7 +15,7 @@
>
> from abc import ABC
> from ipaddress import IPv4Interface, IPv6Interface
> -from typing import Any, Callable, Type, Union
> +from typing import Any, Callable, Union
>
> from framework.config import (
> OS,
> @@ -25,7 +25,6 @@
> )
> from framework.exception import ConfigurationError
> from framework.logger import DTSLogger, get_dts_logger
> -from framework.params import Params
> from framework.settings import SETTINGS
>
> from .cpu import (
> @@ -36,7 +35,7 @@
> lcore_filter,
> )
> from .linux_session import LinuxSession
> -from .os_session import InteractiveShellType, OSSession
> +from .os_session import OSSession
> from .port import Port
> from .virtual_device import VirtualDevice
>
> @@ -196,37 +195,6 @@ def create_session(self, name: str) -> OSSession:
> self._other_sessions.append(connection)
> return connection
>
> - def create_interactive_shell(
> - self,
> - shell_cls: Type[InteractiveShellType],
> - timeout: float =3D SETTINGS.timeout,
> - privileged: bool =3D False,
> - app_params: Params =3D Params(),
> - ) -> InteractiveShellType:
> - """Factory for interactive session handlers.
> -
> - Instantiate `shell_cls` according to the remote OS specifics.
> -
> - Args:
> - shell_cls: The class of the shell.
> - timeout: Timeout for reading output from the SSH channel. If=
you are reading from
> - the buffer and don't receive any data within the timeout=
it will throw an error.
> - privileged: Whether to run the shell with administrative pri=
vileges.
> - app_args: The arguments to be passed to the application.
> -
> - Returns:
> - An instance of the desired interactive application shell.
> - """
> - if not shell_cls.dpdk_app:
> - shell_cls.path =3D self.main_session.join_remote_path(shell_=
cls.path)
> -
> - return self.main_session.create_interactive_shell(
> - shell_cls,
> - timeout,
> - privileged,
> - app_params,
> - )
> -
> def filter_lcores(
> self,
> filter_specifier: LogicalCoreCount | LogicalCoreList,
> diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/te=
stbed_model/os_session.py
> index e5f5fcbe0e..e7e6c9d670 100644
> --- a/dts/framework/testbed_model/os_session.py
> +++ b/dts/framework/testbed_model/os_session.py
> @@ -26,18 +26,16 @@
> from collections.abc import Iterable
> from ipaddress import IPv4Interface, IPv6Interface
> from pathlib import PurePath
> -from typing import Type, TypeVar, Union
> +from typing import Union
>
> from framework.config import Architecture, NodeConfiguration, NodeInfo
> from framework.logger import DTSLogger
> -from framework.params import Params
> from framework.remote_session import (
> InteractiveRemoteSession,
> RemoteSession,
> create_interactive_session,
> create_remote_session,
> )
> -from framework.remote_session.interactive_shell import InteractiveShell
> from framework.remote_session.remote_session import CommandResult
> from framework.settings import SETTINGS
> from framework.utils import MesonArgs
> @@ -45,8 +43,6 @@
> from .cpu import LogicalCore
> from .port import Port
>
> -InteractiveShellType =3D TypeVar("InteractiveShellType", bound=3DInterac=
tiveShell)
> -
>
> class OSSession(ABC):
> """OS-unaware to OS-aware translation API definition.
> @@ -131,36 +127,6 @@ def send_command(
>
> return self.remote_session.send_command(command, timeout, verify=
, env)
>
> - def create_interactive_shell(
> - self,
> - shell_cls: Type[InteractiveShellType],
> - timeout: float,
> - privileged: bool,
> - app_args: Params,
> - ) -> InteractiveShellType:
> - """Factory for interactive session handlers.
> -
> - Instantiate `shell_cls` according to the remote OS specifics.
> -
> - Args:
> - shell_cls: The class of the shell.
> - timeout: Timeout for reading output from the SSH channel. If=
you are
> - reading from the buffer and don't receive any data withi=
n the timeout
> - it will throw an error.
> - privileged: Whether to run the shell with administrative pri=
vileges.
> - app_args: The arguments to be passed to the application.
> -
> - Returns:
> - An instance of the desired interactive application shell.
> - """
> - return shell_cls(
> - self.interactive_session.session,
> - self._logger,
> - self._get_privileged_command if privileged else None,
> - app_args,
> - timeout,
> - )
> -
> @staticmethod
> @abstractmethod
> def _get_privileged_command(command: str) -> str:
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/test=
bed_model/sut_node.py
> index 83ad06ae2d..d231a01425 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -16,7 +16,6 @@
> import tarfile
> import time
> from pathlib import PurePath
> -from typing import Type
>
> from framework.config import (
> BuildTargetConfiguration,
> @@ -24,17 +23,13 @@
> NodeInfo,
> SutNodeConfiguration,
> )
> -from framework.params import Params, Switch
> from framework.params.eal import EalParams
> from framework.remote_session.remote_session import CommandResult
> from framework.settings import SETTINGS
> from framework.utils import MesonArgs
>
> -from .cpu import LogicalCoreCount, LogicalCoreList
> from .node import Node
> -from .os_session import InteractiveShellType, OSSession
> -from .port import Port
> -from .virtual_device import VirtualDevice
> +from .os_session import OSSession
>
>
> class SutNode(Node):
> @@ -56,8 +51,8 @@ class SutNode(Node):
> """
>
> config: SutNodeConfiguration
> - _dpdk_prefix_list: list[str]
> - _dpdk_timestamp: str
> + dpdk_prefix_list: list[str]
> + dpdk_timestamp: str
> _build_target_config: BuildTargetConfiguration | None
> _env_vars: dict
> _remote_tmp_dir: PurePath
> @@ -76,14 +71,14 @@ def __init__(self, node_config: SutNodeConfiguration)=
:
> node_config: The SUT node's test run configuration.
> """
> super(SutNode, self).__init__(node_config)
> - self._dpdk_prefix_list =3D []
> + self.dpdk_prefix_list =3D []
> self._build_target_config =3D None
> self._env_vars =3D {}
> self._remote_tmp_dir =3D self.main_session.get_remote_tmp_dir()
> self.__remote_dpdk_dir =3D None
> self._app_compile_timeout =3D 90
> self._dpdk_kill_session =3D None
> - self._dpdk_timestamp =3D (
> + self.dpdk_timestamp =3D (
> f"{str(os.getpid())}_{time.strftime('%Y%m%d%H%M%S', time.loc=
altime())}"
> )
> self._dpdk_version =3D None
> @@ -283,73 +278,11 @@ def kill_cleanup_dpdk_apps(self) -> None:
> """Kill all dpdk applications on the SUT, then clean up hugepage=
s."""
> if self._dpdk_kill_session and self._dpdk_kill_session.is_alive(=
):
> # we can use the session if it exists and responds
> - self._dpdk_kill_session.kill_cleanup_dpdk_apps(self._dpdk_pr=
efix_list)
> + self._dpdk_kill_session.kill_cleanup_dpdk_apps(self.dpdk_pre=
fix_list)
> else:
> # otherwise, we need to (re)create it
> self._dpdk_kill_session =3D self.create_session("dpdk_kill")
> - self._dpdk_prefix_list =3D []
> -
> - def create_eal_parameters(
> - self,
> - lcore_filter_specifier: LogicalCoreCount | LogicalCoreList =3D L=
ogicalCoreCount(),
> - ascending_cores: bool =3D True,
> - prefix: str =3D "dpdk",
> - append_prefix_timestamp: bool =3D True,
> - no_pci: Switch =3D None,
> - vdevs: list[VirtualDevice] | None =3D None,
> - ports: list[Port] | None =3D None,
> - other_eal_param: str =3D "",
> - ) -> EalParams:
> - """Compose the EAL parameters.
> -
> - Process the list of cores and the DPDK prefix and pass that alon=
g with
> - the rest of the arguments.
> -
> - Args:
> - 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 hi=
ghest IDs).
> - If :data:`False`, sort in descending order.
> - prefix: Set the file prefix string with which to start DPDK,=
e.g.: ``prefix=3D'vf'``.
> - append_prefix_timestamp: If :data:`True`, will append a time=
stamp to DPDK file prefix.
> - no_pci: Switch to disable PCI bus e.g.: ``no_pci=3DTrue``.
> - vdevs: Virtual devices, e.g.::
> -
> - vdevs=3D[
> - VirtualDevice('net_ring0'),
> - VirtualDevice('net_ring1')
> - ]
> - ports: The list of ports to allow. If :data:`None`, all port=
s listed in `self.ports`
> - will be allowed.
> - other_eal_param: user defined DPDK EAL parameters, e.g.:
> - ``other_eal_param=3D'--single-file-segments'``.
> -
> - Returns:
> - An EAL param string, such as
> - ``-c 0xf -a 0000:88:00.0 --file-prefix=3Ddpdk_1112_201908091=
43420``.
> - """
> - lcore_list =3D LogicalCoreList(self.filter_lcores(lcore_filter_s=
pecifier, ascending_cores))
> -
> - if append_prefix_timestamp:
> - prefix =3D f"{prefix}_{self._dpdk_timestamp}"
> - prefix =3D self.main_session.get_dpdk_file_prefix(prefix)
> - if prefix:
> - self._dpdk_prefix_list.append(prefix)
> -
> - if ports is None:
> - ports =3D self.ports
> -
> - return EalParams(
> - lcore_list=3Dlcore_list,
> - memory_channels=3Dself.config.memory_channels,
> - prefix=3Dprefix,
> - no_pci=3Dno_pci,
> - vdevs=3Dvdevs,
> - ports=3Dports,
> - other_eal_param=3DParams.from_str(other_eal_param),
> - )
> + self.dpdk_prefix_list =3D []
>
> def run_dpdk_app(
> self, app_path: PurePath, eal_params: EalParams, timeout: float =
=3D 30
> @@ -379,49 +312,6 @@ def configure_ipv4_forwarding(self, enable: bool) ->=
None:
> """
> self.main_session.configure_ipv4_forwarding(enable)
>
> - def create_interactive_shell(
> - self,
> - shell_cls: Type[InteractiveShellType],
> - timeout: float =3D SETTINGS.timeout,
> - privileged: bool =3D False,
> - app_params: Params =3D Params(),
> - eal_params: EalParams | None =3D None,
> - ) -> InteractiveShellType:
> - """Extend the factory for interactive session handlers.
> -
> - The extensions are SUT node specific:
> -
> - * The default for `eal_parameters`,
> - * The interactive shell path `shell_cls.path` is prepended w=
ith path to the remote
> - DPDK build directory for DPDK apps.
> -
> - Args:
> - shell_cls: The class of the shell.
> - timeout: Timeout for reading output from the SSH channel. If=
you are
> - reading from the buffer and don't receive any data withi=
n the timeout
> - it will throw an error.
> - privileged: Whether to run the shell with administrative pri=
vileges.
> - app_params: The parameters to be passed to the application.
> - eal_params: List of EAL parameters to use to launch the app.=
If this
> - isn't provided or an empty string is passed, it will def=
ault to calling
> - :meth:`create_eal_parameters`.
> -
> - Returns:
> - An instance of the desired interactive application shell.
> - """
> - # We need to append the build directory and add EAL parameters f=
or DPDK apps
> - if shell_cls.dpdk_app:
> - if eal_params is None:
> - eal_params =3D self.create_eal_parameters()
> - eal_params.append_str(str(app_params))
> - app_params =3D eal_params
> -
> - shell_cls.path =3D self.main_session.join_remote_path(
> - self.remote_dpdk_build_dir, shell_cls.path
> - )
> -
> - return super().create_interactive_shell(shell_cls, timeout, priv=
ileged, app_params)
> -
> def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None:
> """Bind all ports on the SUT to a driver.
>
> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts=
/framework/testbed_model/traffic_generator/scapy.py
> index 7bc1c2cc08..bf58ad1c5e 100644
> --- a/dts/framework/testbed_model/traffic_generator/scapy.py
> +++ b/dts/framework/testbed_model/traffic_generator/scapy.py
> @@ -217,9 +217,7 @@ def __init__(self, tg_node: Node, config: ScapyTraffi=
cGeneratorConfig):
> self._tg_node.config.os =3D=3D OS.linux
> ), "Linux is the only supported OS for scapy traffic generation"
>
> - self.session =3D self._tg_node.create_interactive_shell(
> - PythonShell, timeout=3D5, privileged=3DTrue
> - )
> + self.session =3D PythonShell(self._tg_node, timeout=3D5, privile=
ged=3DTrue)
>
> # import libs in remote python console
> for import_statement in SCAPY_RPC_SERVER_IMPORTS:
> diff --git a/dts/tests/TestSuite_hello_world.py b/dts/tests/TestSuite_hel=
lo_world.py
> index 0d6995f260..d958f99030 100644
> --- a/dts/tests/TestSuite_hello_world.py
> +++ b/dts/tests/TestSuite_hello_world.py
> @@ -7,6 +7,7 @@
> No other EAL parameters apart from cores are used.
> """
>
> +from framework.remote_session.dpdk_shell import compute_eal_params
> from framework.test_suite import TestSuite
> from framework.testbed_model.cpu import (
> LogicalCoreCount,
> @@ -38,7 +39,7 @@ def test_hello_world_single_core(self) -> None:
> # get the first usable core
> lcore_amount =3D LogicalCoreCount(1, 1, 1)
> lcores =3D LogicalCoreCountFilter(self.sut_node.lcores, lcore_am=
ount).filter()
> - eal_para =3D self.sut_node.create_eal_parameters(lcore_filter_sp=
ecifier=3Dlcore_amount)
> + eal_para =3D compute_eal_params(self.sut_node, lcore_filter_spec=
ifier=3Dlcore_amount)
> result =3D self.sut_node.run_dpdk_app(self.app_helloworld_path, =
eal_para)
> self.verify(
> f"hello from core {int(lcores[0])}" in result.stdout,
> @@ -55,8 +56,8 @@ def test_hello_world_all_cores(self) -> None:
> "hello from core <core_id>"
> """
> # get the maximum logical core number
> - eal_para =3D self.sut_node.create_eal_parameters(
> - lcore_filter_specifier=3DLogicalCoreList(self.sut_node.lcore=
s)
> + eal_para =3D compute_eal_params(
> + self.sut_node, lcore_filter_specifier=3DLogicalCoreList(self=
.sut_node.lcores)
> )
> result =3D self.sut_node.run_dpdk_app(self.app_helloworld_path, =
eal_para, 50)
> for lcore in self.sut_node.lcores:
> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSu=
ite_pmd_buffer_scatter.py
> index 6d206c1a40..43cf5c61eb 100644
> --- a/dts/tests/TestSuite_pmd_buffer_scatter.py
> +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
> @@ -16,14 +16,13 @@
> """
>
> import struct
> -from dataclasses import asdict
>
> from scapy.layers.inet import IP # type: ignore[import-untyped]
> from scapy.layers.l2 import Ether # type: ignore[import-untyped]
> from scapy.packet import Raw # type: ignore[import-untyped]
> from scapy.utils import hexstr # type: ignore[import-untyped]
>
> -from framework.params.testpmd import SimpleForwardingModes, TestPmdParam=
s
> +from framework.params.testpmd import SimpleForwardingModes
> from framework.remote_session.testpmd_shell import TestPmdShell
> from framework.test_suite import TestSuite
>
> @@ -103,17 +102,13 @@ def pmd_scatter(self, mbsize: int) -> None:
> Test:
> Start testpmd and run functional test with preset mbsize.
> """
> - testpmd =3D self.sut_node.create_interactive_shell(
> - TestPmdShell,
> - app_params=3DTestPmdParams(
> - forward_mode=3DSimpleForwardingModes.mac,
> - mbcache=3D200,
> - mbuf_size=3D[mbsize],
> - max_pkt_len=3D9000,
> - tx_offloads=3D0x00008000,
> - **asdict(self.sut_node.create_eal_parameters()),
> - ),
> - privileged=3DTrue,
> + testpmd =3D TestPmdShell(
> + self.sut_node,
> + forward_mode=3DSimpleForwardingModes.mac,
> + mbcache=3D200,
> + mbuf_size=3D[mbsize],
> + max_pkt_len=3D9000,
> + tx_offloads=3D0x00008000,
> )
> testpmd.start()
>
> diff --git a/dts/tests/TestSuite_smoke_tests.py b/dts/tests/TestSuite_smo=
ke_tests.py
> index ca678f662d..eca27acfd8 100644
> --- a/dts/tests/TestSuite_smoke_tests.py
> +++ b/dts/tests/TestSuite_smoke_tests.py
> @@ -99,7 +99,7 @@ def test_devices_listed_in_testpmd(self) -> None:
> Test:
> List all devices found in testpmd and verify the configured =
devices are among them.
> """
> - testpmd_driver =3D self.sut_node.create_interactive_shell(TestPm=
dShell, privileged=3DTrue)
> + testpmd_driver =3D TestPmdShell(self.sut_node)
> dev_list =3D [str(x) for x in testpmd_driver.get_devices()]
> for nic in self.nics_in_node:
> self.verify(
> --
> 2.34.1
>