[RFC,v3,2/2] dts: Initial Implementation For Jumbo Frames Test Suite

The following test suite reflects the fundamental outline for how the
jumbo frames test suite may be designed. The test suite consists of five
individual test cases, each of which assesses the behavior of packet
transmissions for both 1518 byte and 9000 byte frames.

The edge cases are ripped directly from the old DTS framework, and the
general methodology is the same as well. The process, at this point, has
been refactored to operate within the new DTS framework.

Bugzilla ID: 1421

Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_jumboframes.py         | 182 +++++++++++++++++++++
 2 files changed, 184 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_jumboframes.py
  

Just some very light comments here as well, mostly about doc-string
but also a few questions/suggestions.

On Fri, Jul 26, 2024 at 10:14 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
<snip>
> diff --git a/dts/tests/TestSuite_jumboframes.py b/dts/tests/TestSuite_jumboframes.py
> new file mode 100644
> index 0000000000..dd8092f2a4
> --- /dev/null
> +++ b/dts/tests/TestSuite_jumboframes.py
> @@ -0,0 +1,182 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023-2024 University of New Hampshire

It's probably fine to just make the date on this copyright 2024.

> +"""Jumbo frame consistency and compatibility test suite.
> +
> +The test suite ensures the consistency of jumbo frames transmission within
> +Poll Mode Drivers using a series of individual test cases. If a Poll Mode
> +Driver receives a packet that is greater than its assigned MTU length, then
> +that packet will be dropped, and thus not received. Likewise, if a Poll Mode Driver

This sentence is a little confusing because you're saying "if a packet
is received with X condition then it isn't received." Maybe it would
be more clear to say it isn't forwarded?

> +receives a packet that is less than or equal to a its designated MTU length, then the

I think this was a typo, this should probably be either "equal to its"
or "equal to a" rather than both.

> +packet should be transmitted by the Poll Mode Driver, completing a cycle within the
> +testbed and getting received by the traffic generator. Thus, the following test suite
> +evaluates the behavior within all possible edge cases, ensuring that a test Poll
> +Mode Driver strictly abides by the above implications.
> +"""
<snip>
> +
> +class TestJumboframes(TestSuite):
> +    """DPDK PMD jumbo frames test suite.
> +
> +    Asserts the expected behavior of frames greater than, less then, or equal to

I think this should be "less than" rather than "less then".

> +    a designated MTU size in the testpmd application. If a packet size greater
> +    than the designated testpmd MTU length is retrieved, the test fails. If a
> +    packet size less than or equal to the designated testpmd MTU length is retrieved,
> +    the test passes.
> +    """
> +
> +    def set_up_suite(self) -> None:
> +        """Set up the test suite.
> +
> +        Setup:
> +            Set traffic generator MTU lengths to a size greater than scope of all
> +            test cases.
> +        """
> +        self.tg_node.main_session.configure_port_mtu(
> +            ETHER_JUMBO_FRAME_MTU + 200, self._tg_port_egress
> +        )
> +        self.tg_node.main_session.configure_port_mtu(
> +            ETHER_JUMBO_FRAME_MTU + 200, self._tg_port_ingress
> +        )

I know 9000 is a common jumboframe MTU to support, but do we have to
worry about a case where some NICs wouldn't support an MTU this high?
That could also be a case of the NICs where that would be a problem
are maybe older and not expected to be as common? I'm not completely
sure what the standards and expectations are.

> +
> +    def send_packet_and_verify(self, pktsize: int, should_receive: bool = True) -> None:
> +        """Generate, send, and capture packets to verify that the sent packet was received or not.

I feel like the "verify that..." asserts that you are verifying
something positive when it could be positive or negative. Maybe
"verify whether" would fit better here.

> +
> +        Generates a packet based on a specified size and sends it to the SUT. The desired packet's
> +        payload size is calculated, and arbitrary, byte-sized characters are inserted into the
> +        packet before sending. Packets are captured, and depending on the test case, packet
> +        payloads are checked to determine if the sent payload was received.
> +
> +        Args:
> +            pktsize: Size of packet to be generated and sent.
> +            should_receive: Indicate whether the test case expects to receive the packet or not.
> +        """
> +        padding = pktsize - IP_HEADER_LEN
> +        # Insert extra space for placeholder 'CRC' Error correction.
> +        packet = Ether() / Raw("    ") / IP(len=pktsize) / Raw(load="X" * padding)

This CRC error correction is interesting, I thought generally that the
Ether header length included the CRC, but it makes sense to try and
account for it if it isn't .

> +        received_packets = self.send_packet_and_capture(packet)
> +        found = any(
> +            ("X" * padding) in str(packets.load)
> +            for packets in received_packets
> +            if hasattr(packets, "load")
> +        )
> +
> +        if should_receive:
> +            self.verify(found, "Did not receive packet")
> +        else:
> +            self.verify(not found, "Received packet")
> +
<snip>
> +
> +    def test_jumboframes_jumbo_nojumbo(self) -> None:
> +        """Assess the boundaries of packets sent greater than standard MTU length.
> +
> +        PMDs are set to the standard MTU length of 1518 bytes to assess behavior of sent packets
> +        greater than this size. Sends one packet with a frame size of 1519. The test cases does

Since the bounds were increased to account for the difference in PMDs,
we should probably update this number in the doc-string accordingly.

> +        not expect to receive this packet.
> +
> +        Test:
> +            Start testpmd with standard MTU size of 1518. Send a packet of 1519 and verify it was
> +            not received.
> +        """
> +        with TestPmdShell(
> +            self.sut_node, tx_offloads=0x8000, mbuf_size=[9200], mbcache=200
> +        ) as testpmd:
> +            testpmd.configure_port_mtu_all(ETHER_STANDARD_FRAME)
> +            testpmd.start()
> +
> +            self.send_packet_and_verify(ETHER_STANDARD_FRAME + 5, False)
> +
> +    def test_jumboframes_normal_jumbo(self) -> None:
> +        """Assess the consistency of standard 1518 byte packets using a 9000 byte jumbo MTU length.
> +
> +        PMDs are set to a jumbo frame size of 9000 bytes. Packets of sizes 1517 and 1518 are sent
> +        to assess the boundaries of packets less than or equal to the standard MTU length of 1518.
> +        The test case expects to receive both packets.
> +
> +        Test:
> +            Start testpmd with a jumbo frame size of 9000 bytes. Send a packet of 1517 and 1518
> +            and verify they were received.
> +        """
> +        with TestPmdShell(
> +            self.sut_node, tx_offloads=0x8000, mbuf_size=[9200], mbcache=200
> +        ) as testpmd:
> +            testpmd.configure_port_mtu_all(ETHER_JUMBO_FRAME_MTU)
> +            testpmd.start()
> +
> +            self.send_packet_and_verify(ETHER_STANDARD_FRAME - 5)

Does it still make sense to take off extra bytes here since we are so
far away from the MTU boundary? I think this would be consistent still
even if it were 1517.

> +            self.send_packet_and_verify(ETHER_STANDARD_FRAME)
> +
> +    def test_jumboframes_jumbo_jumbo(self) -> None:
> +        """Assess the boundaries packets sent at an MTU size of 9000 bytes.

Should this first line have something like "Asses the boundaries with
packets..." to tie the sentence together a little more?

> +
> +        PMDs are set to a jumbo frames size of 9000 bytes. Packets of size 1519, 8999, and 9000

It would probably good to also reflect here that it is 1523, 8995 and 9000.

> +        are sent. The test expects to receive all packets.
> +
> +        Test:
> +            Start testpmd with an MTU length of 9000 bytes. Send packets of size 1519, 8999,
> +            and 9000 and verify that all packets were received.

and here as well.

> +        """
> +        with TestPmdShell(
> +            self.sut_node, tx_offloads=0x8000, mbuf_size=[9200], mbcache=200
> +        ) as testpmd:
> +            testpmd.configure_port_mtu_all(ETHER_JUMBO_FRAME_MTU)
> +            testpmd.start()
> +
> +            self.send_packet_and_verify(ETHER_STANDARD_FRAME + 5)
> +            self.send_packet_and_verify(ETHER_JUMBO_FRAME_MTU - 5)
> +            self.send_packet_and_verify(ETHER_JUMBO_FRAME_MTU)
> +
> +    def test_jumboframes_bigger_jumbo(self) -> None:
> +        """Assess the behavior of packets send greater than a specified MTU length of 9000 bytes.

Typo: send should probably be sent.

> +
> +        PMDs are set to a jumbo frames size of 9000 bytes. A packet of size 9001 is sent to the SUT.

We should probably also reflect here that the packet size is really 9005.

> +        The test case does not expect to receive the packet.
> +
> +        Test:
> +            Start testpmd with an MTU length of 9000 bytes. Send a packet of 9001 bytes and verify

Here as well.


> +            it was not received.
> +        """
> +        with TestPmdShell(
> +            self.sut_node, tx_offloads=0x8000, mbuf_size=[9200], mbcache=200
> +        ) as testpmd:
> +            testpmd.configure_port_mtu_all(ETHER_JUMBO_FRAME_MTU)
> +            testpmd.start()
> +
> +            self.send_packet_and_verify(ETHER_JUMBO_FRAME_MTU + 5, False)
> +
> +    def tear_down_suite(self) -> None:
> +        """Tear down the test suite.
> +
> +        Teardown:
> +            Set the MTU size of the traffic generator back to the standard 1518 byte size.
> +        """
> +        self.tg_node.main_session.configure_port_mtu(ETHER_STANDARD_FRAME, self._tg_port_egress)
> +        self.tg_node.main_session.configure_port_mtu(ETHER_STANDARD_FRAME, self._tg_port_ingress)
> --
> 2.44.0
>
  

Hi,
Ive been looking into the MTU terminology and would just like to clarify 
some naming conventions and doc strings.

On 7/26/24 15:13, Nicholas Pratte wrote:
<snip>
> +IP_HEADER_LEN = 20
> +ETHER_STANDARD_FRAME = 1500
> +ETHER_JUMBO_FRAME_MTU = 9000

For these constants, I am confused why one is "FRAME" and the other is 
"MTU". The value of 'ETHER_STANDARD_FRAME' is 1500 (the standard MTU 
size), it would make sense to rename it to 'ETHER_STANDARD_MTU', to keep 
naming consistent.

If the value was 1518 instead of 1500, then `ETHER_STANDARD_FRAME` would 
be appropriate.

<snip>
> +    def test_jumboframes_normal_nojumbo(self) -> None:
> +        """Assess the boundaries of packets sent less than or equal to the standard MTU length.
> +
> +        PMDs are set to the standard MTU length of 1518 to assess behavior of sent packets less than
> +        or equal to this size. Sends two packets: one that is less than 1518 bytes, and another that
> +        is equal to 1518 bytes. The test case expects to receive both packets.
> +
> +        Test:
> +            Start testpmd and send packets of sizes 1517 and 1518.
> +        """
> +        with TestPmdShell(
> +            self.sut_node, tx_offloads=0x8000, mbuf_size=[9200], mbcache=200
> +        ) as testpmd:
> +            testpmd.configure_port_mtu_all(ETHER_STANDARD_FRAME)
> +            testpmd.start()

Renaming 'ETHER_STANDARD_FRAME' to 'ETHER_STANDARD_MTU' would reduce 
confusion here too.
e.g.
`testpmd.configure_port_mtu_all(ETHER_STANDARD_MTU)`

Additionally, you state you are sending packets of sizes 1517 and 1518. 
but you then call:
`self.send_packet_and_verify(ETHER_STANDARD_FRAME - 5)`
`self.send_packet_and_verify(ETHER_STANDARD_FRAME)`

Calculating to:
`self.send_packet_and_verify(1495)`
`self.send_packet_and_verify(1500)`

Which is confusing.
I believe this is because you are accounting for the 4 bytes of VLAN's 
in your calculations, but you might want to explain this.


Overall very solid and clean test suite, just wanted to get 
clarification on a few areas 🙂.
Alex
  

Hi Alex, thanks for the review!

See my comments below.

<snip>
> > +IP_HEADER_LEN = 20
> > +ETHER_STANDARD_FRAME = 1500
> > +ETHER_JUMBO_FRAME_MTU = 9000
>
> For these constants, I am confused why one is "FRAME" and the other is
> "MTU". The value of 'ETHER_STANDARD_FRAME' is 1500 (the standard MTU
> size), it would make sense to rename it to 'ETHER_STANDARD_MTU', to keep
> naming consistent.
>
> If the value was 1518 instead of 1500, then `ETHER_STANDARD_FRAME` would
> be appropriate.

You are correct! I will make the changes.
>
<snip>
> Renaming 'ETHER_STANDARD_FRAME' to 'ETHER_STANDARD_MTU' would reduce
> confusion here too.
> e.g.
> `testpmd.configure_port_mtu_all(ETHER_STANDARD_MTU)`
>
> Additionally, you state you are sending packets of sizes 1517 and 1518.
> but you then call:
> `self.send_packet_and_verify(ETHER_STANDARD_FRAME - 5)`
> `self.send_packet_and_verify(ETHER_STANDARD_FRAME)`

Ack. I must have missed the docstring when I adjusted the boundaries
at which we test jumbo frame sizes ethernet overhead issues we've been
having. Adding the +5, -5 bytes is sort of a temporary
measure/placeholder while we wait to gather more information on how to
properly assess and test the ethernet overhead issue. See my next
comment for more clarification.
>
> Calculating to:
> `self.send_packet_and_verify(1495)`
> `self.send_packet_and_verify(1500)`
>
> Which is confusing.
> I believe this is because you are accounting for the 4 bytes of VLAN's
> in your calculations, but you might want to explain this.

What the +5 bytes situation really is for, currently, is to assess the
boundaries of a set MTU size, sorry for the confusion there. For
example, if we have a MTU of 1500, some vendors assume an ethernet
overhead of +22 bytes on top of the 1500 byte MTU (for a total frame
size of 1522 bytes), and other vendors, such as Mellanox, add +18
bytes of ethernet overhead +1500 byte MTU (for a total frame size of
1518). The +5 tries to compensate for this by adding +5 or -5 bytes to
properly test greater than or less than a 1500 byte MTU for all
vendors, but this gets tricky when you are trying to run tests at the
MTU size itself. If you look back to the oldest version I have of this
suite, you'll see that each test case was originally sending 1499 byte
packets, 1501 packets, and 1500 packets in some cases, but we can't
run tests like this because of the differing assumptions from vendor
to vendor (you can find the old suite in a different email thread, I
messed up the sending of the series so I apologize for that).

Here's the original version of this patch:
https://inbox.dpdk.org/dev/20240524183604.6925-2-npratte@iol.unh.edu/

The calculation of ethernet overhead basically comes down to a single
'if' statement in testpmd's code. You can find this method in
testpmd.c and do some digging if you're interested (look under
'app/test-pmd/testpmd.c' and search for 'get_eth_overhead')

>
>
> Overall very solid and clean test suite, just wanted to get
> clarification on a few areas 🙂.
> Alex

I wrote this test suite a while back when I was just starting out, and
a lot of this information was new to me at the time, so it's not
surprising to see some of the misuse of definitions you've laid out
here; I appreciate the feedback!

-Nicholas