Thank you for the feedback, see my comments below!

<snip>
>> +        current_mtu = testpmd_shell.show_port_info(0).mtu
>> +        self.verify(current_mtu is not None, "Error grabbing testpmd MTU 
>> value.")
>> +        if current_mtu and (
>> +            current_mtu >= STANDARD_MTU + VENDOR_AGNOSTIC_PADDING and mtu 
>> == STANDARD_MTU
>> +        ):
>> +            self.send_packet_and_verify(pkt_size=larger_frame_size, 
>> should_receive=True)
>
>
> I don't understand when this condition may be true - can you explain? Thanks!

I added some additional testing to Alex's MTU update tests to check
the forwarding of standard 1500 byte packets for MTU sizes greater
than 1500. Practically speaking this means that the suite will check
1500 byte packets for 2400, 4800 and 9000 byte MTU sizes. In all these
cases, packets of size 1509 should be forwarded. In the case of a 1500
byte MTU, this 1509 byte packet should be dropped.

Now the reason I decided to put these test 1500 byte packet tests in
the test suite at all is because the 'jumbo_frames' suite performs a
similar test; an MTU size of 9000 bytes is set, and standard 1500 byte
packets are checked for forwarding. The original 'mtu_update' doesn't
include this sort of testing in its test plan, but I figured I'd add
on this testing so that both MTU tests are more uniform in their
functional tests. It's a good question because this exact bit of
change in the test is what transformed this suite into an MTU test for
both '--max-pkt-len' and 'port config mtu' since each of these options
use different components of ethdev.

>
>>
>> +        else:
>> +            self.send_packet_and_verify(pkt_size=larger_frame_size, 
>> should_receive=False)
>> +
>> +    @func_test
>> +    def test_runtime_mtu_updating_and_forwarding(self) -> None:
>> +        """Verify runtime MTU adjustments and assess packet forwarding 
>> behavior.
>> +
>> +        Test:
>> +            Start TestPMD in a paired topology.
>> +            Set port MTU to 1500.
>> +            Send packets of 1493, 1500 and 1509 bytes.
>
>
> I think 1493 should be 1491.

Nice one! I'll fix that.

>
>>
>> --
>> 2.47.1
>>
>
> Thanks, other than a couple questions here and in the associated patch this 
> looks good. I can merge on Tuesday.
>
> Reviewed-by: Patrick Robb <pr...@iol.unh.edu>
> Tested-by: Patrick Robb <pr...@iol.unh.edu>

Reply via email to