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 5769A45720; Fri, 2 Aug 2024 21:54:59 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 43FF0410F1; Fri, 2 Aug 2024 21:54:59 +0200 (CEST) Received: from mail-pg1-f171.google.com (mail-pg1-f171.google.com [209.85.215.171]) by mails.dpdk.org (Postfix) with ESMTP id C1979410D5 for ; Fri, 2 Aug 2024 21:54:57 +0200 (CEST) Received: by mail-pg1-f171.google.com with SMTP id 41be03b00d2f7-7a1c7857a49so5563282a12.1 for ; Fri, 02 Aug 2024 12:54:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1722628497; x=1723233297; 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=RZXYBiRAA941adlNfa/h4tkCpGE9suPXLbuDjRR9fck=; b=GTujKyuWOp2FNeRLcL9OIyM3dtTz4uo5COa5sXsWn6t85DkRFMolOJMcZCYi3MpDgF 4xiRUWu8WLZYOz8GnVw94bbd/QyD8f3il+dq38gJ/MsOT0T/4zylwJF6NSNDp0cfCiQf PRY5wMUoO0j9pFUafqnx1urhY5/DqSRa/3p+g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722628497; x=1723233297; 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=RZXYBiRAA941adlNfa/h4tkCpGE9suPXLbuDjRR9fck=; b=pQkM+mKChS73TjBDYJRWGZo8mn426lTyDVDwhkcQvaBC9RPMJFv9aWqqWGPw8VJcWY W4MDfMDB8ftFjjg6Nqt+7CUxKT/x7MlTKYVnHUVT+Y0stLb2ePB67hdBurS+VAzzimh/ JWgUIVGnM1ntdUrcLzJisSOWAI++OYK6QPWGmENBEBWFoasKPwRAv/DFIHK+Fc1aq2qV kAOlDvewwdwqXI/4t7u8SrSBvlxaLfCXEvagGRcAGOzyl9rJcKxhhXRIH7J+cXSh+Fxn ie8zWYtGb+3S3rejAQ6a3D95JxDttWHJp8Tik26AU9Q03Kk0yPr83bIQ67OLp5Jd1Ug2 9mAQ== X-Forwarded-Encrypted: i=1; AJvYcCVxr5ndRjjMLngOmfQgt7EMBGMWBzcLFHuYCPTRdN1CHDT8jULbBw527KOgL8AOrolGeDrc/ovq+gn6HS4= X-Gm-Message-State: AOJu0Yxkcf4NXsUoF8+nvM2GV3yiiE4XaOdwMGJ+LniexfPYr1C1rgBf k9eky4lUmX0wmXplecPVdzLM5coR3bQMbie2iVife9y687s+Wbt0cvWGyVVFblitfxsfCF9QD0v 8tNxr3VhKPJ0wjqWZzLlCRQX3j/F5hDSvJwJffQ== X-Google-Smtp-Source: AGHT+IGTKlcVAB+r4qObNquppNKzGuoxiWgA50fv9q/KL0V389qIGvPEoD7dnG4fnPly1AIsLgeVgd+ii9H32aQFZTo= X-Received: by 2002:a05:6a20:394a:b0:1c2:96f1:a2ce with SMTP id adf61e73a8af0-1c6995244e7mr6518092637.3.1722628496670; Fri, 02 Aug 2024 12:54:56 -0700 (PDT) MIME-Version: 1.0 References: <20240524183604.6925-1-npratte@iol.unh.edu> <20240726141307.14410-1-npratte@iol.unh.edu> <20240726141307.14410-3-npratte@iol.unh.edu> In-Reply-To: <20240726141307.14410-3-npratte@iol.unh.edu> From: Jeremy Spewock Date: Fri, 2 Aug 2024 15:54:44 -0400 Message-ID: Subject: Re: [RFC PATCH v3 2/2] dts: Initial Implementation For Jumbo Frames Test Suite To: Nicholas Pratte Cc: probb@iol.unh.edu, dmarx@iol.unh.edu, luca.vizzarro@arm.com, yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com, juraj.linkes@pantheon.tech, 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 Just some very light comments here as well, mostly about doc-string but also a few questions/suggestions. On Fri, Jul 26, 2024 at 10:14=E2=80=AFAM Nicholas Pratte wrote: > diff --git a/dts/tests/TestSuite_jumboframes.py b/dts/tests/TestSuite_jum= boframes.py > new file mode 100644 > index 0000000000..dd8092f2a4 > --- /dev/null > +++ b/dts/tests/TestSuite_jumboframes.py > @@ -0,0 +1,182 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2023-2024 University of New Hampshire It's probably fine to just make the date on this copyright 2024. > +"""Jumbo frame consistency and compatibility test suite. > + > +The test suite ensures the consistency of jumbo frames transmission with= in > +Poll Mode Drivers using a series of individual test cases. If a Poll Mod= e > +Driver receives a packet that is greater than its assigned MTU length, t= hen > +that packet will be dropped, and thus not received. Likewise, if a Poll = Mode Driver This sentence is a little confusing because you're saying "if a packet is received with X condition then it isn't received." Maybe it would be more clear to say it isn't forwarded? > +receives a packet that is less than or equal to a its designated MTU len= gth, then the I think this was a typo, this should probably be either "equal to its" or "equal to a" rather than both. > +packet should be transmitted by the Poll Mode Driver, completing a cycle= within the > +testbed and getting received by the traffic generator. Thus, the followi= ng test suite > +evaluates the behavior within all possible edge cases, ensuring that a t= est Poll > +Mode Driver strictly abides by the above implications. > +""" > + > +class TestJumboframes(TestSuite): > + """DPDK PMD jumbo frames test suite. > + > + Asserts the expected behavior of frames greater than, less then, or = equal to I think this should be "less than" rather than "less then". > + a designated MTU size in the testpmd application. If a packet size g= reater > + than the designated testpmd MTU length is retrieved, the test fails.= If a > + packet size less than or equal to the designated testpmd MTU length = is retrieved, > + the test passes. > + """ > + > + def set_up_suite(self) -> None: > + """Set up the test suite. > + > + Setup: > + Set traffic generator MTU lengths to a size greater than sco= pe of all > + test cases. > + """ > + self.tg_node.main_session.configure_port_mtu( > + ETHER_JUMBO_FRAME_MTU + 200, self._tg_port_egress > + ) > + self.tg_node.main_session.configure_port_mtu( > + ETHER_JUMBO_FRAME_MTU + 200, self._tg_port_ingress > + ) I know 9000 is a common jumboframe MTU to support, but do we have to worry about a case where some NICs wouldn't support an MTU this high? That could also be a case of the NICs where that would be a problem are maybe older and not expected to be as common? I'm not completely sure what the standards and expectations are. > + > + def send_packet_and_verify(self, pktsize: int, should_receive: bool = =3D True) -> None: > + """Generate, send, and capture packets to verify that the sent p= acket was received or not. I feel like the "verify that..." asserts that you are verifying something positive when it could be positive or negative. Maybe "verify whether" would fit better here. > + > + Generates a packet based on a specified size and sends it to the= SUT. The desired packet's > + payload size is calculated, and arbitrary, byte-sized characters= are inserted into the > + packet before sending. Packets are captured, and depending on th= e test case, packet > + payloads are checked to determine if the sent payload was receiv= ed. > + > + Args: > + pktsize: Size of packet to be generated and sent. > + should_receive: Indicate whether the test case expects to re= ceive the packet or not. > + """ > + padding =3D pktsize - IP_HEADER_LEN > + # Insert extra space for placeholder 'CRC' Error correction. > + packet =3D Ether() / Raw(" ") / IP(len=3Dpktsize) / Raw(load= =3D"X" * padding) This CRC error correction is interesting, I thought generally that the Ether header length included the CRC, but it makes sense to try and account for it if it isn't . > + received_packets =3D self.send_packet_and_capture(packet) > + found =3D any( > + ("X" * padding) in str(packets.load) > + for packets in received_packets > + if hasattr(packets, "load") > + ) > + > + if should_receive: > + self.verify(found, "Did not receive packet") > + else: > + self.verify(not found, "Received packet") > + > + > + def test_jumboframes_jumbo_nojumbo(self) -> None: > + """Assess the boundaries of packets sent greater than standard M= TU length. > + > + PMDs are set to the standard MTU length of 1518 bytes to assess = behavior of sent packets > + greater than this size. Sends one packet with a frame size of 15= 19. The test cases does Since the bounds were increased to account for the difference in PMDs, we should probably update this number in the doc-string accordingly. > + not expect to receive this packet. > + > + Test: > + Start testpmd with standard MTU size of 1518. Send a packet = of 1519 and verify it was > + not received. > + """ > + with TestPmdShell( > + self.sut_node, tx_offloads=3D0x8000, mbuf_size=3D[9200], mbc= ache=3D200 > + ) as testpmd: > + testpmd.configure_port_mtu_all(ETHER_STANDARD_FRAME) > + testpmd.start() > + > + self.send_packet_and_verify(ETHER_STANDARD_FRAME + 5, False) > + > + def test_jumboframes_normal_jumbo(self) -> None: > + """Assess the consistency of standard 1518 byte packets using a = 9000 byte jumbo MTU length. > + > + PMDs are set to a jumbo frame size of 9000 bytes. Packets of siz= es 1517 and 1518 are sent > + to assess the boundaries of packets less than or equal to the st= andard MTU length of 1518. > + The test case expects to receive both packets. > + > + Test: > + Start testpmd with a jumbo frame size of 9000 bytes. Send a = packet of 1517 and 1518 > + and verify they were received. > + """ > + with TestPmdShell( > + self.sut_node, tx_offloads=3D0x8000, mbuf_size=3D[9200], mbc= ache=3D200 > + ) as testpmd: > + testpmd.configure_port_mtu_all(ETHER_JUMBO_FRAME_MTU) > + testpmd.start() > + > + self.send_packet_and_verify(ETHER_STANDARD_FRAME - 5) Does it still make sense to take off extra bytes here since we are so far away from the MTU boundary? I think this would be consistent still even if it were 1517. > + self.send_packet_and_verify(ETHER_STANDARD_FRAME) > + > + def test_jumboframes_jumbo_jumbo(self) -> None: > + """Assess the boundaries packets sent at an MTU size of 9000 byt= es. Should this first line have something like "Asses the boundaries with packets..." to tie the sentence together a little more? > + > + PMDs are set to a jumbo frames size of 9000 bytes. Packets of si= ze 1519, 8999, and 9000 It would probably good to also reflect here that it is 1523, 8995 and 9000. > + are sent. The test expects to receive all packets. > + > + Test: > + Start testpmd with an MTU length of 9000 bytes. Send packets= of size 1519, 8999, > + and 9000 and verify that all packets were received. and here as well. > + """ > + with TestPmdShell( > + self.sut_node, tx_offloads=3D0x8000, mbuf_size=3D[9200], mbc= ache=3D200 > + ) as testpmd: > + testpmd.configure_port_mtu_all(ETHER_JUMBO_FRAME_MTU) > + testpmd.start() > + > + self.send_packet_and_verify(ETHER_STANDARD_FRAME + 5) > + self.send_packet_and_verify(ETHER_JUMBO_FRAME_MTU - 5) > + self.send_packet_and_verify(ETHER_JUMBO_FRAME_MTU) > + > + def test_jumboframes_bigger_jumbo(self) -> None: > + """Assess the behavior of packets send greater than a specified = MTU length of 9000 bytes. Typo: send should probably be sent. > + > + PMDs are set to a jumbo frames size of 9000 bytes. A packet of s= ize 9001 is sent to the SUT. We should probably also reflect here that the packet size is really 9005. > + The test case does not expect to receive the packet. > + > + Test: > + Start testpmd with an MTU length of 9000 bytes. Send a packe= t of 9001 bytes and verify Here as well. > + it was not received. > + """ > + with TestPmdShell( > + self.sut_node, tx_offloads=3D0x8000, mbuf_size=3D[9200], mbc= ache=3D200 > + ) as testpmd: > + testpmd.configure_port_mtu_all(ETHER_JUMBO_FRAME_MTU) > + testpmd.start() > + > + self.send_packet_and_verify(ETHER_JUMBO_FRAME_MTU + 5, False= ) > + > + def tear_down_suite(self) -> None: > + """Tear down the test suite. > + > + Teardown: > + Set the MTU size of the traffic generator back to the standa= rd 1518 byte size. > + """ > + self.tg_node.main_session.configure_port_mtu(ETHER_STANDARD_FRAM= E, self._tg_port_egress) > + self.tg_node.main_session.configure_port_mtu(ETHER_STANDARD_FRAM= E, self._tg_port_ingress) > -- > 2.44.0 >