From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Mon, 17 Jun 2024 17:25:19 +0200 (CEST) Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2ebdd684ef6so5326491fa.1 for ; 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 Date: Mon, 17 Jun 2024 11:25:06 -0400 Message-ID: Subject: Re: [PATCH v5 7/8] dts: rework interactive shells To: Luca Vizzarro Cc: dev@dpdk.org, Jeremy Spewock , =?UTF-8?Q?Juraj_Linke=C5=A1?= , Paul Szczepanek 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Tested-by: Nicholas Pratte Reviewed-by: Nicholas Pratte On Mon, Jun 17, 2024 at 10:54=E2=80=AFAM Luca Vizzarro 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 > Reviewed-by: Paul Szczepanek > --- > 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 " > """ > # 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 >