On 11/22/23 11:38, Juraj Linkeš wrote:
On Tue, Nov 21, 2023 at 5:20 PM Yoan Picchi <yoan.pic...@foss.arm.com> wrote:
On 11/15/23 13:09, Juraj Linkeš wrote:
Format according to the Google format and PEP257, with slight
deviations.
Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech>
---
.../traffic_generator/__init__.py | 22 ++++++++-
.../capturing_traffic_generator.py | 46 +++++++++++--------
.../traffic_generator/traffic_generator.py | 33 +++++++------
3 files changed, 68 insertions(+), 33 deletions(-)
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py
b/dts/framework/testbed_model/traffic_generator/__init__.py
index 11bfa1ee0f..51cca77da4 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -1,6 +1,19 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2023 PANTHEON.tech s.r.o.
+"""DTS traffic generators.
+
+A traffic generator is capable of generating traffic and then monitor
returning traffic.
+A traffic generator may just count the number of received packets
+and it may additionally capture individual packets.
The sentence feels odd. Isn't it supposed to be "or" here? and no need
for that early of a line break
There are two mays, so there probably should be an or. But I'd like to
reword it to this:
All traffic generators count the number of received packets, and they
may additionally
capture individual packets.
What do you think?
I think it's better with the new sentence. But I think it'd be even
better to split into two sentences to highlight the must/may:
All traffic generators must count the number of received packets. Some
may additionally capture individual packets.
+
+A traffic generator may be software running on generic hardware or it could be
specialized hardware.
+
+The traffic generators that only count the number of received packets are
suitable only for
+performance testing. In functional testing, we need to be able to dissect each
arrived packet
+and a capturing traffic generator is required.
+"""
+
from framework.config import ScapyTrafficGeneratorConfig,
TrafficGeneratorType
from framework.exception import ConfigurationError
from framework.testbed_model.node import Node
@@ -12,8 +25,15 @@
def create_traffic_generator(
tg_node: Node, traffic_generator_config: ScapyTrafficGeneratorConfig
) -> CapturingTrafficGenerator:
- """A factory function for creating traffic generator object from user
config."""
+ """The factory function for creating traffic generator objects from the
test run configuration.
+
+ Args:
+ tg_node: The traffic generator node where the created traffic
generator will be running.
+ traffic_generator_config: The traffic generator config.
+ Returns:
+ A traffic generator capable of capturing received packets.
+ """
match traffic_generator_config.traffic_generator_type:
case TrafficGeneratorType.SCAPY:
return ScapyTrafficGenerator(tg_node, traffic_generator_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 e521211ef0..b0a43ad003 100644
---
a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
+++
b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
@@ -23,19 +23,22 @@
def _get_default_capture_name() -> str:
- """
- This is the function used for the default implementation of capture names.
- """
return str(uuid.uuid4())
class CapturingTrafficGenerator(TrafficGenerator):
"""Capture packets after sending traffic.
- A mixin interface which enables a packet generator to declare that it can
capture
+ The intermediary interface which enables a packet generator to declare
that it can capture
packets and return them to the user.
+ Similarly to
+
:class:`~framework.testbed_model.traffic_generator.traffic_generator.TrafficGenerator`,
+ this class exposes the public methods specific to capturing traffic
generators and defines
+ a private method that must implement the traffic generation and capturing
logic in subclasses.
+
The methods of capturing traffic generators obey the following workflow:
+
1. send packets
2. capture packets
3. write the capture to a .pcap file
@@ -44,6 +47,7 @@ class CapturingTrafficGenerator(TrafficGenerator):
@property
def is_capturing(self) -> bool:
+ """This traffic generator can capture traffic."""
return True
def send_packet_and_capture(
@@ -54,11 +58,12 @@ def send_packet_and_capture(
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
- """Send a packet, return received traffic.
+ """Send `packet` and capture received traffic.
+
+ Send `packet` on `send_port` and then return all traffic captured
+ on `receive_port` for the given `duration`.
- Send a packet on the send_port and then return all traffic captured
- on the receive_port for the given duration. Also record the captured
traffic
- in a pcap file.
+ The captured traffic is recorded in the `capture_name`.pcap file.
Args:
packet: The packet to send.
@@ -68,7 +73,7 @@ def send_packet_and_capture(
capture_name: The name of the .pcap file where to store the
capture.
Returns:
- A list of received packets. May be empty if no packets are
captured.
+ The received packets. May be empty if no packets are captured.
"""
return self.send_packets_and_capture(
[packet], send_port, receive_port, duration, capture_name
@@ -82,11 +87,14 @@ def send_packets_and_capture(
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
- """Send packets, return received traffic.
+ """Send `packets` and capture received traffic.
- Send packets on the send_port and then return all traffic captured
- on the receive_port for the given duration. Also record the captured
traffic
- in a pcap file.
+ Send `packets` 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. The
target directory
+ can be configured with the :option:`--output-dir` command line
argument or
+ the :envvar:`DTS_OUTPUT_DIR` environment variable.
Args:
packets: The packets to send.
@@ -96,7 +104,7 @@ def send_packets_and_capture(
capture_name: The name of the .pcap file where to store the
capture.
Returns:
- A list of received packets. May be empty if no packets are
captured.
+ The received packets. May be empty if no packets are captured.
"""
self._logger.debug(get_packet_summaries(packets))
self._logger.debug(
@@ -124,10 +132,12 @@ def _send_packets_and_capture(
receive_port: Port,
duration: float,
) -> list[Packet]:
- """
- The extended classes must implement this method which
- sends packets on send_port and receives packets on the receive_port
- for the specified duration. It must be able to handle no received
packets.
+ """The implementation of :method:`send_packets_and_capture`.
+
+ The subclasses must implement this method which sends `packets` on
`send_port`
+ and receives packets on `receive_port` for the specified `duration`.
+
+ It must be able to handle no received packets.
This sentence feels odd too. Maybe "It must be able to handle receiving
no packets."
Right, your suggestion is better.
"""
def _write_capture_from_packets(
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index ea7c3963da..ed396c6a2f 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -22,7 +22,8 @@
class TrafficGenerator(ABC):
"""The base traffic generator.
- Defines the few basic methods that each traffic generator must implement.
+ Exposes the common public methods of all traffic generators and defines
private methods
+ that must implement the traffic generation logic in subclasses.
"""
_config: TrafficGeneratorConfig
@@ -30,6 +31,12 @@ class TrafficGenerator(ABC):
_logger: DTSLOG
def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
+ """Initialize the traffic generator.
+
+ Args:
+ tg_node: The traffic generator node where the created traffic
generator will be running.
+ config: The traffic generator's test run configuration.
+ """
self._config = config
self._tg_node = tg_node
self._logger = getLogger(
@@ -37,9 +44,9 @@ def __init__(self, tg_node: Node, config:
TrafficGeneratorConfig):
)
def send_packet(self, packet: Packet, port: Port) -> None:
- """Send a packet and block until it is fully sent.
+ """Send `packet` and block until it is fully sent.
- What fully sent means is defined by the traffic generator.
+ Send `packet` on `port`, then wait until `packet` is fully sent.
Args:
packet: The packet to send.
@@ -48,9 +55,9 @@ def send_packet(self, packet: Packet, port: Port) -> None:
self.send_packets([packet], port)
def send_packets(self, packets: list[Packet], port: Port) -> None:
- """Send packets and block until they are fully sent.
+ """Send `packets` and block until they are fully sent.
- What fully sent means is defined by the traffic generator.
+ Send `packets` on `port`, then wait until `packets` are fully sent.
Args:
packets: The packets to send.
@@ -62,19 +69,17 @@ def send_packets(self, packets: list[Packet], port: Port)
-> None:
@abstractmethod
def _send_packets(self, packets: list[Packet], port: Port) -> None:
- """
- The extended classes must implement this method which
- sends packets on send_port. The method should block until all packets
- are fully sent.
+ """The implementation of :method:`send_packets`.
+
+ The subclasses must implement this method which sends `packets` on
`port`.
+ The method should block until all `packets` are fully sent.
+
+ What full sent means is defined by the traffic generator.
full -> fully
"""
@property
def is_capturing(self) -> bool:
- """Whether this traffic generator can capture traffic.
-
- Returns:
- True if the traffic generator can capture traffic, False otherwise.
- """
+ """This traffic generator can't capture traffic."""
return False
@abstractmethod
