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 ACE0545945; Mon, 9 Sep 2024 12:44:06 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4D5E0402BB; Mon, 9 Sep 2024 12:44:06 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id C352440299 for ; Mon, 9 Sep 2024 12:44:04 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9E925FEC; Mon, 9 Sep 2024 03:44:32 -0700 (PDT) Received: from [10.1.25.50] (JR4XG4HTQC.cambridge.arm.com [10.1.25.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 64CBC3F66E; Mon, 9 Sep 2024 03:44:03 -0700 (PDT) Message-ID: Date: Mon, 9 Sep 2024 11:44:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] dts: add l2fwd test suite Content-Language: en-GB To: Jeremy Spewock Cc: dev@dpdk.org, =?UTF-8?Q?Juraj_Linke=C5=A1?= , Honnappa Nagarahalli , Paul Szczepanek References: <20240806125140.2582859-1-luca.vizzarro@arm.com> From: Luca Vizzarro In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 09/08/2024 16:27, Jeremy Spewock wrote: >> Signed-off-by: Luca Vizzarro >> Reviewed-by: Paul Szczepanek >> --- >> Depends-on: series-32714 ("dts: add pktgen and testpmd changes") > > Out of my own curiosity, are depends on supposed to be outside of the > commit body? I don't think it really matters for automation or > anything regardless, but I just didn't know if there was a rule about > it. Depends-on tags are metadata for Patchwork, they don't belong in the repository. So there shouldn't be any depends-on committed as it wouldn't make any sense in Git. My reply aside, the contributing guidelines also specify how to do it[1]. >> new file mode 100644 >> index 0000000000..46f07b78eb >> --- /dev/null >> +++ b/dts/tests/TestSuite_l2fwd.py >> @@ -0,0 +1,58 @@ > > This file looks like it is missing the copyright information at the top. Argh! Good catch, thank you! >> +"""Basic L2 forwarding test suite. >> + >> +This testing suites runs basic L2 forwarding on testpmd with different queue sizes per port. > > The phrasing of "different queue sizes per port" makes me initially > think that like, port 0 will have 2 queues and port 1 will have 4. > Maybe something like "This testing suites runs basic L2 forwarding on > testpmd across multiple different queue sizes" would make this more > clear. > Ack. >> + def set_up_suite(self) -> None: >> + """Set up the test suite. >> + >> + Setup: >> + Verify that we have at least 2 ports in the current test. Generate the random packets >> + that will be sent and spawn a reusable testpmd shell. > > Seems like this method is no longer spawning a testpmd shell, so this > part of the doc-string is no longer relevant. Ack. Remnants of an earlier version... :') >> + def test_l2fwd_integrity(self) -> None: >> + """Test the L2 forwarding integrity. >> + >> + Test: >> + Configure a testpmd shell with a different numbers of queues per run. Start up L2 > > It might make sense to name the numbers of queues in the doc-string > just so that the rst for the suite is more clear. Ack. >> + expected_packets = [self.get_expected_packet(packet) for packet in self.packets] > > Ahh, the get_expected_packet method also sheds some light on how the > match_all_packets could be useful. > :) [1] https://doc.dpdk.org/guides/contributing/patches.html#patch-dependencies