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 A2A4D424C3; Tue, 11 Jun 2024 17:33:28 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9239C40A7D; Tue, 11 Jun 2024 17:33:28 +0200 (CEST) Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by mails.dpdk.org (Postfix) with ESMTP id E742840A75 for ; Tue, 11 Jun 2024 17:33:26 +0200 (CEST) Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2c2ecbc109fso2462686a91.1 for ; Tue, 11 Jun 2024 08:33:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718120006; x=1718724806; 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=l42OmOlOzcRUKNYolkTjCQfxr63TKGagPE/IRguvP20=; b=b3ve2r2mVJpAM1zcFn4nOAoOdh/jvKGUvg1U/o2zeHMcMyU9k6dX1ZBfBiw0Aqhexx 7eMLoF8O7bhi3I3HpajAS3U+Mgmr5kaszgfQJY2LkhnRiO+04XCzNeVSmfIp4OERYNH0 nVMzuaPeJPqhGYdLWAMG45OBx7QHYlp1HoKAw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718120006; x=1718724806; 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=l42OmOlOzcRUKNYolkTjCQfxr63TKGagPE/IRguvP20=; b=aYcnE4fShbRjOguctfi6ZxS/0HLHmTkiUu2a1dTeBszs5u2Js7XXl/Jxc6EbL9M+Fr mwvJ9l1yEhCZkuLYb47zoq2vePPsgWhCKkQc7tT6wDaOxpzPksxVBHJHghJpis3AmHDq qeJJYeGoLJFV6RIkELCASsi24RzhhyGIeONIdDO/OSKjHVX+RvbImh4g4NEs6hZNpUcy ODvDkmsDTAby3pJFl+qPm6Cj2jGj0ZsGyZobvB1cQqAxyKELqK1Sj3OWagJ6skRYj2n4 eh81g8varm/32pWNq7ZBBJ9DD0XYtx5QI4rq9VamcL+CP5MBj0SR+XJAXt5fH4ESC41j bMyg== X-Forwarded-Encrypted: i=1; AJvYcCX+68zSSvlp4FGVrjTxtgtTEyTiN++sEbCM+rvIxWMp+EeHTHdEFyzud6/7PzRgN4tak1LpfOSQ7dVt490= X-Gm-Message-State: AOJu0YzYU3sM3L0nJWFbUFKmpvJXD7qfA3mTDV/zkjjKkXlJiyTRY8bQ OEZzrmCsBb2j7IPH+AI5Q6APmteNJICOpBrYoDESQqW23pIiKQa3aQGRHsZ7zkzIxfkR4Zdcw90 gS6E2kwTaBGU7cSaiBtoHiwDxoAR8x2/E3VOq9Q== X-Google-Smtp-Source: AGHT+IEEmTY2CxsZ1Ip79v7o4J3yrkpRPnPWQhRrjvrRuZmlJ+4BIUlLW/gp8RiJduFwUrVRKyGCrZ2RBrl5klrn31k= X-Received: by 2002:a17:90a:c48:b0:2bf:c6fb:ec34 with SMTP id 98e67ed59e1d1-2c2bc9bc84cmr12234960a91.8.1718120005970; Tue, 11 Jun 2024 08:33:25 -0700 (PDT) MIME-Version: 1.0 References: <20240514201436.2496-1-jspewock@iol.unh.edu> <20240605213148.21371-1-jspewock@iol.unh.edu> <20240605213148.21371-5-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Tue, 11 Jun 2024 11:33:14 -0400 Message-ID: Subject: Re: [PATCH v3 4/4] dts: add test case that utilizes offload to pmd_buffer_scatter To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, thomas@monjalon.net, wathsala.vithanage@arm.com, npratte@iol.unh.edu, yoan.picchi@foss.arm.com, Luca.Vizzarro@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, Jun 11, 2024 at 5:22=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > > > On 10. 6. 2024 22:08, Jeremy Spewock wrote: > > On Mon, Jun 10, 2024 at 11:22=E2=80=AFAM Juraj Linke=C5=A1 > > wrote: > >> > >>> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/Te= stSuite_pmd_buffer_scatter.py > >>> index 41f6090a7e..76eabb51f6 100644 > >>> --- a/dts/tests/TestSuite_pmd_buffer_scatter.py > >>> +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py > >> > >>> @@ -86,12 +99,15 @@ def scatter_pktgen_send_packet(self, pktsize: int= ) -> str: > >>> for X_in_hex in payload: > >>> packet.load +=3D struct.pack("=3DB", int("%s%s" % (X_i= n_hex[0], X_in_hex[1]), 16)) > >>> received_packets =3D self.send_packet_and_capture(packet) > >>> + # filter down the list to packets that have the appropriate = structure > >>> + received_packets =3D list( > >>> + filter(lambda p: Ether in p and IP in p and Raw in p, re= ceived_packets) > >>> + ) > >>> self.verify(len(received_packets) > 0, "Did not receive an= y packets.") > >>> - load =3D hexstr(received_packets[0].getlayer(2), onlyhex=3D1= ) > >>> > >>> - return load > >>> + return received_packets > >>> > >>> - def pmd_scatter(self, mbsize: int) -> None: > >>> + def pmd_scatter(self, mbsize: int, testpmd_params: list[str]) ->= None: > >> > >> Since base_testpmd_parameters is a class var, the method is always goi= ng > >> to have access to it and we only need to pass the extra parameters. > >> There's not much of a point in passing what's common to all tests to > >> this method, as it should contain the common parts. > > > > Ack. > > > >> > >>> """Testpmd support of receiving and sending scattered mult= i-segment packets. > >>> > >>> Support for scattered packets is shown by sending 5 packet= s of differing length > >>> @@ -103,34 +119,53 @@ def pmd_scatter(self, mbsize: int) -> None: > >>> """ > >>> testpmd_shell =3D self.sut_node.create_interactive_shell( > >>> TestPmdShell, > >>> - app_parameters=3D( > >>> - "--mbcache=3D200 " > >>> - f"--mbuf-size=3D{mbsize} " > >>> - "--max-pkt-len=3D9000 " > >>> - "--port-topology=3Dpaired " > >>> - "--tx-offloads=3D0x00008000" > >>> - ), > >>> + app_parameters=3D" ".join(testpmd_params), > >>> privileged=3DTrue, > >>> ) > >>> with testpmd_shell as testpmd: > >>> testpmd.set_forward_mode(TestPmdForwardingModes.mac) > >>> + # adjust the MTU of the SUT ports > >>> + for port_id in range(testpmd.number_of_ports): > >>> + testpmd.set_port_mtu(port_id, 9000) > >>> testpmd.start() > >>> > >>> for offset in [-1, 0, 1, 4, 5]: > >>> - recv_payload =3D self.scatter_pktgen_send_packet(mbs= ize + offset) > >>> + # This list should only ever contain one element > >> > >> Which list is the comment referring to? recv_packets? There could be > >> more than just one packet, right? > > > > There technically could be in very strange cases, but this change also > > adds a filter to `scatter_pktgen_send_packet()` that filters the list > > before it is returned here. I imagine there wouldn't be (and in my > > testing there aren't) any other packets that have the structure > > Ether() / IP() / Raw() getting sent by anything on the wire, so I just > > noted it to make it more clear that the call to `any()` probably isn't > > going to have to consume much. I did the filtering in the other method > > because I wanted to be able to distinguish between getting nothing, > > and getting something that has the right structure but not the right > > payload (as, presumably, if this test were to fail it would be shown > > in the payload). > > > > Right, but maybe in other setups this won't be true. We can just make > the comment say the list contains filtered packets with the expected > structure, as that would be more in line with the verification code > (where we don't assume it's just one packet). That's fair, it's probably better to be more clear here, I'll update this. > > >> > > > >>> + @requires(NicCapability.scattered_rx) > >>> def test_scatter_mbuf_2048(self) -> None: > >>> """Run the :meth:`pmd_scatter` test with `mbsize` set to 2= 048.""" > >>> - self.pmd_scatter(mbsize=3D2048) > >>> + self.pmd_scatter( > >>> + mbsize=3D2048, testpmd_params=3D[*(self.base_testpmd_par= ameters), "--mbuf-size=3D2048"] > >>> + ) > >>> + > >> > >> I'm curious why you moved the --mbuf-size parameter here. It's always > >> going to be (or should be) equal to mbsize, which we already pass (and > >> now we're essentially passing the same thing twice), so I feel this ju= st > >> creates opportunities for mistakes. > > > > Honestly, when it's phrased like that, I have no good reason at all, > > haha. I just put it there because I got stuck in some mentality of > > "testpmd parameters go in this list, so it has to go here", but it did > > feel weird to hardcode the same value twice like that. I'll adjust > > this. > > > > > >> > >>> + def test_scatter_mbuf_2048_with_offload(self) -> None: > >>> + """Run the :meth:`pmd_scatter` test with `mbsize` set to 204= 8 and rx_scatter offload.""" > >>> + self.pmd_scatter( > >>> + mbsize=3D2048, > >>> + testpmd_params=3D[ > >>> + *(self.base_testpmd_parameters), > >>> + "--mbuf-size=3D2048", > >>> + "--enable-scatter", > >>> + ], > >>> + ) > >>> > >>> def tear_down_suite(self) -> None: > >>> """Tear down the test suite.