From patchwork Thu Jun 13 18:15:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jeremy Spewock X-Patchwork-Id: 141126 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 57FBF4541F; Thu, 13 Jun 2024 20:15:49 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4581D410F9; Thu, 13 Jun 2024 20:15:49 +0200 (CEST) Received: from mail-io1-f97.google.com (mail-io1-f97.google.com [209.85.166.97]) by mails.dpdk.org (Postfix) with ESMTP id 2C634410F9 for ; Thu, 13 Jun 2024 20:15:48 +0200 (CEST) Received: by mail-io1-f97.google.com with SMTP id ca18e2360f4ac-7ea7bcc72caso51442039f.0 for ; Thu, 13 Jun 2024 11:15:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718302547; x=1718907347; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=sgYogJ/6V0PoGvHvPVF6plskM2WtRi1zHugAykgt6uI=; b=RZO4DN8BNCDQKq5bjjDOlBAhVN6Wl3pt7cusR+R1b0HaAehk+sN4BbUCZOoC0ZzqQX YEYLL8nLiDpbaM2Zb5U1iwzS/rrYx1h636onZfBNrgbBJRR4CxIH2IMj78t1SknkZtwV VGEKhs8Nqxgf0hWWsHiH6QLpmxQ6yxCDohGv4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718302547; x=1718907347; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=sgYogJ/6V0PoGvHvPVF6plskM2WtRi1zHugAykgt6uI=; b=Ga0gTIllqaQDyXg1Mxy/qOjoxIQJhMh19hfPzgqKv0A6NPo3XtCO+0mSPSFsbz9tzl vCte7evN8coOSrJYMGwovpTMbhnKuLWSNb2fvkV9V3vJ3jj7wChahvHr99sWNFG2u2yh zL56PUpWfTsr7PQkf2CLoV3gOMm5qwMtk3rakHCB0cNEwOsJgjwlvQhPVaH4LGgNnzJd dZvWMfvWNP2VrZs6zRHrYYIX35JrBjKSp2t5TmaPGRsfdB+zC/eUdWM4hyp3fkuaAnlf Z6sjit0lVGvlYqHsVLIDGdkesTrpKMUeuPTMZGnOHHziPn4R3VFCh2JGI39lXkPlMDuC 8BAQ== X-Gm-Message-State: AOJu0YyV8JgWpq2Fp8VNQjzC8YGfxLgpygfYK34mTUtdLPtvebeqxQ9/ EHQCdIQbFKxImc+m7GhtFcqAx+5P60PcAkVPKa1Dy8FLR5erf0pKe4oV4vsmt3Vm68MLNzZrbCZ A2qR0nlWrfk4/oXxb6hXZ35VS28Akx5zH X-Google-Smtp-Source: AGHT+IH54XWtWcCk6UrOw9dh7YlSKhmotZQEjh31t5HXg/jASNpKOjbYC4sLm3Ezhqg0xF0j/0/51n3p2RWs X-Received: by 2002:a05:6602:13c8:b0:7eb:4a5b:8602 with SMTP id ca18e2360f4ac-7ebeb4ba6bdmr34697139f.6.1718302547239; Thu, 13 Jun 2024 11:15:47 -0700 (PDT) Received: from postal.iol.unh.edu (postal.iol.unh.edu. [132.177.123.84]) by smtp-relay.gmail.com with ESMTPS id ca18e2360f4ac-7ebdba5e94esm5860339f.15.2024.06.13.11.15.47 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Jun 2024 11:15:47 -0700 (PDT) X-Relaying-Domain: iol.unh.edu Received: from iol.unh.edu (unknown [IPv6:2606:4100:3880:1257::1083]) by postal.iol.unh.edu (Postfix) with ESMTP id 41857605C380; Thu, 13 Jun 2024 14:15:46 -0400 (EDT) From: jspewock@iol.unh.edu To: juraj.linkes@pantheon.tech, probb@iol.unh.edu, yoan.picchi@foss.arm.com, npratte@iol.unh.edu, Honnappa.Nagarahalli@arm.com, wathsala.vithanage@arm.com, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, thomas@monjalon.net Cc: dev@dpdk.org, Jeremy Spewock Subject: [PATCH v4 1/4] dts: add context manager for interactive shells Date: Thu, 13 Jun 2024 14:15:07 -0400 Message-ID: <20240613181510.30135-2-jspewock@iol.unh.edu> X-Mailer: git-send-email 2.45.1 In-Reply-To: <20240613181510.30135-1-jspewock@iol.unh.edu> References: <20240514201436.2496-1-jspewock@iol.unh.edu> <20240613181510.30135-1-jspewock@iol.unh.edu> 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 From: Jeremy Spewock Interactive shells are managed in a way currently where they are closed and cleaned up at the time of garbage collection. Due to there being no guarantee of when this garbage collection happens in Python, there is no way to consistently know when an application will be closed without manually closing the application yourself when you are done with it. This doesn't cause a problem in cases where you can start another instance of the same application multiple times on a server, but this isn't the case for primary applications in DPDK. The introduction of primary applications, such as testpmd, adds a need for knowing previous instances of the application have been stopped and cleaned up before starting a new one, which the garbage collector does not provide. To solve this problem, a new class is added which acts as a base class for interactive shells that enforces that instances of the application be managed using a context manager. Using a context manager guarantees that once you leave the scope of the block where the application is being used for any reason, the application will be closed immediately. This avoids the possibility of the shell not being closed due to an exception being raised or user error. The interactive shell class then becomes shells that can be started/stopped manually or at the time of garbage collection rather than through a context manager. depends-on: patch-139227 ("dts: skip test cases based on capabilities") Signed-off-by: Jeremy Spewock Reviewed-by: Juraj Linkeš --- dts/framework/remote_session/__init__.py | 1 + .../remote_session/interactive_shell.py | 146 ++------------ .../single_active_interactive_shell.py | 188 ++++++++++++++++++ dts/framework/remote_session/testpmd_shell.py | 9 +- dts/framework/testbed_model/os_session.py | 4 +- dts/framework/testbed_model/sut_node.py | 8 +- .../testbed_model/traffic_generator/scapy.py | 2 + dts/tests/TestSuite_pmd_buffer_scatter.py | 27 +-- dts/tests/TestSuite_smoke_tests.py | 3 +- 9 files changed, 233 insertions(+), 155 deletions(-) create mode 100644 dts/framework/remote_session/single_active_interactive_shell.py diff --git a/dts/framework/remote_session/__init__.py b/dts/framework/remote_session/__init__.py index f18a9f2259..81839410b9 100644 --- a/dts/framework/remote_session/__init__.py +++ b/dts/framework/remote_session/__init__.py @@ -21,6 +21,7 @@ from .interactive_shell import InteractiveShell from .python_shell import PythonShell from .remote_session import CommandResult, RemoteSession +from .single_active_interactive_shell import SingleActiveInteractiveShell from .ssh_session import SSHSession from .testpmd_shell import NicCapability, TestPmdShell diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py index 5cfe202e15..9d124b8245 100644 --- a/dts/framework/remote_session/interactive_shell.py +++ b/dts/framework/remote_session/interactive_shell.py @@ -1,149 +1,31 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2023 University of New Hampshire -"""Common functionality for interactive shell handling. +"""Interactive shell with manual stop/start functionality. -The base class, :class:`InteractiveShell`, is meant to be extended by subclasses that contain -functionality specific to that shell type. These subclasses will often modify things like -the prompt to expect or the arguments to pass into the application, but still utilize -the same method for sending a command and collecting output. How this output is handled however -is often application specific. If an application needs elevated privileges to start it is expected -that the method for gaining those privileges is provided when initializing the class. - -The :option:`--timeout` command line argument and the :envvar:`DTS_TIMEOUT` -environment variable configure the timeout of getting the output from command execution. +Provides a class that doesn't require being started/stopped using a context manager and can instead +be started and stopped manually, or have the stopping process be handled at the time of garbage +collection. """ -from abc import ABC -from pathlib import PurePath -from typing import Callable, ClassVar - -from paramiko import Channel, SSHClient, channel # type: ignore[import] - -from framework.logger import DTSLogger -from framework.settings import SETTINGS +from .single_active_interactive_shell import SingleActiveInteractiveShell -class InteractiveShell(ABC): - """The base class for managing interactive shells. +class InteractiveShell(SingleActiveInteractiveShell): + """Adds manual start and stop functionality to interactive shells. - This class shouldn't be instantiated directly, but instead be extended. It contains - methods for starting interactive shells as well as sending commands to these shells - and collecting input until reaching a certain prompt. All interactive applications - will use the same SSH connection, but each will create their own channel on that - session. + Like its super-class, this class should not be instantiated directly and should instead be + extended. This class also provides an option for automated cleanup of the application through + the garbage collector. """ - _interactive_session: SSHClient - _stdin: channel.ChannelStdinFile - _stdout: channel.ChannelFile - _ssh_channel: Channel - _logger: DTSLogger - _timeout: float - _app_args: str - - #: Prompt to expect at the end of output when sending a command. - #: This is often overridden by subclasses. - _default_prompt: ClassVar[str] = "" - - #: Extra characters to add to the end of every command - #: before sending them. This is often overridden by subclasses and is - #: most commonly an additional newline character. - _command_extra_chars: ClassVar[str] = "" - - #: Path to the executable to start the interactive application. - path: ClassVar[PurePath] - - #: Whether this application is a DPDK app. If it is, the build directory - #: for DPDK on the node will be prepended to the path to the executable. - dpdk_app: ClassVar[bool] = False - - def __init__( - self, - interactive_session: SSHClient, - logger: DTSLogger, - get_privileged_command: Callable[[str], str] | None, - app_args: str = "", - timeout: float = SETTINGS.timeout, - ) -> None: - """Create an SSH channel during initialization. - - Args: - interactive_session: The SSH session dedicated to interactive 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 will not be started - with elevated privileges. - app_args: The command line arguments 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. - """ - self._interactive_session = interactive_session - self._ssh_channel = self._interactive_session.invoke_shell() - self._stdin = self._ssh_channel.makefile_stdin("w") - self._stdout = self._ssh_channel.makefile("r") - self._ssh_channel.settimeout(timeout) - self._ssh_channel.set_combine_stderr(True) # combines stdout and stderr streams - self._logger = logger - self._timeout = timeout - self._app_args = app_args - self._start_application(get_privileged_command) - - def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None: - """Starts a new interactive application based on the path to the app. - - This method is often overridden by subclasses as their process for - starting may look different. - - Args: - 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}" - if get_privileged_command is not None: - start_command = get_privileged_command(start_command) - self.send_command(start_command) - - def send_command(self, command: str, prompt: str | None = None) -> str: - """Send `command` and get all output before the expected ending string. - - Lines that expect input are not included in the stdout buffer, so they cannot - be used for expect. - - Example: - If you were prompted to log into something with a username and password, - you cannot expect ``username:`` because it won't yet be in the stdout buffer. - A workaround for this could be consuming an extra newline character to force - the current `prompt` into the stdout buffer. - - Args: - command: The command to send. - prompt: After sending the command, `send_command` will be expecting this string. - If :data:`None`, will use the class's default prompt. - - Returns: - All output in the buffer before expected string. - """ - self._logger.info(f"Sending: '{command}'") - if prompt is None: - prompt = self._default_prompt - self._stdin.write(f"{command}{self._command_extra_chars}\n") - self._stdin.flush() - out: str = "" - for line in self._stdout: - out += line - if prompt in line and not line.rstrip().endswith( - command.rstrip() - ): # ignore line that sent command - break - self._logger.debug(f"Got output: {out}") - return out + def start_application(self) -> None: + """Start the application.""" + self._start_application(self._get_privileged_command) def close(self) -> None: """Properly free all resources.""" - self._stdin.close() - self._ssh_channel.close() + self._close() def __del__(self) -> None: """Make sure the session is properly closed before deleting the object.""" diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py new file mode 100644 index 0000000000..74060be8a7 --- /dev/null +++ b/dts/framework/remote_session/single_active_interactive_shell.py @@ -0,0 +1,188 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2024 University of New Hampshire + +"""Common functionality for interactive shell handling. + +The base class, :class:`SingleActiveInteractiveShell`, is meant to be extended by subclasses that +contain functionality specific to that shell type. These subclasses will often modify things like +the prompt to expect or the arguments to pass into the application, but still utilize +the same method for sending a command and collecting output. How this output is handled however +is often application specific. If an application needs elevated privileges to start it is expected +that the method for gaining those privileges is provided when initializing the class. + +This class is designed for applications like primary applications in DPDK where only one instance +of the application can be running at a given time and, for this reason, is managed using a context +manager. This context manager starts the application when you enter the context and cleans up the +application when you exit. Using a context manager for this is useful since it allows us to ensure +the application is cleaned up as soon as you leave the block regardless of the reason. + +The :option:`--timeout` command line argument and the :envvar:`DTS_TIMEOUT` +environment variable configure the timeout of getting the output from command execution. +""" + +from abc import ABC +from pathlib import PurePath +from typing import Callable, ClassVar + +from paramiko import Channel, SSHClient, channel # type: ignore[import] +from typing_extensions import Self + +from framework.exception import InteractiveCommandExecutionError +from framework.logger import DTSLogger +from framework.settings import SETTINGS + + +class SingleActiveInteractiveShell(ABC): + """The base class for managing interactive shells. + + This class shouldn't be instantiated directly, but instead be extended. It contains + methods for starting interactive shells as well as sending commands to these shells + and collecting input until reaching a certain prompt. All interactive applications + will use the same SSH connection, but each will create their own channel on that + session. + + Interactive shells are started and stopped using a context manager. This allows for the start + and cleanup of the application to happen at predictable times regardless of exceptions or + interrupts. + """ + + _interactive_session: SSHClient + _stdin: channel.ChannelStdinFile + _stdout: channel.ChannelFile + _ssh_channel: Channel + _logger: DTSLogger + _timeout: float + _app_args: str + _get_privileged_command: Callable[[str], str] | None + + #: Prompt to expect at the end of output when sending a command. + #: This is often overridden by subclasses. + _default_prompt: ClassVar[str] = "" + + #: Extra characters to add to the end of every command + #: before sending them. This is often overridden by subclasses and is + #: most commonly an additional newline character. + _command_extra_chars: ClassVar[str] = "" + + #: Path to the executable to start the interactive application. + path: ClassVar[PurePath] + + #: Whether this application is a DPDK app. If it is, the build directory + #: for DPDK on the node will be prepended to the path to the executable. + dpdk_app: ClassVar[bool] = False + + def __init__( + self, + interactive_session: SSHClient, + logger: DTSLogger, + get_privileged_command: Callable[[str], str] | None, + app_args: str = "", + timeout: float = SETTINGS.timeout, + ) -> None: + """Create an SSH channel during initialization. + + Args: + interactive_session: The SSH session dedicated to interactive 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 will not be started + with elevated privileges. + app_args: The command line arguments 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. + """ + self._interactive_session = interactive_session + self._logger = logger + self._timeout = timeout + self._app_args = app_args + self._get_privileged_command = get_privileged_command + + def _init_channel(self): + self._ssh_channel = self._interactive_session.invoke_shell() + self._stdin = self._ssh_channel.makefile_stdin("w") + self._stdout = self._ssh_channel.makefile("r") + self._ssh_channel.settimeout(self._timeout) + self._ssh_channel.set_combine_stderr(True) # combines stdout and stderr streams + + def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None: + """Starts a new interactive application based on the path to the app. + + This method is often overridden by subclasses as their process for starting may look + different. A new SSH channel is initialized for the application to run on, then the + application is started. + + Args: + get_privileged_command: A function (but could be any callable) that produces + the version of the command with elevated privileges. + """ + self._init_channel() + start_command = f"{self.path} {self._app_args}" + if get_privileged_command is not None: + start_command = get_privileged_command(start_command) + self.send_command(start_command) + + def send_command(self, command: str, prompt: str | None = None) -> str: + """Send `command` and get all output before the expected ending string. + + Lines that expect input are not included in the stdout buffer, so they cannot + be used for expect. + + Example: + If you were prompted to log into something with a username and password, + you cannot expect ``username:`` because it won't yet be in the stdout buffer. + A workaround for this could be consuming an extra newline character to force + the current `prompt` into the stdout buffer. + + Args: + command: The command to send. + prompt: After sending the command, `send_command` will be expecting this string. + If :data:`None`, will use the class's default prompt. + + Returns: + All output in the buffer before expected string. + """ + self._logger.info(f"Sending: '{command}'") + if prompt is None: + prompt = self._default_prompt + self._stdin.write(f"{command}{self._command_extra_chars}\n") + self._stdin.flush() + out: str = "" + for line in self._stdout: + out += line + if prompt in line and not line.rstrip().endswith( + command.rstrip() + ): # ignore line that sent command + break + self._logger.debug(f"Got output: {out}") + return out + + def _close(self) -> None: + self._stdin.close() + self._ssh_channel.close() + + def __enter__(self) -> Self: + """Enter the context block. + + Upon entering a context block with this class, the desired behavior is to create the + channel for the application to use, and then start the application. + + Returns: + Reference to the object for the application after it has been started. + """ + self._start_application(self._get_privileged_command) + return self + + def __exit__(self, *_) -> None: + """Exit the context block. + + Upon exiting a context block with this class, we want to ensure that the instance of the + application is explicitly closed and properly cleaned up using its close method. Note that + because this method returns :data:`None` if an exception was raised within the block, it is + not handled and will be re-raised after the application is closed. + + The desired behavior is to close the application regardless of the reason for exiting the + context and then recreate that reason afterwards. All method arguments are ignored for + this reason. + """ + self._close() diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py index f6783af621..17561d4dae 100644 --- a/dts/framework/remote_session/testpmd_shell.py +++ b/dts/framework/remote_session/testpmd_shell.py @@ -26,7 +26,7 @@ from framework.settings import SETTINGS from framework.utils import StrEnum -from .interactive_shell import InteractiveShell +from .single_active_interactive_shell import SingleActiveInteractiveShell class TestPmdDevice(object): @@ -82,7 +82,7 @@ class TestPmdForwardingModes(StrEnum): recycle_mbufs = auto() -class TestPmdShell(InteractiveShell): +class TestPmdShell(SingleActiveInteractiveShell): """Testpmd interactive shell. The testpmd shell users should never use @@ -227,10 +227,11 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True): f"Test pmd failed to set fwd mode to {mode.value}" ) - def close(self) -> None: + def _close(self) -> None: """Overrides :meth:`~.interactive_shell.close`.""" + self.stop() self.send_command("quit", "") - return super().close() + return super()._close() def get_capas_rxq( self, supported_capabilities: MutableSet, unsupported_capabilities: MutableSet diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py index d5bf7e0401..745f0317f8 100644 --- a/dts/framework/testbed_model/os_session.py +++ b/dts/framework/testbed_model/os_session.py @@ -32,8 +32,8 @@ from framework.remote_session import ( CommandResult, InteractiveRemoteSession, - InteractiveShell, RemoteSession, + SingleActiveInteractiveShell, create_interactive_session, create_remote_session, ) @@ -43,7 +43,7 @@ from .cpu import LogicalCore from .port import Port -InteractiveShellType = TypeVar("InteractiveShellType", bound=InteractiveShell) +InteractiveShellType = TypeVar("InteractiveShellType", bound=SingleActiveInteractiveShell) class OSSession(ABC): diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py index 1fb536735d..7dd39fd735 100644 --- a/dts/framework/testbed_model/sut_node.py +++ b/dts/framework/testbed_model/sut_node.py @@ -243,10 +243,10 @@ def get_supported_capabilities( unsupported_capas: set[NicCapability] = set() self._logger.debug(f"Checking which capabilities from {capabilities} NIC are supported.") testpmd_shell = self.create_interactive_shell(TestPmdShell, privileged=True) - for capability in capabilities: - if capability not in supported_capas or capability not in unsupported_capas: - capability.value(testpmd_shell, supported_capas, unsupported_capas) - del testpmd_shell + with testpmd_shell as running_testpmd: + for capability in capabilities: + if capability not in supported_capas or capability not in unsupported_capas: + capability.value(running_testpmd, supported_capas, unsupported_capas) return supported_capas def _set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None: diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py index df3069d516..ba3a56df79 100644 --- a/dts/framework/testbed_model/traffic_generator/scapy.py +++ b/dts/framework/testbed_model/traffic_generator/scapy.py @@ -221,6 +221,8 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig): PythonShell, timeout=5, privileged=True ) + self.session.start_application() + # import libs in remote python console for import_statement in SCAPY_RPC_SERVER_IMPORTS: self.session.send_command(import_statement) diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py index 3701c47408..645a66b607 100644 --- a/dts/tests/TestSuite_pmd_buffer_scatter.py +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py @@ -101,7 +101,7 @@ def pmd_scatter(self, mbsize: int) -> None: Test: Start testpmd and run functional test with preset mbsize. """ - testpmd = self.sut_node.create_interactive_shell( + testpmd_shell = self.sut_node.create_interactive_shell( TestPmdShell, app_parameters=( "--mbcache=200 " @@ -112,17 +112,20 @@ def pmd_scatter(self, mbsize: int) -> None: ), privileged=True, ) - testpmd.set_forward_mode(TestPmdForwardingModes.mac) - testpmd.start() - - for offset in [-1, 0, 1, 4, 5]: - recv_payload = self.scatter_pktgen_send_packet(mbsize + offset) - self._logger.debug(f"Payload of scattered packet after forwarding: \n{recv_payload}") - self.verify( - ("58 " * 8).strip() in recv_payload, - f"Payload of scattered packet did not match expected payload with offset {offset}.", - ) - testpmd.stop() + with testpmd_shell as testpmd: + testpmd.set_forward_mode(TestPmdForwardingModes.mac) + testpmd.start() + + for offset in [-1, 0, 1, 4, 5]: + recv_payload = self.scatter_pktgen_send_packet(mbsize + offset) + self._logger.debug( + f"Payload of scattered packet after forwarding: \n{recv_payload}" + ) + self.verify( + ("58 " * 8).strip() in recv_payload, + "Payload of scattered packet did not match expected payload with offset " + f"{offset}.", + ) def test_scatter_mbuf_2048(self) -> None: """Run the :meth:`pmd_scatter` test with `mbsize` set to 2048.""" diff --git a/dts/tests/TestSuite_smoke_tests.py b/dts/tests/TestSuite_smoke_tests.py index a553e89662..360e64eb5a 100644 --- a/dts/tests/TestSuite_smoke_tests.py +++ b/dts/tests/TestSuite_smoke_tests.py @@ -100,7 +100,8 @@ def test_devices_listed_in_testpmd(self) -> None: List all devices found in testpmd and verify the configured devices are among them. """ testpmd_driver = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True) - dev_list = [str(x) for x in testpmd_driver.get_devices()] + with testpmd_driver as testpmd: + dev_list = [str(x) for x in testpmd.get_devices()] for nic in self.nics_in_node: self.verify( nic.pci in dev_list, From patchwork Thu Jun 13 18:15:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Spewock X-Patchwork-Id: 141128 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 823074541F; Thu, 13 Jun 2024 20:16:05 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 542BD427D5; Thu, 13 Jun 2024 20:15:54 +0200 (CEST) Received: from mail-yw1-f230.google.com (mail-yw1-f230.google.com [209.85.128.230]) by mails.dpdk.org (Postfix) with ESMTP id 6CAE7427B9 for ; Thu, 13 Jun 2024 20:15:52 +0200 (CEST) Received: by mail-yw1-f230.google.com with SMTP id 00721157ae682-62f8dcbd4b5so15925867b3.0 for ; Thu, 13 Jun 2024 11:15:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718302551; x=1718907351; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=8lpCoC6hjD9iiAger6N81O2cRJjsWsj/wb+XJqDHNbU=; b=En4rU9Wv/w2jAebbHUBjn57cCh6hIxoj+rPaEX4vRpmWVO45ZlL3iUhkQKYjFAKbsm LCv/K0U/J5VA4V+L4EjtOtIQAOxYZ4Xpb62/hkulQTPEPhQWPVWD/kL3MkEvx5T8O9Rj +YM/npJ7DFqvgV2PEt3M0ken66nSaKIzjVN0A= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718302551; x=1718907351; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8lpCoC6hjD9iiAger6N81O2cRJjsWsj/wb+XJqDHNbU=; b=Ts6Qdp5YlYgWJ183bWvJQ4BB3ByDGgytvBNfOzkHEhZwdtCgagQqELPKiLKHI6g+0q YTVxfxTGjM+DOf8K+O0AB9rkaNK1Qdue+fs1zOfy3tc0wiBM5hOpUfsQPDQXUQVTLeCO PjexK0uMedD7vP07Yv9DndPHIsh4lr+VQxyvfEF/bIwoGoBhxeSGHfrML9KglLq+xzOL T379zq1RDSFpOPIuKkDy99nR+YO5ADRkxOKRkVFEdJ4JECjijazpn2FsAJd9k+MXixJK EEai23Olr7aXC8zWMl0wYbmyGMlGmVxfNcLTr+f8hnkoELyjbrPsX7P0q2EzxyQ0+Gih oZQA== X-Gm-Message-State: AOJu0Ywg7Jhgg0mPzzsTsXfOj4Nn8ml6L3eZlYJLPOo4DCvcCoKPe9aN ARxXUDooPRRokTtFg8EgPxEDwwEjmnx815pDKNIURFXLT0a86a6jQdxBL8W5u4c5qWm7bQTE3Bd hykb+PCcQ+8bXtHqZrS9O6ebRqzlhyhslCG64agOiwZYgPbdm X-Google-Smtp-Source: AGHT+IGanawBRyGWHqdJeGsIqXzxkvw3jfmIICnCEIEZ2NhRzIgcKJEhjs1Kjdc6bjumWnDBFMzeI0V0fk0j X-Received: by 2002:a0d:f703:0:b0:631:45f6:7850 with SMTP id 00721157ae682-6322480cf1dmr1682767b3.39.1718302550124; Thu, 13 Jun 2024 11:15:50 -0700 (PDT) Received: from postal.iol.unh.edu (postal.iol.unh.edu. [2606:4100:3880:1234::84]) by smtp-relay.gmail.com with ESMTPS id 00721157ae682-63118da40a0sm595227b3.33.2024.06.13.11.15.49 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Jun 2024 11:15:50 -0700 (PDT) X-Relaying-Domain: iol.unh.edu Received: from iol.unh.edu (unknown [IPv6:2606:4100:3880:1257::1083]) by postal.iol.unh.edu (Postfix) with ESMTP id 38D38605C380; Thu, 13 Jun 2024 14:15:49 -0400 (EDT) From: jspewock@iol.unh.edu To: juraj.linkes@pantheon.tech, probb@iol.unh.edu, yoan.picchi@foss.arm.com, npratte@iol.unh.edu, Honnappa.Nagarahalli@arm.com, wathsala.vithanage@arm.com, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, thomas@monjalon.net Cc: dev@dpdk.org, Jeremy Spewock Subject: [PATCH v4 2/4] dts: improve starting and stopping interactive shells Date: Thu, 13 Jun 2024 14:15:08 -0400 Message-ID: <20240613181510.30135-3-jspewock@iol.unh.edu> X-Mailer: git-send-email 2.45.1 In-Reply-To: <20240613181510.30135-1-jspewock@iol.unh.edu> References: <20240514201436.2496-1-jspewock@iol.unh.edu> <20240613181510.30135-1-jspewock@iol.unh.edu> 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 From: Jeremy Spewock The InteractiveShell class currently relies on being cleaned up and shutdown at the time of garbage collection, but this cleanup of the class does no verification that the session is still running prior to cleanup. So, if a user were to call this method themselves prior to garbage collection, it would be called twice and throw an exception when the desired behavior is to do nothing since the session is already cleaned up. This is solved by using a weakref and a finalize class which achieves the same result of calling the method at garbage collection, but also ensures that it is called exactly once. Additionally, this fixes issues regarding starting a primary DPDK application while another is still cleaning up via a retry when starting interactive shells. It also adds catch for attempting to send a command to an interactive shell that is not running to create a more descriptive error message. Signed-off-by: Jeremy Spewock --- .../remote_session/interactive_shell.py | 35 ++++++++++++++----- .../single_active_interactive_shell.py | 34 ++++++++++++++++-- dts/framework/remote_session/testpmd_shell.py | 2 +- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py index 9d124b8245..5b6f5c2a41 100644 --- a/dts/framework/remote_session/interactive_shell.py +++ b/dts/framework/remote_session/interactive_shell.py @@ -8,6 +8,9 @@ collection. """ +import weakref +from typing import Callable, ClassVar + from .single_active_interactive_shell import SingleActiveInteractiveShell @@ -15,18 +18,34 @@ class InteractiveShell(SingleActiveInteractiveShell): """Adds manual start and stop functionality to interactive shells. Like its super-class, this class should not be instantiated directly and should instead be - extended. This class also provides an option for automated cleanup of the application through - the garbage collector. + extended. This class also provides an option for automated cleanup of the application using a + weakref and a finalize class. This finalize class allows for cleanup of the class at the time + of garbage collection and also ensures that cleanup only happens once. This way if a user + initiates the closing of the shell manually it is not repeated at the time of garbage + collection. """ + _finalizer: weakref.finalize + #: Shells that do not require only one instance to be running shouldn't need more than 1 + #: attempt to start. + _init_attempts: ClassVar[int] = 1 + + def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None: + """Overrides :meth:`_start_application` in the parent class. + + Add a weakref finalize class after starting the application. + + Args: + get_privileged_command: A function (but could be any callable) that produces + the version of the command with elevated privileges. + """ + super()._start_application(get_privileged_command) + self._finalizer = weakref.finalize(self, self._close) + def start_application(self) -> None: """Start the application.""" self._start_application(self._get_privileged_command) def close(self) -> None: - """Properly free all resources.""" - self._close() - - def __del__(self) -> None: - """Make sure the session is properly closed before deleting the object.""" - self.close() + """Free all resources using finalize class.""" + self._finalizer() diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py index 74060be8a7..282ceec483 100644 --- a/dts/framework/remote_session/single_active_interactive_shell.py +++ b/dts/framework/remote_session/single_active_interactive_shell.py @@ -44,6 +44,10 @@ class SingleActiveInteractiveShell(ABC): Interactive shells are started and stopped using a context manager. This allows for the start and cleanup of the application to happen at predictable times regardless of exceptions or interrupts. + + Attributes: + is_alive: :data:`True` if the application has started successfully, :data:`False` + otherwise. """ _interactive_session: SSHClient @@ -55,6 +59,9 @@ class SingleActiveInteractiveShell(ABC): _app_args: str _get_privileged_command: Callable[[str], str] | None + #: The number of times to try starting the application before considering it a failure. + _init_attempts: ClassVar[int] = 5 + #: Prompt to expect at the end of output when sending a command. #: This is often overridden by subclasses. _default_prompt: ClassVar[str] = "" @@ -71,6 +78,8 @@ class SingleActiveInteractiveShell(ABC): #: for DPDK on the node will be prepended to the path to the executable. dpdk_app: ClassVar[bool] = False + is_alive: bool = False + def __init__( self, interactive_session: SSHClient, @@ -110,17 +119,34 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None This method is often overridden by subclasses as their process for starting may look different. A new SSH channel is initialized for the application to run on, then the - application is started. + application is started. Initialization of the shell on the host can be retried up to + `self._init_attempts` - 1 times. This is done because some DPDK applications need slightly + more time after exiting their script to clean up EAL before others can start. Args: get_privileged_command: A function (but could be any callable) that produces the version of the command with elevated privileges. """ self._init_channel() + self._ssh_channel.settimeout(5) start_command = f"{self.path} {self._app_args}" if get_privileged_command is not None: start_command = get_privileged_command(start_command) - self.send_command(start_command) + self.is_alive = True + for attempt in range(self._init_attempts): + try: + self.send_command(start_command) + break + except TimeoutError: + self._logger.info( + f"Interactive shell failed to start (attempt {attempt+1} out of " + f"{self._init_attempts})" + ) + else: + self._ssh_channel.settimeout(self._timeout) + self.is_alive = False # update state on failure to start + raise InteractiveCommandExecutionError("Failed to start application.") + self._ssh_channel.settimeout(self._timeout) def send_command(self, command: str, prompt: str | None = None) -> str: """Send `command` and get all output before the expected ending string. @@ -142,6 +168,10 @@ def send_command(self, command: str, prompt: str | None = None) -> str: Returns: All output in the buffer before expected string. """ + if not self.is_alive: + raise InteractiveCommandExecutionError( + f"Cannot send command {command} to application because the shell is not running." + ) self._logger.info(f"Sending: '{command}'") if prompt is None: prompt = self._default_prompt diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py index 17561d4dae..805bb3a77d 100644 --- a/dts/framework/remote_session/testpmd_shell.py +++ b/dts/framework/remote_session/testpmd_shell.py @@ -230,7 +230,7 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True): def _close(self) -> None: """Overrides :meth:`~.interactive_shell.close`.""" self.stop() - self.send_command("quit", "") + self.send_command("quit", "Bye...") return super()._close() def get_capas_rxq( From patchwork Thu Jun 13 18:15:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Spewock X-Patchwork-Id: 141127 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 2F4E84541F; Thu, 13 Jun 2024 20:15:58 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 14D48427CC; Thu, 13 Jun 2024 20:15:53 +0200 (CEST) Received: from mail-io1-f98.google.com (mail-io1-f98.google.com [209.85.166.98]) by mails.dpdk.org (Postfix) with ESMTP id 0CD97427D1 for ; Thu, 13 Jun 2024 20:15:52 +0200 (CEST) Received: by mail-io1-f98.google.com with SMTP id ca18e2360f4ac-7ebcc7676c3so53908639f.0 for ; Thu, 13 Jun 2024 11:15:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718302551; x=1718907351; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=w4zL3mL74Mm7K31s43fnUGoAWiml9wzu58xUoK8N2Fg=; b=CUOTxa5FnBGNpEZnDHH9U6b40TOHPeaRoUOYJdoDQu0kVinNGyVVlSRnlbtPu+HALo qR2jQZ7eNPgOER6uPCCpzj06Plk2UwvRrmz5Wn6T+Vb5jRETXnsejB3huu0nvksHnSYT 74O5iHPoN2K59oIR8lVRZbkCmLIFvT1+9O6nU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718302551; x=1718907351; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=w4zL3mL74Mm7K31s43fnUGoAWiml9wzu58xUoK8N2Fg=; b=iupaqIgPwe5demRS6PtutrHoQDJzgx6OSQ/lEG1q7Pq/oGnKkY8N9JcQchJgdgyRdo YD/rekmgZuJBSsWl/v5HWKTB7WuzMMjLbocG9Ul9ppvDFXKu/LkjnWXXnc4qL/3HJWJC 5Z/g08pYOr3CPHtDHczJ2Q4uPSchDQJtLedmfqIgd7QECsQtuKTwO/Lz7gSSVd9MxA40 a3SZgZvSNhjwV1fABluGwfcl+4WtaCSaNhYjTRD6eMnZ20+pG5zAnVQ9OgZJYRAkgGwy +7Du8SVfw9GKgpYUOHMvJMuhQq3X/B7EvOW9syI4GGLVOzV/YHxXFWZHEpIKgSCnT0kk nQAg== X-Gm-Message-State: AOJu0Yzm/VZ1ZgTgKOHt4F0f3/yiCCxst2udxlMVgg/T1gwZfPs+19B6 E09IVai72lDTB8Z9OzVJ6uXmOhF8C7zghoxkyVXdGRd4Fv+7AD1XE7FKhdHdZH19ecHf+fMDVa/ eUCsWX4lqmH1vLwDbJaJosexOuXBxgwdNshveg2650he80r4k X-Google-Smtp-Source: AGHT+IGX6Grppy14yJV8f/ALXuWTWhxcFs58DoXEsAdkinUQAZy8kul+zW+gkHQUx8NBBEvHbCvJdPjELvSl X-Received: by 2002:a05:6602:60cb:b0:7eb:7ebd:1fa6 with SMTP id ca18e2360f4ac-7ebeaef0e30mr34160839f.0.1718302551285; Thu, 13 Jun 2024 11:15:51 -0700 (PDT) Received: from postal.iol.unh.edu (postal.iol.unh.edu. [2606:4100:3880:1234::84]) by smtp-relay.gmail.com with ESMTPS id 8926c6da1cb9f-4b956992175sm41217173.30.2024.06.13.11.15.50 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Jun 2024 11:15:51 -0700 (PDT) X-Relaying-Domain: iol.unh.edu Received: from iol.unh.edu (unknown [IPv6:2606:4100:3880:1257::1083]) by postal.iol.unh.edu (Postfix) with ESMTP id 591F3605C380; Thu, 13 Jun 2024 14:15:50 -0400 (EDT) From: jspewock@iol.unh.edu To: juraj.linkes@pantheon.tech, probb@iol.unh.edu, yoan.picchi@foss.arm.com, npratte@iol.unh.edu, Honnappa.Nagarahalli@arm.com, wathsala.vithanage@arm.com, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, thomas@monjalon.net Cc: dev@dpdk.org, Jeremy Spewock Subject: [PATCH v4 3/4] dts: add methods for modifying MTU to testpmd shell Date: Thu, 13 Jun 2024 14:15:09 -0400 Message-ID: <20240613181510.30135-4-jspewock@iol.unh.edu> X-Mailer: git-send-email 2.45.1 In-Reply-To: <20240613181510.30135-1-jspewock@iol.unh.edu> References: <20240514201436.2496-1-jspewock@iol.unh.edu> <20240613181510.30135-1-jspewock@iol.unh.edu> 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 From: Jeremy Spewock There are methods within DTS currently that support updating the MTU of ports on a node, but the methods for doing this in a linux session rely on the ip command and the port being bound to the kernel driver. Since test suites are run while bound to the driver for DPDK, there needs to be a way to modify the value while bound to said driver as well. This is done by using testpmd to modify the MTU. Signed-off-by: Jeremy Spewock --- dts/framework/remote_session/testpmd_shell.py | 102 +++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py index 805bb3a77d..09f80cb250 100644 --- a/dts/framework/remote_session/testpmd_shell.py +++ b/dts/framework/remote_session/testpmd_shell.py @@ -20,7 +20,7 @@ from enum import Enum, auto from functools import partial from pathlib import PurePath -from typing import Callable, ClassVar +from typing import Any, Callable, ClassVar from framework.exception import InteractiveCommandExecutionError from framework.settings import SETTINGS @@ -82,6 +82,39 @@ class TestPmdForwardingModes(StrEnum): recycle_mbufs = auto() +def stop_then_start_port_decorator( + func: Callable[["TestPmdShell", int, Any, bool], None] +) -> Callable[["TestPmdShell", int, Any, bool], None]: + """Decorator that stops a port, runs decorated function, then starts the port. + + The function being decorated must be a method defined in :class:`TestPmdShell` that takes a + port ID (as an int) as its first parameter and has a "verify" parameter (as a bool) as its last + parameter. The port ID and verify parameters will be passed into + :meth:`TestPmdShell._stop_port` so that the correct port is stopped/started and verification + takes place if desired. + + Args: + func: The function to run while the port is stopped. + + Returns: + Wrapper function that stops a port, runs the decorated function, then starts the port. + """ + + def wrapper(shell: "TestPmdShell", port_id: int, *args, **kwargs) -> None: + """Function that wraps the instance method of :class:`TestPmdShell`. + + Args: + shell: Instance of the shell containing the method to decorate. + port_id: ID of the port to stop/start. + """ + verify_value = kwargs["verify"] if "verify" in kwargs else args[-1] + shell._stop_port(port_id, verify_value) + func(shell, port_id, *args, **kwargs) + shell._start_port(port_id, verify_value) + + return wrapper + + class TestPmdShell(SingleActiveInteractiveShell): """Testpmd interactive shell. @@ -227,6 +260,73 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True): f"Test pmd failed to set fwd mode to {mode.value}" ) + def _stop_port(self, port_id: int, verify: bool = True) -> None: + """Stop port with `port_id` in testpmd. + + Depending on the PMD, the port may need to be stopped before configuration can take place. + This method wraps the command needed to properly stop ports and take their link down. + + Args: + port_id: ID of the port to take down. + verify: If :data:`True` the output will be scanned in an attempt to verify that the + stopping of ports was successful. Defaults to True. + + Raises: + InteractiveCommandExecutionError: If `verify` is :data:`True` and the port did not + successfully stop. + """ + stop_port_output = self.send_command(f"port stop {port_id}") + if verify and ("Done" not in stop_port_output): + self._logger.debug(f"Failed to stop port {port_id}. Output was:\n{stop_port_output}") + raise InteractiveCommandExecutionError(f"Test pmd failed to stop port {port_id}.") + + def _start_port(self, port_id: int, verify: bool = True) -> None: + """Start port `port_id` in testpmd. + + Because the port may need to be stopped to make some configuration changes, it naturally + follows that it will need to be started again once those changes have been made. + + Args: + port_id: ID of the port to start. + verify: If :data:`True` the output will be scanned in an attempt to verify that the + port came back up without error. Defaults to True. + + Raises: + InteractiveCommandExecutionError: If `verify` is :data:`True` and the port did not come + back up. + """ + start_port_output = self.send_command(f"port start {port_id}") + if verify and ("Done" not in start_port_output): + self._logger.debug(f"Failed to start port {port_id}. Output was:\n{start_port_output}") + raise InteractiveCommandExecutionError(f"Test pmd failed to start port {port_id}.") + + @stop_then_start_port_decorator + def set_port_mtu(self, port_id: int, mtu: int, verify: bool = True) -> None: + """Change the MTU of a port using testpmd. + + Some PMDs require that the port be stopped before changing the MTU, and it does no harm to + stop the port before configuring in cases where it isn't required, so we first stop ports, + then update the MTU, then start the ports again afterwards. + + Args: + port_id: ID of the port to adjust the MTU on. + mtu: Desired value for the MTU to be set to. + verify: If `verify` is :data:`True` then the output will be scanned in an attempt to + verify that the mtu was properly set on the port. Defaults to :data:`True`. + + Raises: + InteractiveCommandExecutionError: If `verify` is :data:`True` and the MTU was not + properly updated on the port matching `port_id`. + """ + set_mtu_output = self.send_command(f"port config mtu {port_id} {mtu}") + if verify and (f"MTU: {mtu}" not in self.send_command(f"show port info {port_id}")): + self._logger.debug( + f"Failed to set mtu to {mtu} on port {port_id}." f" Output was:\n{set_mtu_output}" + ) + raise InteractiveCommandExecutionError( + f"Test pmd failed to update mtu of port {port_id} to {mtu}" + ) + def _close(self) -> None: """Overrides :meth:`~.interactive_shell.close`.""" self.stop() From patchwork Thu Jun 13 18:15:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeremy Spewock X-Patchwork-Id: 141129 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 0FEED4541F; Thu, 13 Jun 2024 20:16:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B0D29427DA; Thu, 13 Jun 2024 20:15:56 +0200 (CEST) Received: from mail-io1-f103.google.com (mail-io1-f103.google.com [209.85.166.103]) by mails.dpdk.org (Postfix) with ESMTP id 6F764427D2 for ; Thu, 13 Jun 2024 20:15:53 +0200 (CEST) Received: by mail-io1-f103.google.com with SMTP id ca18e2360f4ac-7eba486df76so46567939f.0 for ; Thu, 13 Jun 2024 11:15:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718302553; x=1718907353; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=NeTldC4siRoVk7rZWm+npftl6M9/8F9jB3FO5a7MUKI=; b=aoCiTHR9entncCM5hPj6rhLc7qQ9vWnDN3dNDJ8gLyWUdvC1brZkyXmHHDYSYUeboM rHNPHYJCM894Y8pUlKYHFyNtlLpugLiG9Ek7EwqoAJEK9VhJLCu3f8lgHb9Gj0gjsUXm NDIqXWXkQcZ6rgv+uwccDjgZAFaWilMb+G7e0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718302553; x=1718907353; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NeTldC4siRoVk7rZWm+npftl6M9/8F9jB3FO5a7MUKI=; b=uROK9xzuAX22pN+0kvPm26TJD1XRCRcHNVc0f2vQfUbERTG7x6fZLBLCJJbiSzNN97 7kMu+Nk6GyyhgbCEbdv772WleNbAjVWVwBGvbbgS80IEZG1c/5Tizd6RCHZXJAI9Ghag qj9vuMo67nwO5z1udn0THe/TyhzAeDRDkECU71B2nTanOkDNiWxYOGyujt2OwnlO+yL+ WXlnyalYyHuE3sllBtu16xarQK00sa1+BVeCkpKN1c9/MCHn6yIq2bD+ov7wrM++fSGJ M02Akpgs1X0VScHLCcupOl+5XL1xU+lOD5HNvt15iZyZc6wiwcX/0Co6mEyjt2F8cyDb 8UzA== X-Gm-Message-State: AOJu0YxYYWTk5okge7vaoSqPzAOyCA5wFV0QTKjrSm7Q0s1aAC8el+Xb 1nGyKrq/l1h7UQsMgn6mNGOi+uzRQxUf/7VBU6kaGeTso8qDr6UBPjgJRPslB/pMFORBHIJjTzf u2XKup6lZ6Ol6qB+z81ARHy2sDmM2WKOQ X-Google-Smtp-Source: AGHT+IG2cpJqUkyGdWIdAyivkqkeyu6X0NMmqg6qL3HM/p2VDnuRZEuFnNniQw3Zh3vwClSsttH/6DpUe845 X-Received: by 2002:a05:6602:640a:b0:7eb:7995:e488 with SMTP id ca18e2360f4ac-7ebea3dbefcmr62828039f.6.1718302552682; Thu, 13 Jun 2024 11:15:52 -0700 (PDT) Received: from postal.iol.unh.edu (postal.iol.unh.edu. [132.177.123.84]) by smtp-relay.gmail.com with ESMTPS id ca18e2360f4ac-7ebdbaf11b6sm5813639f.30.2024.06.13.11.15.52 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Jun 2024 11:15:52 -0700 (PDT) X-Relaying-Domain: iol.unh.edu Received: from iol.unh.edu (unknown [IPv6:2606:4100:3880:1257::1083]) by postal.iol.unh.edu (Postfix) with ESMTP id 85266605C380; Thu, 13 Jun 2024 14:15:51 -0400 (EDT) From: jspewock@iol.unh.edu To: juraj.linkes@pantheon.tech, probb@iol.unh.edu, yoan.picchi@foss.arm.com, npratte@iol.unh.edu, Honnappa.Nagarahalli@arm.com, wathsala.vithanage@arm.com, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, thomas@monjalon.net Cc: dev@dpdk.org, Jeremy Spewock Subject: [PATCH v4 4/4] dts: add test case that utilizes offload to pmd_buffer_scatter Date: Thu, 13 Jun 2024 14:15:10 -0400 Message-ID: <20240613181510.30135-5-jspewock@iol.unh.edu> X-Mailer: git-send-email 2.45.1 In-Reply-To: <20240613181510.30135-1-jspewock@iol.unh.edu> References: <20240514201436.2496-1-jspewock@iol.unh.edu> <20240613181510.30135-1-jspewock@iol.unh.edu> 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 From: Jeremy Spewock Some NICs tested in DPDK allow for the scattering of packets without an offload and others enforce that you enable the scattered_rx offload in testpmd. The current version of the suite for testing support of scattering packets only tests the case where the NIC supports testing without the offload, so an expansion of coverage is needed to cover the second case as well. depends-on: patch-139227 ("dts: skip test cases based on capabilities") Signed-off-by: Jeremy Spewock --- dts/tests/TestSuite_pmd_buffer_scatter.py | 75 ++++++++++++++++------- 1 file changed, 53 insertions(+), 22 deletions(-) diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py index 645a66b607..f7bdd4fbcf 100644 --- a/dts/tests/TestSuite_pmd_buffer_scatter.py +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py @@ -16,14 +16,19 @@ """ import struct +from typing import ClassVar from scapy.layers.inet import IP # type: ignore[import] from scapy.layers.l2 import Ether # type: ignore[import] -from scapy.packet import Raw # type: ignore[import] +from scapy.packet import Packet, Raw # type: ignore[import] from scapy.utils import hexstr # type: ignore[import] -from framework.remote_session.testpmd_shell import TestPmdForwardingModes, TestPmdShell -from framework.test_suite import TestSuite +from framework.remote_session.testpmd_shell import ( + NicCapability, + TestPmdForwardingModes, + TestPmdShell, +) +from framework.test_suite import TestSuite, requires class TestPmdBufferScatter(TestSuite): @@ -48,6 +53,14 @@ class TestPmdBufferScatter(TestSuite): and a single byte of packet data stored in a second buffer alongside the CRC. """ + #: Parameters for testing scatter using testpmd which are universal across all test cases. + base_testpmd_parameters: ClassVar[list[str]] = [ + "--mbcache=200", + "--max-pkt-len=9000", + "--port-topology=paired", + "--tx-offloads=0x00008000", + ] + def set_up_suite(self) -> None: """Set up the test suite. @@ -64,19 +77,19 @@ def set_up_suite(self) -> None: self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_egress) self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_ingress) - def scatter_pktgen_send_packet(self, pktsize: int) -> str: + def scatter_pktgen_send_packet(self, pktsize: int) -> list[Packet]: """Generate and send a packet to the SUT then capture what is forwarded back. Generate an IP packet of a specific length and send it to the SUT, - then capture the resulting received packet and extract its payload. - The desired length of the packet is met by packing its payload + then capture the resulting received packets and filter them down to the ones that have the + correct layers. The desired length of the packet is met by packing its payload with the letter "X" in hexadecimal. Args: pktsize: Size of the packet to generate and send. Returns: - The payload of the received packet as a string. + The filtered down list of received packets. """ packet = Ether() / IP() / Raw() packet.getlayer(2).load = "" @@ -86,51 +99,69 @@ def scatter_pktgen_send_packet(self, pktsize: int) -> str: for X_in_hex in payload: packet.load += struct.pack("=B", int("%s%s" % (X_in_hex[0], X_in_hex[1]), 16)) received_packets = self.send_packet_and_capture(packet) + # filter down the list to packets that have the appropriate structure + received_packets = list( + filter(lambda p: Ether in p and IP in p and Raw in p, received_packets) + ) self.verify(len(received_packets) > 0, "Did not receive any packets.") - load = hexstr(received_packets[0].getlayer(2), onlyhex=1) - return load + return received_packets - def pmd_scatter(self, mbsize: int) -> None: + def pmd_scatter(self, mbsize: int, extra_testpmd_params: list[str] = []) -> None: """Testpmd support of receiving and sending scattered multi-segment packets. Support for scattered packets is shown by sending 5 packets of differing length where the length of the packet is calculated by taking mbuf-size + an offset. The offsets used in the test are -1, 0, 1, 4, 5 respectively. + Args: + mbsize: Size to set memory buffers to when starting testpmd. + extra_testpmd_params: Additional parameters to add to the base list when starting + testpmd. + Test: - Start testpmd and run functional test with preset mbsize. + Start testpmd and run functional test with preset `mbsize`. """ testpmd_shell = self.sut_node.create_interactive_shell( TestPmdShell, - app_parameters=( - "--mbcache=200 " - f"--mbuf-size={mbsize} " - "--max-pkt-len=9000 " - "--port-topology=paired " - "--tx-offloads=0x00008000" + app_parameters=" ".join( + [*self.base_testpmd_parameters, f"--mbuf-size={mbsize}", *extra_testpmd_params] ), privileged=True, ) with testpmd_shell as testpmd: testpmd.set_forward_mode(TestPmdForwardingModes.mac) + # adjust the MTU of the SUT ports + for port_id in range(testpmd.number_of_ports): + testpmd.set_port_mtu(port_id, 9000) testpmd.start() for offset in [-1, 0, 1, 4, 5]: - recv_payload = self.scatter_pktgen_send_packet(mbsize + offset) - self._logger.debug( - f"Payload of scattered packet after forwarding: \n{recv_payload}" - ) + recv_packets = self.scatter_pktgen_send_packet(mbsize + offset) + self._logger.debug(f"Relevant captured packets: \n{recv_packets}") + self.verify( - ("58 " * 8).strip() in recv_payload, + any( + " ".join(["58"] * 8) in hexstr(pakt.getlayer(2), onlyhex=1) + for pakt in recv_packets + ), "Payload of scattered packet did not match expected payload with offset " f"{offset}.", ) + testpmd.stop() + # reset the MTU of the SUT ports + for port_id in range(testpmd.number_of_ports): + testpmd.set_port_mtu(port_id, 1500) + @requires(NicCapability.scattered_rx) def test_scatter_mbuf_2048(self) -> None: """Run the :meth:`pmd_scatter` test with `mbsize` set to 2048.""" self.pmd_scatter(mbsize=2048) + def test_scatter_mbuf_2048_with_offload(self) -> None: + """Run the :meth:`pmd_scatter` test with `mbsize` set to 2048 and rx_scatter offload.""" + self.pmd_scatter(mbsize=2048, extra_testpmd_params=["--enable-scatter"]) + def tear_down_suite(self) -> None: """Tear down the test suite.