Tested-by: Nicholas Pratte <npra...@iol.unh.edu> Reviewed-by: Nicholas Pratte <npra...@iol.unh.edu>
On Tue, Mar 26, 2024 at 3:04 PM Luca Vizzarro <luca.vizza...@arm.com> 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 <luca.vizza...@arm.com> > Reviewed-by: Jack Bond-Preston <jack.bond-pres...@arm.com> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> > --- > .../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 " > -- > 2.34.1 >