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

[Alex Chapman <alex.chapman@arm.com>]

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