Re: [PATCH v3 1/7] dts: Add scatter test suite

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> This test suite provides testing the support of scattered packets by
> Poll Mode Drivers using testpmd. It incorporates 5 different test cases
> which test the sending and receiving of packets with lengths that are
> less than the mbuf data buffer size, the same as the mbuf data buffer
> size, and the mbuf data buffer size plus 1, 4, and 5. The goal of this
> test suite is to align with the existing dts test plan for scattered
> packets within DTS.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/tests/TestSuite_scatter.py | 86 ++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 dts/tests/TestSuite_scatter.py
>
> diff --git a/dts/tests/TestSuite_scatter.py b/dts/tests/TestSuite_scatter.py
> new file mode 100644
> index 0000000000..217f465e92
> --- /dev/null
> +++ b/dts/tests/TestSuite_scatter.py
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +import struct
> +
> +from scapy.layers.inet import IP  # type: ignore[import]
> +from scapy.layers.l2 import Ether  # type: ignore[import]
> +from scapy.packet import Raw  # type: ignore[import]
> +from scapy.utils import hexstr  # type: ignore[import]
> +
> +from framework.remote_session import TestPmdShell
> +from framework.test_suite import TestSuite
> +
> +
> +class Scatter(TestSuite):
> +    mbsize: int
> +
> +    def set_up_suite(self) -> None:
> +        self.verify(
> +            len(self._port_links) > 1,
> +            "Must have at least two port links to run scatter",
> +        )
> +        if self._sut_port_egress.os_driver in ["i40e", "ixgbe", "ice"]:
> +            self.mbsize = 2048
> +        else:
> +            self.mbsize = 1024
> +

How exactly should mbsize determined? Is os_driver the sole
determinant? Or is the mbsize also somehow specific to the suite?

If it's just about the driver, then let's move the mbsize calculation
to Port and use that.

> +        self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_egress)
> +        self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_ingress)
> +
> +    def scatter_pktgen_send_packet(self, pktsize: int) -> str:
> +        """Generate and send packet to the SUT.
> +
> +        Functional test for scatter packets.
> +
> +        Args:
> +            pktsize: Size of the packet to generate and send.
> +        """
> +        packet = Ether() / IP() / Raw()
> +        packet.getlayer(2).load = ""
> +        payload_len = pktsize - len(packet) - 4
> +        payload = ["58"] * payload_len
> +        # pack the payload
> +        for X_in_hex in payload:
> +            packet.load += struct.pack(
> +                "=B", int("%s%s" % (X_in_hex[0], X_in_hex[1]), 16)
> +            )
> +        load = hexstr(packet.getlayer(2), onlyhex=1)
> +        received_packets = self.send_packet_and_capture(packet)
> +        self.verify(len(received_packets) > 0, "Did not receive any packets.")
> +        load = hexstr(received_packets[0].getlayer(2), onlyhex=1)
> +
> +        return load

I guess this is where the discussion about packet verification
originates. Once we conclude that discussion, let's revisit this.

> +
> +    def test_scatter_mbuf_2048(self) -> None:

This has 2048 in name, but the mbsize doesn't have to be 2048. Do we
have to have mbuf_2048 in the name?

> +        """
> +        Test:
> +            Start testpmd and run functional test with preset mbsize.
> +        """

The one-line description is missing.

> +        testpmd = self.sut_node.create_interactive_shell(
> +            TestPmdShell,
> +            app_parameters=(
> +                "--mbcache=200 "
> +                f"--mbuf-size={self.mbsize} "
> +                "--max-pkt-len=9000 "
> +                "--port-topology=paired "
> +                "--tx-offloads=0x00008000"
> +            ),
> +            privileged=True,
> +        )
> +        testpmd.send_command("set fwd mac")
> +        testpmd.send_command("start")
> +        link_is_up = testpmd.wait_link_status_up(0) and testpmd.wait_link_status_up(1)

Other test suites may use this or something similar to it. I'd move
the logic to the class so that we don't have to call send_command().
I'd also look into making the start/stop cycle a context manager (so
that's usable with the "with" statement), if at all possible.

> +        self.verify(link_is_up, "Links never came up in TestPMD.")
> +
> +        for offset in [-1, 0, 1, 4, 5]:
> +            recv_payload = self.scatter_pktgen_send_packet(self.mbsize + offset)
> +            self.verify(
> +                ("58 " * 8).strip() in recv_payload,
> +                "Received packet had incorrect payload",
> +            )
> +        testpmd.send_command("stop")
> +
> +    def tear_down_suite(self) -> None:
> +        self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_egress)
> +        self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_ingress)

We're assuming the original mtu was 1500. That's a fine assumption,
but let's keep it in mind just in case.

> --
> 2.42.0
>