<snip> 
You could probably combine this line with the previous since they are
from the same module.

> +from scapy.packet import Raw  # type: ignore[import-untyped]

I think you can also import `Packet` from this module if you wanted to
combine another two lines as well.


Wow I didn't even notice that good catch
 
> +
> +from framework.remote_session.testpmd_shell import (
> +    SimpleForwardingModes,

This reminds me of a question I've had for a little while now which
is, should this be imported from the module that it originates from
(params) or is it fine to just grab it from the testpmd shell where it
is also imported? I guess I don't really see this causing a problem at
all since there isn't really a chance of any circular imports in this
case or things that would be breaking, but I just don't know if there
is any kind of guideline regarding these scenarios.

I briefly looked for some best practice guidelines about this kind of thing but I couldn't find anything explicit. However, I'm going to assume it's probably preferred to do a direct import like you mentioned so I'm going to change that.
 
<snip>
> +        testpmd.start()
> +        self.send_packet_and_capture(packet=packet)
> +        verbose_output = testpmd.extract_verbose_output(testpmd.stop())
> +        for packet in verbose_output:
> +            if packet.dst_mac == "00:00:00:00:00:01":

Since this method is the one that relies on this MAC address being set
on the packet, it might be helpful to set that MAC on the packet
before sending it in the same method. Why this address is the one you
were searching for would then be clear at just a glance at the send
method which could be useful. That or you could note in the doc-string
what you expect the MAC to be.

Funny that you mentioned this because I actually tried to set the destination mac address within the send_packet_and_verify_checksums method and I couldn't get it to work no matter what I tried. For some reason even though the mac address was the one I set after send_packet_and_capture, none of the packets in verbose_output would have that mac address. I tried debugging for a while but I just couldn't get it to work, even with the adjust_addresses patch applied it was breaking so I just removed it entirely and set them all in the test cases, which worked fine. I did add an extra arg to the send_and_verify_checksums method called ID, which I passed the mac_id variable to so that it's cleaner.
 

> +                if OLFlag.RTE_MBUF_F_RX_L4_CKSUM_GOOD in packet.ol_flags:
> +                    isIP = True
> +                else:
> +                    isIP = False
> +                if OLFlag.RTE_MBUF_F_RX_L4_CKSUM_GOOD in packet.ol_flags:
> +                    isL4 = True
> +                else:
> +                    isL4 = False
> +            else:
> +                isIP = False
> +                isL4 = False

Would having this else statement break the booleans if there was
another noise packet after the one that you sent in the verbose
output? I think that would make both of these booleans false by the
end of the loop even if one was supposed to be true. You might be able
to fix this however with either a break statement after you find the
first packet with the right MAC address, or you could use the built-in
`any()` function that python offers.

Yeah you're right, not sure what I was thinking there haha. I simplified it back down to two lines in the new version.
 
<snip>

Not really important at all (especially because it won't affect the
order that the cases are run in) but it might make sense to put the
two individual tests of l3_rx and l4_rx before the test of both of
them combined. Again, super nit-picky, but it makes sense in my head
to see the individual parts tested and then see them tested together.
I'll leave it up to you if you think it makes sense to just leave it
as is :).

Makes sense to me, swapped them
 

> +
> +    def test_vlan_checksum(self) -> None:
> +        """Tests VLAN Rx checksum hardware offload and verify packet reception."""

What is this testing? Just that the checksums work with VLANs also
set? That's fine if so I just wasn't sure initially since it looks
like the method is checking to see if you can receive the packets and
then if the checksums are right.

Yes it's just testing to ensure checksums work with VLAN packets according to the test plan. I'm not entirely sure why they also check to make sure they're received, but it was in the test plan so I just left it in there.
 
<snip>

Other than that, all the docstring errors and function names were fixed in both patches. Thanks for the review Jeremy!