From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 459C74339B; Wed, 22 Nov 2023 12:39:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D64A540DDC; Wed, 22 Nov 2023 12:39:02 +0100 (CET) Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by mails.dpdk.org (Postfix) with ESMTP id 439464028C for ; Wed, 22 Nov 2023 12:39:00 +0100 (CET) Received: by mail-ed1-f49.google.com with SMTP id 4fb4d7f45d1cf-548a2c20f50so4958140a12.1 for ; Wed, 22 Nov 2023 03:39:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700653140; x=1701257940; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=PO5SJazTh187AwMTPJGUXaXm5ytcNNcWwFEIaAXetg8=; b=FLXgxwx4z3ixLpI6F8VOx1Umgd7puiT5UqKCqZorqnU21h50/rhEAU60ZO+lhXjr+5 whEoOYwi/JCVn9g1s1UwTAG8BpYhtsQgdnJZXdiy5qw9GbY0lFznEbCqOwYili/7bkFT BjvWEVgI+dJSYNXSq+gaRG9ayYs+vVP8My5NW/P1HQpFnQ1l+xNm3CX5RQRQ+LDBexje 45pkk55mS395E0z0keSGpVHZGuXSrVhXNxfqsUnXOADl/0N8nkWnO8uql97OEGqyqYFa hCqEd0ZuwyxX3RH+ZMGrEsv86vmXv8MtSzmWVS8lCqi3kg1n9DCYNeWj2tDOYVu5C3wo XVYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700653140; x=1701257940; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PO5SJazTh187AwMTPJGUXaXm5ytcNNcWwFEIaAXetg8=; b=xEQUaTAi7kSoQL9EJB/y7KJrcVsWrxXs69bS+KLmO4d7qB1ugdj56eGHPcMbCAOtz8 L4zqjrySoD+hgIv2oJf76V/rtG13oaBvsui02UBYwnktnVlPuDSUTQw2NoaHcJtRQOd+ p9x21kEm/v4HBzS/izEewHcrWAwc4pCTUtS9grAE0pUVSgsX1Hwry0YPOljDm8bJVgc3 ozZrok4FLTlVPuIcTWyGbEwJTQleBs6dqifDzDSRK28NQcq4fF9DUCD/EYYEJg//o1Vk GEvBlTRzpu8BaQIWLaxkDORhmxEVIZ5amd2oEstmxr1vF+84mLGETv9Ev8iQSIMeJbQd ptXA== X-Gm-Message-State: AOJu0Yzkw25c+jY/l/kJSKeD/kreT4rYeEjwJNQ5SuCQ/PkskssoOxAZ G29U+PpqTdfM1C9v/rO4P8qSM9cKxl0ZnYKhCq56Jw== X-Google-Smtp-Source: AGHT+IEX09Xiilf2tazDvnvC/6PlCBWdvJ+HhqgkL5p+drDAPYLEa1UoZ8hAKxtlNsuqzkaWbPrHjhhs9ib9/oy+BVo= X-Received: by 2002:a17:906:209e:b0:9fe:38b7:4278 with SMTP id 30-20020a170906209e00b009fe38b74278mr1923672ejq.16.1700653139951; Wed, 22 Nov 2023 03:38:59 -0800 (PST) MIME-Version: 1.0 References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-20-juraj.linkes@pantheon.tech> <77f91fce-7208-4918-badc-0213f254a75e@foss.arm.com> In-Reply-To: <77f91fce-7208-4918-badc-0213f254a75e@foss.arm.com> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Wed, 22 Nov 2023 12:38:48 +0100 Message-ID: Subject: Re: [PATCH v7 19/21] dts: base traffic generators docstring update To: Yoan Picchi Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Nov 21, 2023 at 5:20=E2=80=AFPM Yoan Picchi wrote: > > On 11/15/23 13:09, Juraj Linke=C5=A1 wrote: > > Format according to the Google format and PEP257, with slight > > deviations. > > > > Signed-off-by: Juraj Linke=C5=A1 > > --- > > .../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? > > + > > +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 diss= ect each arrived packet > > +and a capturing traffic generator is required. > > +""" > > + > > from framework.config import ScapyTrafficGeneratorConfig, TrafficGene= ratorType > > 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: ScapyTrafficGeneratorCon= fig > > ) -> CapturingTrafficGenerator: > > - """A factory function for creating traffic generator object from u= ser config.""" > > + """The factory function for creating traffic generator objects fro= m 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_c= onfig) > > diff --git a/dts/framework/testbed_model/traffic_generator/capturing_tr= affic_generator.py b/dts/framework/testbed_model/traffic_generator/capturin= g_traffic_generator.py > > index e521211ef0..b0a43ad003 100644 > > --- a/dts/framework/testbed_model/traffic_generator/capturing_traffic_g= enerator.py > > +++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_g= enerator.py > > @@ -23,19 +23,22 @@ > > > > > > def _get_default_capture_name() -> str: > > - """ > > - This is the function used for the default implementation of captur= e 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 dec= lare that it can capture > > packets and return them to the user. > > > > + Similarly to > > + :class:`~framework.testbed_model.traffic_generator.traffic_generat= or.TrafficGenerator`, > > + this class exposes the public methods specific to capturing traffi= c generators and defines > > + a private method that must implement the traffic generation and ca= pturing logic in subclasses. > > + > > The methods of capturing traffic generators obey the following wo= rkflow: > > + > > 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 =3D _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 captu= red > > + on `receive_port` for the given `duration`. > > > > - Send a packet on the send_port and then return all traffic cap= tured > > - on the receive_port for the given duration. Also record the ca= ptured traffic > > - in a pcap file. > > + The captured traffic is recorded in the `capture_name`.pcap fi= le. > > > > 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 t= he capture. > > > > Returns: > > - A list of received packets. May be empty if no packets ar= e captured. > > + The received packets. May be empty if no packets are capt= ured. > > """ > > 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 =3D _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 capt= ured > > - on the receive_port for the given duration. Also record the ca= ptured traffic > > - in a pcap file. > > + Send `packets` on `send_port` and then return all traffic capt= ured > > + on `receive_port` for the given `duration`. > > + > > + The captured traffic is recorded in the `capture_name`.pcap fi= le. 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 t= he capture. > > > > Returns: > > - A list of received packets. May be empty if no packets ar= e captured. > > + The received packets. May be empty if no packets are capt= ured. > > """ > > 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 recei= ved 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 `dura= tion`. > > + > > + 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_gene= rator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.= py > > index ea7c3963da..ed396c6a2f 100644 > > --- a/dts/framework/testbed_model/traffic_generator/traffic_generator.p= y > > +++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.p= y > > @@ -22,7 +22,8 @@ > > class TrafficGenerator(ABC): > > """The base traffic generator. > > > > - Defines the few basic methods that each traffic generator must imp= lement. > > + Exposes the common public methods of all traffic generators and de= fines 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 traf= fic generator will be running. > > + config: The traffic generator's test run configuration. > > + """ > > self._config =3D config > > self._tg_node =3D tg_node > > self._logger =3D getLogger( > > @@ -37,9 +44,9 @@ def __init__(self, tg_node: Node, config: TrafficGene= ratorConfig): > > ) > > > > 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 sen= t. > > > > 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) -> Non= e: > > - """ > > - 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 o= therwise. > > - """ > > + """This traffic generator can't capture traffic.""" > > return False > > > > @abstractmethod >