<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:arial,sans-serif"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 8, 2024 at 10:47 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Jan 3, 2024 at 11:33 PM <<a href="mailto:jspewock@iol.unh.edu" target="_blank">jspewock@iol.unh.edu</a>> wrote:<br>
><br>
> From: Jeremy Spewock <<a href="mailto:jspewock@iol.unh.edu" target="_blank">jspewock@iol.unh.edu</a>><br>
><br>
> This test suite provides testing of the support of scattered packets by<br>
> Poll Mode Drivers using testpmd, verifying the ability to receive and<br>
> transmit scattered multi-segment packets made up of multiple<br>
> non-contiguous memory buffers. This is tested through 5 different cases<br>
> in which the length of the packets sent are less than the mbuf size,<br>
> equal to the mbuf size, and 1, 4, and 5 bytes greater than the mbuf size<br>
> in order to show both the CRC and the packet data are capable of<br>
> existing in the first, second, or both buffers.<br>
><br>
> Naturally, if the PMD is capable of forwarding scattered packets which<br>
> it receives as input, this shows it is capable of both receiving and<br>
> transmitting scattered packets.<br>
><br>
> Signed-off-by: Jeremy Spewock <<a href="mailto:jspewock@iol.unh.edu" target="_blank">jspewock@iol.unh.edu</a>><br>
> ---<br>
>  dts/tests/TestSuite_pmd_buffer_scatter.py | 126 ++++++++++++++++++++++<br>
>  1 file changed, 126 insertions(+)<br>
>  create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py<br>
><br>
> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py<br>
> new file mode 100644<br>
> index 0000000000..8838c3404f<br>
> --- /dev/null<br>
> +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py<br>
> @@ -0,0 +1,126 @@<br>
> +# SPDX-License-Identifier: BSD-3-Clause<br>
> +# Copyright(c) 2023-2024 University of New Hampshire<br>
> +<br>
> +"""Multi-segment packet scattering testing suite.<br>
<br>
Test suite - I guess this is a copy-paste?<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">Good catch, it probably was.<br></div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +This testing suite tests the support of transmitting and receiving scattered packets. This is shown<br>
> +by the Poll Mode Driver being able to forward scattered multi-segment packets composed of multiple<br>
> +non-contiguous memory buffers. To ensure the receipt of scattered packets, the DMA rings of the<br>
> +port's RX queues must be configured with mbuf data buffers whose size is less than the maximum<br>
> +length.<br>
> +<br>
> +If it is the case that the Poll Mode Driver can forward scattered packets which it receives, then<br>
> +this suffices to show the Poll Mode Driver is capable of both receiving and transmitting scattered<br>
> +packets.<br>
> +"""<br>
<br>
We have a newline between the docstring and the import everywhere.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">You're right, I must have missed it here, I'll add that.<br></div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +import struct<br>
> +<br>
> +from scapy.layers.inet import IP  # type: ignore[import]<br>
> +from scapy.layers.l2 import Ether  # type: ignore[import]<br>
> +from scapy.packet import Raw  # type: ignore[import]<br>
> +from scapy.utils import hexstr  # type: ignore[import]<br>
> +<br>
> +from framework.remote_session.testpmd_shell import TestPmdForwardingModes, TestPmdShell<br>
> +from framework.test_suite import TestSuite<br>
> +<br>
> +<br>
> +class PmdBufferScatter(TestSuite):<br>
> +    """DPDK PMD packet scattering test suite.<br>
> +<br>
> +    Configure the Rx queues to have mbuf data buffers whose sizes are smaller than the maximum<br>
> +    packet size. Specifically, set mbuf data buffers to have a size of 2048 to fit a full 1512-byte<br>
> +    (CRC included) ethernet frame in a mono-segment packet. The testing of scattered packets is<br>
> +    done by sending a packet whose length is greater than the size of the configured size of mbuf<br>
> +    data buffers. There are a total of 5 packets sent within test cases which have lengths less<br>
> +    than, equal to, and greater than the mbuf size. There are multiple packets sent with lengths<br>
> +    greater than the mbuf size in order to test cases such as:<br>
> +<br>
> +    1. A single byte of the CRC being in a second buffer while the remaining 3 bytes are stored in<br>
> +        the first buffer alongside packet data.<br>
> +    2. The entire CRC being stored in a second buffer while all of the packet data is stored in the<br>
> +        first.<br>
> +    3. Most of the packet data being stored in the first buffer and a single byte of packet data<br>
> +        stored in a second buffer alongside the CRC.<br>
> +    """<br>
> +<br>
> +    def set_up_suite(self) -> None:<br>
> +        """Set up the test suite.<br>
> +<br>
> +        Setup:<br>
> +            Verify they we have at least 2 port links in the current execution and increase the MTU<br>
<br>
Typo - they.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">Oops, thank you!<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +            of both ports on the tg_node to 9000 to support larger packet sizes.<br>
<br>
The description should be code agnostic, so let's use traffic<br>
generator node instead of tg_node.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">Good point, I'll update this.<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +        """<br>
> +        self.verify(<br>
> +            len(self._port_links) > 1,<br>
> +            "Must have at least two port links to run scatter",<br>
<br>
I'd like this to be at least "Must have at least two port links to run<br>
the scatter test suite" so that it's immediately obvious where this<br>
comes from. I'm also debating which of these is better: "Must have at<br>
least" or "There must be at least", but I'm not sure.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">I think reading it over that "There must be at least" sounds better, I'll update this as well.<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +        )<br>
> +<br>
> +        self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_egress)<br>
> +        self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_ingress)<br>
> +<br>
> +    def scatter_pktgen_send_packet(self, pktsize: int) -> str:<br>
> +        """Generate and send packet to the SUT.<br>
<br>
send a packet<br>
<br>
But this also captures a packet, so let's mention that.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">Good point, I'll add this.<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +        Functional test for scatter packets.<br>
<br>
This is just part of the test. The actual test is the pmd_scatter<br>
method with test cases being the callers of pmd_scatter.<br>
We should improve this. We mentioned a packet, so let's describe it.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">This definitely could be expanded, this likely just came from pulling from old DTS or before the docstrings were expanded. Good catch!<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +        Args:<br>
> +            pktsize: Size of the packet to generate and send.<br>
> +        """<br>
> +        packet = Ether() / IP() / Raw()<br>
> +        packet.getlayer(2).load = ""<br>
> +        payload_len = pktsize - len(packet) - 4<br>
> +        payload = ["58"] * payload_len<br>
> +        # pack the payload<br>
> +        for X_in_hex in payload:<br>
> +            packet.load += struct.pack("=B", int("%s%s" % (X_in_hex[0], X_in_hex[1]), 16))<br>
> +        received_packets = self.send_packet_and_capture(packet)<br>
> +        self.verify(len(received_packets) > 0, "Did not receive any packets.")<br>
> +        load = hexstr(received_packets[0].getlayer(2), onlyhex=1)<br>
> +<br>
> +        return load<br>
> +<br>
> +    def pmd_scatter(self, mbsize: int) -> None:<br>
> +        """Testpmd support of receiving and sending scattered multi-segment packets.<br>
> +<br>
> +        Support for scattered packets is shown by sending 5 packets of differing length<br>
> +        where the length of the packet is calculated by taking mbuf-size + an offset. The<br>
> +        offsets used in the test case are -1, 0, 1, 4, 5 respectively.<br>
> +<br>
<br>
In the test.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">Good point, I'll change this.<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +        Test:<br>
> +            Start testpmd and run functional test with preset mbsize.<br>
> +        """<br>
> +        testpmd = self.sut_node.create_interactive_shell(<br>
> +            TestPmdShell,<br>
> +            app_parameters=(<br>
> +                "--mbcache=200 "<br>
> +                f"--mbuf-size={mbsize} "<br>
> +                "--max-pkt-len=9000 "<br>
> +                "--port-topology=paired "<br>
> +                "--tx-offloads=0x00008000"<br>
> +            ),<br>
> +            privileged=True,<br>
> +        )<br>
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)<br>
> +        testpmd.start()<br>
> +<br>
> +        for offset in [-1, 0, 1, 4, 5]:<br>
> +            recv_payload = self.scatter_pktgen_send_packet(mbsize + offset)<br>
> +            self._logger.debug(f"Payload of scattered packet after forwarding: \n{recv_payload}")<br>
> +            self.verify(<br>
> +                ("58 " * 8).strip() in recv_payload,<br>
> +                f"Payload of scattered packet did not match expected payload with offset {offset}.",<br>
> +            )<br>
> +        testpmd.stop()<br>
> +<br>
> +    def test_scatter_mbuf_2048(self) -> None:<br>
> +        """Run :func:`~PmdBufferScatter.pmd_scatter` function with `mbsize` set to 2048."""<br>
<br>
This would probably read better as "Run the pmd_scatter test"<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">I agree, I'll update this.<br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +        self.pmd_scatter(mbsize=2048)<br>
> +<br>
> +    def tear_down_suite(self) -> None:<br>
> +        """Tear down the test suite.<br>
> +<br>
> +        Teardown:<br>
> +            Set the MTU of the tg_node back to a more standard size of 1500<br>
> +        """<br>
> +        self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_egress)<br>
> +        self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_ingress)<br>
> --<br>
> 2.43.0<br>
><br></blockquote><div><br></div><div style="font-family:arial,sans-serif" class="gmail_default">Thank you again for the thorough feedback!<br></div></div></div>