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 9C1FE45459; Wed, 19 Jun 2024 12:25:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1BE1F427DE; Wed, 19 Jun 2024 12:24:55 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id C991640B99 for ; Wed, 19 Jun 2024 12:24:52 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0A1C9DA7; Wed, 19 Jun 2024 03:25:17 -0700 (PDT) Received: from localhost.localdomain (FVFG51LCQ05N.cambridge.arm.com [10.1.35.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 745873F64C; Wed, 19 Jun 2024 03:24:51 -0700 (PDT) From: Luca Vizzarro To: dev@dpdk.org Cc: Jeremy Spewock , =?UTF-8?q?Juraj=20Linke=C5=A1?= , Luca Vizzarro , Paul Szczepanek , Nicholas Pratte Subject: [PATCH v6 2/8] dts: use Params for interactive shells Date: Wed, 19 Jun 2024 11:23:23 +0100 Message-Id: <20240619102329.250263-3-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240619102329.250263-1-luca.vizzarro@arm.com> References: <20240326190422.577028-1-luca.vizzarro@arm.com> <20240619102329.250263-1-luca.vizzarro@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Make it so that interactive shells accept an implementation of `Params` for app arguments. Convert EalParameters to use `Params` instead. String command line parameters can still be supplied by using the `Params.from_str()` method. Signed-off-by: Luca Vizzarro Reviewed-by: Paul Szczepanek Reviewed-by: Juraj Linkeš Reviewed-by: Jeremy Spewock Reviewed-by: Nicholas Pratte --- .../remote_session/interactive_shell.py | 12 +- dts/framework/remote_session/testpmd_shell.py | 11 +- dts/framework/testbed_model/node.py | 6 +- dts/framework/testbed_model/os_session.py | 4 +- dts/framework/testbed_model/sut_node.py | 124 ++++++++---------- dts/tests/TestSuite_pmd_buffer_scatter.py | 3 +- 6 files changed, 77 insertions(+), 83 deletions(-) diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py index c025c52ba3..8191b36630 100644 --- a/dts/framework/remote_session/interactive_shell.py +++ b/dts/framework/remote_session/interactive_shell.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2023 University of New Hampshire +# Copyright(c) 2024 Arm Limited """Common functionality for interactive shell handling. @@ -21,6 +22,7 @@ from paramiko import Channel, SSHClient, channel # type: ignore[import-untyped] from framework.logger import DTSLogger +from framework.params import Params from framework.settings import SETTINGS @@ -40,7 +42,7 @@ class InteractiveShell(ABC): _ssh_channel: Channel _logger: DTSLogger _timeout: float - _app_args: str + _app_params: Params #: Prompt to expect at the end of output when sending a command. #: This is often overridden by subclasses. @@ -63,7 +65,7 @@ def __init__( interactive_session: SSHClient, logger: DTSLogger, get_privileged_command: Callable[[str], str] | None, - app_args: str = "", + app_params: Params = Params(), timeout: float = SETTINGS.timeout, ) -> None: """Create an SSH channel during initialization. @@ -74,7 +76,7 @@ def __init__( get_privileged_command: A method for modifying a command to allow it to use elevated privileges. If :data:`None`, the application will not be started with elevated privileges. - app_args: The command line arguments to be passed to the application on startup. + app_params: The command line parameters to be passed to the application on startup. timeout: The timeout used for the SSH channel that is dedicated to this interactive shell. This timeout is for collecting output, so if reading from the buffer and no output is gathered within the timeout, an exception is thrown. @@ -87,7 +89,7 @@ def __init__( self._ssh_channel.set_combine_stderr(True) # combines stdout and stderr streams self._logger = logger self._timeout = timeout - self._app_args = app_args + self._app_params = app_params self._start_application(get_privileged_command) def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None: @@ -100,7 +102,7 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None 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_args}" + start_command = f"{self.path} {self._app_params}" if get_privileged_command is not None: start_command = get_privileged_command(start_command) self.send_command(start_command) diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py index d413bf2cc7..2836ed5c48 100644 --- a/dts/framework/remote_session/testpmd_shell.py +++ b/dts/framework/remote_session/testpmd_shell.py @@ -28,6 +28,7 @@ from framework.exception import InteractiveCommandExecutionError from framework.parser import ParserFn, TextParser from framework.settings import SETTINGS +from framework.testbed_model.sut_node import EalParams from framework.utils import StrEnum from .interactive_shell import InteractiveShell @@ -645,8 +646,14 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None Also find the number of pci addresses which were allowed on the command line when the app was started. """ - self._app_args += " -i --mask-event intr_lsc" - self.number_of_ports = self._app_args.count("-a ") + self._app_params += " -i --mask-event intr_lsc" + + assert isinstance(self._app_params, EalParams) + + self.number_of_ports = ( + len(self._app_params.ports) if self._app_params.ports is not None else 0 + ) + super()._start_application(get_privileged_command) def start(self, verify: bool = True) -> None: diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py index 74061f6262..6af4f25a3c 100644 --- a/dts/framework/testbed_model/node.py +++ b/dts/framework/testbed_model/node.py @@ -2,6 +2,7 @@ # Copyright(c) 2010-2014 Intel Corporation # Copyright(c) 2022-2023 PANTHEON.tech s.r.o. # Copyright(c) 2022-2023 University of New Hampshire +# Copyright(c) 2024 Arm Limited """Common functionality for node management. @@ -24,6 +25,7 @@ ) 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 ( @@ -199,7 +201,7 @@ def create_interactive_shell( shell_cls: Type[InteractiveShellType], timeout: float = SETTINGS.timeout, privileged: bool = False, - app_args: str = "", + app_params: Params = Params(), ) -> InteractiveShellType: """Factory for interactive session handlers. @@ -222,7 +224,7 @@ def create_interactive_shell( shell_cls, timeout, privileged, - app_args, + app_params, ) def filter_lcores( diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py index d5bf7e0401..1a77aee532 100644 --- a/dts/framework/testbed_model/os_session.py +++ b/dts/framework/testbed_model/os_session.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2023 PANTHEON.tech s.r.o. # Copyright(c) 2023 University of New Hampshire +# Copyright(c) 2024 Arm Limited """OS-aware remote session. @@ -29,6 +30,7 @@ from framework.config import Architecture, NodeConfiguration, NodeInfo from framework.logger import DTSLogger +from framework.params import Params from framework.remote_session import ( CommandResult, InteractiveRemoteSession, @@ -134,7 +136,7 @@ def create_interactive_shell( shell_cls: Type[InteractiveShellType], timeout: float, privileged: bool, - app_args: str, + app_args: Params, ) -> InteractiveShellType: """Factory for interactive session handlers. diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py index 97aa26d419..c886590979 100644 --- a/dts/framework/testbed_model/sut_node.py +++ b/dts/framework/testbed_model/sut_node.py @@ -2,6 +2,7 @@ # Copyright(c) 2010-2014 Intel Corporation # Copyright(c) 2023 PANTHEON.tech s.r.o. # Copyright(c) 2023 University of New Hampshire +# Copyright(c) 2024 Arm Limited """System under test (DPDK + hardware) node. @@ -14,8 +15,9 @@ import os import tarfile import time +from dataclasses import dataclass, field from pathlib import PurePath -from typing import Type +from typing import Literal, Type from framework.config import ( BuildTargetConfiguration, @@ -23,6 +25,7 @@ NodeInfo, SutNodeConfiguration, ) +from framework.params import Params, Switch from framework.remote_session import CommandResult from framework.settings import SETTINGS from framework.utils import MesonArgs @@ -34,62 +37,42 @@ from .virtual_device import VirtualDevice -class EalParameters(object): - """The environment abstraction layer parameters. - - The string representation can be created by converting the instance to a string. - """ +def _port_to_pci(port: Port) -> str: + return port.pci - def __init__( - self, - lcore_list: LogicalCoreList, - memory_channels: int, - prefix: str, - no_pci: bool, - vdevs: list[VirtualDevice], - ports: list[Port], - other_eal_param: str, - ): - """Initialize the parameters according to inputs. - - Process the parameters into the format used on the command line. - Args: - lcore_list: The list of logical cores to use. - memory_channels: The number of memory channels to use. - prefix: Set the file prefix string with which to start DPDK, e.g.: ``prefix='vf'``. - no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``. - vdevs: Virtual devices, e.g.:: +@dataclass(kw_only=True) +class EalParams(Params): + """The environment abstraction layer parameters. - vdevs=[ - VirtualDevice('net_ring0'), - VirtualDevice('net_ring1') - ] - ports: The list of ports to allow. - other_eal_param: user defined DPDK EAL parameters, e.g.: + Attributes: + lcore_list: The list of logical cores to use. + memory_channels: The number of memory channels to use. + prefix: Set the file prefix string with which to start DPDK, e.g.: ``prefix="vf"``. + no_pci: Switch to disable PCI bus, e.g.: ``no_pci=True``. + vdevs: Virtual devices, e.g.:: + vdevs=[ + VirtualDevice('net_ring0'), + VirtualDevice('net_ring1') + ] + ports: The list of ports to allow. + other_eal_param: user defined DPDK EAL parameters, e.g.: ``other_eal_param='--single-file-segments'`` - """ - self._lcore_list = f"-l {lcore_list}" - self._memory_channels = f"-n {memory_channels}" - self._prefix = prefix - if prefix: - self._prefix = f"--file-prefix={prefix}" - self._no_pci = "--no-pci" if no_pci else "" - self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs) - self._ports = " ".join(f"-a {port.pci}" for port in ports) - self._other_eal_param = other_eal_param - - def __str__(self) -> str: - """Create the EAL string.""" - return ( - f"{self._lcore_list} " - f"{self._memory_channels} " - f"{self._prefix} " - f"{self._no_pci} " - f"{self._vdevs} " - f"{self._ports} " - f"{self._other_eal_param}" - ) + """ + + lcore_list: LogicalCoreList = field(metadata=Params.short("l")) + memory_channels: int = field(metadata=Params.short("n")) + prefix: str = field(metadata=Params.long("file-prefix")) + no_pci: Switch + vdevs: list[VirtualDevice] | None = field( + default=None, metadata=Params.multiple() | Params.long("vdev") + ) + ports: list[Port] | None = field( + default=None, + metadata=Params.convert_value(_port_to_pci) | Params.multiple() | Params.short("a"), + ) + other_eal_param: Params | None = None + _separator: Literal[True] = field(default=True, init=False, metadata=Params.short("-")) class SutNode(Node): @@ -350,11 +333,11 @@ def create_eal_parameters( ascending_cores: bool = True, prefix: str = "dpdk", append_prefix_timestamp: bool = True, - no_pci: bool = False, + no_pci: Switch = None, vdevs: list[VirtualDevice] | None = None, ports: list[Port] | None = None, other_eal_param: str = "", - ) -> "EalParameters": + ) -> EalParams: """Compose the EAL parameters. Process the list of cores and the DPDK prefix and pass that along with @@ -393,24 +376,21 @@ def create_eal_parameters( if prefix: self._dpdk_prefix_list.append(prefix) - if vdevs is None: - vdevs = [] - if ports is None: ports = self.ports - return EalParameters( + return EalParams( lcore_list=lcore_list, memory_channels=self.config.memory_channels, prefix=prefix, no_pci=no_pci, vdevs=vdevs, ports=ports, - other_eal_param=other_eal_param, + other_eal_param=Params.from_str(other_eal_param), ) def run_dpdk_app( - self, app_path: PurePath, eal_args: "EalParameters", timeout: float = 30 + self, app_path: PurePath, eal_params: EalParams, timeout: float = 30 ) -> CommandResult: """Run DPDK application on the remote node. @@ -419,14 +399,14 @@ def run_dpdk_app( Args: app_path: The remote path to the DPDK application. - eal_args: EAL parameters to run the DPDK application with. + eal_params: EAL parameters to run the DPDK application with. timeout: Wait at most this long in seconds for `command` execution to complete. Returns: The result of the DPDK app execution. """ return self.main_session.send_command( - f"{app_path} {eal_args}", timeout, privileged=True, verify=True + f"{app_path} {eal_params}", timeout, privileged=True, verify=True ) def configure_ipv4_forwarding(self, enable: bool) -> None: @@ -442,8 +422,8 @@ def create_interactive_shell( shell_cls: Type[InteractiveShellType], timeout: float = SETTINGS.timeout, privileged: bool = False, - app_parameters: str = "", - eal_parameters: EalParameters | None = None, + app_params: Params = Params(), + eal_params: EalParams | None = None, ) -> InteractiveShellType: """Extend the factory for interactive session handlers. @@ -459,26 +439,26 @@ def create_interactive_shell( 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 privileges. - eal_parameters: List of EAL parameters to use to launch the app. If this + 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 default to calling :meth:`create_eal_parameters`. - app_parameters: Additional arguments to pass into the application on the - command-line. Returns: An instance of the desired interactive application shell. """ # We need to append the build directory and add EAL parameters for DPDK apps if shell_cls.dpdk_app: - if not eal_parameters: - eal_parameters = self.create_eal_parameters() - app_parameters = f"{eal_parameters} -- {app_parameters}" + if eal_params is None: + eal_params = self.create_eal_parameters() + eal_params.append_str(str(app_params)) + app_params = eal_params shell_cls.path = self.main_session.join_remote_path( self.remote_dpdk_build_dir, shell_cls.path ) - return super().create_interactive_shell(shell_cls, timeout, privileged, app_parameters) + return super().create_interactive_shell(shell_cls, timeout, privileged, app_params) def bind_ports_to_driver(self, for_dpdk: bool = True) -> None: """Bind all ports on the SUT to a driver. diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py index a020682e8d..c6e93839cb 100644 --- a/dts/tests/TestSuite_pmd_buffer_scatter.py +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py @@ -22,6 +22,7 @@ from scapy.packet import Raw # type: ignore[import-untyped] from scapy.utils import hexstr # type: ignore[import-untyped] +from framework.params import Params from framework.remote_session.testpmd_shell import TestPmdForwardingModes, TestPmdShell from framework.test_suite import TestSuite @@ -103,7 +104,7 @@ def pmd_scatter(self, mbsize: int) -> None: """ testpmd = self.sut_node.create_interactive_shell( TestPmdShell, - app_parameters=( + app_params=Params.from_str( "--mbcache=200 " f"--mbuf-size={mbsize} " "--max-pkt-len=9000 " -- 2.34.1