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 5DB76424C9; Tue, 11 Jun 2024 11:22:07 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D9BC54067C; Tue, 11 Jun 2024 11:22:06 +0200 (CEST) Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by mails.dpdk.org (Postfix) with ESMTP id F01764021F for ; Tue, 11 Jun 2024 11:22:04 +0200 (CEST) Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-57a52dfd081so981857a12.2 for ; Tue, 11 Jun 2024 02:22:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1718097724; x=1718702524; darn=dpdk.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=VsQuXogx7+XYyzXnXCC6iHMRrACmd0yDb9GLaEvUVgU=; b=no+TZAhMbldnMNa/iWcjjcgREEanHevGYg/VZRnclOmPEKTIB8w92ILgVA9wBbpYvb 53EuImLJWd7jAKJR/jutoSjX/PCcwCdfHaiLldkTM69AQU+PIdEObs96jdAOrkh/6YAi zBCnaQ3xUsy8z1Ec8/EOBV4A7gCT+AeVO5BbNlTk2qazWFOM+klEu4QJlN4stiGpTriX qzLUK2rRIaHMYRro+nQ6F4H07QimHv4C2w+ReB/BNnyeHNWMiEU0B7vzm7Bo6CxfmTYD aPn93uoR2P8T23j4fSvNIOUCgJ9ABcOjZKtBHCxrkuAwcTKncsGuxm/BZCE/Ai+86AHs M8BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718097724; x=1718702524; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VsQuXogx7+XYyzXnXCC6iHMRrACmd0yDb9GLaEvUVgU=; b=G+1ygEB396tptzbq4wC9Dw2jsGN7DHeb0HAV9R3Xml2G7EnaboXbcJz1869xxW/uVq eLIxikwXb01ef86jbZp5qEM/P68VxDuiwDHqluTpmmGZAJXf3h9xE5vsL7SoTz0wIRxN ItmOHCjcjwYTDLFCZ6JXnW5pkZGdcilnFT/HWp3C2d8vmTuErA5Kj9lT9A3DZ24yd3Hi Xveih11b6lxa2DgnCBUPtvcfBKUxiCBIo4KIZ1HH0npBkVN0yd/kJE3wqE/Q8I7OUjZJ 0LVB+NboNRp/jZimEJn+IjjIZn/DSri3NLDMm0385flRom8SCKgd3dTh5aEMF8NPjRqm +NwA== X-Forwarded-Encrypted: i=1; AJvYcCXGfAsPGem3QtgbfaLMl/WYZcsp6+8KoRULhHzV1uRou3PuCsLZJzreyWws7xzNd+KP6u9Cm3/lgED0noo= X-Gm-Message-State: AOJu0Yy+mDLuYWDKcmAoesQHkojY6GxS7PLxNaeVO+4cXMhhhA9OD4Xj HGNHbo+WzDZo/GloCfioGyjWtuKcSRTifZxVjhxo3lJKMx2G+eYpXKrhuoF13dlzJO/ssjf5tLZ Htzg= X-Google-Smtp-Source: AGHT+IHTZPbSb0D93fzInd8cVzLyaILRAukUngk53UFM6QdW45FXmMHrKKVckPciqR9mFLWZZ/tq2w== X-Received: by 2002:a50:9b5a:0:b0:57c:77c6:e301 with SMTP id 4fb4d7f45d1cf-57c77c6e314mr3835200a12.35.1718097724429; Tue, 11 Jun 2024 02:22:04 -0700 (PDT) Received: from [10.12.0.137] (81.89.53.154.host.vnet.sk. [81.89.53.154]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-57aae2323f7sm8956346a12.85.2024.06.11.02.22.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 11 Jun 2024 02:22:04 -0700 (PDT) Message-ID: Date: Tue, 11 Jun 2024 11:22:03 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 4/4] dts: add test case that utilizes offload to pmd_buffer_scatter To: Jeremy Spewock 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 References: <20240514201436.2496-1-jspewock@iol.unh.edu> <20240605213148.21371-1-jspewock@iol.unh.edu> <20240605213148.21371-5-jspewock@iol.unh.edu> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 10. 6. 2024 22:08, Jeremy Spewock wrote: > On Mon, Jun 10, 2024 at 11:22 AM Juraj Linkeš > wrote: >> >>> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_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 += struct.pack("=B", int("%s%s" % (X_in_hex[0], X_in_hex[1]), 16)) >>> received_packets = self.send_packet_and_capture(packet) >>> + # filter down the list to packets that have the appropriate structure >>> + received_packets = list( >>> + filter(lambda p: Ether in p and IP in p and Raw in p, received_packets) >>> + ) >>> self.verify(len(received_packets) > 0, "Did not receive any packets.") >>> - load = hexstr(received_packets[0].getlayer(2), onlyhex=1) >>> >>> - 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 going >> 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 multi-segment packets. >>> >>> Support for scattered packets is shown by sending 5 packets of differing length >>> @@ -103,34 +119,53 @@ def pmd_scatter(self, mbsize: int) -> None: >>> """ >>> testpmd_shell = self.sut_node.create_interactive_shell( >>> TestPmdShell, >>> - app_parameters=( >>> - "--mbcache=200 " >>> - f"--mbuf-size={mbsize} " >>> - "--max-pkt-len=9000 " >>> - "--port-topology=paired " >>> - "--tx-offloads=0x00008000" >>> - ), >>> + app_parameters=" ".join(testpmd_params), >>> privileged=True, >>> ) >>> 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 = self.scatter_pktgen_send_packet(mbsize + 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). >> > >>> + @requires(NicCapability.scattered_rx) >>> def test_scatter_mbuf_2048(self) -> None: >>> """Run the :meth:`pmd_scatter` test with `mbsize` set to 2048.""" >>> - self.pmd_scatter(mbsize=2048) >>> + self.pmd_scatter( >>> + mbsize=2048, testpmd_params=[*(self.base_testpmd_parameters), "--mbuf-size=2048"] >>> + ) >>> + >> >> 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 just >> 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 2048 and rx_scatter offload.""" >>> + self.pmd_scatter( >>> + mbsize=2048, >>> + testpmd_params=[ >>> + *(self.base_testpmd_parameters), >>> + "--mbuf-size=2048", >>> + "--enable-scatter", >>> + ], >>> + ) >>> >>> def tear_down_suite(self) -> None: >>> """Tear down the test suite.