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 464BA440EC; Tue, 28 May 2024 17:44:05 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 02F35402E8; Tue, 28 May 2024 17:44:05 +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 0F6A4402E4 for ; Tue, 28 May 2024 17:44:04 +0200 (CEST) Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2e95ab5a45dso786801fa.3 for ; Tue, 28 May 2024 08:44:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1716911043; x=1717515843; 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=sz87nv8Wqy+X0B12PkAvzIe3/4XbIjZE2wzMmtUx5HA=; b=eEW1z/oKC0F6t+Z0OAQ48tog7wxs+YcRMTNVqTbu/8eqwSpMmvrCnFSEQf1ZzoFatJ kuUw78bg57ggOKzr8s6/NDV5+pwgw7ojLQjintJtPpoqGhABUR1R2whtEa6SQML10t0T sdNhxf3sbD8UwrrbM+h62JclDd4rS7nAAQFUM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716911043; x=1717515843; 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=sz87nv8Wqy+X0B12PkAvzIe3/4XbIjZE2wzMmtUx5HA=; b=OHbuk6LN5U5XcUDTz5RetN7FuJdsbJgQcV4RRV3YAr+ITOi+cwa2Qa4j1cqFlCfoHk r0YFIrIrCEkKNS6dFXPsnerYss+ByodHUlzMzdT/5L0ua+wDFZixSctPYGJEz9MB7A8K HGpTbpRfaKfobJtGBp2FzIbT0DMtVRdRXLOMWvjI8K16NM6cJ18d0QSnE9t0AviU+tfX EyH6USNGNvj66LwPzfbfOhhmCH58gVqdExhVD+PQGQf10NhYBi2Xm5u+CiAaiANatn6O unexbQxxJZ904LHuZgnoQioTGbOTzTeY1W8pNNtrQHxSbqdaD7UZ0s0oOy6916bMyxCL RFQg== X-Gm-Message-State: AOJu0YxmqHRV9rP5hIHZI96G66UEF86hU7ASOXfczJpGBzg5SvyHv1go reST9amdd36RPc/6cVqeckwNyS/XH6jhNE3V/1EvtbSiEQZjSfqH/m2mgG6/QnPy0Os61hEdLCX LcpjwVWEiI4VjndDKV/mTgcObNaJoPRDWzqrSGQ== X-Google-Smtp-Source: AGHT+IHkBQcaTH2yib+ZK8nlwudITKlRrD/sRin9IciE4prkfDujSEMinHmnjb5YQ0Sb2DRrnrYkucYiQn+sxXWqmcU= X-Received: by 2002:a2e:b88d:0:b0:2e5:2aa1:a76a with SMTP id 38308e7fff4ca-2e95b30d8c3mr94873231fa.5.1716911043267; Tue, 28 May 2024 08:44:03 -0700 (PDT) MIME-Version: 1.0 References: <20240326190422.577028-1-luca.vizzarro@arm.com> <20240326190422.577028-3-luca.vizzarro@arm.com> In-Reply-To: <20240326190422.577028-3-luca.vizzarro@arm.com> From: Nicholas Pratte Date: Tue, 28 May 2024 11:43:52 -0400 Message-ID: Subject: Re: [PATCH 2/6] dts: use Params for interactive shells To: Luca Vizzarro Cc: dev@dpdk.org, =?UTF-8?Q?Juraj_Linke=C5=A1?= , Jack Bond-Preston , Honnappa Nagarahalli 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 Tue, Mar 26, 2024 at 3:04=E2=80=AFPM Luca Vizzarro wrote: > > 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/fram= ework/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 =3D "", > + app_args: Params | None =3D None, > timeout: float =3D 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 callabl= e) that produces > the version of the command with elevated privileges. > """ > - start_command =3D f"{self.path} {self._app_args}" > + start_command =3D f"{self.path} {self._app_args or ''}" > if get_privileged_command is not None: > start_command =3D get_privileged_command(start_command) > self.send_command(start_command) > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor= k/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 +=3D " -i --mask-event intr_lsc" > - self.number_of_ports =3D 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 +=3D " -i --mask-event intr_= lsc" > + > + self.number_of_ports =3D 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 =3D True) -> None: > diff --git a/dts/framework/testbed_model/__init__.py b/dts/framework/test= bed_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, LogicalCoreLi= st > 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 =3D SETTINGS.timeout, > privileged: bool =3D False, > - app_args: str =3D "", > + app_args: Params | None =3D None, > ) -> InteractiveShellType: > """Factory for interactive session handlers. > > diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/te= stbed_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/test= bed_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=3DTrue) > +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 =3D field(metadata=3Dparams.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=3D'vf'``. > - no_pci: Switch to disable PCI bus e.g.: ``no_pci=3DTrue``. > - vdevs: Virtual devices, e.g.:: > + memory_channels: int =3D field(metadata=3Dparams.short("n")) > + """The number of memory channels to use.""" > > - vdevs=3D[ > - 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=3D'--single-file-segments'`` > - """ > - self._lcore_list =3D f"-l {lcore_list}" > - self._memory_channels =3D f"-n {memory_channels}" > - self._prefix =3D prefix > - if prefix: > - self._prefix =3D f"--file-prefix=3D{prefix}" > - self._no_pci =3D "--no-pci" if no_pci else "" > - self._vdevs =3D " ".join(f"--vdev {vdev}" for vdev in vdevs) > - self._ports =3D " ".join(f"-a {port.pci}" for port in ports) > - self._other_eal_param =3D 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 =3D field(metadata=3Dparams.long("file-prefix")) > + """Set the file prefix string with which to start DPDK, e.g.: ``pref= ix=3D"vf"``.""" > + > + no_pci: params.Option > + """Switch to disable PCI bus e.g.: ``no_pci=3DTrue``.""" > + > + vdevs: list[VirtualDevice] | None =3D field( > + default=3DNone, metadata=3Dparams.multiple(params.long("vdev")) > + ) > + """Virtual devices, e.g.:: > + > + vdevs=3D[ > + VirtualDevice("net_ring0"), > + VirtualDevice("net_ring1") > + ] > + """ > + > + ports: list[Port] | None =3D field( > + default=3DNone, > + metadata=3Dparams.field_mixins(_port_to_pci, metadata=3Dparams.m= ultiple(params.short("a"))), > + ) > + """The list of ports to allow.""" > + > + other_eal_param: StrParams | None =3D None > + """Any other EAL parameter(s).""" > + > + app_params: Params | None =3D field(default=3DNone, metadata=3Dparam= s.options_end()) > + """Parameters to pass to the underlying DPDK app.""" > > > class SutNode(Node): > @@ -350,7 +343,7 @@ def create_eal_parameters( > ascending_cores: bool =3D True, > prefix: str =3D "dpdk", > append_prefix_timestamp: bool =3D True, > - no_pci: bool =3D False, > + no_pci: params.Option =3D None, > vdevs: list[VirtualDevice] | None =3D None, > ports: list[Port] | None =3D None, > other_eal_param: str =3D "", > @@ -393,9 +386,6 @@ def create_eal_parameters( > if prefix: > self._dpdk_prefix_list.append(prefix) > > - if vdevs is None: > - vdevs =3D [] > - > if ports is None: > ports =3D self.ports > > @@ -406,7 +396,7 @@ def create_eal_parameters( > no_pci=3Dno_pci, > vdevs=3Dvdevs, > ports=3Dports, > - other_eal_param=3Dother_eal_param, > + other_eal_param=3DStrParams(other_eal_param), > ) > > def run_dpdk_app( > @@ -442,7 +432,7 @@ def create_interactive_shell( > shell_cls: Type[InteractiveShellType], > timeout: float =3D SETTINGS.timeout, > privileged: bool =3D False, > - app_parameters: str =3D "", > + app_parameters: Params | None =3D None, > eal_parameters: EalParameters | None =3D 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 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. > 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 def= ault 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 f= or DPDK apps > if shell_cls.dpdk_app: > - if not eal_parameters: > + if eal_parameters is None: > eal_parameters =3D self.create_eal_parameters() > - app_parameters =3D f"{eal_parameters} -- {app_parameters}" > + eal_parameters.app_params =3D app_parameters > + app_parameters =3D eal_parameters > > shell_cls.path =3D 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/TestSu= ite_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 TestPmdForwardingMode= s, TestPmdShell > from framework.test_suite import TestSuite > > @@ -103,7 +104,7 @@ def pmd_scatter(self, mbsize: int) -> None: > """ > testpmd =3D self.sut_node.create_interactive_shell( > TestPmdShell, > - app_parameters=3D( > + app_parameters=3DStrParams( > "--mbcache=3D200 " > f"--mbuf-size=3D{mbsize} " > "--max-pkt-len=3D9000 " > -- > 2.34.1 >