[PATCH 0/4] dts: add pktgen and testpmd changes

[PATCH 0/4] dts: add pktgen and testpmd changes

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

Hello,

sending some framework changes that will be required in my upcoming
l2fwd test suite.

Best,
Luca

Luca Vizzarro (5):
  dts: add ability to send/receive multiple packets
  dts: add random generation seed setting
  dts: add random packet generator
  dts: add ability to start/stop testpmd ports
  dts: add testpmd set ports queues

 doc/guides/tools/dts.rst                      |   5 +
 dts/framework/config/__init__.py              |   4 +
 dts/framework/config/conf_yaml_schema.json    |   4 +
 dts/framework/config/types.py                 |   2 +
 dts/framework/remote_session/testpmd_shell.py | 102 +++++++++++++++++-
 dts/framework/runner.py                       |   8 ++
 dts/framework/settings.py                     |  17 +++
 dts/framework/test_suite.py                   |  68 +++++++++++-
 dts/framework/testbed_model/tg_node.py        |  14 +--
 .../capturing_traffic_generator.py            |  31 ------
 dts/framework/utils.py                        |  79 +++++++++++++-
 11 files changed, 288 insertions(+), 46 deletions(-)

-- 
2.34.1

[PATCH 1/5] dts: add ability to send/receive multiple packets

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

The framework allows only to send one packet at once via Scapy. This
change adds the ability to send multiple packets, and also introduces a
new fast way to verify if we received several expected packets.

Moreover, it reduces code duplication by keeping a single packet sending
method only at the test suite level.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczpanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 dts/framework/test_suite.py                   | 68 +++++++++++++++++--
 dts/framework/testbed_model/tg_node.py        | 14 ++--
 .../capturing_traffic_generator.py            | 31 ---------
 3 files changed, 71 insertions(+), 42 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..051509fb86 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -13,12 +13,13 @@
     * Test case verification.
 """
 
+from collections import Counter
 from ipaddress import IPv4Interface, IPv6Interface, ip_interface
 from typing import ClassVar, Union
 
 from scapy.layers.inet import IP  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
-from scapy.packet import Packet, Padding  # type: ignore[import-untyped]
+from scapy.packet import Packet, Padding, raw  # type: ignore[import-untyped]
 
 from framework.testbed_model.port import Port, PortLink
 from framework.testbed_model.sut_node import SutNode
@@ -199,9 +200,34 @@ def send_packet_and_capture(
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
-        return self.tg_node.send_packet_and_capture(
-            packet,
+        return self.send_packets_and_capture(
+            [packet],
+            filter_config,
+            duration,
+        )
+
+    def send_packets_and_capture(
+        self,
+        packets: list[Packet],
+        filter_config: PacketFilteringConfig = PacketFilteringConfig(),
+        duration: float = 1,
+    ) -> list[Packet]:
+        """Send and receive `packets` using the associated TG.
+
+        Send `packets` through the appropriate interface and receive on the appropriate interface.
+        Modify the packets with l3/l2 addresses corresponding to the testbed and desired traffic.
+
+        Args:
+            packets: The packets to send.
+            filter_config: The filter to use when capturing packets.
+            duration: Capture traffic for this amount of time after sending `packet`.
+
+        Returns:
+            A list of received packets.
+        """
+        packets = [self._adjust_addresses(packet) for packet in packets]
+        return self.tg_node.send_packets_and_capture(
+            packets,
             self._tg_port_egress,
             self._tg_port_ingress,
             filter_config,
@@ -303,6 +329,40 @@ def verify_packets(self, expected_packet: Packet, received_packets: list[Packet]
             )
             self._fail_test_case_verify("An expected packet not found among received packets.")
 
+    def match_all_packets(
+        self, expected_packets: list[Packet], received_packets: list[Packet]
+    ) -> None:
+        """Matches all the expected packets against the received ones.
+
+        Matching is performed by counting down the occurrences in a dictionary which keys are the
+        raw packet bytes. No deep packet comparison is performed. All the unexpected packets (noise)
+        are automatically ignored.
+
+        Args:
+            expected_packets: The packets we are expecting to receive.
+            received_packets: All the packets that were received.
+
+        Raises:
+            TestCaseVerifyError: if and not all the `expected_packets` were found in
+                `received_packets`.
+        """
+        expected_packets_counters = Counter(map(raw, expected_packets))
+        received_packets_counters = Counter(map(raw, received_packets))
+        # The number of expected packets is subtracted by the number of received packets, ignoring
+        # any unexpected packets and capping at zero.
+        missing_packets_counters = expected_packets_counters - received_packets_counters
+        missing_packets_count = missing_packets_counters.total()
+        self._logger.debug(
+            f"match_all_packets: expected {len(expected_packets)}, "
+            f"received {len(received_packets)}, missing {missing_packets_count}"
+        )
+
+        if missing_packets_count != 0:
+            self._fail_test_case_verify(
+                f"Not all packets were received, expected {len(expected_packets)} "
+                f"but {missing_packets_count} were missing."
+            )
+
     def _compare_packets(self, expected_packet: Packet, received_packet: Packet) -> bool:
         self._logger.debug(
             f"Comparing packets: \n{expected_packet.summary()}\n{received_packet.summary()}"
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 4ee326e99c..19b5b6e74c 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -51,22 +51,22 @@ def __init__(self, node_config: TGNodeConfiguration):
         self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
         self._logger.info(f"Created node: {self.name}")
 
-    def send_packet_and_capture(
+    def send_packets_and_capture(
         self,
-        packet: Packet,
+        packets: list[Packet],
         send_port: Port,
         receive_port: Port,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
     ) -> list[Packet]:
-        """Send `packet`, return received traffic.
+        """Send `packets`, return received traffic.
 
-        Send `packet` on `send_port` and then return all traffic captured
+        Send `packets` on `send_port` and then return all traffic captured
         on `receive_port` for the given duration. Also record the captured traffic
         in a pcap file.
 
         Args:
-            packet: The packet to send.
+            packets: The packets to send.
             send_port: The egress port on the TG node.
             receive_port: The ingress port in the TG node.
             filter_config: The filter to use when capturing packets.
@@ -75,8 +75,8 @@ def send_packet_and_capture(
         Returns:
              A list of received packets. May be empty if no packets are captured.
         """
-        return self.traffic_generator.send_packet_and_capture(
-            packet,
+        return self.traffic_generator.send_packets_and_capture(
+            packets,
             send_port,
             receive_port,
             filter_config,
diff --git a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
index c8380b7d57..66a77da9c4 100644
--- a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
@@ -63,37 +63,6 @@ def is_capturing(self) -> bool:
         """This traffic generator can capture traffic."""
         return True
 
-    def send_packet_and_capture(
-        self,
-        packet: Packet,
-        send_port: Port,
-        receive_port: Port,
-        filter_config: PacketFilteringConfig,
-        duration: float,
-        capture_name: str = _get_default_capture_name(),
-    ) -> list[Packet]:
-        """Send `packet` and capture received traffic.
-
-        Send `packet` on `send_port` and then return all traffic captured
-        on `receive_port` for the given `duration`.
-
-        The captured traffic is recorded in the `capture_name`.pcap file.
-
-        Args:
-            packet: The packet to send.
-            send_port: The egress port on the TG node.
-            receive_port: The ingress port in the TG node.
-            filter_config: Filters to apply when capturing packets.
-            duration: Capture traffic for this amount of time after sending the packet.
-            capture_name: The name of the .pcap file where to store the capture.
-
-        Returns:
-             The received packets. May be empty if no packets are captured.
-        """
-        return self.send_packets_and_capture(
-            [packet], send_port, receive_port, filter_config, duration, capture_name
-        )
-
     def send_packets_and_capture(
         self,
         packets: list[Packet],
-- 
2.34.1

[PATCH 2/5] dts: add random generation seed setting

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

When introducing pseudo-random generation in the test runs we need to
ensure that these can be reproduced by setting a pre-defined seed.
This commits adds the ability to set one or allow for one to be
generated and reported back to the user.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczpanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 doc/guides/tools/dts.rst                   |  5 +++++
 dts/framework/config/__init__.py           |  4 ++++
 dts/framework/config/conf_yaml_schema.json |  4 ++++
 dts/framework/config/types.py              |  2 ++
 dts/framework/runner.py                    |  8 ++++++++
 dts/framework/settings.py                  | 17 +++++++++++++++++
 6 files changed, 40 insertions(+)

diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 515b15e4d8..9b5ea9779c 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -251,6 +251,8 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet
                            ... | DTS_TEST_SUITES='suite, suite case, ...' (default: [])
      --re-run N_TIMES, --re_run N_TIMES
                            [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0)
+     --random-seed NUMBER  [DTS_RANDOM_SEED] The seed to use with the pseudo-random generator. If not specified, the configuration value is
+                           used instead. If that's also not specified, a random seed is generated. (default: None)
 
 
 The brackets contain the names of environment variables that set the same thing.
@@ -548,6 +550,9 @@ involved in the testing. These can be defined with the following mappings:
    +----------------------------+---------------+---------------------------------------------------+
    | ``traffic_generator_node`` | Node name for the traffic generator node.                         |
    +----------------------------+-------------------------------------------------------------------+
+   | ``random_seed``            | (*optional*) *int* – Allows you to set a seed for pseudo-random   |
+   |                            | generation.                                                       |
+   +----------------------------+-------------------------------------------------------------------+
 
 ``nodes``
    `sequence <https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range>`_ listing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..269d9ec318 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -445,6 +445,7 @@ class TestRunConfiguration:
         system_under_test_node: The SUT node to use in this test run.
         traffic_generator_node: The TG node to use in this test run.
         vdevs: The names of virtual devices to test.
+        random_seed: The seed to use for pseudo-random generation.
     """
 
     build_targets: list[BuildTargetConfiguration]
@@ -455,6 +456,7 @@ class TestRunConfiguration:
     system_under_test_node: SutNodeConfiguration
     traffic_generator_node: TGNodeConfiguration
     vdevs: list[str]
+    random_seed: int | None
 
     @classmethod
     def from_dict(
@@ -497,6 +499,7 @@ def from_dict(
         vdevs = (
             d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
         )
+        random_seed = d.get("random_seed", None)
         return cls(
             build_targets=build_targets,
             perf=d["perf"],
@@ -506,6 +509,7 @@ def from_dict(
             system_under_test_node=system_under_test_node,
             traffic_generator_node=traffic_generator_node,
             vdevs=vdevs,
+            random_seed=random_seed,
         )
 
     def copy_and_modify(self, **kwargs) -> Self:
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..df390e8ae2 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -379,6 +379,10 @@
           },
           "traffic_generator_node": {
             "$ref": "#/definitions/node_name"
+          },
+          "random_seed": {
+            "type": "integer",
+            "description": "Optional field. Allows you to set a seed for pseudo-random generation."
           }
         },
         "additionalProperties": false,
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index cf16556403..ce7b784ac8 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -121,6 +121,8 @@ class TestRunConfigDict(TypedDict):
     system_under_test_node: TestRunSUTConfigDict
     #:
     traffic_generator_node: str
+    #:
+    random_seed: int
 
 
 class ConfigurationDict(TypedDict):
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..34b1dad5c4 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -20,6 +20,7 @@
 import importlib
 import inspect
 import os
+import random
 import re
 import sys
 from pathlib import Path
@@ -147,6 +148,7 @@ def run(self) -> None:
                 self._logger.info(
                     f"Running test run with SUT '{test_run_config.system_under_test_node.name}'."
                 )
+                self._init_random_seed(test_run_config)
                 test_run_result = self._result.add_test_run(test_run_config)
                 # we don't want to modify the original config, so create a copy
                 test_run_test_suites = list(
@@ -723,3 +725,9 @@ def _exit_dts(self) -> None:
             self._logger.info("DTS execution has ended.")
 
         sys.exit(self._result.get_return_code())
+
+    def _init_random_seed(self, conf: TestRunConfiguration) -> None:
+        """Initialize the random seed to use for the test run."""
+        seed = SETTINGS.random_seed or conf.random_seed or random.randrange(0xFFFF_FFFF)
+        self._logger.info(f"Initializing test run with random seed {seed}")
+        random.seed(seed)
diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index f6303066d4..7744e37f54 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -66,6 +66,12 @@
 
     Re-run each test case this many times in case of a failure.
 
+.. option:: --random-seed
+.. envvar:: DTS_RANDOM_SEED
+
+    The seed to use with the pseudo-random generator. If not specified, the configuration value is
+    used instead. If that's also not specified, a random seed is generated.
+
 The module provides one key module-level variable:
 
 Attributes:
@@ -115,6 +121,8 @@ class Settings:
     test_suites: list[TestSuiteConfig] = field(default_factory=list)
     #:
     re_run: int = 0
+    #:
+    random_seed: int | None = None
 
 
 SETTINGS: Settings = Settings()
@@ -375,6 +383,15 @@ def _get_parser() -> _DTSArgumentParser:
     )
     _add_env_var_to_action(action, "RERUN")
 
+    action = parser.add_argument(
+        "--random-seed",
+        type=int,
+        help="The seed to use with the pseudo-random generator. If not specified, the configuration"
+        " value is used instead. If that's also not specified, a random seed is generated.",
+        metavar="NUMBER",
+    )
+    _add_env_var_to_action(action)
+
     return parser
 
 
-- 
2.34.1

[PATCH 3/5] dts: add random packet generator

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

Add a basic utility that can create random L3 and L4 packets with random
payloads and port numbers (if L4).

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczpanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 dts/framework/utils.py | 79 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index 6b5d5a805f..c768dd0c99 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -17,14 +17,16 @@
 import atexit
 import json
 import os
+import random
 import subprocess
-from enum import Enum
+from enum import Enum, Flag
 from pathlib import Path
 from subprocess import SubprocessError
 
+from scapy.layers.inet import IP, TCP, UDP, Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
-from .exception import ConfigurationError
+from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
 
@@ -244,3 +246,76 @@ def _delete_tarball(self) -> None:
     def __fspath__(self) -> str:
         """The os.PathLike protocol implementation."""
         return str(self._tarball_path)
+
+
+class PacketProtocols(Flag):
+    """Flag specifying which protocols to use for packet generation."""
+
+    #:
+    IP = 1
+    #:
+    TCP = 2 | IP
+    #:
+    UDP = 4 | IP
+    #:
+    ALL = TCP | UDP
+
+
+def generate_random_packets(
+    number_of: int,
+    payload_size: int = 1500,
+    protocols: PacketProtocols = PacketProtocols.ALL,
+    ports_range: range = range(1024, 49152),
+    mtu: int = 1500,
+) -> list[Packet]:
+    """Generate a number of random packets.
+
+    The payload of the packets will consist of random bytes. If `payload_size` is too big, then the
+    maximum payload size allowed for the specific packet type is used. The size is calculated based
+    on the specified `mtu`, therefore it is essential that `mtu` is set correctly to match the MTU
+    of the port that will send out the generated packets.
+
+    If `protocols` has any L4 protocol enabled then all the packets are generated with any of
+    the specified L4 protocols chosen at random. If only :attr:`~PacketProtocols.IP` is set, then
+    only L3 packets are generated.
+
+    If L4 packets will be generated, then the TCP/UDP ports to be used will be chosen at random from
+    `ports_range`.
+
+    Args:
+        number_of: The number of packets to generate.
+        payload_size: The packet payload size to generate, capped based on `mtu`.
+        protocols: The protocols to use for the generated packets.
+        ports_range: The range of L4 port numbers to use. Used only if `protocols` has L4 protocols.
+        mtu: The MTU of the NIC port that will send out the generated packets.
+
+    Raises:
+        InternalError: If the `payload_size` is invalid.
+
+    Returns:
+        A list containing the randomly generated packets.
+    """
+    if payload_size < 0:
+        raise InternalError(f"An invalid payload_size of {payload_size} was given.")
+
+    l4_factories = []
+    if protocols & PacketProtocols.TCP:
+        l4_factories.append(TCP)
+    if protocols & PacketProtocols.UDP:
+        l4_factories.append(UDP)
+
+    def _make_packet() -> Packet:
+        packet = Ether()
+
+        if protocols & PacketProtocols.IP:
+            packet /= IP()
+
+        if len(l4_factories) > 0:
+            src_port, dst_port = random.choices(ports_range, k=2)
+            packet /= random.choice(l4_factories)(sport=src_port, dport=dst_port)
+
+        max_payload_size = mtu - len(packet)
+        usable_payload_size = payload_size if payload_size < max_payload_size else max_payload_size
+        return packet / random.randbytes(usable_payload_size)
+
+    return [_make_packet() for _ in range(number_of)]
-- 
2.34.1

[PATCH 4/5] dts: add ability to start/stop ports

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

Add the commands to start and stop all the ports, so that they can be
configured. Because there is a distinction of commands that require the
ports to be stopped and started, also add decorators for commands that
require a specific state, removing this logic from the test writer's
duty.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczpanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 86 ++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index eda6eb320f..293c7b9dff 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -14,16 +14,17 @@
     testpmd_shell.close()
 """
 
+import functools
 import re
 import time
 from dataclasses import dataclass, field
 from enum import Flag, auto
 from pathlib import PurePath
-from typing import ClassVar
+from typing import Any, Callable, ClassVar, Concatenate, ParamSpec
 
 from typing_extensions import Self, Unpack
 
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import InteractiveCommandExecutionError, InternalError
 from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
 from framework.params.types import TestPmdParamsDict
 from framework.parser import ParserFn, TextParser
@@ -33,6 +34,9 @@
 from framework.testbed_model.sut_node import SutNode
 from framework.utils import StrEnum
 
+P = ParamSpec("P")
+TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]
+
 
 class TestPmdDevice:
     """The data of a device that testpmd can recognize.
@@ -577,12 +581,51 @@ class TestPmdPortStats(TextParser):
     tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
 
 
+def requires_stopped_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require stopped ports.
+
+    If the decorated method is called while the ports are started, then these are stopped before
+    continuing.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if self.ports_started:
+            self._logger.debug("Ports need to be stopped to continue")
+            self.stop_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
+def requires_started_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require started ports.
+
+    If the decorated method is called while the ports are stopped, then these are started before
+    continuing.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if not self.ports_started:
+            self._logger.debug("Ports need to be started to continue")
+            self.start_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
 class TestPmdShell(DPDKShell):
     """Testpmd interactive shell.
 
     The testpmd shell users should never use
     the :meth:`~.interactive_shell.InteractiveShell.send_command` method directly, but rather
     call specialized methods. If there isn't one that satisfies a need, it should be added.
+
+    Attributes:
+        ports_started: Indicates whether the ports are started.
     """
 
     _app_params: TestPmdParams
@@ -617,6 +660,9 @@ def __init__(
             TestPmdParams(**app_params),
         )
 
+        self.ports_started = not self._app_params.disable_device_start
+
+    @requires_started_ports
     def start(self, verify: bool = True) -> None:
         """Start packet forwarding with the current configuration.
 
@@ -721,6 +767,42 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def stop_all_ports(self, verify: bool = True) -> None:
+        """Stops all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                stopped successfully.
+        """
+        self._logger.debug("Stopping all the ports...")
+        output = self.send_command("port stop all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not stopped successfully")
+
+        self.ports_started = False
+
+    def start_all_ports(self, verify: bool = True) -> None:
+        """Starts all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                started successfully.
+        """
+        self._logger.debug("Starting all the ports...")
+        output = self.send_command("port start all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not started successfully")
+
+        self.ports_started = True
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

[PATCH 4/5] dts: add ability to start/stop testpmd ports

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

Add testpmd commands to start and stop all the ports, so that they can
be configured. Because there is a distinction of commands that require
the ports to be stopped and started, also add decorators for commands
that require a specific state, removing this logic from the test
writer's duty.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczpanek <paul.szczepanek@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 86 ++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index eda6eb320f..293c7b9dff 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -14,16 +14,17 @@
     testpmd_shell.close()
 """
 
+import functools
 import re
 import time
 from dataclasses import dataclass, field
 from enum import Flag, auto
 from pathlib import PurePath
-from typing import ClassVar
+from typing import Any, Callable, ClassVar, Concatenate, ParamSpec
 
 from typing_extensions import Self, Unpack
 
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import InteractiveCommandExecutionError, InternalError
 from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
 from framework.params.types import TestPmdParamsDict
 from framework.parser import ParserFn, TextParser
@@ -33,6 +34,9 @@
 from framework.testbed_model.sut_node import SutNode
 from framework.utils import StrEnum
 
+P = ParamSpec("P")
+TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]
+
 
 class TestPmdDevice:
     """The data of a device that testpmd can recognize.
@@ -577,12 +581,51 @@ class TestPmdPortStats(TextParser):
     tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
 
 
+def requires_stopped_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require stopped ports.
+
+    If the decorated method is called while the ports are started, then these are stopped before
+    continuing.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if self.ports_started:
+            self._logger.debug("Ports need to be stopped to continue")
+            self.stop_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
+def requires_started_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require started ports.
+
+    If the decorated method is called while the ports are stopped, then these are started before
+    continuing.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if not self.ports_started:
+            self._logger.debug("Ports need to be started to continue")
+            self.start_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
 class TestPmdShell(DPDKShell):
     """Testpmd interactive shell.
 
     The testpmd shell users should never use
     the :meth:`~.interactive_shell.InteractiveShell.send_command` method directly, but rather
     call specialized methods. If there isn't one that satisfies a need, it should be added.
+
+    Attributes:
+        ports_started: Indicates whether the ports are started.
     """
 
     _app_params: TestPmdParams
@@ -617,6 +660,9 @@ def __init__(
             TestPmdParams(**app_params),
         )
 
+        self.ports_started = not self._app_params.disable_device_start
+
+    @requires_started_ports
     def start(self, verify: bool = True) -> None:
         """Start packet forwarding with the current configuration.
 
@@ -721,6 +767,42 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def stop_all_ports(self, verify: bool = True) -> None:
+        """Stops all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                stopped successfully.
+        """
+        self._logger.debug("Stopping all the ports...")
+        output = self.send_command("port stop all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not stopped successfully")
+
+        self.ports_started = False
+
+    def start_all_ports(self, verify: bool = True) -> None:
+        """Starts all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                started successfully.
+        """
+        self._logger.debug("Starting all the ports...")
+        output = self.send_command("port start all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not started successfully")
+
+        self.ports_started = True
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

[PATCH 5/5] dts: add testpmd set ports queues

[Luca Vizzarro]

From: Luca Vizzarro <luca.vizzarro@arm.com>

Add a facility to update the number of TX/RX queues during the runtime
of testpmd.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczpanek <paul.szczepanek@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 293c7b9dff..40e850502c 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -803,6 +803,22 @@ def start_all_ports(self, verify: bool = True) -> None:
 
         self.ports_started = True
 
+    @requires_stopped_ports
+    def set_ports_queues(self, number_of: int) -> None:
+        """Sets the number of queues per port.
+
+        Args:
+            number_of: The number of RX/TX queues to create per port.
+
+        Raises:
+            InternalError: If `number_of` is invalid.
+        """
+        if number_of < 1:
+            raise InternalError("The number of queues must be positive and non-zero")
+
+        self.send_command(f"port config all rxq {number_of}")
+        self.send_command(f"port config all txq {number_of}")
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

[PATCH v2 0/5] dts: add pktgen and testpmd changes

[Luca Vizzarro]

Apologies, re-sending again due to errors in sending v1.

v2:
- rebased

Luca Vizzarro (5):
  dts: add ability to send/receive multiple packets
  dts: add random generation seed setting
  dts: add random packet generator
  dts: add ability to start/stop testpmd ports
  dts: add testpmd set ports queues

 doc/guides/tools/dts.rst                      |   5 +
 dts/framework/config/__init__.py              |   4 +
 dts/framework/config/conf_yaml_schema.json    |   4 +
 dts/framework/config/types.py                 |   2 +
 dts/framework/remote_session/testpmd_shell.py | 102 +++++++++++++++++-
 dts/framework/runner.py                       |   8 ++
 dts/framework/settings.py                     |  17 +++
 dts/framework/test_suite.py                   |  68 +++++++++++-
 dts/framework/testbed_model/tg_node.py        |  14 +--
 .../capturing_traffic_generator.py            |  31 ------
 dts/framework/utils.py                        |  79 +++++++++++++-
 11 files changed, 288 insertions(+), 46 deletions(-)

-- 
2.34.1

[PATCH v2 1/5] dts: add ability to send/receive multiple packets

[Luca Vizzarro]

The framework allows only to send one packet at once via Scapy. This
change adds the ability to send multiple packets, and also introduces a
new fast way to verify if we received several expected packets.

Moreover, it reduces code duplication by keeping a single packet sending
method only at the test suite level.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 dts/framework/test_suite.py                   | 68 +++++++++++++++++--
 dts/framework/testbed_model/tg_node.py        | 14 ++--
 .../capturing_traffic_generator.py            | 31 ---------
 3 files changed, 71 insertions(+), 42 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..051509fb86 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -13,12 +13,13 @@
     * Test case verification.
 """
 
+from collections import Counter
 from ipaddress import IPv4Interface, IPv6Interface, ip_interface
 from typing import ClassVar, Union
 
 from scapy.layers.inet import IP  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
-from scapy.packet import Packet, Padding  # type: ignore[import-untyped]
+from scapy.packet import Packet, Padding, raw  # type: ignore[import-untyped]
 
 from framework.testbed_model.port import Port, PortLink
 from framework.testbed_model.sut_node import SutNode
@@ -199,9 +200,34 @@ def send_packet_and_capture(
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
-        return self.tg_node.send_packet_and_capture(
-            packet,
+        return self.send_packets_and_capture(
+            [packet],
+            filter_config,
+            duration,
+        )
+
+    def send_packets_and_capture(
+        self,
+        packets: list[Packet],
+        filter_config: PacketFilteringConfig = PacketFilteringConfig(),
+        duration: float = 1,
+    ) -> list[Packet]:
+        """Send and receive `packets` using the associated TG.
+
+        Send `packets` through the appropriate interface and receive on the appropriate interface.
+        Modify the packets with l3/l2 addresses corresponding to the testbed and desired traffic.
+
+        Args:
+            packets: The packets to send.
+            filter_config: The filter to use when capturing packets.
+            duration: Capture traffic for this amount of time after sending `packet`.
+
+        Returns:
+            A list of received packets.
+        """
+        packets = [self._adjust_addresses(packet) for packet in packets]
+        return self.tg_node.send_packets_and_capture(
+            packets,
             self._tg_port_egress,
             self._tg_port_ingress,
             filter_config,
@@ -303,6 +329,40 @@ def verify_packets(self, expected_packet: Packet, received_packets: list[Packet]
             )
             self._fail_test_case_verify("An expected packet not found among received packets.")
 
+    def match_all_packets(
+        self, expected_packets: list[Packet], received_packets: list[Packet]
+    ) -> None:
+        """Matches all the expected packets against the received ones.
+
+        Matching is performed by counting down the occurrences in a dictionary which keys are the
+        raw packet bytes. No deep packet comparison is performed. All the unexpected packets (noise)
+        are automatically ignored.
+
+        Args:
+            expected_packets: The packets we are expecting to receive.
+            received_packets: All the packets that were received.
+
+        Raises:
+            TestCaseVerifyError: if and not all the `expected_packets` were found in
+                `received_packets`.
+        """
+        expected_packets_counters = Counter(map(raw, expected_packets))
+        received_packets_counters = Counter(map(raw, received_packets))
+        # The number of expected packets is subtracted by the number of received packets, ignoring
+        # any unexpected packets and capping at zero.
+        missing_packets_counters = expected_packets_counters - received_packets_counters
+        missing_packets_count = missing_packets_counters.total()
+        self._logger.debug(
+            f"match_all_packets: expected {len(expected_packets)}, "
+            f"received {len(received_packets)}, missing {missing_packets_count}"
+        )
+
+        if missing_packets_count != 0:
+            self._fail_test_case_verify(
+                f"Not all packets were received, expected {len(expected_packets)} "
+                f"but {missing_packets_count} were missing."
+            )
+
     def _compare_packets(self, expected_packet: Packet, received_packet: Packet) -> bool:
         self._logger.debug(
             f"Comparing packets: \n{expected_packet.summary()}\n{received_packet.summary()}"
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 4ee326e99c..19b5b6e74c 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -51,22 +51,22 @@ def __init__(self, node_config: TGNodeConfiguration):
         self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
         self._logger.info(f"Created node: {self.name}")
 
-    def send_packet_and_capture(
+    def send_packets_and_capture(
         self,
-        packet: Packet,
+        packets: list[Packet],
         send_port: Port,
         receive_port: Port,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
     ) -> list[Packet]:
-        """Send `packet`, return received traffic.
+        """Send `packets`, return received traffic.
 
-        Send `packet` on `send_port` and then return all traffic captured
+        Send `packets` on `send_port` and then return all traffic captured
         on `receive_port` for the given duration. Also record the captured traffic
         in a pcap file.
 
         Args:
-            packet: The packet to send.
+            packets: The packets to send.
             send_port: The egress port on the TG node.
             receive_port: The ingress port in the TG node.
             filter_config: The filter to use when capturing packets.
@@ -75,8 +75,8 @@ def send_packet_and_capture(
         Returns:
              A list of received packets. May be empty if no packets are captured.
         """
-        return self.traffic_generator.send_packet_and_capture(
-            packet,
+        return self.traffic_generator.send_packets_and_capture(
+            packets,
             send_port,
             receive_port,
             filter_config,
diff --git a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
index c8380b7d57..66a77da9c4 100644
--- a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
@@ -63,37 +63,6 @@ def is_capturing(self) -> bool:
         """This traffic generator can capture traffic."""
         return True
 
-    def send_packet_and_capture(
-        self,
-        packet: Packet,
-        send_port: Port,
-        receive_port: Port,
-        filter_config: PacketFilteringConfig,
-        duration: float,
-        capture_name: str = _get_default_capture_name(),
-    ) -> list[Packet]:
-        """Send `packet` and capture received traffic.
-
-        Send `packet` on `send_port` and then return all traffic captured
-        on `receive_port` for the given `duration`.
-
-        The captured traffic is recorded in the `capture_name`.pcap file.
-
-        Args:
-            packet: The packet to send.
-            send_port: The egress port on the TG node.
-            receive_port: The ingress port in the TG node.
-            filter_config: Filters to apply when capturing packets.
-            duration: Capture traffic for this amount of time after sending the packet.
-            capture_name: The name of the .pcap file where to store the capture.
-
-        Returns:
-             The received packets. May be empty if no packets are captured.
-        """
-        return self.send_packets_and_capture(
-            [packet], send_port, receive_port, filter_config, duration, capture_name
-        )
-
     def send_packets_and_capture(
         self,
         packets: list[Packet],
-- 
2.34.1

[PATCH v2 2/5] dts: add random generation seed setting

[Luca Vizzarro]

When introducing pseudo-random generation in the test runs we need to
ensure that these can be reproduced by setting a pre-defined seed.
This commits adds the ability to set one or allow for one to be
generated and reported back to the user.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 doc/guides/tools/dts.rst                   |  5 +++++
 dts/framework/config/__init__.py           |  4 ++++
 dts/framework/config/conf_yaml_schema.json |  4 ++++
 dts/framework/config/types.py              |  2 ++
 dts/framework/runner.py                    |  8 ++++++++
 dts/framework/settings.py                  | 17 +++++++++++++++++
 6 files changed, 40 insertions(+)

diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 515b15e4d8..9b5ea9779c 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -251,6 +251,8 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet
                            ... | DTS_TEST_SUITES='suite, suite case, ...' (default: [])
      --re-run N_TIMES, --re_run N_TIMES
                            [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0)
+     --random-seed NUMBER  [DTS_RANDOM_SEED] The seed to use with the pseudo-random generator. If not specified, the configuration value is
+                           used instead. If that's also not specified, a random seed is generated. (default: None)
 
 
 The brackets contain the names of environment variables that set the same thing.
@@ -548,6 +550,9 @@ involved in the testing. These can be defined with the following mappings:
    +----------------------------+---------------+---------------------------------------------------+
    | ``traffic_generator_node`` | Node name for the traffic generator node.                         |
    +----------------------------+-------------------------------------------------------------------+
+   | ``random_seed``            | (*optional*) *int* – Allows you to set a seed for pseudo-random   |
+   |                            | generation.                                                       |
+   +----------------------------+-------------------------------------------------------------------+
 
 ``nodes``
    `sequence <https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range>`_ listing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..269d9ec318 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -445,6 +445,7 @@ class TestRunConfiguration:
         system_under_test_node: The SUT node to use in this test run.
         traffic_generator_node: The TG node to use in this test run.
         vdevs: The names of virtual devices to test.
+        random_seed: The seed to use for pseudo-random generation.
     """
 
     build_targets: list[BuildTargetConfiguration]
@@ -455,6 +456,7 @@ class TestRunConfiguration:
     system_under_test_node: SutNodeConfiguration
     traffic_generator_node: TGNodeConfiguration
     vdevs: list[str]
+    random_seed: int | None
 
     @classmethod
     def from_dict(
@@ -497,6 +499,7 @@ def from_dict(
         vdevs = (
             d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
         )
+        random_seed = d.get("random_seed", None)
         return cls(
             build_targets=build_targets,
             perf=d["perf"],
@@ -506,6 +509,7 @@ def from_dict(
             system_under_test_node=system_under_test_node,
             traffic_generator_node=traffic_generator_node,
             vdevs=vdevs,
+            random_seed=random_seed,
         )
 
     def copy_and_modify(self, **kwargs) -> Self:
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..df390e8ae2 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -379,6 +379,10 @@
           },
           "traffic_generator_node": {
             "$ref": "#/definitions/node_name"
+          },
+          "random_seed": {
+            "type": "integer",
+            "description": "Optional field. Allows you to set a seed for pseudo-random generation."
           }
         },
         "additionalProperties": false,
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index cf16556403..ce7b784ac8 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -121,6 +121,8 @@ class TestRunConfigDict(TypedDict):
     system_under_test_node: TestRunSUTConfigDict
     #:
     traffic_generator_node: str
+    #:
+    random_seed: int
 
 
 class ConfigurationDict(TypedDict):
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..34b1dad5c4 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -20,6 +20,7 @@
 import importlib
 import inspect
 import os
+import random
 import re
 import sys
 from pathlib import Path
@@ -147,6 +148,7 @@ def run(self) -> None:
                 self._logger.info(
                     f"Running test run with SUT '{test_run_config.system_under_test_node.name}'."
                 )
+                self._init_random_seed(test_run_config)
                 test_run_result = self._result.add_test_run(test_run_config)
                 # we don't want to modify the original config, so create a copy
                 test_run_test_suites = list(
@@ -723,3 +725,9 @@ def _exit_dts(self) -> None:
             self._logger.info("DTS execution has ended.")
 
         sys.exit(self._result.get_return_code())
+
+    def _init_random_seed(self, conf: TestRunConfiguration) -> None:
+        """Initialize the random seed to use for the test run."""
+        seed = SETTINGS.random_seed or conf.random_seed or random.randrange(0xFFFF_FFFF)
+        self._logger.info(f"Initializing test run with random seed {seed}")
+        random.seed(seed)
diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index f6303066d4..7744e37f54 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -66,6 +66,12 @@
 
     Re-run each test case this many times in case of a failure.
 
+.. option:: --random-seed
+.. envvar:: DTS_RANDOM_SEED
+
+    The seed to use with the pseudo-random generator. If not specified, the configuration value is
+    used instead. If that's also not specified, a random seed is generated.
+
 The module provides one key module-level variable:
 
 Attributes:
@@ -115,6 +121,8 @@ class Settings:
     test_suites: list[TestSuiteConfig] = field(default_factory=list)
     #:
     re_run: int = 0
+    #:
+    random_seed: int | None = None
 
 
 SETTINGS: Settings = Settings()
@@ -375,6 +383,15 @@ def _get_parser() -> _DTSArgumentParser:
     )
     _add_env_var_to_action(action, "RERUN")
 
+    action = parser.add_argument(
+        "--random-seed",
+        type=int,
+        help="The seed to use with the pseudo-random generator. If not specified, the configuration"
+        " value is used instead. If that's also not specified, a random seed is generated.",
+        metavar="NUMBER",
+    )
+    _add_env_var_to_action(action)
+
     return parser
 
 
-- 
2.34.1

[PATCH v2 3/5] dts: add random packet generator

[Luca Vizzarro]

Add a basic utility that can create random L3 and L4 packets with random
payloads and port numbers (if L4).

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
---
 dts/framework/utils.py | 79 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index 6b5d5a805f..c768dd0c99 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -17,14 +17,16 @@
 import atexit
 import json
 import os
+import random
 import subprocess
-from enum import Enum
+from enum import Enum, Flag
 from pathlib import Path
 from subprocess import SubprocessError
 
+from scapy.layers.inet import IP, TCP, UDP, Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
-from .exception import ConfigurationError
+from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
 
@@ -244,3 +246,76 @@ def _delete_tarball(self) -> None:
     def __fspath__(self) -> str:
         """The os.PathLike protocol implementation."""
         return str(self._tarball_path)
+
+
+class PacketProtocols(Flag):
+    """Flag specifying which protocols to use for packet generation."""
+
+    #:
+    IP = 1
+    #:
+    TCP = 2 | IP
+    #:
+    UDP = 4 | IP
+    #:
+    ALL = TCP | UDP
+
+
+def generate_random_packets(
+    number_of: int,
+    payload_size: int = 1500,
+    protocols: PacketProtocols = PacketProtocols.ALL,
+    ports_range: range = range(1024, 49152),
+    mtu: int = 1500,
+) -> list[Packet]:
+    """Generate a number of random packets.
+
+    The payload of the packets will consist of random bytes. If `payload_size` is too big, then the
+    maximum payload size allowed for the specific packet type is used. The size is calculated based
+    on the specified `mtu`, therefore it is essential that `mtu` is set correctly to match the MTU
+    of the port that will send out the generated packets.
+
+    If `protocols` has any L4 protocol enabled then all the packets are generated with any of
+    the specified L4 protocols chosen at random. If only :attr:`~PacketProtocols.IP` is set, then
+    only L3 packets are generated.
+
+    If L4 packets will be generated, then the TCP/UDP ports to be used will be chosen at random from
+    `ports_range`.
+
+    Args:
+        number_of: The number of packets to generate.
+        payload_size: The packet payload size to generate, capped based on `mtu`.
+        protocols: The protocols to use for the generated packets.
+        ports_range: The range of L4 port numbers to use. Used only if `protocols` has L4 protocols.
+        mtu: The MTU of the NIC port that will send out the generated packets.
+
+    Raises:
+        InternalError: If the `payload_size` is invalid.
+
+    Returns:
+        A list containing the randomly generated packets.
+    """
+    if payload_size < 0:
+        raise InternalError(f"An invalid payload_size of {payload_size} was given.")
+
+    l4_factories = []
+    if protocols & PacketProtocols.TCP:
+        l4_factories.append(TCP)
+    if protocols & PacketProtocols.UDP:
+        l4_factories.append(UDP)
+
+    def _make_packet() -> Packet:
+        packet = Ether()
+
+        if protocols & PacketProtocols.IP:
+            packet /= IP()
+
+        if len(l4_factories) > 0:
+            src_port, dst_port = random.choices(ports_range, k=2)
+            packet /= random.choice(l4_factories)(sport=src_port, dport=dst_port)
+
+        max_payload_size = mtu - len(packet)
+        usable_payload_size = payload_size if payload_size < max_payload_size else max_payload_size
+        return packet / random.randbytes(usable_payload_size)
+
+    return [_make_packet() for _ in range(number_of)]
-- 
2.34.1

[PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Luca Vizzarro]

Add testpmd commands to start and stop all the ports, so that they can
be configured. Because there is a distinction of commands that require
the ports to be stopped and started, also add decorators for commands
that require a specific state, removing this logic from the test
writer's duty.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 86 ++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 43e9f56517..ca24b28070 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -14,16 +14,17 @@
     testpmd_shell.close()
 """
 
+import functools
 import re
 import time
 from dataclasses import dataclass, field
 from enum import Flag, auto
 from pathlib import PurePath
-from typing import ClassVar
+from typing import Any, Callable, ClassVar, Concatenate, ParamSpec
 
 from typing_extensions import Self, Unpack
 
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import InteractiveCommandExecutionError, InternalError
 from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
 from framework.params.types import TestPmdParamsDict
 from framework.parser import ParserFn, TextParser
@@ -33,6 +34,9 @@
 from framework.testbed_model.sut_node import SutNode
 from framework.utils import StrEnum
 
+P = ParamSpec("P")
+TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]
+
 
 class TestPmdDevice:
     """The data of a device that testpmd can recognize.
@@ -577,12 +581,51 @@ class TestPmdPortStats(TextParser):
     tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
 
 
+def requires_stopped_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require stopped ports.
+
+    If the decorated method is called while the ports are started, then these are stopped before
+    continuing.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if self.ports_started:
+            self._logger.debug("Ports need to be stopped to continue")
+            self.stop_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
+def requires_started_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require started ports.
+
+    If the decorated method is called while the ports are stopped, then these are started before
+    continuing.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if not self.ports_started:
+            self._logger.debug("Ports need to be started to continue")
+            self.start_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
 class TestPmdShell(DPDKShell):
     """Testpmd interactive shell.
 
     The testpmd shell users should never use
     the :meth:`~.interactive_shell.InteractiveShell.send_command` method directly, but rather
     call specialized methods. If there isn't one that satisfies a need, it should be added.
+
+    Attributes:
+        ports_started: Indicates whether the ports are started.
     """
 
     _app_params: TestPmdParams
@@ -619,6 +662,9 @@ def __init__(
             name,
         )
 
+        self.ports_started = not self._app_params.disable_device_start
+
+    @requires_started_ports
     def start(self, verify: bool = True) -> None:
         """Start packet forwarding with the current configuration.
 
@@ -723,6 +769,42 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def stop_all_ports(self, verify: bool = True) -> None:
+        """Stops all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                stopped successfully.
+        """
+        self._logger.debug("Stopping all the ports...")
+        output = self.send_command("port stop all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not stopped successfully")
+
+        self.ports_started = False
+
+    def start_all_ports(self, verify: bool = True) -> None:
+        """Starts all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                started successfully.
+        """
+        self._logger.debug("Starting all the ports...")
+        output = self.send_command("port start all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not started successfully")
+
+        self.ports_started = True
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

[PATCH v2 5/5] dts: add testpmd set ports queues

[Luca Vizzarro]

Add a facility to update the number of TX/RX queues during the runtime
of testpmd.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index ca24b28070..85fbc42696 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> None:
 
         self.ports_started = True
 
+    @requires_stopped_ports
+    def set_ports_queues(self, number_of: int) -> None:
+        """Sets the number of queues per port.
+
+        Args:
+            number_of: The number of RX/TX queues to create per port.
+
+        Raises:
+            InternalError: If `number_of` is invalid.
+        """
+        if number_of < 1:
+            raise InternalError("The number of queues must be positive and non-zero")
+
+        self.send_command(f"port config all rxq {number_of}")
+        self.send_command(f"port config all txq {number_of}")
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets

[Jeremy Spewock]

Hey Luca,

Thanks for the patch! I think this change makes a lot of sense in
general, and I like the idea of removing the duplication between
capturing traffic generators and the test suite. I just had one
thought that I left below, but otherwise:

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> The framework allows only to send one packet at once via Scapy. This
> change adds the ability to send multiple packets, and also introduces a
> new fast way to verify if we received several expected packets.
>
> Moreover, it reduces code duplication by keeping a single packet sending
> method only at the test suite level.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Alex Chapman <alex.chapman@arm.com>
> ---
<snip>
> +    def match_all_packets(
> +        self, expected_packets: list[Packet], received_packets: list[Packet]
> +    ) -> None:

This is a very interesting approach to comparing what you expect to
what you received. I hadn't seen counters used before but they seem
useful for this purpose. This method reminds me a lot of the filtering
process that was discussed in bugzilla ticket 1438
(https://bugs.dpdk.org/show_bug.cgi?id=1438) where there was some talk
about how to handle the noise vs what you received. This is different
in a few ways though of course in that it allows you to specify
multiple different criteria that are acceptable rather than just
getting the payload and really only concerns itself with matching
lists instead of doing any filtering.

The main reason I mention this is it brought up a question for me: Do
you think, in the future when the payload filtering method is
implemented, these two methods will co-exist or it would make more
sense to just let test suites get their noise-free list of packets and
then check that what they care about is present? I think a method like
this might be useful in some more niche cases where you have multiple
packet structures to look for, so I don't doubt there are some reasons
to have both, I was just wondering if you had thought about it or had
an opinion.

> +        """Matches all the expected packets against the received ones.
> +
> +        Matching is performed by counting down the occurrences in a dictionary which keys are the
> +        raw packet bytes. No deep packet comparison is performed. All the unexpected packets (noise)
> +        are automatically ignored.
> +
> +        Args:
> +            expected_packets: The packets we are expecting to receive.
> +            received_packets: All the packets that were received.
> +
> +        Raises:
> +            TestCaseVerifyError: if and not all the `expected_packets` were found in
> +                `received_packets`.
> +        """
> +        expected_packets_counters = Counter(map(raw, expected_packets))
> +        received_packets_counters = Counter(map(raw, received_packets))
> +        # The number of expected packets is subtracted by the number of received packets, ignoring
> +        # any unexpected packets and capping at zero.
> +        missing_packets_counters = expected_packets_counters - received_packets_counters
> +        missing_packets_count = missing_packets_counters.total()
> +        self._logger.debug(
> +            f"match_all_packets: expected {len(expected_packets)}, "
> +            f"received {len(received_packets)}, missing {missing_packets_count}"
> +        )
> +
> +        if missing_packets_count != 0:
> +            self._fail_test_case_verify(
> +                f"Not all packets were received, expected {len(expected_packets)} "
> +                f"but {missing_packets_count} were missing."
> +            )
> +
<snip>
> --
> 2.34.1
>

Re: [PATCH v2 2/5] dts: add random generation seed setting

[Jeremy Spewock]

Makes a lot of sense to me.

On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> When introducing pseudo-random generation in the test runs we need to
> ensure that these can be reproduced by setting a pre-defined seed.
> This commits adds the ability to set one or allow for one to be
> generated and reported back to the user.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Alex Chapman <alex.chapman@arm.com>
> ---

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

Re: [PATCH v2 3/5] dts: add random packet generator

[Jeremy Spewock]

On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add a basic utility that can create random L3 and L4 packets with random
> payloads and port numbers (if L4).
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Alex Chapman <alex.chapman@arm.com>
> ---
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Jeremy Spewock]

I tried to address this very same problem in the scatter expansion
patch series but, admittedly, I like your approach better. There is
one thing that Juraj and I discussed on that thread however that is
probably worth noting here regarding verification of the stopping and
starting ports. The main idea behind that being if the user calls a
testpmd method and specifically specifies they don't want to verify
the outcome of that method, but then we still verify whether we
stopped the ports or not, that could cause confusion. The compromise
we ended up settling for was just to make a parameter to the decorator
that allows you to statically decide which decorated methods should
verify the ports stopping and which shouldn't. It was mainly because
of the ability to specify "verify" as a kwarg or an arg and the types
being messy. The discussion we had about it starts here if you were
interested in reading it:
http://inbox.dpdk.org/dev/dff89e16-0173-4069-878c-d1c34df1ae13@pantheon.tech/

On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add testpmd commands to start and stop all the ports, so that they can
> be configured. Because there is a distinction of commands that require
> the ports to be stopped and started, also add decorators for commands
> that require a specific state, removing this logic from the test
> writer's duty.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
<snip>
> +P = ParamSpec("P")
> +TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]

Agggh, I spent so much time trying to figure out how to do exactly
this when I was working on the scatter series but I couldn't get a
nice typehint while specifying that I wanted a TestPmdShell first. I
had no idea that Concatenate was a type but I will definitely keep
that one in mind.

> +
>
>  class TestPmdDevice:
>      """The data of a device that testpmd can recognize.
> @@ -577,12 +581,51 @@ class TestPmdPortStats(TextParser):
>      tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
>
>
> +def requires_stopped_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
> +    """Decorator for :class:`TestPmdShell` commands methods that require stopped ports.
> +
> +    If the decorated method is called while the ports are started, then these are stopped before
> +    continuing.
> +    """
> +
> +    @functools.wraps(func)
> +    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
> +        if self.ports_started:
> +            self._logger.debug("Ports need to be stopped to continue")
> +            self.stop_all_ports()
> +
> +        return func(self, *args, **kwargs)
> +
> +    return _wrapper
> +
> +
> +def requires_started_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
> +    """Decorator for :class:`TestPmdShell` commands methods that require started ports.
> +
> +    If the decorated method is called while the ports are stopped, then these are started before
> +    continuing.
> +    """
> +
> +    @functools.wraps(func)
> +    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
> +        if not self.ports_started:
> +            self._logger.debug("Ports need to be started to continue")
> +            self.start_all_ports()
> +
> +        return func(self, *args, **kwargs)
> +
> +    return _wrapper
> +

I'm not sure how much it is necessary since it is pretty clear what
these decorators are trying to do with the parameter, but since these
are public methods I think not having an "Args:" section for the func
might trip up the doc generation.

> +
>  class TestPmdShell(DPDKShell):
>      """Testpmd interactive shell.
>
>      The testpmd shell users should never use
>      the :meth:`~.interactive_shell.InteractiveShell.send_command` method directly, but rather
>      call specialized methods. If there isn't one that satisfies a need, it should be added.
> +
> +    Attributes:
> +        ports_started: Indicates whether the ports are started.
>      """
>
>      _app_params: TestPmdParams
> @@ -619,6 +662,9 @@ def __init__(
>              name,
>          )
>
> +        self.ports_started = not self._app_params.disable_device_start
> +

It might be useful to add this attribute as a stub in the class just
to be clear on the typing. It also helps me understand things at more
of a glance personally.

> +    @requires_started_ports
>      def start(self, verify: bool = True) -> None:
>          """Start packet forwarding with the current configuration.
<snip>

> --
> 2.34.1
>

Re: [PATCH v2 5/5] dts: add testpmd set ports queues

[Jeremy Spewock]

On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add a facility to update the number of TX/RX queues during the runtime
> of testpmd.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index ca24b28070..85fbc42696 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> None:
>
>          self.ports_started = True
>
> +    @requires_stopped_ports
> +    def set_ports_queues(self, number_of: int) -> None:

Was there a reason to not add a verify option to this method? It seems
like it could be useful, and the general pattern with testpmd methods
so far is verifying the outcome wherever we can.

> +        """Sets the number of queues per port.
> +
> +        Args:
> +            number_of: The number of RX/TX queues to create per port.

Is there any value in allowing users to set either Rx or Tx rather
than enforcing they change both? I guess I can't think of a specific
reason to do one and not the other off the top of my head, but it
seems like the option could be useful.

> +
> +        Raises:
> +            InternalError: If `number_of` is invalid.

I might be more in favor of this being an
InteractiveCommandExecutionError or some other DTSError just to have a
little more control over the error codes and to make it more clear
that the error came specifically from the framework.

> +        """
> +        if number_of < 1:
> +            raise InternalError("The number of queues must be positive and non-zero")
> +
> +        self.send_command(f"port config all rxq {number_of}")
> +        self.send_command(f"port config all txq {number_of}")
> +
>      def show_port_info_all(self) -> list[TestPmdPort]:
>          """Returns the information of all the ports.
>
> --
> 2.34.1
>

Re: [PATCH v2 5/5] dts: add testpmd set ports queues

[Jeremy Spewock]

On Fri, Aug 9, 2024 at 11:14 AM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> > +
> > +        Raises:
> > +            InternalError: If `number_of` is invalid.
>
> I might be more in favor of this being an
> InteractiveCommandExecutionError or some other DTSError just to have a
> little more control over the error codes and to make it more clear
> that the error came specifically from the framework.

Sorry, I am just realizing this is a DTS error that you added two
months ago, this comment is invalid.

>
> > +        """
> > +        if number_of < 1:
> > +            raise InternalError("The number of queues must be positive and non-zero")
> > +
> > +        self.send_command(f"port config all rxq {number_of}")
> > +        self.send_command(f"port config all txq {number_of}")
> > +
> >      def show_port_info_all(self) -> list[TestPmdPort]:
> >          """Returns the information of all the ports.
> >
> > --
> > 2.34.1
> >

Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets

[Juraj Linkeš]

This is a nice improvement. The comment below won't necessarily results 
in any changes, so:
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py

> @@ -303,6 +329,40 @@ def verify_packets(self, expected_packet: Packet, received_packets: list[Packet]
>               )
>               self._fail_test_case_verify("An expected packet not found among received packets.")
>   
> +    def match_all_packets(
> +        self, expected_packets: list[Packet], received_packets: list[Packet]
> +    ) -> None:
> +        """Matches all the expected packets against the received ones.
> +
> +        Matching is performed by counting down the occurrences in a dictionary which keys are the
> +        raw packet bytes. No deep packet comparison is performed. All the unexpected packets (noise)
> +        are automatically ignored.
> +
> +        Args:
> +            expected_packets: The packets we are expecting to receive.
> +            received_packets: All the packets that were received.
> +
> +        Raises:
> +            TestCaseVerifyError: if and not all the `expected_packets` were found in
> +                `received_packets`.
> +        """
> +        expected_packets_counters = Counter(map(raw, expected_packets))
> +        received_packets_counters = Counter(map(raw, received_packets))
> +        # The number of expected packets is subtracted by the number of received packets, ignoring
> +        # any unexpected packets and capping at zero.
> +        missing_packets_counters = expected_packets_counters - received_packets_counters
> +        missing_packets_count = missing_packets_counters.total()
> +        self._logger.debug(
> +            f"match_all_packets: expected {len(expected_packets)}, "
> +            f"received {len(received_packets)}, missing {missing_packets_count}"
> +        )
> +
> +        if missing_packets_count != 0:
> +            self._fail_test_case_verify(
> +                f"Not all packets were received, expected {len(expected_packets)} "
> +                f"but {missing_packets_count} were missing."
> +            )
> +

Is it worthwhile to log the missing packets? It's not necessary, as the 
received packets are logged elsewhere, but it would be convenient. On 
the other hand, it could just unnecessarily bloat logs.

Re: [PATCH v2 2/5] dts: add random generation seed setting

[Juraj Linkeš]

This is a great idea. I originally didn't think the config file option 
would be very useful, but thinking a bit more about it, when debugging 
and trying to find a solution the devs need to run with the same seed 
over and over, so it's going to have a place.

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

On 6. 8. 2024 14:46, Luca Vizzarro wrote:
> When introducing pseudo-random generation in the test runs we need to
> ensure that these can be reproduced by setting a pre-defined seed.
> This commits adds the ability to set one or allow for one to be
> generated and reported back to the user.
> 
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Alex Chapman <alex.chapman@arm.com>
> ---
>   doc/guides/tools/dts.rst                   |  5 +++++
>   dts/framework/config/__init__.py           |  4 ++++
>   dts/framework/config/conf_yaml_schema.json |  4 ++++
>   dts/framework/config/types.py              |  2 ++
>   dts/framework/runner.py                    |  8 ++++++++
>   dts/framework/settings.py                  | 17 +++++++++++++++++
>   6 files changed, 40 insertions(+)
> 
> diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> index 515b15e4d8..9b5ea9779c 100644
> --- a/doc/guides/tools/dts.rst
> +++ b/doc/guides/tools/dts.rst
> @@ -251,6 +251,8 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet
>                              ... | DTS_TEST_SUITES='suite, suite case, ...' (default: [])
>        --re-run N_TIMES, --re_run N_TIMES
>                              [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0)
> +     --random-seed NUMBER  [DTS_RANDOM_SEED] The seed to use with the pseudo-random generator. If not specified, the configuration value is
> +                           used instead. If that's also not specified, a random seed is generated. (default: None)
>   
>   
>   The brackets contain the names of environment variables that set the same thing.
> @@ -548,6 +550,9 @@ involved in the testing. These can be defined with the following mappings:
>      +----------------------------+---------------+---------------------------------------------------+
>      | ``traffic_generator_node`` | Node name for the traffic generator node.                         |
>      +----------------------------+-------------------------------------------------------------------+
> +   | ``random_seed``            | (*optional*) *int* – Allows you to set a seed for pseudo-random   |
> +   |                            | generation.                                                       |
> +   +----------------------------+-------------------------------------------------------------------+
>   
>   ``nodes``
>      `sequence <https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range>`_ listing
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index df60a5030e..269d9ec318 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -445,6 +445,7 @@ class TestRunConfiguration:
>           system_under_test_node: The SUT node to use in this test run.
>           traffic_generator_node: The TG node to use in this test run.
>           vdevs: The names of virtual devices to test.
> +        random_seed: The seed to use for pseudo-random generation.
>       """
>   
>       build_targets: list[BuildTargetConfiguration]
> @@ -455,6 +456,7 @@ class TestRunConfiguration:
>       system_under_test_node: SutNodeConfiguration
>       traffic_generator_node: TGNodeConfiguration
>       vdevs: list[str]
> +    random_seed: int | None
>   
>       @classmethod
>       def from_dict(
> @@ -497,6 +499,7 @@ def from_dict(
>           vdevs = (
>               d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
>           )
> +        random_seed = d.get("random_seed", None)
>           return cls(
>               build_targets=build_targets,
>               perf=d["perf"],
> @@ -506,6 +509,7 @@ def from_dict(
>               system_under_test_node=system_under_test_node,
>               traffic_generator_node=traffic_generator_node,
>               vdevs=vdevs,
> +            random_seed=random_seed,
>           )
>   
>       def copy_and_modify(self, **kwargs) -> Self:
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index f02a310bb5..df390e8ae2 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -379,6 +379,10 @@
>             },
>             "traffic_generator_node": {
>               "$ref": "#/definitions/node_name"
> +          },
> +          "random_seed": {
> +            "type": "integer",
> +            "description": "Optional field. Allows you to set a seed for pseudo-random generation."
>             }
>           },
>           "additionalProperties": false,
> diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
> index cf16556403..ce7b784ac8 100644
> --- a/dts/framework/config/types.py
> +++ b/dts/framework/config/types.py
> @@ -121,6 +121,8 @@ class TestRunConfigDict(TypedDict):
>       system_under_test_node: TestRunSUTConfigDict
>       #:
>       traffic_generator_node: str
> +    #:
> +    random_seed: int
>   
>   
>   class ConfigurationDict(TypedDict):
> diff --git a/dts/framework/runner.py b/dts/framework/runner.py
> index 6b6f6a05f5..34b1dad5c4 100644
> --- a/dts/framework/runner.py
> +++ b/dts/framework/runner.py
> @@ -20,6 +20,7 @@
>   import importlib
>   import inspect
>   import os
> +import random
>   import re
>   import sys
>   from pathlib import Path
> @@ -147,6 +148,7 @@ def run(self) -> None:
>                   self._logger.info(
>                       f"Running test run with SUT '{test_run_config.system_under_test_node.name}'."
>                   )
> +                self._init_random_seed(test_run_config)
>                   test_run_result = self._result.add_test_run(test_run_config)
>                   # we don't want to modify the original config, so create a copy
>                   test_run_test_suites = list(
> @@ -723,3 +725,9 @@ def _exit_dts(self) -> None:
>               self._logger.info("DTS execution has ended.")
>   
>           sys.exit(self._result.get_return_code())
> +
> +    def _init_random_seed(self, conf: TestRunConfiguration) -> None:
> +        """Initialize the random seed to use for the test run."""
> +        seed = SETTINGS.random_seed or conf.random_seed or random.randrange(0xFFFF_FFFF)
> +        self._logger.info(f"Initializing test run with random seed {seed}")
> +        random.seed(seed)
> diff --git a/dts/framework/settings.py b/dts/framework/settings.py
> index f6303066d4..7744e37f54 100644
> --- a/dts/framework/settings.py
> +++ b/dts/framework/settings.py
> @@ -66,6 +66,12 @@
>   
>       Re-run each test case this many times in case of a failure.
>   
> +.. option:: --random-seed
> +.. envvar:: DTS_RANDOM_SEED
> +
> +    The seed to use with the pseudo-random generator. If not specified, the configuration value is
> +    used instead. If that's also not specified, a random seed is generated.
> +
>   The module provides one key module-level variable:
>   
>   Attributes:
> @@ -115,6 +121,8 @@ class Settings:
>       test_suites: list[TestSuiteConfig] = field(default_factory=list)
>       #:
>       re_run: int = 0
> +    #:
> +    random_seed: int | None = None
>   
>   
>   SETTINGS: Settings = Settings()
> @@ -375,6 +383,15 @@ def _get_parser() -> _DTSArgumentParser:
>       )
>       _add_env_var_to_action(action, "RERUN")
>   
> +    action = parser.add_argument(
> +        "--random-seed",
> +        type=int,
> +        help="The seed to use with the pseudo-random generator. If not specified, the configuration"
> +        " value is used instead. If that's also not specified, a random seed is generated.",
> +        metavar="NUMBER",
> +    )
> +    _add_env_var_to_action(action)
> +
>       return parser
>   
>   

Re: [PATCH v2 3/5] dts: add random packet generator

[Juraj Linkeš]

On 6. 8. 2024 14:46, Luca Vizzarro wrote:
> Add a basic utility that can create random L3 and L4 packets with random
> payloads and port numbers (if L4).
> 
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Alex Chapman <alex.chapman@arm.com>

I believe this is to be used with a method that fills in the blanks of a 
passed packet (such as mac addresses). If so,
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Juraj Linkeš]

As Jeremy mentioned, adding the verify argument may be worthwhile, but 
maybe only if we actually identify a usecase where we wouldn't want to 
do the verification.

I also have two other points:
1. Do we want a decorator that does both - starting and stopping? Is the 
idea to have all methods that require a certain state to be decorated 
and in that case we don't need this? E.g. if there are multiple 
configuration steps, only the first one stops the ports (if started) and 
we start the ports only when a method needs that (such as starting pkt 
fwd)? I think this workflow should be documented in the class docstring 
(the important part being that starting and stopping of ports is done 
automatically).
2. Do we want decorators that start/stop just one port? I guess this is 
basically useless with the above workflow.

Re: [PATCH v2 5/5] dts: add testpmd set ports queues

[Juraj Linkeš]

On 6. 8. 2024 14:46, Luca Vizzarro wrote:
> Add a facility to update the number of TX/RX queues during the runtime
> of testpmd.
> 
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
>   dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index ca24b28070..85fbc42696 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> None:
>   
>           self.ports_started = True
>   
> +    @requires_stopped_ports
> +    def set_ports_queues(self, number_of: int) -> None:
> +        """Sets the number of queues per port.
> +
> +        Args:
> +            number_of: The number of RX/TX queues to create per port.
> +
> +        Raises:
> +            InternalError: If `number_of` is invalid.
> +        """
> +        if number_of < 1:
> +            raise InternalError("The number of queues must be positive and non-zero")

I don't think we have talked about the message formatting policy, so 
here's a suggestion: Let's end all exception messages with a dot. This 
could probably extend to log messages as well. Thomas mentioned in the 
past that it reads better and I concur - it is an actual sentence.

Also, this being an InternalError, do you think the invalid input could 
only happen because of something going wrong in DTS? This seems 
reasonable from the possible usecases.

> +
> +        self.send_command(f"port config all rxq {number_of}")
> +        self.send_command(f"port config all txq {number_of}")
> +
>       def show_port_info_all(self) -> list[TestPmdPort]:
>           """Returns the information of all the ports.
>   

Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets

[Luca Vizzarro]

On 09/08/2024 16:10, Jeremy Spewock wrote:
> <snip>
>> +    def match_all_packets(
>> +        self, expected_packets: list[Packet], received_packets: list[Packet]
>> +    ) -> None:
> 
> This is a very interesting approach to comparing what you expect to
> what you received. I hadn't seen counters used before but they seem
> useful for this purpose. This method reminds me a lot of the filtering
> process that was discussed in bugzilla ticket 1438
> (https://bugs.dpdk.org/show_bug.cgi?id=1438) where there was some talk
> about how to handle the noise vs what you received. This is different
> in a few ways though of course in that it allows you to specify
> multiple different criteria that are acceptable rather than just
> getting the payload and really only concerns itself with matching
> lists instead of doing any filtering.
> 
> The main reason I mention this is it brought up a question for me: Do
> you think, in the future when the payload filtering method is
> implemented, these two methods will co-exist or it would make more
> sense to just let test suites get their noise-free list of packets and
> then check that what they care about is present? I think a method like
> this might be useful in some more niche cases where you have multiple
> packet structures to look for, so I don't doubt there are some reasons
> to have both, I was just wondering if you had thought about it or had
> an opinion.

I am not actually entirely sure, it sounds as if everything has space 
for their own case. This method is useful to just plainly check that all 
the packets we sent came back, simple and straightforward. Payload 
filtering can be more niche and complex. I think if it can provide the 
same level of functionality as this function, it can be replaced for 
sure. But in that case it sounds like the test developer would be tied 
to a specific payload. In the case of random packet generation I don't 
think it would work, in which case they could both coexist.

Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets

[Luca Vizzarro]

On 23/08/2024 11:17, Juraj Linkeš wrote:
> Is it worthwhile to log the missing packets? It's not necessary, as the 
> received packets are logged elsewhere, but it would be convenient. On 
> the other hand, it could just unnecessarily bloat logs.

I considered it but as you said I think it may unecessarily bloat the 
logs. The "unfiltered" packets can already be retrieved anyways if needed.

Re: [PATCH v2 3/5] dts: add random packet generator

[Luca Vizzarro]

On 23/08/2024 12:58, Juraj Linkeš wrote:

> I believe this is to be used with a method that fills in the blanks of a 
> passed packet (such as mac addresses). If so,
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Yes, that's correct :D

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Luca Vizzarro]

On 09/08/2024 16:13, Jeremy Spewock wrote:
> I tried to address this very same problem in the scatter expansion
> patch series but, admittedly, I like your approach better. There is
> one thing that Juraj and I discussed on that thread however that is
> probably worth noting here regarding verification of the stopping and
> starting ports. The main idea behind that being if the user calls a
> testpmd method and specifically specifies they don't want to verify
> the outcome of that method, but then we still verify whether we
> stopped the ports or not, that could cause confusion. The compromise
> we ended up settling for was just to make a parameter to the decorator
> that allows you to statically decide which decorated methods should
> verify the ports stopping and which shouldn't. It was mainly because
> of the ability to specify "verify" as a kwarg or an arg and the types
> being messy. The discussion we had about it starts here if you were
> interested in reading it:
> http://inbox.dpdk.org/dev/dff89e16-0173-4069-878c-d1c34df1ae13@pantheon.tech/

When it comes to starting and stopping ports, we may need to ensure that 
the operations were done correctly. Surely we don't want to start 
forwarding thinking that the ports are initialised, when they could be 
not... I guess the point here is that it may be important to save the 
correct state. Not sure, I guess verify could always be added later if 
needed.

> On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>>
>> Add testpmd commands to start and stop all the ports, so that they can
>> be configured. Because there is a distinction of commands that require
>> the ports to be stopped and started, also add decorators for commands
>> that require a specific state, removing this logic from the test
>> writer's duty.
>>
>> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
>> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
>> ---
> <snip>
>> +P = ParamSpec("P")
>> +TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]
> 
> Agggh, I spent so much time trying to figure out how to do exactly
> this when I was working on the scatter series but I couldn't get a
> nice typehint while specifying that I wanted a TestPmdShell first. I
> had no idea that Concatenate was a type but I will definitely keep
> that one in mind.
> 

Glad I could help! :D

>> +def requires_started_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
>> +    """Decorator for :class:`TestPmdShell` commands methods that require started ports.
>> +
>> +    If the decorated method is called while the ports are stopped, then these are started before
>> +    continuing.
>> +    """
>> +
>> +    @functools.wraps(func)
>> +    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
>> +        if not self.ports_started:
>> +            self._logger.debug("Ports need to be started to continue")
>> +            self.start_all_ports()
>> +
>> +        return func(self, *args, **kwargs)
>> +
>> +    return _wrapper
>> +
> 
> I'm not sure how much it is necessary since it is pretty clear what
> these decorators are trying to do with the parameter, but since these
> are public methods I think not having an "Args:" section for the func
> might trip up the doc generation.

We have several methods that have one-liner docstrings... so not really 
sure. I was hoping to have the doc generation merged as testing what's 
correct and what's not is currently complicated.

>>
>> +        self.ports_started = not self._app_params.disable_device_start
>> +
> 
> It might be useful to add this attribute as a stub in the class just
> to be clear on the typing. It also helps me understand things at more
> of a glance personally.

You make a fair point here.

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Luca Vizzarro]

On 23/08/2024 13:16, Juraj Linkeš wrote:
> As Jeremy mentioned, adding the verify argument may be worthwhile, but 
> maybe only if we actually identify a usecase where we wouldn't want to 
> do the verification.

Yeah, as I pointed out, it feels unlikely to pretend that they are 
started (or stopped). I think that could cause my unpredicted behaviour 
and confusion. But also as said already, it can always be added on demand.

> I also have two other points:
> 1. Do we want a decorator that does both - starting and stopping? Is the 
> idea to have all methods that require a certain state to be decorated 
> and in that case we don't need this? E.g. if there are multiple 
> configuration steps, only the first one stops the ports (if started) and 
> we start the ports only when a method needs that (such as starting pkt 
> fwd)? I think this workflow should be documented in the class docstring 
> (the important part being that starting and stopping of ports is done 
> automatically).

The way I envisioned these decorators is by using a lazy approach. I 
don't think it may be right to eagerly restart ports after stopping 
them, because maybe we want to run another command after that will stop 
them again. So only start and stop as needed. Ideally every port that 
requires a specific state of the ports need to be decorated. I went 
through all the methods in the class to check which would and which 
wouldn't, and it seems alright like this.

> 2. Do we want decorators that start/stop just one port? I guess this is 
> basically useless with the above workflow.

Would it actually matter? We are not really using the other ports in 
parallel here I believe. May bring unnecessary complexity. I am thinking 
that maybe if needed it could be added later.

Re: [PATCH v2 5/5] dts: add testpmd set ports queues

[Luca Vizzarro]

On 09/08/2024 16:14, Jeremy Spewock wrote:
> On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>>
>> Add a facility to update the number of TX/RX queues during the runtime
>> of testpmd.
>>
>> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
>> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
>> ---
>>   dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
>> index ca24b28070..85fbc42696 100644
>> --- a/dts/framework/remote_session/testpmd_shell.py
>> +++ b/dts/framework/remote_session/testpmd_shell.py
>> @@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> None:
>>
>>           self.ports_started = True
>>
>> +    @requires_stopped_ports
>> +    def set_ports_queues(self, number_of: int) -> None:
> 
> Was there a reason to not add a verify option to this method? It seems
> like it could be useful, and the general pattern with testpmd methods
> so far is verifying the outcome wherever we can.

Yes, I've noticed that the value (in show port info) doesn't get updated 
until actually restarting the ports. So it wouldn't work here 
unfortunately. Internally it looks like it's only updating a variable, 
the actual operation can only be verified upon ports starting.

>> +        """Sets the number of queues per port.
>> +
>> +        Args:
>> +            number_of: The number of RX/TX queues to create per port.
> 
> Is there any value in allowing users to set either Rx or Tx rather
> than enforcing they change both? I guess I can't think of a specific
> reason to do one and not the other off the top of my head, but it
> seems like the option could be useful.

My initial code was only updating the RX queues for my l2fwd test suite, 
and I was having lots of unpredictable and flaky behaviour. Sometimes it 
would work other times it wouldn't. When setting different values for RX 
and TX queues you actually get a warning from the driver to expect 
unexpected behaviour by doing this. And indeed I did get it. From this I 
gathered that we don't want to have different values, unless we 
explicitly want to make the drivers unhappy. I had unexpected and 
different behaviour on both ice and mlx5_core.

> I might be more in favor of this being an
> InteractiveCommandExecutionError or some other DTSError just to have a
> little more control over the error codes and to make it more clear
> that the error came specifically from the framework.

I noticed you already gave yourself an answer :D

Re: [PATCH v2 5/5] dts: add testpmd set ports queues

[Luca Vizzarro]

On 23/08/2024 13:22, Juraj Linkeš wrote:
> 
> 
> On 6. 8. 2024 14:46, Luca Vizzarro wrote:
>> Add a facility to update the number of TX/RX queues during the runtime
>> of testpmd.
>>
>> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
>> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
>> ---
>>   dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/ 
>> framework/remote_session/testpmd_shell.py
>> index ca24b28070..85fbc42696 100644
>> --- a/dts/framework/remote_session/testpmd_shell.py
>> +++ b/dts/framework/remote_session/testpmd_shell.py
>> @@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> 
>> None:
>>           self.ports_started = True
>> +    @requires_stopped_ports
>> +    def set_ports_queues(self, number_of: int) -> None:
>> +        """Sets the number of queues per port.
>> +
>> +        Args:
>> +            number_of: The number of RX/TX queues to create per port.
>> +
>> +        Raises:
>> +            InternalError: If `number_of` is invalid.
>> +        """
>> +        if number_of < 1:
>> +            raise InternalError("The number of queues must be 
>> positive and non-zero")
> 
> I don't think we have talked about the message formatting policy, so 
> here's a suggestion: Let's end all exception messages with a dot. This 
> could probably extend to log messages as well. Thomas mentioned in the 
> past that it reads better and I concur - it is an actual sentence.

I gathered the same too, so this was an unvoluntary miss.

> Also, this being an InternalError, do you think the invalid input could 
> only happen because of something going wrong in DTS? This seems 
> reasonable from the possible usecases.

Yes, this is just input sanity check. The only case this would fail is 
if the callee of this function supplied the wrong value, and that would 
be a bug in DTS.

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Juraj Linkeš]

On 6. 9. 2024 18:46, Luca Vizzarro wrote:
> On 23/08/2024 13:16, Juraj Linkeš wrote:
>> As Jeremy mentioned, adding the verify argument may be worthwhile, but 
>> maybe only if we actually identify a usecase where we wouldn't want to 
>> do the verification.
> 
> Yeah, as I pointed out, it feels unlikely to pretend that they are 
> started (or stopped). I think that could cause my unpredicted behaviour 
> and confusion. But also as said already, it can always be added on demand.
> 
>> I also have two other points:
>> 1. Do we want a decorator that does both - starting and stopping? Is 
>> the idea to have all methods that require a certain state to be 
>> decorated and in that case we don't need this? E.g. if there are 
>> multiple configuration steps, only the first one stops the ports (if 
>> started) and we start the ports only when a method needs that (such as 
>> starting pkt fwd)? I think this workflow should be documented in the 
>> class docstring (the important part being that starting and stopping 
>> of ports is done automatically).
> 
> The way I envisioned these decorators is by using a lazy approach. I 
> don't think it may be right to eagerly restart ports after stopping 
> them, because maybe we want to run another command after that will stop 
> them again. So only start and stop as needed. Ideally every port that 
> requires a specific state of the ports need to be decorated. I went 
> through all the methods in the class to check which would and which 
> wouldn't, and it seems alright like this.
> 

I agree. I was initially thinking about this from the point of view of 
making sure we don't change things for other users, but that doesn't 
apply here, as we control the whole testpmd workflow. Your approach is 
actually great - simple and effective.

>> 2. Do we want decorators that start/stop just one port? I guess this 
>> is basically useless with the above workflow.
> 
> Would it actually matter? We are not really using the other ports in 
> parallel here I believe. May bring unnecessary complexity. I am thinking 
> that maybe if needed it could be added later.

I think the only benefit would be that we're sending less commands so 
it'll be a bit faster. Maybe we just make a mental note of this for the 
future.

[PATCH v3 0/5] dts: add pktgen and testpmd changes

[Luca Vizzarro]

v3:
- add Args to decorators docstring
- added `ports_started` class stub
- rebased
v2:
- rebased

Luca Vizzarro (6):
  dts: add ability to send/receive multiple packets
  dts: add random generation seed setting
  dts: add random packet generator
  dts: add ability to start/stop testpmd ports
  dts: add testpmd set ports queues
  dts: add l2fwd test suite

 doc/guides/tools/dts.rst                      |   5 +
 dts/framework/config/__init__.py              |   4 +
 dts/framework/config/conf_yaml_schema.json    |   7 +-
 dts/framework/config/types.py                 |   2 +
 dts/framework/remote_session/testpmd_shell.py | 110 +++++++++++++++++-
 dts/framework/runner.py                       |   8 ++
 dts/framework/settings.py                     |  17 +++
 dts/framework/test_suite.py                   |  68 ++++++++++-
 dts/framework/testbed_model/tg_node.py        |  14 +--
 .../capturing_traffic_generator.py            |  31 -----
 dts/framework/utils.py                        |  79 ++++++++++++-
 dts/tests/TestSuite_l2fwd.py                  |  58 +++++++++
 12 files changed, 356 insertions(+), 47 deletions(-)
 create mode 100644 dts/tests/TestSuite_l2fwd.py

-- 
2.34.1

[PATCH v3 1/5] dts: add ability to send/receive multiple packets

[Luca Vizzarro]

The framework allows only to send one packet at once via Scapy. This
change adds the ability to send multiple packets, and also introduces a
new fast way to verify if we received several expected packets.

Moreover, it reduces code duplication by keeping a single packet sending
method only at the test suite level.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/test_suite.py                   | 68 +++++++++++++++++--
 dts/framework/testbed_model/tg_node.py        | 14 ++--
 .../capturing_traffic_generator.py            | 31 ---------
 3 files changed, 71 insertions(+), 42 deletions(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..051509fb86 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -13,12 +13,13 @@
     * Test case verification.
 """
 
+from collections import Counter
 from ipaddress import IPv4Interface, IPv6Interface, ip_interface
 from typing import ClassVar, Union
 
 from scapy.layers.inet import IP  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
-from scapy.packet import Packet, Padding  # type: ignore[import-untyped]
+from scapy.packet import Packet, Padding, raw  # type: ignore[import-untyped]
 
 from framework.testbed_model.port import Port, PortLink
 from framework.testbed_model.sut_node import SutNode
@@ -199,9 +200,34 @@ def send_packet_and_capture(
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
-        return self.tg_node.send_packet_and_capture(
-            packet,
+        return self.send_packets_and_capture(
+            [packet],
+            filter_config,
+            duration,
+        )
+
+    def send_packets_and_capture(
+        self,
+        packets: list[Packet],
+        filter_config: PacketFilteringConfig = PacketFilteringConfig(),
+        duration: float = 1,
+    ) -> list[Packet]:
+        """Send and receive `packets` using the associated TG.
+
+        Send `packets` through the appropriate interface and receive on the appropriate interface.
+        Modify the packets with l3/l2 addresses corresponding to the testbed and desired traffic.
+
+        Args:
+            packets: The packets to send.
+            filter_config: The filter to use when capturing packets.
+            duration: Capture traffic for this amount of time after sending `packet`.
+
+        Returns:
+            A list of received packets.
+        """
+        packets = [self._adjust_addresses(packet) for packet in packets]
+        return self.tg_node.send_packets_and_capture(
+            packets,
             self._tg_port_egress,
             self._tg_port_ingress,
             filter_config,
@@ -303,6 +329,40 @@ def verify_packets(self, expected_packet: Packet, received_packets: list[Packet]
             )
             self._fail_test_case_verify("An expected packet not found among received packets.")
 
+    def match_all_packets(
+        self, expected_packets: list[Packet], received_packets: list[Packet]
+    ) -> None:
+        """Matches all the expected packets against the received ones.
+
+        Matching is performed by counting down the occurrences in a dictionary which keys are the
+        raw packet bytes. No deep packet comparison is performed. All the unexpected packets (noise)
+        are automatically ignored.
+
+        Args:
+            expected_packets: The packets we are expecting to receive.
+            received_packets: All the packets that were received.
+
+        Raises:
+            TestCaseVerifyError: if and not all the `expected_packets` were found in
+                `received_packets`.
+        """
+        expected_packets_counters = Counter(map(raw, expected_packets))
+        received_packets_counters = Counter(map(raw, received_packets))
+        # The number of expected packets is subtracted by the number of received packets, ignoring
+        # any unexpected packets and capping at zero.
+        missing_packets_counters = expected_packets_counters - received_packets_counters
+        missing_packets_count = missing_packets_counters.total()
+        self._logger.debug(
+            f"match_all_packets: expected {len(expected_packets)}, "
+            f"received {len(received_packets)}, missing {missing_packets_count}"
+        )
+
+        if missing_packets_count != 0:
+            self._fail_test_case_verify(
+                f"Not all packets were received, expected {len(expected_packets)} "
+                f"but {missing_packets_count} were missing."
+            )
+
     def _compare_packets(self, expected_packet: Packet, received_packet: Packet) -> bool:
         self._logger.debug(
             f"Comparing packets: \n{expected_packet.summary()}\n{received_packet.summary()}"
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 4ee326e99c..19b5b6e74c 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -51,22 +51,22 @@ def __init__(self, node_config: TGNodeConfiguration):
         self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
         self._logger.info(f"Created node: {self.name}")
 
-    def send_packet_and_capture(
+    def send_packets_and_capture(
         self,
-        packet: Packet,
+        packets: list[Packet],
         send_port: Port,
         receive_port: Port,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
     ) -> list[Packet]:
-        """Send `packet`, return received traffic.
+        """Send `packets`, return received traffic.
 
-        Send `packet` on `send_port` and then return all traffic captured
+        Send `packets` on `send_port` and then return all traffic captured
         on `receive_port` for the given duration. Also record the captured traffic
         in a pcap file.
 
         Args:
-            packet: The packet to send.
+            packets: The packets to send.
             send_port: The egress port on the TG node.
             receive_port: The ingress port in the TG node.
             filter_config: The filter to use when capturing packets.
@@ -75,8 +75,8 @@ def send_packet_and_capture(
         Returns:
              A list of received packets. May be empty if no packets are captured.
         """
-        return self.traffic_generator.send_packet_and_capture(
-            packet,
+        return self.traffic_generator.send_packets_and_capture(
+            packets,
             send_port,
             receive_port,
             filter_config,
diff --git a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
index c8380b7d57..66a77da9c4 100644
--- a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
@@ -63,37 +63,6 @@ def is_capturing(self) -> bool:
         """This traffic generator can capture traffic."""
         return True
 
-    def send_packet_and_capture(
-        self,
-        packet: Packet,
-        send_port: Port,
-        receive_port: Port,
-        filter_config: PacketFilteringConfig,
-        duration: float,
-        capture_name: str = _get_default_capture_name(),
-    ) -> list[Packet]:
-        """Send `packet` and capture received traffic.
-
-        Send `packet` on `send_port` and then return all traffic captured
-        on `receive_port` for the given `duration`.
-
-        The captured traffic is recorded in the `capture_name`.pcap file.
-
-        Args:
-            packet: The packet to send.
-            send_port: The egress port on the TG node.
-            receive_port: The ingress port in the TG node.
-            filter_config: Filters to apply when capturing packets.
-            duration: Capture traffic for this amount of time after sending the packet.
-            capture_name: The name of the .pcap file where to store the capture.
-
-        Returns:
-             The received packets. May be empty if no packets are captured.
-        """
-        return self.send_packets_and_capture(
-            [packet], send_port, receive_port, filter_config, duration, capture_name
-        )
-
     def send_packets_and_capture(
         self,
         packets: list[Packet],
-- 
2.34.1

[PATCH v3 2/5] dts: add random generation seed setting

[Luca Vizzarro]

When introducing pseudo-random generation in the test runs we need to
ensure that these can be reproduced by setting a pre-defined seed.
This commits adds the ability to set one or allow for one to be
generated and reported back to the user.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 doc/guides/tools/dts.rst                   |  5 +++++
 dts/framework/config/__init__.py           |  4 ++++
 dts/framework/config/conf_yaml_schema.json |  4 ++++
 dts/framework/config/types.py              |  2 ++
 dts/framework/runner.py                    |  8 ++++++++
 dts/framework/settings.py                  | 17 +++++++++++++++++
 6 files changed, 40 insertions(+)

diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 515b15e4d8..9b5ea9779c 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -251,6 +251,8 @@ DTS is run with ``main.py`` located in the ``dts`` directory after entering Poet
                            ... | DTS_TEST_SUITES='suite, suite case, ...' (default: [])
      --re-run N_TIMES, --re_run N_TIMES
                            [DTS_RERUN] Re-run each test case the specified number of times if a test failure occurs. (default: 0)
+     --random-seed NUMBER  [DTS_RANDOM_SEED] The seed to use with the pseudo-random generator. If not specified, the configuration value is
+                           used instead. If that's also not specified, a random seed is generated. (default: None)
 
 
 The brackets contain the names of environment variables that set the same thing.
@@ -548,6 +550,9 @@ involved in the testing. These can be defined with the following mappings:
    +----------------------------+---------------+---------------------------------------------------+
    | ``traffic_generator_node`` | Node name for the traffic generator node.                         |
    +----------------------------+-------------------------------------------------------------------+
+   | ``random_seed``            | (*optional*) *int* – Allows you to set a seed for pseudo-random   |
+   |                            | generation.                                                       |
+   +----------------------------+-------------------------------------------------------------------+
 
 ``nodes``
    `sequence <https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range>`_ listing
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..269d9ec318 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -445,6 +445,7 @@ class TestRunConfiguration:
         system_under_test_node: The SUT node to use in this test run.
         traffic_generator_node: The TG node to use in this test run.
         vdevs: The names of virtual devices to test.
+        random_seed: The seed to use for pseudo-random generation.
     """
 
     build_targets: list[BuildTargetConfiguration]
@@ -455,6 +456,7 @@ class TestRunConfiguration:
     system_under_test_node: SutNodeConfiguration
     traffic_generator_node: TGNodeConfiguration
     vdevs: list[str]
+    random_seed: int | None
 
     @classmethod
     def from_dict(
@@ -497,6 +499,7 @@ def from_dict(
         vdevs = (
             d["system_under_test_node"]["vdevs"] if "vdevs" in d["system_under_test_node"] else []
         )
+        random_seed = d.get("random_seed", None)
         return cls(
             build_targets=build_targets,
             perf=d["perf"],
@@ -506,6 +509,7 @@ def from_dict(
             system_under_test_node=system_under_test_node,
             traffic_generator_node=traffic_generator_node,
             vdevs=vdevs,
+            random_seed=random_seed,
         )
 
     def copy_and_modify(self, **kwargs) -> Self:
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..df390e8ae2 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -379,6 +379,10 @@
           },
           "traffic_generator_node": {
             "$ref": "#/definitions/node_name"
+          },
+          "random_seed": {
+            "type": "integer",
+            "description": "Optional field. Allows you to set a seed for pseudo-random generation."
           }
         },
         "additionalProperties": false,
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index cf16556403..ce7b784ac8 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -121,6 +121,8 @@ class TestRunConfigDict(TypedDict):
     system_under_test_node: TestRunSUTConfigDict
     #:
     traffic_generator_node: str
+    #:
+    random_seed: int
 
 
 class ConfigurationDict(TypedDict):
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..34b1dad5c4 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -20,6 +20,7 @@
 import importlib
 import inspect
 import os
+import random
 import re
 import sys
 from pathlib import Path
@@ -147,6 +148,7 @@ def run(self) -> None:
                 self._logger.info(
                     f"Running test run with SUT '{test_run_config.system_under_test_node.name}'."
                 )
+                self._init_random_seed(test_run_config)
                 test_run_result = self._result.add_test_run(test_run_config)
                 # we don't want to modify the original config, so create a copy
                 test_run_test_suites = list(
@@ -723,3 +725,9 @@ def _exit_dts(self) -> None:
             self._logger.info("DTS execution has ended.")
 
         sys.exit(self._result.get_return_code())
+
+    def _init_random_seed(self, conf: TestRunConfiguration) -> None:
+        """Initialize the random seed to use for the test run."""
+        seed = SETTINGS.random_seed or conf.random_seed or random.randrange(0xFFFF_FFFF)
+        self._logger.info(f"Initializing test run with random seed {seed}")
+        random.seed(seed)
diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index f6303066d4..7744e37f54 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -66,6 +66,12 @@
 
     Re-run each test case this many times in case of a failure.
 
+.. option:: --random-seed
+.. envvar:: DTS_RANDOM_SEED
+
+    The seed to use with the pseudo-random generator. If not specified, the configuration value is
+    used instead. If that's also not specified, a random seed is generated.
+
 The module provides one key module-level variable:
 
 Attributes:
@@ -115,6 +121,8 @@ class Settings:
     test_suites: list[TestSuiteConfig] = field(default_factory=list)
     #:
     re_run: int = 0
+    #:
+    random_seed: int | None = None
 
 
 SETTINGS: Settings = Settings()
@@ -375,6 +383,15 @@ def _get_parser() -> _DTSArgumentParser:
     )
     _add_env_var_to_action(action, "RERUN")
 
+    action = parser.add_argument(
+        "--random-seed",
+        type=int,
+        help="The seed to use with the pseudo-random generator. If not specified, the configuration"
+        " value is used instead. If that's also not specified, a random seed is generated.",
+        metavar="NUMBER",
+    )
+    _add_env_var_to_action(action)
+
     return parser
 
 
-- 
2.34.1

[PATCH v3 3/5] dts: add random packet generator

[Luca Vizzarro]

Add a basic utility that can create random L3 and L4 packets with random
payloads and port numbers (if L4).

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Alex Chapman <alex.chapman@arm.com>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/utils.py | 79 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index 6b5d5a805f..c768dd0c99 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -17,14 +17,16 @@
 import atexit
 import json
 import os
+import random
 import subprocess
-from enum import Enum
+from enum import Enum, Flag
 from pathlib import Path
 from subprocess import SubprocessError
 
+from scapy.layers.inet import IP, TCP, UDP, Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
-from .exception import ConfigurationError
+from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
 
@@ -244,3 +246,76 @@ def _delete_tarball(self) -> None:
     def __fspath__(self) -> str:
         """The os.PathLike protocol implementation."""
         return str(self._tarball_path)
+
+
+class PacketProtocols(Flag):
+    """Flag specifying which protocols to use for packet generation."""
+
+    #:
+    IP = 1
+    #:
+    TCP = 2 | IP
+    #:
+    UDP = 4 | IP
+    #:
+    ALL = TCP | UDP
+
+
+def generate_random_packets(
+    number_of: int,
+    payload_size: int = 1500,
+    protocols: PacketProtocols = PacketProtocols.ALL,
+    ports_range: range = range(1024, 49152),
+    mtu: int = 1500,
+) -> list[Packet]:
+    """Generate a number of random packets.
+
+    The payload of the packets will consist of random bytes. If `payload_size` is too big, then the
+    maximum payload size allowed for the specific packet type is used. The size is calculated based
+    on the specified `mtu`, therefore it is essential that `mtu` is set correctly to match the MTU
+    of the port that will send out the generated packets.
+
+    If `protocols` has any L4 protocol enabled then all the packets are generated with any of
+    the specified L4 protocols chosen at random. If only :attr:`~PacketProtocols.IP` is set, then
+    only L3 packets are generated.
+
+    If L4 packets will be generated, then the TCP/UDP ports to be used will be chosen at random from
+    `ports_range`.
+
+    Args:
+        number_of: The number of packets to generate.
+        payload_size: The packet payload size to generate, capped based on `mtu`.
+        protocols: The protocols to use for the generated packets.
+        ports_range: The range of L4 port numbers to use. Used only if `protocols` has L4 protocols.
+        mtu: The MTU of the NIC port that will send out the generated packets.
+
+    Raises:
+        InternalError: If the `payload_size` is invalid.
+
+    Returns:
+        A list containing the randomly generated packets.
+    """
+    if payload_size < 0:
+        raise InternalError(f"An invalid payload_size of {payload_size} was given.")
+
+    l4_factories = []
+    if protocols & PacketProtocols.TCP:
+        l4_factories.append(TCP)
+    if protocols & PacketProtocols.UDP:
+        l4_factories.append(UDP)
+
+    def _make_packet() -> Packet:
+        packet = Ether()
+
+        if protocols & PacketProtocols.IP:
+            packet /= IP()
+
+        if len(l4_factories) > 0:
+            src_port, dst_port = random.choices(ports_range, k=2)
+            packet /= random.choice(l4_factories)(sport=src_port, dport=dst_port)
+
+        max_payload_size = mtu - len(packet)
+        usable_payload_size = payload_size if payload_size < max_payload_size else max_payload_size
+        return packet / random.randbytes(usable_payload_size)
+
+    return [_make_packet() for _ in range(number_of)]
-- 
2.34.1

[PATCH v3 4/5] dts: add ability to start/stop testpmd ports

[Luca Vizzarro]

Add testpmd commands to start and stop all the ports, so that they can
be configured. Because there is a distinction of commands that require
the ports to be stopped and started, also add decorators for commands
that require a specific state, removing this logic from the test
writer's duty.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 94 ++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 43e9f56517..57501eae18 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -14,16 +14,17 @@
     testpmd_shell.close()
 """
 
+import functools
 import re
 import time
 from dataclasses import dataclass, field
 from enum import Flag, auto
 from pathlib import PurePath
-from typing import ClassVar
+from typing import Any, Callable, ClassVar, Concatenate, ParamSpec
 
 from typing_extensions import Self, Unpack
 
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import InteractiveCommandExecutionError, InternalError
 from framework.params.testpmd import SimpleForwardingModes, TestPmdParams
 from framework.params.types import TestPmdParamsDict
 from framework.parser import ParserFn, TextParser
@@ -33,6 +34,9 @@
 from framework.testbed_model.sut_node import SutNode
 from framework.utils import StrEnum
 
+P = ParamSpec("P")
+TestPmdShellMethod = Callable[Concatenate["TestPmdShell", P], Any]
+
 
 class TestPmdDevice:
     """The data of a device that testpmd can recognize.
@@ -577,12 +581,57 @@ class TestPmdPortStats(TextParser):
     tx_bps: int = field(metadata=TextParser.find_int(r"Tx-bps:\s+(\d+)"))
 
 
+def requires_stopped_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require stopped ports.
+
+    If the decorated method is called while the ports are started, then these are stopped before
+    continuing.
+
+    Args:
+        func: The :class:`TestPmdShell` method to decorate.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if self.ports_started:
+            self._logger.debug("Ports need to be stopped to continue")
+            self.stop_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
+def requires_started_ports(func: TestPmdShellMethod) -> TestPmdShellMethod:
+    """Decorator for :class:`TestPmdShell` commands methods that require started ports.
+
+    If the decorated method is called while the ports are stopped, then these are started before
+    continuing.
+
+    Args:
+        func: The :class:`TestPmdShell` method to decorate.
+    """
+
+    @functools.wraps(func)
+    def _wrapper(self: "TestPmdShell", *args: P.args, **kwargs: P.kwargs):
+        if not self.ports_started:
+            self._logger.debug("Ports need to be started to continue")
+            self.start_all_ports()
+
+        return func(self, *args, **kwargs)
+
+    return _wrapper
+
+
 class TestPmdShell(DPDKShell):
     """Testpmd interactive shell.
 
     The testpmd shell users should never use
     the :meth:`~.interactive_shell.InteractiveShell.send_command` method directly, but rather
     call specialized methods. If there isn't one that satisfies a need, it should be added.
+
+    Attributes:
+        ports_started: Indicates whether the ports are started.
     """
 
     _app_params: TestPmdParams
@@ -596,6 +645,8 @@ class TestPmdShell(DPDKShell):
     #: This forces the prompt to appear after sending a command.
     _command_extra_chars: ClassVar[str] = "\n"
 
+    ports_started: bool
+
     def __init__(
         self,
         node: SutNode,
@@ -619,6 +670,9 @@ def __init__(
             name,
         )
 
+        self.ports_started = not self._app_params.disable_device_start
+
+    @requires_started_ports
     def start(self, verify: bool = True) -> None:
         """Start packet forwarding with the current configuration.
 
@@ -723,6 +777,42 @@ def set_forward_mode(self, mode: SimpleForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def stop_all_ports(self, verify: bool = True) -> None:
+        """Stops all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                stopped successfully.
+        """
+        self._logger.debug("Stopping all the ports...")
+        output = self.send_command("port stop all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not stopped successfully")
+
+        self.ports_started = False
+
+    def start_all_ports(self, verify: bool = True) -> None:
+        """Starts all the ports.
+
+        Args:
+            verify: If :data:`True`, the output of the command will be checked for a successful
+                execution.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the ports were not
+                started successfully.
+        """
+        self._logger.debug("Starting all the ports...")
+        output = self.send_command("port start all")
+        if verify and not output.strip().endswith("Done"):
+            raise InteractiveCommandExecutionError("Ports were not started successfully")
+
+        self.ports_started = True
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

[PATCH v3 5/5] dts: add testpmd set ports queues

[Luca Vizzarro]

Add a facility to update the number of TX/RX queues during the runtime
of testpmd.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 57501eae18..5b911ce538 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -813,6 +813,22 @@ def start_all_ports(self, verify: bool = True) -> None:
 
         self.ports_started = True
 
+    @requires_stopped_ports
+    def set_ports_queues(self, number_of: int) -> None:
+        """Sets the number of queues per port.
+
+        Args:
+            number_of: The number of RX/TX queues to create per port.
+
+        Raises:
+            InternalError: If `number_of` is invalid.
+        """
+        if number_of < 1:
+            raise InternalError("The number of queues must be positive and non-zero")
+
+        self.send_command(f"port config all rxq {number_of}")
+        self.send_command(f"port config all txq {number_of}")
+
     def show_port_info_all(self) -> list[TestPmdPort]:
         """Returns the information of all the ports.
 
-- 
2.34.1

Re: [PATCH v3 4/5] dts: add ability to start/stop testpmd ports

[Juraj Linkeš]

On 9. 9. 2024 13:01, Luca Vizzarro wrote:
> Add testpmd commands to start and stop all the ports, so that they can
> be configured. Because there is a distinction of commands that require
> the ports to be stopped and started, also add decorators for commands
> that require a specific state, removing this logic from the test
> writer's duty.
> 
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Re: [PATCH v3 5/5] dts: add testpmd set ports queues

[Juraj Linkeš]

On 9. 9. 2024 13:01, Luca Vizzarro wrote:
> Add a facility to update the number of TX/RX queues during the runtime
> of testpmd.
> 
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets

[Jeremy Spewock]

On Fri, Sep 6, 2024 at 12:23 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> On 09/08/2024 16:10, Jeremy Spewock wrote:
> > <snip>
> >> +    def match_all_packets(
> >> +        self, expected_packets: list[Packet], received_packets: list[Packet]
> >> +    ) -> None:
> >
> > This is a very interesting approach to comparing what you expect to
> > what you received. I hadn't seen counters used before but they seem
> > useful for this purpose. This method reminds me a lot of the filtering
> > process that was discussed in bugzilla ticket 1438
> > (https://bugs.dpdk.org/show_bug.cgi?id=1438) where there was some talk
> > about how to handle the noise vs what you received. This is different
> > in a few ways though of course in that it allows you to specify
> > multiple different criteria that are acceptable rather than just
> > getting the payload and really only concerns itself with matching
> > lists instead of doing any filtering.
> >
> > The main reason I mention this is it brought up a question for me: Do
> > you think, in the future when the payload filtering method is
> > implemented, these two methods will co-exist or it would make more
> > sense to just let test suites get their noise-free list of packets and
> > then check that what they care about is present? I think a method like
> > this might be useful in some more niche cases where you have multiple
> > packet structures to look for, so I don't doubt there are some reasons
> > to have both, I was just wondering if you had thought about it or had
> > an opinion.
>
> I am not actually entirely sure, it sounds as if everything has space
> for their own case. This method is useful to just plainly check that all
> the packets we sent came back, simple and straightforward. Payload
> filtering can be more niche and complex. I think if it can provide the
> same level of functionality as this function, it can be replaced for
> sure. But in that case it sounds like the test developer would be tied
> to a specific payload. In the case of random packet generation I don't
> think it would work, in which case they could both coexist.

Good points, this makes sense to me!

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Jeremy Spewock]

On Fri, Sep 6, 2024 at 12:46 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> On 23/08/2024 13:16, Juraj Linkeš wrote:
> > As Jeremy mentioned, adding the verify argument may be worthwhile, but
> > maybe only if we actually identify a usecase where we wouldn't want to
> > do the verification.
>
> Yeah, as I pointed out, it feels unlikely to pretend that they are
> started (or stopped). I think that could cause my unpredicted behaviour
> and confusion. But also as said already, it can always be added on demand.
>

I agree that we can save this as something to add as we need it and
that there could be some confusion to pretending the ports are in a
certain state. Since we actually save the state in testpmd, we
definitely want that to be accurate. The main thing I was thinking
about this for however was less about the state tracking and more
about what the expectation is for developers when they specifically
specify they do not want a method to be verified. There aren't many
cases where you wouldn't want to verify the outcome of a command, but
I would imagine the two reasons you would do so is either you don't
care about the result, or you specifically don't want to be disruptive
to the rest of the run if this command fails (like if the framework
has a testpmd shell running for some kind of process but doesn't want
to end the run if it fails). When we enforce the verification of some
part of the process like stopping the port in this case, that
assumption about not verifying changes. The second reason I mentioned
is why I believe capabilities checking specifies no-verify when it
updates the MTU, for example. I imagine we don't want the test run to
end if a testpmd method fails its check with capabilities, we would
want the capability to just be unsupported. Of course the developer
could catch the exception themselves, but in that case I think it
would make more sense for catching the exception to be the primary way
to avoid verification and we could just remove the parameter all
together so there isn't this edge-case for avoiding verification with
decorated methods. I guess even the static parameter that specifies
whether or not to verify doesn't really address this problem either,
haha. Maybe it would even make more sense for the decorator to assume
that it shouldn't throw an exception if the ports state changing
failed (but still avoid updating the port state) and let the verifying
process be completely down to if the method being decorated succeeded
or not with the ports in whatever state they were at the time since, I
would assume, if the ports fail to update their state the method will
also fail anyway.

All-in-all however, there likely won't be many cases where the ports
actually fail to stop in the first place, and not verifying in general
is uncommon. If what I mentioned above makes sense and we're fine with
the slight change in thinking, I think it is fine to merge this and
just keep in the back of our minds that this is the case, and maybe
either document somewhere that you have to catch the exception with
decorated methods, or just remove the verify parameter from decorated
methods all together and enforce that they must be verified. As
mentioned, it's not like we are forcing ourselves to stick with it,
adding the parameter is pretty easy to do on demand.

> > I also have two other points:
> > 1. Do we want a decorator that does both - starting and stopping? Is the
> > idea to have all methods that require a certain state to be decorated
> > and in that case we don't need this? E.g. if there are multiple
> > configuration steps, only the first one stops the ports (if started) and
> > we start the ports only when a method needs that (such as starting pkt
> > fwd)? I think this workflow should be documented in the class docstring
> > (the important part being that starting and stopping of ports is done
> > automatically).
>
> The way I envisioned these decorators is by using a lazy approach. I
> don't think it may be right to eagerly restart ports after stopping
> them, because maybe we want to run another command after that will stop
> them again. So only start and stop as needed. Ideally every port that
> requires a specific state of the ports need to be decorated. I went
> through all the methods in the class to check which would and which
> wouldn't, and it seems alright like this.
>
> > 2. Do we want decorators that start/stop just one port? I guess this is
> > basically useless with the above workflow.
>
> Would it actually matter? We are not really using the other ports in
> parallel here I believe. May bring unnecessary complexity. I am thinking
> that maybe if needed it could be added later.

Re: [PATCH v3 4/5] dts: add ability to start/stop testpmd ports

[Jeremy Spewock]

On Mon, Sep 9, 2024 at 7:03 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add testpmd commands to start and stop all the ports, so that they can
> be configured. Because there is a distinction of commands that require
> the ports to be stopped and started, also add decorators for commands
> that require a specific state, removing this logic from the test
> writer's duty.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

Re: [PATCH v3 5/5] dts: add testpmd set ports queues

[Jeremy Spewock]

On Mon, Sep 9, 2024 at 7:03 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add a facility to update the number of TX/RX queues during the runtime
> of testpmd.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports

[Juraj Linkeš]

On 9. 9. 2024 16:20, Jeremy Spewock wrote:
> On Fri, Sep 6, 2024 at 12:46 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>>
>> On 23/08/2024 13:16, Juraj Linkeš wrote:
>>> As Jeremy mentioned, adding the verify argument may be worthwhile, but
>>> maybe only if we actually identify a usecase where we wouldn't want to
>>> do the verification.
>>
>> Yeah, as I pointed out, it feels unlikely to pretend that they are
>> started (or stopped). I think that could cause my unpredicted behaviour
>> and confusion. But also as said already, it can always be added on demand.
>>
> 
> I agree that we can save this as something to add as we need it and
> that there could be some confusion to pretending the ports are in a
> certain state. Since we actually save the state in testpmd, we
> definitely want that to be accurate. The main thing I was thinking
> about this for however was less about the state tracking and more
> about what the expectation is for developers when they specifically
> specify they do not want a method to be verified. There aren't many
> cases where you wouldn't want to verify the outcome of a command, but
> I would imagine the two reasons you would do so is either you don't
> care about the result, or you specifically don't want to be disruptive
> to the rest of the run if this command fails (like if the framework
> has a testpmd shell running for some kind of process but doesn't want
> to end the run if it fails). When we enforce the verification of some
> part of the process like stopping the port in this case, that
> assumption about not verifying changes. The second reason I mentioned
> is why I believe capabilities checking specifies no-verify when it
> updates the MTU, for example. I imagine we don't want the test run to
> end if a testpmd method fails its check with capabilities, we would
> want the capability to just be unsupported. Of course the developer
> could catch the exception themselves, but in that case I think it
> would make more sense for catching the exception to be the primary way
> to avoid verification and we could just remove the parameter all
> together so there isn't this edge-case for avoiding verification with
> decorated methods. I guess even the static parameter that specifies
> whether or not to verify doesn't really address this problem either,
> haha. Maybe it would even make more sense for the decorator to assume
> that it shouldn't throw an exception if the ports state changing
> failed (but still avoid updating the port state) and let the verifying
> process be completely down to if the method being decorated succeeded
> or not with the ports in whatever state they were at the time since, I
> would assume, if the ports fail to update their state the method will
> also fail anyway.
> 
> All-in-all however, there likely won't be many cases where the ports
> actually fail to stop in the first place, and not verifying in general
> is uncommon. If what I mentioned above makes sense and we're fine with
> the slight change in thinking, I think it is fine to merge this and
> just keep in the back of our minds that this is the case, and maybe
> either document somewhere that you have to catch the exception with
> decorated methods, or just remove the verify parameter from decorated
> methods all together and enforce that they must be verified. As
> mentioned, it's not like we are forcing ourselves to stick with it,
> adding the parameter is pretty easy to do on demand.
> 

These are pretty good points. It makes sense to remove the verify 
parameter and if we don't care about the result (or the result should 
not be disrupting), we just catch the exception.

This patchset is not the place to settle this, so please open a bugzilla 
ticket for this. In the worst case scenario we just close the ticket, 
but it's good to have these thoughts in the ticket as well.

>>> I also have two other points:
>>> 1. Do we want a decorator that does both - starting and stopping? Is the
>>> idea to have all methods that require a certain state to be decorated
>>> and in that case we don't need this? E.g. if there are multiple
>>> configuration steps, only the first one stops the ports (if started) and
>>> we start the ports only when a method needs that (such as starting pkt
>>> fwd)? I think this workflow should be documented in the class docstring
>>> (the important part being that starting and stopping of ports is done
>>> automatically).
>>
>> The way I envisioned these decorators is by using a lazy approach. I
>> don't think it may be right to eagerly restart ports after stopping
>> them, because maybe we want to run another command after that will stop
>> them again. So only start and stop as needed. Ideally every port that
>> requires a specific state of the ports need to be decorated. I went
>> through all the methods in the class to check which would and which
>> wouldn't, and it seems alright like this.
>>
>>> 2. Do we want decorators that start/stop just one port? I guess this is
>>> basically useless with the above workflow.
>>
>> Would it actually matter? We are not really using the other ports in
>> parallel here I believe. May bring unnecessary complexity. I am thinking
>> that maybe if needed it could be added later.

Re: [PATCH 0/4] dts: add pktgen and testpmd changes

[Juraj Linkeš]

Series applied to next-dts, thanks.

On 6. 8. 2024 14:14, Luca Vizzarro wrote:
> From: Luca Vizzarro <luca.vizzarro@arm.com>
> 
> Hello,
> 
> sending some framework changes that will be required in my upcoming
> l2fwd test suite.
> 
> Best,
> Luca
> 
> Luca Vizzarro (5):
>    dts: add ability to send/receive multiple packets
>    dts: add random generation seed setting
>    dts: add random packet generator
>    dts: add ability to start/stop testpmd ports
>    dts: add testpmd set ports queues
> 
>   doc/guides/tools/dts.rst                      |   5 +
>   dts/framework/config/__init__.py              |   4 +
>   dts/framework/config/conf_yaml_schema.json    |   4 +
>   dts/framework/config/types.py                 |   2 +
>   dts/framework/remote_session/testpmd_shell.py | 102 +++++++++++++++++-
>   dts/framework/runner.py                       |   8 ++
>   dts/framework/settings.py                     |  17 +++
>   dts/framework/test_suite.py                   |  68 +++++++++++-
>   dts/framework/testbed_model/tg_node.py        |  14 +--
>   .../capturing_traffic_generator.py            |  31 ------
>   dts/framework/utils.py                        |  79 +++++++++++++-
>   11 files changed, 288 insertions(+), 46 deletions(-)
> 

Re: [PATCH v3 1/5] dts: add ability to send/receive multiple packets

[Patrick Robb]

Reviewed-by: Patrick Robb <probb@iol.unh.edu>

The only thought I had from this was that with more methods being
added to the framework for packet construction and validation (in this
case, your random packets utils.py method and the
testsuite.match_all_packets() strategy), and with multiple reference
solutions being available for traffic and testsuite validation, these
common strategies should be more comprehensively documented. In my
opinion the best case is immediately following "How to write a
testsuite," since having these reference strategies for validation
readily available is obviously helpful for new users. I've created a
DTS Bugzilla ticket for this, preliminarily assigning it to me:
https://bugs.dpdk.org/show_bug.cgi?id=1538

Thanks Luca.

On Mon, Sep 9, 2024 at 7:03 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> The framework allows only to send one packet at once via Scapy. This
> change adds the ability to send multiple packets, and also introduces a
> new fast way to verify if we received several expected packets.
>
> Moreover, it reduces code duplication by keeping a single packet sending
> method only at the test suite level.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Reviewed-by: Alex Chapman <alex.chapman@arm.com>
> Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  dts/framework/test_suite.py                   | 68 +++++++++++++++++--
>  dts/framework/testbed_model/tg_node.py        | 14 ++--
>  .../capturing_traffic_generator.py            | 31 ---------
>  3 files changed, 71 insertions(+), 42 deletions(-)
>
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 694b2eba65..051509fb86 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -13,12 +13,13 @@
>      * Test case verification.
>  """
>
> +from collections import Counter
>  from ipaddress import IPv4Interface, IPv6Interface, ip_interface
>  from typing import ClassVar, Union
>
>  from scapy.layers.inet import IP  # type: ignore[import-untyped]
>  from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
> -from scapy.packet import Packet, Padding  # type: ignore[import-untyped]
> +from scapy.packet import Packet, Padding, raw  # type: ignore[import-untyped]
>
>  from framework.testbed_model.port import Port, PortLink
>  from framework.testbed_model.sut_node import SutNode
> @@ -199,9 +200,34 @@ def send_packet_and_capture(
>          Returns:
>              A list of received packets.
>          """
> -        packet = self._adjust_addresses(packet)
> -        return self.tg_node.send_packet_and_capture(
> -            packet,
> +        return self.send_packets_and_capture(
> +            [packet],
> +            filter_config,
> +            duration,
> +        )
> +
> +    def send_packets_and_capture(
> +        self,
> +        packets: list[Packet],
> +        filter_config: PacketFilteringConfig = PacketFilteringConfig(),
> +        duration: float = 1,
> +    ) -> list[Packet]:
> +        """Send and receive `packets` using the associated TG.
> +
> +        Send `packets` through the appropriate interface and receive on the appropriate interface.
> +        Modify the packets with l3/l2 addresses corresponding to the testbed and desired traffic.
> +
> +        Args:
> +            packets: The packets to send.
> +            filter_config: The filter to use when capturing packets.
> +            duration: Capture traffic for this amount of time after sending `packet`.
> +
> +        Returns:
> +            A list of received packets.
> +        """
> +        packets = [self._adjust_addresses(packet) for packet in packets]
> +        return self.tg_node.send_packets_and_capture(
> +            packets,
>              self._tg_port_egress,
>              self._tg_port_ingress,
>              filter_config,
> @@ -303,6 +329,40 @@ def verify_packets(self, expected_packet: Packet, received_packets: list[Packet]
>              )
>              self._fail_test_case_verify("An expected packet not found among received packets.")
>
> +    def match_all_packets(
> +        self, expected_packets: list[Packet], received_packets: list[Packet]
> +    ) -> None:
> +        """Matches all the expected packets against the received ones.
> +
> +        Matching is performed by counting down the occurrences in a dictionary which keys are the
> +        raw packet bytes. No deep packet comparison is performed. All the unexpected packets (noise)
> +        are automatically ignored.
> +
> +        Args:
> +            expected_packets: The packets we are expecting to receive.
> +            received_packets: All the packets that were received.
> +
> +        Raises:
> +            TestCaseVerifyError: if and not all the `expected_packets` were found in
> +                `received_packets`.
> +        """
> +        expected_packets_counters = Counter(map(raw, expected_packets))
> +        received_packets_counters = Counter(map(raw, received_packets))
> +        # The number of expected packets is subtracted by the number of received packets, ignoring
> +        # any unexpected packets and capping at zero.
> +        missing_packets_counters = expected_packets_counters - received_packets_counters
> +        missing_packets_count = missing_packets_counters.total()
> +        self._logger.debug(
> +            f"match_all_packets: expected {len(expected_packets)}, "
> +            f"received {len(received_packets)}, missing {missing_packets_count}"
> +        )
> +
> +        if missing_packets_count != 0:
> +            self._fail_test_case_verify(
> +                f"Not all packets were received, expected {len(expected_packets)} "
> +                f"but {missing_packets_count} were missing."
> +            )
> +
>      def _compare_packets(self, expected_packet: Packet, received_packet: Packet) -> bool:
>          self._logger.debug(
>              f"Comparing packets: \n{expected_packet.summary()}\n{received_packet.summary()}"
> diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
> index 4ee326e99c..19b5b6e74c 100644
> --- a/dts/framework/testbed_model/tg_node.py
> +++ b/dts/framework/testbed_model/tg_node.py
> @@ -51,22 +51,22 @@ def __init__(self, node_config: TGNodeConfiguration):
>          self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
>          self._logger.info(f"Created node: {self.name}")
>
> -    def send_packet_and_capture(
> +    def send_packets_and_capture(
>          self,
> -        packet: Packet,
> +        packets: list[Packet],
>          send_port: Port,
>          receive_port: Port,
>          filter_config: PacketFilteringConfig = PacketFilteringConfig(),
>          duration: float = 1,
>      ) -> list[Packet]:
> -        """Send `packet`, return received traffic.
> +        """Send `packets`, return received traffic.
>
> -        Send `packet` on `send_port` and then return all traffic captured
> +        Send `packets` on `send_port` and then return all traffic captured
>          on `receive_port` for the given duration. Also record the captured traffic
>          in a pcap file.
>
>          Args:
> -            packet: The packet to send.
> +            packets: The packets to send.
>              send_port: The egress port on the TG node.
>              receive_port: The ingress port in the TG node.
>              filter_config: The filter to use when capturing packets.
> @@ -75,8 +75,8 @@ def send_packet_and_capture(
>          Returns:
>               A list of received packets. May be empty if no packets are captured.
>          """
> -        return self.traffic_generator.send_packet_and_capture(
> -            packet,
> +        return self.traffic_generator.send_packets_and_capture(
> +            packets,
>              send_port,
>              receive_port,
>              filter_config,
> diff --git a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> index c8380b7d57..66a77da9c4 100644
> --- a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> +++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> @@ -63,37 +63,6 @@ def is_capturing(self) -> bool:
>          """This traffic generator can capture traffic."""
>          return True
>
> -    def send_packet_and_capture(
> -        self,
> -        packet: Packet,
> -        send_port: Port,
> -        receive_port: Port,
> -        filter_config: PacketFilteringConfig,
> -        duration: float,
> -        capture_name: str = _get_default_capture_name(),
> -    ) -> list[Packet]:
> -        """Send `packet` and capture received traffic.
> -
> -        Send `packet` on `send_port` and then return all traffic captured
> -        on `receive_port` for the given `duration`.
> -
> -        The captured traffic is recorded in the `capture_name`.pcap file.
> -
> -        Args:
> -            packet: The packet to send.
> -            send_port: The egress port on the TG node.
> -            receive_port: The ingress port in the TG node.
> -            filter_config: Filters to apply when capturing packets.
> -            duration: Capture traffic for this amount of time after sending the packet.
> -            capture_name: The name of the .pcap file where to store the capture.
> -
> -        Returns:
> -             The received packets. May be empty if no packets are captured.
> -        """
> -        return self.send_packets_and_capture(
> -            [packet], send_port, receive_port, filter_config, duration, capture_name
> -        )
> -
>      def send_packets_and_capture(
>          self,
>          packets: list[Packet],
> --
> 2.34.1
>