From patchwork Tue Mar 26 19:04:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Vizzarro X-Patchwork-Id: 138828 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 034C043D5B; Tue, 26 Mar 2024 20:04:48 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0EAB0410E6; Tue, 26 Mar 2024 20:04:36 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 46028410D3 for ; Tue, 26 Mar 2024 20:04:34 +0100 (CET) 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 6E5952F4; Tue, 26 Mar 2024 12:05:07 -0700 (PDT) Received: from localhost.localdomain (unknown [10.57.16.115]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id AB0643F64C; Tue, 26 Mar 2024 12:04:32 -0700 (PDT) From: Luca Vizzarro To: dev@dpdk.org Cc: =?utf-8?q?Juraj_Linke=C5=A1?= , Luca Vizzarro , Jack Bond-Preston , Honnappa Nagarahalli Subject: [PATCH 2/6] dts: use Params for interactive shells Date: Tue, 26 Mar 2024 19:04:18 +0000 Message-Id: <20240326190422.577028-3-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240326190422.577028-1-luca.vizzarro@arm.com> References: <20240326190422.577028-1-luca.vizzarro@arm.com> MIME-Version: 1.0 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 `StrParams` implementation. Signed-off-by: Luca Vizzarro Reviewed-by: Jack Bond-Preston Reviewed-by: Honnappa Nagarahalli --- .../remote_session/interactive_shell.py | 8 +- dts/framework/remote_session/testpmd_shell.py | 12 +- dts/framework/testbed_model/__init__.py | 2 +- dts/framework/testbed_model/node.py | 4 +- dts/framework/testbed_model/os_session.py | 4 +- dts/framework/testbed_model/sut_node.py | 106 ++++++++---------- dts/tests/TestSuite_pmd_buffer_scatter.py | 3 +- 7 files changed, 73 insertions(+), 66 deletions(-) diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py index 5cfe202e15..a2c7b30d9f 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] 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_args: Params | None #: 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_args: Params | None = None, timeout: float = SETTINGS.timeout, ) -> None: """Create an SSH channel during initialization. @@ -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_args or ''}" 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 cb2ab6bd00..db3abb7600 100644 --- a/dts/framework/remote_session/testpmd_shell.py +++ b/dts/framework/remote_session/testpmd_shell.py @@ -21,6 +21,7 @@ from typing import Callable, ClassVar from framework.exception import InteractiveCommandExecutionError +from framework.params import StrParams from framework.settings import SETTINGS from framework.utils import StrEnum @@ -118,8 +119,15 @@ 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 ") + from framework.testbed_model.sut_node import EalParameters + + assert isinstance(self._app_args, EalParameters) + + if isinstance(self._app_args.app_params, StrParams): + self._app_args.app_params.value += " -i --mask-event intr_lsc" + + self.number_of_ports = len(self._app_args.ports) if self._app_args.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/__init__.py b/dts/framework/testbed_model/__init__.py index 6086512ca2..ef9520df4c 100644 --- a/dts/framework/testbed_model/__init__.py +++ b/dts/framework/testbed_model/__init__.py @@ -23,6 +23,6 @@ from .cpu import LogicalCoreCount, LogicalCoreCountFilter, LogicalCoreList from .node import Node from .port import Port, PortLink -from .sut_node import SutNode +from .sut_node import SutNode, EalParameters from .tg_node import TGNode from .virtual_device import VirtualDevice diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py index 74061f6262..ec9512d618 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_args: Params | None = None, ) -> InteractiveShellType: """Factory for interactive session handlers. diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py index d5bf7e0401..7234c975c8 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 | None, ) -> 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..3f8c3807b3 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. @@ -11,6 +12,7 @@ """ +from dataclasses import dataclass, field import os import tarfile import time @@ -23,6 +25,8 @@ NodeInfo, SutNodeConfiguration, ) +from framework import params +from framework.params import Params, StrParams from framework.remote_session import CommandResult from framework.settings import SETTINGS from framework.utils import MesonArgs @@ -34,62 +38,51 @@ from .virtual_device import VirtualDevice -class EalParameters(object): +def _port_to_pci(port: Port) -> str: + return port.pci + + +@dataclass(kw_only=True) +class EalParameters(Params): """The environment abstraction layer parameters. The string representation can be created by converting the instance to a string. """ - 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. + lcore_list: LogicalCoreList = field(metadata=params.short("l")) + """The list of logical cores to use.""" - 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.:: + memory_channels: int = field(metadata=params.short("n")) + """The number of memory channels to use.""" - 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}" - ) + prefix: str = field(metadata=params.long("file-prefix")) + """Set the file prefix string with which to start DPDK, e.g.: ``prefix="vf"``.""" + + no_pci: params.Option + """Switch to disable PCI bus e.g.: ``no_pci=True``.""" + + vdevs: list[VirtualDevice] | None = field( + default=None, metadata=params.multiple(params.long("vdev")) + ) + """Virtual devices, e.g.:: + + vdevs=[ + VirtualDevice("net_ring0"), + VirtualDevice("net_ring1") + ] + """ + + ports: list[Port] | None = field( + default=None, + metadata=params.field_mixins(_port_to_pci, metadata=params.multiple(params.short("a"))), + ) + """The list of ports to allow.""" + + other_eal_param: StrParams | None = None + """Any other EAL parameter(s).""" + + app_params: Params | None = field(default=None, metadata=params.options_end()) + """Parameters to pass to the underlying DPDK app.""" class SutNode(Node): @@ -350,7 +343,7 @@ def create_eal_parameters( ascending_cores: bool = True, prefix: str = "dpdk", append_prefix_timestamp: bool = True, - no_pci: bool = False, + no_pci: params.Option = None, vdevs: list[VirtualDevice] | None = None, ports: list[Port] | None = None, other_eal_param: str = "", @@ -393,9 +386,6 @@ def create_eal_parameters( if prefix: self._dpdk_prefix_list.append(prefix) - if vdevs is None: - vdevs = [] - if ports is None: ports = self.ports @@ -406,7 +396,7 @@ def create_eal_parameters( no_pci=no_pci, vdevs=vdevs, ports=ports, - other_eal_param=other_eal_param, + other_eal_param=StrParams(other_eal_param), ) def run_dpdk_app( @@ -442,7 +432,7 @@ def create_interactive_shell( shell_cls: Type[InteractiveShellType], timeout: float = SETTINGS.timeout, privileged: bool = False, - app_parameters: str = "", + app_parameters: Params | None = None, eal_parameters: EalParameters | None = None, ) -> InteractiveShellType: """Extend the factory for interactive session handlers. @@ -459,6 +449,7 @@ 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. + app_args: The arguments to be passed to the application. eal_parameters: 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`. @@ -470,9 +461,10 @@ def create_interactive_shell( """ # We need to append the build directory and add EAL parameters for DPDK apps if shell_cls.dpdk_app: - if not eal_parameters: + if eal_parameters is None: eal_parameters = self.create_eal_parameters() - app_parameters = f"{eal_parameters} -- {app_parameters}" + eal_parameters.app_params = app_parameters + app_parameters = eal_parameters shell_cls.path = self.main_session.join_remote_path( self.remote_dpdk_build_dir, shell_cls.path diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py index 3701c47408..4cdbdc4272 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] from scapy.utils import hexstr # type: ignore[import] +from framework.params import StrParams 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_parameters=StrParams( "--mbcache=200 " f"--mbuf-size={mbsize} " "--max-pkt-len=9000 "