* Re: [PATCH] dts: add l2fwd test suite
2024-08-06 12:51 [PATCH] dts: add l2fwd test suite Luca Vizzarro
@ 2024-08-09 15:27 ` Jeremy Spewock
2024-09-09 10:44 ` Luca Vizzarro
2024-09-04 18:11 ` Dean Marx
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Jeremy Spewock @ 2024-08-09 15:27 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
Hey Luca,
Thanks for the patch! Just a few pretty minor comments below.
On Tue, Aug 6, 2024 at 8:53 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Add a basic L2 forwarding test suite which tests the correct
> functionality of the forwarding facility built-in in the DPDK.
>
> The tests are performed with different queues numbers per port.
>
> Bugzilla ID: 1481
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
> 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.
> ---
> dts/framework/config/conf_yaml_schema.json | 3 +-
> dts/tests/TestSuite_l2fwd.py | 58 ++++++++++++++++++++++
> 2 files changed, 60 insertions(+), 1 deletion(-)
> create mode 100644 dts/tests/TestSuite_l2fwd.py
>
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index df390e8ae2..58a719a923 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -187,7 +187,8 @@
> "enum": [
> "hello_world",
> "os_udp",
> - "pmd_buffer_scatter"
> + "pmd_buffer_scatter",
> + "l2fwd"
> ]
> },
> "test_target": {
> diff --git a/dts/tests/TestSuite_l2fwd.py b/dts/tests/TestSuite_l2fwd.py
> 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.
> +"""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.
> +The forwarding test is performed with several packets being sent at once.
> +"""
> +
> +from framework.params.testpmd import EthPeer, SimpleForwardingModes
> +from framework.remote_session.testpmd_shell import TestPmdShell
> +from framework.test_suite import TestSuite
> +from framework.testbed_model.cpu import LogicalCoreCount
> +from framework.utils import generate_random_packets
> +
> +
> +class TestL2fwd(TestSuite):
> + """L2 forwarding test suite."""
> +
> + #: The total number of packets to generate and send for forwarding.
> + NUMBER_OF_PACKETS_TO_SEND = 50
> + #: The payload size to use for the generated packets in bytes.
> + PAYLOAD_SIZE = 100
> +
> + 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.
> + """
> + self.verify(len(self.sut_node.ports) >= 2, "At least 2 ports are required for this test.")
> + self.packets = generate_random_packets(self.NUMBER_OF_PACKETS_TO_SEND, self.PAYLOAD_SIZE)
> +
> + 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.
> + forwarding, send random packets from the TG and verify they were all received back.
> + """
> + queues = [1, 2, 4, 8]
> +
> + with TestPmdShell(
> + self.sut_node,
> + lcore_filter_specifier=LogicalCoreCount(cores_per_socket=4),
> + forward_mode=SimpleForwardingModes.mac,
> + eth_peer=[EthPeer(1, self.tg_node.ports[1].mac_address)],
> + disable_device_start=True,
> + ) as shell:
> + for queues_num in queues:
> + self._logger.info(f"Testing L2 forwarding with {queues_num} queue(s)")
> + shell.set_ports_queues(queues_num)
> + shell.start()
> +
> + received_packets = self.send_packets_and_capture(self.packets)
> +
> + 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.
> + self.match_all_packets(expected_packets, received_packets)
> +
> + shell.stop()
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dts: add l2fwd test suite
2024-08-09 15:27 ` Jeremy Spewock
@ 2024-09-09 10:44 ` Luca Vizzarro
0 siblings, 0 replies; 7+ messages in thread
From: Luca Vizzarro @ 2024-09-09 10:44 UTC (permalink / raw)
To: Jeremy Spewock
Cc: dev, Juraj Linkeš, Honnappa Nagarahalli, Paul Szczepanek
On 09/08/2024 16:27, Jeremy Spewock wrote:
>> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
>> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dts: add l2fwd test suite
2024-08-06 12:51 [PATCH] dts: add l2fwd test suite Luca Vizzarro
2024-08-09 15:27 ` Jeremy Spewock
@ 2024-09-04 18:11 ` Dean Marx
2024-09-09 15:05 ` Patrick Robb
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Dean Marx @ 2024-09-04 18:11 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On Tue, Aug 6, 2024 at 8:53 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
> Add a basic L2 forwarding test suite which tests the correct
> functionality of the forwarding facility built-in in the DPDK.
>
> The tests are performed with different queues numbers per port.
>
> Bugzilla ID: 1481
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
> Depends-on: series-32714 ("dts: add pktgen and testpmd changes")
>
Jeremy already mentioned this but the suite is missing the copyright and
license header, otherwise:
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
[-- Attachment #2: Type: text/html, Size: 1158 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dts: add l2fwd test suite
2024-08-06 12:51 [PATCH] dts: add l2fwd test suite Luca Vizzarro
2024-08-09 15:27 ` Jeremy Spewock
2024-09-04 18:11 ` Dean Marx
@ 2024-09-09 15:05 ` Patrick Robb
2024-09-09 17:54 ` Nicholas Pratte
2024-10-30 15:03 ` [PATCH v2] " Thomas Wilks
4 siblings, 0 replies; 7+ messages in thread
From: Patrick Robb @ 2024-09-09 15:05 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek
Reviewed-by: Patrick Robb <probb@iol.unh.edu>
Tested-by: Patrick Robb <probb@iol.unh.edu>
Ran on a Broadcom 57414 2x25G NIC on an Intel x86 server.
I'll follow up with a review on the pktgen and testpmd changes series
as obviously we want to move that to next-dts. Thanks!
On Tue, Aug 6, 2024 at 8:53 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
> + 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
> + forwarding, send random packets from the TG and verify they were all received back.
> + """
> + queues = [1, 2, 4, 8]
The only question I had was whether we need a show port info
capability check to verify the max queues per port is at least 8, but
I think that 8 is so low that we will not realistically run into this
problem, after a quick 60 second check on a couple NICs.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dts: add l2fwd test suite
2024-08-06 12:51 [PATCH] dts: add l2fwd test suite Luca Vizzarro
` (2 preceding siblings ...)
2024-09-09 15:05 ` Patrick Robb
@ 2024-09-09 17:54 ` Nicholas Pratte
2024-10-30 15:03 ` [PATCH v2] " Thomas Wilks
4 siblings, 0 replies; 7+ messages in thread
From: Nicholas Pratte @ 2024-09-09 17:54 UTC (permalink / raw)
To: Luca Vizzarro
Cc: dev, Jeremy Spewock, Juraj Linkeš,
Honnappa Nagarahalli, Paul Szczepanek
Just a few clarifying questions below. Otherwise,
(Test cases was run on Connect_x5 and Broadcom P225p devices).
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
Tested-by: Nicholas Pratte <npratte@iol.unh.edu>
<snip>
> +
> + with TestPmdShell(
> + self.sut_node,
> + lcore_filter_specifier=LogicalCoreCount(cores_per_socket=4),
> + forward_mode=SimpleForwardingModes.mac,
> + eth_peer=[EthPeer(1, self.tg_node.ports[1].mac_address)],
> + disable_device_start=True,
Would you be able to elaborate on the explicit lcore definition and
the eth_peer defined in the testpmd parameters above? I tried doing
some digging to understand the existence of these parameters, but
couldn't find anything insightful. Were these parameters basically
just copied over from old DTS?
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] dts: add l2fwd test suite
2024-08-06 12:51 [PATCH] dts: add l2fwd test suite Luca Vizzarro
` (3 preceding siblings ...)
2024-09-09 17:54 ` Nicholas Pratte
@ 2024-10-30 15:03 ` Thomas Wilks
4 siblings, 0 replies; 7+ messages in thread
From: Thomas Wilks @ 2024-10-30 15:03 UTC (permalink / raw)
To: dev; +Cc: Thomas Wilks, Luca Vizzarro, Paul Szczepanek
Add a basic L2 forwarding test suite which tests the correct
functionality of the forwarding facility built-in in the DPDK.
The tests are performed with different queues numbers per port.
Bugzilla ID: 1481
Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Signed-off-by: Thomas Wilks <thomas.wilks@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
dts/tests/TestSuite_l2fwd.py | 62 ++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 dts/tests/TestSuite_l2fwd.py
diff --git a/dts/tests/TestSuite_l2fwd.py b/dts/tests/TestSuite_l2fwd.py
new file mode 100644
index 0000000000..07467946e7
--- /dev/null
+++ b/dts/tests/TestSuite_l2fwd.py
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 Arm Limited
+
+"""Basic L2 forwarding test suite.
+
+This testing suites runs basic L2 forwarding on testpmd with different queue sizes per port.
+The forwarding test is performed with several packets being sent at once.
+"""
+
+from framework.params.testpmd import EthPeer, SimpleForwardingModes
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite, func_test
+from framework.testbed_model.cpu import LogicalCoreCount
+from framework.utils import generate_random_packets
+
+
+class TestL2fwd(TestSuite):
+ """L2 forwarding test suite."""
+
+ #: The total number of packets to generate and send for forwarding.
+ NUMBER_OF_PACKETS_TO_SEND = 50
+ #: The payload size to use for the generated packets in bytes.
+ PAYLOAD_SIZE = 100
+
+ 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.
+ """
+ self.verify(len(self.sut_node.ports) >= 2, "At least 2 ports are required for this test.")
+ self.packets = generate_random_packets(self.NUMBER_OF_PACKETS_TO_SEND, self.PAYLOAD_SIZE)
+
+ @func_test
+ 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
+ forwarding, send random packets from the TG and verify they were all received back.
+ """
+ queues = [1, 2, 4, 8]
+
+ with TestPmdShell(
+ self.sut_node,
+ lcore_filter_specifier=LogicalCoreCount(cores_per_socket=4),
+ forward_mode=SimpleForwardingModes.mac,
+ eth_peer=[EthPeer(1, self.tg_node.ports[1].mac_address)],
+ disable_device_start=True,
+ ) as shell:
+ for queues_num in queues:
+ self._logger.info(f"Testing L2 forwarding with {queues_num} queue(s)")
+ shell.set_ports_queues(queues_num)
+ shell.start()
+
+ received_packets = self.send_packets_and_capture(self.packets)
+
+ expected_packets = [self.get_expected_packet(packet) for packet in self.packets]
+ self.match_all_packets(expected_packets, received_packets)
+
+ shell.stop()
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread