On 10. 6. 2024 22:08, Jeremy Spewock wrote:
On Mon, Jun 10, 2024 at 11:22 AM Juraj Linkeš
<juraj.lin...@pantheon.tech> wrote:
diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py
b/dts/tests/TestSuite_pmd_buffer_scatter.py
index 41f6090a7e..76eabb51f6 100644
--- a/dts/tests/TestSuite_pmd_buffer_scatter.py
+++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
@@ -86,12 +99,15 @@ def scatter_pktgen_send_packet(self, pktsize: int) -> str:
for X_in_hex in payload:
packet.load += struct.pack("=B", int("%s%s" % (X_in_hex[0],
X_in_hex[1]), 16))
received_packets = self.send_packet_and_capture(packet)
+ # filter down the list to packets that have the appropriate structure
+ received_packets = list(
+ filter(lambda p: Ether in p and IP in p and Raw in p,
received_packets)
+ )
self.verify(len(received_packets) > 0, "Did not receive any
packets.")
- load = hexstr(received_packets[0].getlayer(2), onlyhex=1)
- return load
+ return received_packets
- def pmd_scatter(self, mbsize: int) -> None:
+ def pmd_scatter(self, mbsize: int, testpmd_params: list[str]) -> None:
Since base_testpmd_parameters is a class var, the method is always going
to have access to it and we only need to pass the extra parameters.
There's not much of a point in passing what's common to all tests to
this method, as it should contain the common parts.
Ack.
"""Testpmd support of receiving and sending scattered multi-segment
packets.
Support for scattered packets is shown by sending 5 packets of
differing length
@@ -103,34 +119,53 @@ def pmd_scatter(self, mbsize: int) -> None:
"""
testpmd_shell = self.sut_node.create_interactive_shell(
TestPmdShell,
- app_parameters=(
- "--mbcache=200 "
- f"--mbuf-size={mbsize} "
- "--max-pkt-len=9000 "
- "--port-topology=paired "
- "--tx-offloads=0x00008000"
- ),
+ app_parameters=" ".join(testpmd_params),
privileged=True,
)
with testpmd_shell as testpmd:
testpmd.set_forward_mode(TestPmdForwardingModes.mac)
+ # adjust the MTU of the SUT ports
+ for port_id in range(testpmd.number_of_ports):
+ testpmd.set_port_mtu(port_id, 9000)
testpmd.start()
for offset in [-1, 0, 1, 4, 5]:
- recv_payload = self.scatter_pktgen_send_packet(mbsize + offset)
+ # This list should only ever contain one element
Which list is the comment referring to? recv_packets? There could be
more than just one packet, right?
There technically could be in very strange cases, but this change also
adds a filter to `scatter_pktgen_send_packet()` that filters the list
before it is returned here. I imagine there wouldn't be (and in my
testing there aren't) any other packets that have the structure
Ether() / IP() / Raw() getting sent by anything on the wire, so I just
noted it to make it more clear that the call to `any()` probably isn't
going to have to consume much. I did the filtering in the other method
because I wanted to be able to distinguish between getting nothing,
and getting something that has the right structure but not the right
payload (as, presumably, if this test were to fail it would be shown
in the payload).
Right, but maybe in other setups this won't be true. We can just make
the comment say the list contains filtered packets with the expected
structure, as that would be more in line with the verification code
(where we don't assume it's just one packet).
<snip>
+ @requires(NicCapability.scattered_rx)
def test_scatter_mbuf_2048(self) -> None:
"""Run the :meth:`pmd_scatter` test with `mbsize` set to 2048."""
- self.pmd_scatter(mbsize=2048)
+ self.pmd_scatter(
+ mbsize=2048, testpmd_params=[*(self.base_testpmd_parameters),
"--mbuf-size=2048"]
+ )
+
I'm curious why you moved the --mbuf-size parameter here. It's always
going to be (or should be) equal to mbsize, which we already pass (and
now we're essentially passing the same thing twice), so I feel this just
creates opportunities for mistakes.
Honestly, when it's phrased like that, I have no good reason at all,
haha. I just put it there because I got stuck in some mentality of
"testpmd parameters go in this list, so it has to go here", but it did
feel weird to hardcode the same value twice like that. I'll adjust
this.
+ def test_scatter_mbuf_2048_with_offload(self) -> None:
+ """Run the :meth:`pmd_scatter` test with `mbsize` set to 2048 and rx_scatter
offload."""
+ self.pmd_scatter(
+ mbsize=2048,
+ testpmd_params=[
+ *(self.base_testpmd_parameters),
+ "--mbuf-size=2048",
+ "--enable-scatter",
+ ],
+ )
def tear_down_suite(self) -> None:
"""Tear down the test suite.
