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.vizza...@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.vizza...@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepa...@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
>
