* [PATCH 1/5] dts: add ability to send/receive multiple packets
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
@ 2024-08-06 12:14 ` Luca Vizzarro
2024-08-06 12:14 ` [PATCH 2/5] dts: add random generation seed setting Luca Vizzarro
` (7 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:14 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczpanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH 2/5] dts: add random generation seed setting
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
2024-08-06 12:14 ` [PATCH 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
@ 2024-08-06 12:14 ` Luca Vizzarro
2024-08-06 12:14 ` [PATCH 3/5] dts: add random packet generator Luca Vizzarro
` (6 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:14 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczpanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH 3/5] dts: add random packet generator
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
2024-08-06 12:14 ` [PATCH 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
2024-08-06 12:14 ` [PATCH 2/5] dts: add random generation seed setting Luca Vizzarro
@ 2024-08-06 12:14 ` Luca Vizzarro
2024-08-06 12:14 ` [PATCH 4/5] dts: add ability to start/stop ports Luca Vizzarro
` (5 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:14 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczpanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH 4/5] dts: add ability to start/stop ports
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
` (2 preceding siblings ...)
2024-08-06 12:14 ` [PATCH 3/5] dts: add random packet generator Luca Vizzarro
@ 2024-08-06 12:14 ` Luca Vizzarro
2024-08-06 12:14 ` [PATCH 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
` (4 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:14 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczpanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH 4/5] dts: add ability to start/stop testpmd ports
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
` (3 preceding siblings ...)
2024-08-06 12:14 ` [PATCH 4/5] dts: add ability to start/stop ports Luca Vizzarro
@ 2024-08-06 12:14 ` Luca Vizzarro
2024-08-06 12:14 ` [PATCH 5/5] dts: add testpmd set ports queues Luca Vizzarro
` (3 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:14 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczpanek
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH 5/5] dts: add testpmd set ports queues
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
` (4 preceding siblings ...)
2024-08-06 12:14 ` [PATCH 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
@ 2024-08-06 12:14 ` Luca Vizzarro
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
` (2 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:14 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczpanek
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v2 0/5] dts: add pktgen and testpmd changes
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
` (5 preceding siblings ...)
2024-08-06 12:14 ` [PATCH 5/5] dts: add testpmd set ports queues Luca Vizzarro
@ 2024-08-06 12:46 ` Luca Vizzarro
2024-08-06 12:46 ` [PATCH v2 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
` (4 more replies)
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
2024-09-09 15:50 ` [PATCH 0/4] dts: add pktgen and testpmd changes Juraj Linkeš
8 siblings, 5 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:46 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš, Honnappa Nagarahalli, 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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v2 1/5] dts: add ability to send/receive multiple packets
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
@ 2024-08-06 12:46 ` Luca Vizzarro
2024-08-09 15:10 ` Jeremy Spewock
2024-08-23 10:17 ` Juraj Linkeš
2024-08-06 12:46 ` [PATCH v2 2/5] dts: add random generation seed setting Luca Vizzarro
` (3 subsequent siblings)
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:46 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets
2024-08-06 12:46 ` [PATCH v2 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
@ 2024-08-09 15:10 ` Jeremy Spewock
2024-09-06 16:23 ` Luca Vizzarro
2024-08-23 10:17 ` Juraj Linkeš
1 sibling, 1 reply; 47+ messages in thread
From: Jeremy Spewock @ 2024-08-09 15:10 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets
2024-08-09 15:10 ` Jeremy Spewock
@ 2024-09-06 16:23 ` Luca Vizzarro
2024-09-09 14:12 ` Jeremy Spewock
0 siblings, 1 reply; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:23 UTC (permalink / raw)
To: Jeremy Spewock
Cc: dev, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets
2024-09-06 16:23 ` Luca Vizzarro
@ 2024-09-09 14:12 ` Jeremy Spewock
0 siblings, 0 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-09-09 14:12 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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!
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets
2024-08-06 12:46 ` [PATCH v2 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
2024-08-09 15:10 ` Jeremy Spewock
@ 2024-08-23 10:17 ` Juraj Linkeš
2024-09-06 16:30 ` Luca Vizzarro
1 sibling, 1 reply; 47+ messages in thread
From: Juraj Linkeš @ 2024-08-23 10:17 UTC (permalink / raw)
To: Luca Vizzarro, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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.
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 1/5] dts: add ability to send/receive multiple packets
2024-08-23 10:17 ` Juraj Linkeš
@ 2024-09-06 16:30 ` Luca Vizzarro
0 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:30 UTC (permalink / raw)
To: Juraj Linkeš, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2 2/5] dts: add random generation seed setting
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
2024-08-06 12:46 ` [PATCH v2 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
@ 2024-08-06 12:46 ` Luca Vizzarro
2024-08-09 15:10 ` Jeremy Spewock
2024-08-23 10:30 ` Juraj Linkeš
2024-08-06 12:46 ` [PATCH v2 3/5] dts: add random packet generator Luca Vizzarro
` (2 subsequent siblings)
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:46 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 2/5] dts: add random generation seed setting
2024-08-06 12:46 ` [PATCH v2 2/5] dts: add random generation seed setting Luca Vizzarro
@ 2024-08-09 15:10 ` Jeremy Spewock
2024-08-23 10:30 ` Juraj Linkeš
1 sibling, 0 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-08-09 15:10 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 2/5] dts: add random generation seed setting
2024-08-06 12:46 ` [PATCH v2 2/5] dts: add random generation seed setting Luca Vizzarro
2024-08-09 15:10 ` Jeremy Spewock
@ 2024-08-23 10:30 ` Juraj Linkeš
1 sibling, 0 replies; 47+ messages in thread
From: Juraj Linkeš @ 2024-08-23 10:30 UTC (permalink / raw)
To: Luca Vizzarro, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2 3/5] dts: add random packet generator
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
2024-08-06 12:46 ` [PATCH v2 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
2024-08-06 12:46 ` [PATCH v2 2/5] dts: add random generation seed setting Luca Vizzarro
@ 2024-08-06 12:46 ` Luca Vizzarro
2024-08-09 15:11 ` Jeremy Spewock
2024-08-23 11:58 ` Juraj Linkeš
2024-08-06 12:46 ` [PATCH v2 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
2024-08-06 12:46 ` [PATCH v2 5/5] dts: add testpmd set ports queues Luca Vizzarro
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:46 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek,
Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 3/5] dts: add random packet generator
2024-08-06 12:46 ` [PATCH v2 3/5] dts: add random packet generator Luca Vizzarro
@ 2024-08-09 15:11 ` Jeremy Spewock
2024-08-23 11:58 ` Juraj Linkeš
1 sibling, 0 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-08-09 15:11 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 3/5] dts: add random packet generator
2024-08-06 12:46 ` [PATCH v2 3/5] dts: add random packet generator Luca Vizzarro
2024-08-09 15:11 ` Jeremy Spewock
@ 2024-08-23 11:58 ` Juraj Linkeš
2024-09-06 16:31 ` Luca Vizzarro
1 sibling, 1 reply; 47+ messages in thread
From: Juraj Linkeš @ 2024-08-23 11:58 UTC (permalink / raw)
To: Luca Vizzarro, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 3/5] dts: add random packet generator
2024-08-23 11:58 ` Juraj Linkeš
@ 2024-09-06 16:31 ` Luca Vizzarro
0 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:31 UTC (permalink / raw)
To: Juraj Linkeš, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
` (2 preceding siblings ...)
2024-08-06 12:46 ` [PATCH v2 3/5] dts: add random packet generator Luca Vizzarro
@ 2024-08-06 12:46 ` Luca Vizzarro
2024-08-09 15:13 ` Jeremy Spewock
2024-08-23 12:16 ` Juraj Linkeš
2024-08-06 12:46 ` [PATCH v2 5/5] dts: add testpmd set ports queues Luca Vizzarro
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:46 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-08-06 12:46 ` [PATCH v2 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
@ 2024-08-09 15:13 ` Jeremy Spewock
2024-09-06 16:41 ` Luca Vizzarro
2024-08-23 12:16 ` Juraj Linkeš
1 sibling, 1 reply; 47+ messages in thread
From: Jeremy Spewock @ 2024-08-09 15:13 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
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
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-08-09 15:13 ` Jeremy Spewock
@ 2024-09-06 16:41 ` Luca Vizzarro
0 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:41 UTC (permalink / raw)
To: Jeremy Spewock
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-08-06 12:46 ` [PATCH v2 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
2024-08-09 15:13 ` Jeremy Spewock
@ 2024-08-23 12:16 ` Juraj Linkeš
2024-09-06 16:46 ` Luca Vizzarro
1 sibling, 1 reply; 47+ messages in thread
From: Juraj Linkeš @ 2024-08-23 12:16 UTC (permalink / raw)
To: Luca Vizzarro, dev; +Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-08-23 12:16 ` Juraj Linkeš
@ 2024-09-06 16:46 ` Luca Vizzarro
2024-09-09 7:32 ` Juraj Linkeš
2024-09-09 14:20 ` Jeremy Spewock
0 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:46 UTC (permalink / raw)
To: Juraj Linkeš, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-09-06 16:46 ` Luca Vizzarro
@ 2024-09-09 7:32 ` Juraj Linkeš
2024-09-09 14:20 ` Jeremy Spewock
1 sibling, 0 replies; 47+ messages in thread
From: Juraj Linkeš @ 2024-09-09 7:32 UTC (permalink / raw)
To: Luca Vizzarro, dev; +Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-09-06 16:46 ` Luca Vizzarro
2024-09-09 7:32 ` Juraj Linkeš
@ 2024-09-09 14:20 ` Jeremy Spewock
2024-09-09 15:16 ` Juraj Linkeš
1 sibling, 1 reply; 47+ messages in thread
From: Jeremy Spewock @ 2024-09-09 14:20 UTC (permalink / raw)
To: Luca Vizzarro
Cc: Juraj Linkeš, dev, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 4/5] dts: add ability to start/stop testpmd ports
2024-09-09 14:20 ` Jeremy Spewock
@ 2024-09-09 15:16 ` Juraj Linkeš
0 siblings, 0 replies; 47+ messages in thread
From: Juraj Linkeš @ 2024-09-09 15:16 UTC (permalink / raw)
To: Jeremy Spewock, Luca Vizzarro; +Cc: dev, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2 5/5] dts: add testpmd set ports queues
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
` (3 preceding siblings ...)
2024-08-06 12:46 ` [PATCH v2 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
@ 2024-08-06 12:46 ` Luca Vizzarro
2024-08-09 15:14 ` Jeremy Spewock
2024-08-23 12:22 ` Juraj Linkeš
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-08-06 12:46 UTC (permalink / raw)
To: dev
Cc: Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 5/5] dts: add testpmd set ports queues
2024-08-06 12:46 ` [PATCH v2 5/5] dts: add testpmd set ports queues Luca Vizzarro
@ 2024-08-09 15:14 ` Jeremy Spewock
2024-08-20 16:04 ` Jeremy Spewock
2024-09-06 16:51 ` Luca Vizzarro
2024-08-23 12:22 ` Juraj Linkeš
1 sibling, 2 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-08-09 15:14 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
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
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 5/5] dts: add testpmd set ports queues
2024-08-09 15:14 ` Jeremy Spewock
@ 2024-08-20 16:04 ` Jeremy Spewock
2024-09-06 16:51 ` Luca Vizzarro
1 sibling, 0 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-08-20 16:04 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
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
> >
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 5/5] dts: add testpmd set ports queues
2024-08-09 15:14 ` Jeremy Spewock
2024-08-20 16:04 ` Jeremy Spewock
@ 2024-09-06 16:51 ` Luca Vizzarro
1 sibling, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:51 UTC (permalink / raw)
To: Jeremy Spewock
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 5/5] dts: add testpmd set ports queues
2024-08-06 12:46 ` [PATCH v2 5/5] dts: add testpmd set ports queues Luca Vizzarro
2024-08-09 15:14 ` Jeremy Spewock
@ 2024-08-23 12:22 ` Juraj Linkeš
2024-09-06 16:53 ` Luca Vizzarro
1 sibling, 1 reply; 47+ messages in thread
From: Juraj Linkeš @ 2024-08-23 12:22 UTC (permalink / raw)
To: Luca Vizzarro, dev; +Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek
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.
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v2 5/5] dts: add testpmd set ports queues
2024-08-23 12:22 ` Juraj Linkeš
@ 2024-09-06 16:53 ` Luca Vizzarro
0 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-06 16:53 UTC (permalink / raw)
To: Juraj Linkeš, dev
Cc: Jeremy Spewock, Honnappa Nagarahalli, Paul Szczepanek
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 0/5] dts: add pktgen and testpmd changes
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
` (6 preceding siblings ...)
2024-08-06 12:46 ` [PATCH v2 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
@ 2024-09-09 11:01 ` Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
` (4 more replies)
2024-09-09 15:50 ` [PATCH 0/4] dts: add pktgen and testpmd changes Juraj Linkeš
8 siblings, 5 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-09 11:01 UTC (permalink / raw)
To: dev; +Cc: Honnappa Nagarahalli, 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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v3 1/5] dts: add ability to send/receive multiple packets
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
@ 2024-09-09 11:01 ` Luca Vizzarro
2024-09-09 17:12 ` Patrick Robb
2024-09-09 11:01 ` [PATCH v3 2/5] dts: add random generation seed setting Luca Vizzarro
` (3 subsequent siblings)
4 siblings, 1 reply; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-09 11:01 UTC (permalink / raw)
To: dev
Cc: Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek,
Alex Chapman, Jeremy Spewock, Juraj Linkeš
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 1/5] dts: add ability to send/receive multiple packets
2024-09-09 11:01 ` [PATCH v3 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
@ 2024-09-09 17:12 ` Patrick Robb
0 siblings, 0 replies; 47+ messages in thread
From: Patrick Robb @ 2024-09-09 17:12 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Honnappa Nagarahalli, Paul Szczepanek, Alex Chapman,
Jeremy Spewock, Juraj Linkeš
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
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 2/5] dts: add random generation seed setting
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
@ 2024-09-09 11:01 ` Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 3/5] dts: add random packet generator Luca Vizzarro
` (2 subsequent siblings)
4 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-09 11:01 UTC (permalink / raw)
To: dev
Cc: Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek,
Alex Chapman, Jeremy Spewock, Juraj Linkeš
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v3 3/5] dts: add random packet generator
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 1/5] dts: add ability to send/receive multiple packets Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 2/5] dts: add random generation seed setting Luca Vizzarro
@ 2024-09-09 11:01 ` Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
2024-09-09 11:01 ` [PATCH v3 5/5] dts: add testpmd set ports queues Luca Vizzarro
4 siblings, 0 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-09 11:01 UTC (permalink / raw)
To: dev
Cc: Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek,
Alex Chapman, Jeremy Spewock, Juraj Linkeš
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v3 4/5] dts: add ability to start/stop testpmd ports
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
` (2 preceding siblings ...)
2024-09-09 11:01 ` [PATCH v3 3/5] dts: add random packet generator Luca Vizzarro
@ 2024-09-09 11:01 ` Luca Vizzarro
2024-09-09 11:54 ` Juraj Linkeš
2024-09-09 14:20 ` Jeremy Spewock
2024-09-09 11:01 ` [PATCH v3 5/5] dts: add testpmd set ports queues Luca Vizzarro
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-09 11:01 UTC (permalink / raw)
To: dev
Cc: Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek, Juraj Linkeš
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 4/5] dts: add ability to start/stop testpmd ports
2024-09-09 11:01 ` [PATCH v3 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
@ 2024-09-09 11:54 ` Juraj Linkeš
2024-09-09 14:20 ` Jeremy Spewock
1 sibling, 0 replies; 47+ messages in thread
From: Juraj Linkeš @ 2024-09-09 11:54 UTC (permalink / raw)
To: Luca Vizzarro, dev; +Cc: Honnappa Nagarahalli, Paul Szczepanek
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 4/5] dts: add ability to start/stop testpmd ports
2024-09-09 11:01 ` [PATCH v3 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
2024-09-09 11:54 ` Juraj Linkeš
@ 2024-09-09 14:20 ` Jeremy Spewock
1 sibling, 0 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-09-09 14:20 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Honnappa Nagarahalli, Paul Szczepanek, Juraj Linkeš
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 5/5] dts: add testpmd set ports queues
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
` (3 preceding siblings ...)
2024-09-09 11:01 ` [PATCH v3 4/5] dts: add ability to start/stop testpmd ports Luca Vizzarro
@ 2024-09-09 11:01 ` Luca Vizzarro
2024-09-09 12:01 ` Juraj Linkeš
2024-09-09 14:20 ` Jeremy Spewock
4 siblings, 2 replies; 47+ messages in thread
From: Luca Vizzarro @ 2024-09-09 11:01 UTC (permalink / raw)
To: dev
Cc: Honnappa Nagarahalli, Luca Vizzarro, Paul Szczepanek, Juraj Linkeš
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 5/5] dts: add testpmd set ports queues
2024-09-09 11:01 ` [PATCH v3 5/5] dts: add testpmd set ports queues Luca Vizzarro
@ 2024-09-09 12:01 ` Juraj Linkeš
2024-09-09 14:20 ` Jeremy Spewock
1 sibling, 0 replies; 47+ messages in thread
From: Juraj Linkeš @ 2024-09-09 12:01 UTC (permalink / raw)
To: Luca Vizzarro, dev; +Cc: Honnappa Nagarahalli, Paul Szczepanek
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 5/5] dts: add testpmd set ports queues
2024-09-09 11:01 ` [PATCH v3 5/5] dts: add testpmd set ports queues Luca Vizzarro
2024-09-09 12:01 ` Juraj Linkeš
@ 2024-09-09 14:20 ` Jeremy Spewock
1 sibling, 0 replies; 47+ messages in thread
From: Jeremy Spewock @ 2024-09-09 14:20 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Honnappa Nagarahalli, Paul Szczepanek, Juraj Linkeš
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>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 0/4] dts: add pktgen and testpmd changes
2024-08-06 12:14 [PATCH 0/4] dts: add pktgen and testpmd changes Luca Vizzarro
` (7 preceding siblings ...)
2024-09-09 11:01 ` [PATCH v3 0/5] dts: add pktgen and testpmd changes Luca Vizzarro
@ 2024-09-09 15:50 ` Juraj Linkeš
8 siblings, 0 replies; 47+ messages in thread
From: Juraj Linkeš @ 2024-09-09 15:50 UTC (permalink / raw)
To: Luca Vizzarro, dev; +Cc: Jeremy Spewock, Honnappa Nagarahalli
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(-)
>
^ permalink raw reply [flat|nested] 47+ messages in thread