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 78E9743747; Tue, 19 Dec 2023 18:29:46 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 65B5A42E11; Tue, 19 Dec 2023 18:29:46 +0100 (CET) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) by mails.dpdk.org (Postfix) with ESMTP id 69A0742DED for ; Tue, 19 Dec 2023 18:29:45 +0100 (CET) Received: by mail-lf1-f52.google.com with SMTP id 2adb3069b0e04-50e2168ab09so5915061e87.0 for ; Tue, 19 Dec 2023 09:29:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1703006985; x=1703611785; 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=YnmQiHGPmEuJjUrUqmxjq+F7TX4VQRWrqZxNtIz397U=; b=Kk3fjK4hayFFWtJHsyU8FiRVnQ/4ZJH11Y3kAPiZeNCxrqdoKbfEst3wKh+C0saEDM UKR8AbEpR7yYUReMido9zekYzc6VIy44XdLYQJeVeC5yzM5g4CWMKCm6mnJopWPdiifu zvQC1fegEZ1QVk2QIPuYwyGScmIuLgdmvjp3WqLMam8JhvVOfHF8GSbwhHQxxE5QKAhl d0l2bhP8itcv93mtCqj07DIHoC0vpPDz66U2Tqydb7uoY8M6+v5rsSINY6aKNycHeK2u hm7EW6jsEBUzv2zRsMrJW9aeNFOdwz4D7xmFajTE43e2Uc1pM76ivGI5MO4XxmErUf6m yuCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703006985; x=1703611785; 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=YnmQiHGPmEuJjUrUqmxjq+F7TX4VQRWrqZxNtIz397U=; b=wWKvypZAd27zK0/m04fXCGEpbDLqjG1InWvGyNPFzEGFmTWin5pz+2JSSQsaOPcvlq ucISiEZ8wqoQ1COCp9Cg/nevAOGkXls27ENa9tGfPHHbXbkZaRGYOMT4sCb1h++Qgc6L VLcnebu1SdbLrBiTEvKwoqWGJEa+03az0zGSQTfpDFe2kiFY/kv9OqVRmYXnREBTlWpA V82UzvsscN2bmOILSKHQLpZDVepF8rc79qFwG/5UQkj8nGuB0pq4YaQqr5DERPl5kLhw KF1CBZRnpE23rs6z3ubVXvHcbuwoJMuVSqyDxxWDpOIzWtJiLuv/MZsseqLES/o1k4k0 ctZA== X-Gm-Message-State: AOJu0Yx7Tm5xvkQvTurmHzibK+7P8kbo4XCy2pdA1RsnCyzJMuy8Pjsr o4aXxhiqTvPL86nOmzrE1kzq5yBUU1glCWPF4fJyZA== X-Google-Smtp-Source: AGHT+IHWYZsxgxbT7gj/gAubhJX3NWIMnlSdQRyxgOPG/kQiYKx0NPhswZtZHuCrWCj+qrTIgLaihKLu6bCfr5DpltQ= X-Received: by 2002:a05:6512:2821:b0:50e:3158:c53a with SMTP id cf33-20020a056512282100b0050e3158c53amr2920107lfb.124.1703006984803; Tue, 19 Dec 2023 09:29:44 -0800 (PST) MIME-Version: 1.0 References: <20231218181221.10057-1-jspewock@iol.unh.edu> <20231218181221.10057-8-jspewock@iol.unh.edu> In-Reply-To: <20231218181221.10057-8-jspewock@iol.unh.edu> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Tue, 19 Dec 2023 18:29:33 +0100 Message-ID: Subject: Re: [PATCH v4 7/7] dts: add scatter test suite To: jspewock@iol.unh.edu Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, wathsala.vithanage@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, 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 Should we use the full name (pmd_buffer_scatter) in the subject? I lean towards the full name. On Mon, Dec 18, 2023 at 7:13=E2=80=AFPM wrote: > > From: Jeremy Spewock > > This test suite provides testing the support of scattered packets by > Poll Mode Drivers using testpmd. It incorporates 5 different test cases > which test the sending and receiving of packets with lengths that are > less than the mbuf data buffer size, the same as the mbuf data buffer > size, and the mbuf data buffer size plus 1, 4, and 5. The goal of this > test suite is to align with the existing dts test plan for scattered > packets within DTS. > Again, we need to describe why we're adding this commit. In the case of test suites, the why is obvious, so we should give a good high level description of what the tests test (something like the test suite tests the x feature by doing y, y being the salient part of the tests). The original test plan is actually pretty good, so we can extract the rationale from it. > Signed-off-by: Jeremy Spewock > --- > dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++++++ > 1 file changed, 105 insertions(+) > create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py > > diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSu= ite_pmd_buffer_scatter.py > new file mode 100644 > index 0000000000..8e2a32a1aa > --- /dev/null > +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py > @@ -0,0 +1,105 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2023 University of New Hampshire > + > +"""Multi-segment packet scattering testing suite. > + > +Configure the Rx queues to have mbuf data buffers whose sizes are smalle= r than the maximum packet > +size (currently set to 2048 to fit a full 1512-byte ethernet frame) and = send a total of 5 packets > +with lengths less than, equal to, and greater than the mbuf size (CRC in= cluded). > +""" Let's expand this. I'll point to the original test plan again, let's use some of it here. I think it makes sense to make this docstring a kind of a test plan with high level description. > +import struct > + > +from scapy.layers.inet import IP # type: ignore[import] > +from scapy.layers.l2 import Ether # type: ignore[import] > +from scapy.packet import Raw # type: ignore[import] > +from scapy.utils import hexstr # type: ignore[import] > + > +from framework.remote_session.remote.testpmd_shell import ( > + TestPmdForwardingModes, > + TestPmdShell, > +) > +from framework.test_suite import TestSuite > + > + > +class PmdBufferScatter(TestSuite): > + """DPDK packet scattering test suite. > + And here we could add some more specifics. I'd like to utilize the original test plans and a split like this makes sense at a first glance. > + Attributes: > + mbsize: The size to se the mbuf to be. > + """ > + > + mbsize: int > + > + def set_up_suite(self) -> None: > + self.verify( > + len(self._port_links) > 1, > + "Must have at least two port links to run scatter", > + ) > + > + self.tg_node.main_session.configure_port_mtu(9000, self._tg_port= _egress) > + self.tg_node.main_session.configure_port_mtu(9000, self._tg_port= _ingress) > + > + def scatter_pktgen_send_packet(self, pktsize: int) -> str: > + """Generate and send packet to the SUT. > + > + Functional test for scatter packets. > + > + Args: > + pktsize: Size of the packet to generate and send. > + """ > + packet =3D Ether() / IP() / Raw() > + packet.getlayer(2).load =3D "" > + payload_len =3D pktsize - len(packet) - 4 > + payload =3D ["58"] * payload_len > + # pack the payload > + for X_in_hex in payload: > + packet.load +=3D struct.pack("=3DB", int("%s%s" % (X_in_hex[= 0], X_in_hex[1]), 16)) > + load =3D hexstr(packet.getlayer(2), onlyhex=3D1) > + received_packets =3D self.send_packet_and_capture(packet) > + self.verify(len(received_packets) > 0, "Did not receive any pack= ets.") > + load =3D hexstr(received_packets[0].getlayer(2), onlyhex=3D1) > + > + return load > + > + def pmd_scatter(self) -> None: > + """Testpmd support of receiving and sending scattered multi-segm= ent packets. > + > + Support for scattered packets is shown by sending 5 packets of d= iffering length > + where the length of the packet is calculated by taking mbuf-size= + an offset. The > + offsets used in the test case are -1, 0, 1, 4, 5 respectively. > + > + Test: > + Start testpmd and run functional test with preset mbsize. > + """ > + testpmd =3D self.sut_node.create_interactive_shell( > + TestPmdShell, > + app_parameters=3D( > + "--mbcache=3D200 " > + f"--mbuf-size=3D{self.mbsize} " > + "--max-pkt-len=3D9000 " > + "--port-topology=3Dpaired " > + "--tx-offloads=3D0x00008000" > + ), > + privileged=3DTrue, > + ) > + testpmd.set_forward_mode(TestPmdForwardingModes.mac) > + testpmd.start() > + link_is_up =3D testpmd.wait_link_status_up(0) and testpmd.wait_l= ink_status_up(1) These two calls should probably be inside testpmd.start(). Looks like we're always going to be doing this. Also, this assumes there will be two ports. Ideally, the shell itself will know which ports to check (that should be in EAL parameters), but that may require a bigger refactor (unless we just parse back the -a options, which could be permissible). > + self.verify(link_is_up, "Links never came up in TestPMD.") > + > + for offset in [-1, 0, 1, 4, 5]: > + recv_payload =3D self.scatter_pktgen_send_packet(self.mbsize= + offset) > + self.verify( > + ("58 " * 8).strip() in recv_payload, > + "Received packet had incorrect payload", > + ) This is still just one test case. We should probably leave it as is (i.e. not split into 5 test cases), but we should add the information about the offset/test case into the message so that we know what actually failed right away. > + testpmd.stop() > + > + def test_scatter_mbuf_2048(self) -> None: > + """Run :func:`~PmdBufferScatter.pmd_scatter` function after sett= ing the `mbsize` to 2048.""" > + self.mbsize =3D 2048 > + self.pmd_scatter() Let's just pass 2048 to the method, I don't see a reason for the instance variable. > + > + def tear_down_suite(self) -> None: > + self.tg_node.main_session.configure_port_mtu(1500, self._tg_port= _egress) > + self.tg_node.main_session.configure_port_mtu(1500, self._tg_port= _ingress) > -- > 2.43.0 >