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 9EFD24594A; Mon, 9 Sep 2024 19:13:21 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 86AE3410FD; Mon, 9 Sep 2024 19:13:21 +0200 (CEST) Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) by mails.dpdk.org (Postfix) with ESMTP id 42E1D40ED0 for ; Mon, 9 Sep 2024 19:13:20 +0200 (CEST) Received: by mail-pf1-f177.google.com with SMTP id d2e1a72fcca58-718a3b8a2dcso2598765b3a.2 for ; Mon, 09 Sep 2024 10:13:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1725901999; x=1726506799; 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=2jO3NxHvf6Y5qrddShBIEch1fpNK3n1BvWCw8Mw2Hzk=; b=eOOTAf+f7zG+XZSSDgar8ywfstasGnnuMIJvNTCdP+FmpR9+6npTrHUTopxdKABO/6 8eh6GrE3klZrukX9EmZl7/C+3VOs/Rd6r88pPPo76UfAkSR/OZX1euJ28V24rHzqJMH5 hFJlPu9KyCAiclahGRk9bvrrNR1+zOX2xUViA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725901999; x=1726506799; 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=2jO3NxHvf6Y5qrddShBIEch1fpNK3n1BvWCw8Mw2Hzk=; b=rcMV3Cq42sbF0ytp8udzmULloMY7SXvC821C9QFJd6k2gNsU2ArpCVZuD4pxOWXaWI XyfvCc8UAx7Gh/almGP8oEjDcF97J6fQwR4l7pSfz0+2AqBhIbSimQi2wAmGhzSCfFta xnTNMhJpwm59TPyRLiElbUjPcmnmqmzHRcBPm2TBlMn4thJM7GsfTNKPL8WBuOb5+rQk 5EUvLx0a/yBMKZbrvpRr89br12afOugkyDh/eEWB1RUtHQ8c0h2DlxRn9d6iD5RH1Vw8 tKrp4TZ3H7CCZbUT8bth8R3cEN39QYFF4Q4sPqcZdoVZc+Ty87wpTOlcFxQcD4Dlo5RP LAwQ== X-Gm-Message-State: AOJu0YxrMb2uinyPj3ArR8YJV7L7Ai3F+3u/Tl99HCN1j5gkIfsE/zhO A18nZbpDSq0KO3q7KPrOYGXgNpWJzzSixUEzH0Z08BqiWggFUGTcjkqWjZhZKwGzugi6kH3dbPJ sRtP0KBMGPs6BQeDOE1ES6hokyMAenHQxYMfBqA== X-Google-Smtp-Source: AGHT+IEm0GN+sWL/HmBB6gd9LifkOPbQ9TMgm/pw8taWrUpPDna7XQ2SvWMGSy5itdQXIlUMLUkVoPZM/c4LHs55KyA= X-Received: by 2002:a05:6a20:2d14:b0:1c6:ba9c:5d7b with SMTP id adf61e73a8af0-1cf1d0f98b5mr10474325637.23.1725901999340; Mon, 09 Sep 2024 10:13:19 -0700 (PDT) MIME-Version: 1.0 References: <20240806121417.2567708-1-Luca.Vizzarro@arm.com> <20240909110158.1009592-1-luca.vizzarro@arm.com> <20240909110158.1009592-2-luca.vizzarro@arm.com> In-Reply-To: <20240909110158.1009592-2-luca.vizzarro@arm.com> From: Patrick Robb Date: Mon, 9 Sep 2024 13:12:31 -0400 Message-ID: Subject: Re: [PATCH v3 1/5] dts: add ability to send/receive multiple packets To: Luca Vizzarro Cc: dev@dpdk.org, Honnappa Nagarahalli , Paul Szczepanek , Alex Chapman , Jeremy Spewock , =?UTF-8?Q?Juraj_Linke=C5=A1?= 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 Reviewed-by: Patrick Robb 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=3D1538 Thanks Luca. On Mon, Sep 9, 2024 at 7:03=E2=80=AFAM Luca Vizzarro 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 > Reviewed-by: Paul Szczepanek > Reviewed-by: Alex Chapman > Reviewed-by: Jeremy Spewock > Reviewed-by: Juraj Linke=C5=A1 > --- > 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-unt= yped] > > 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 =3D 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 =3D PacketFilteringConfig()= , > + duration: float =3D 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 tes= tbed 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 send= ing `packet`. > + > + Returns: > + A list of received packets. > + """ > + packets =3D [self._adjust_addresses(packet) for packet in packet= s] > + 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, re= ceived_packets: list[Packet] > ) > self._fail_test_case_verify("An expected packet not found am= ong received packets.") > > + def match_all_packets( > + self, expected_packets: list[Packet], received_packets: list[Pac= ket] > + ) -> None: > + """Matches all the expected packets against the received ones. > + > + Matching is performed by counting down the occurrences in a dict= ionary which keys are the > + raw packet bytes. No deep packet comparison is performed. All th= e 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` w= ere found in > + `received_packets`. > + """ > + expected_packets_counters =3D Counter(map(raw, expected_packets)= ) > + received_packets_counters =3D 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 =3D expected_packets_counters - receive= d_packets_counters > + missing_packets_count =3D 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 !=3D 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{receive= d_packet.summary()}" > diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testb= ed_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 =3D create_traffic_generator(self, node_c= onfig.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 =3D PacketFilteringConfig()= , > duration: float =3D 1, > ) -> list[Packet]: > - """Send `packet`, return received traffic. > + """Send `packets`, return received traffic. > > - Send `packet` on `send_port` and then return all traffic capture= d > + Send `packets` on `send_port` and then return all traffic captur= ed > on `receive_port` for the given duration. Also record the captur= ed 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_traf= fic_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_gen= erator.py > +++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_gen= erator.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 =3D _get_default_capture_name(), > - ) -> list[Packet]: > - """Send `packet` and capture received traffic. > - > - Send `packet` on `send_port` and then return all traffic capture= d > - 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 send= ing 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 captur= ed. > - """ > - 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 >