[v4,7/7] dts: add scatter test suite
Checks
Commit Message
From: Jeremy Spewock <jspewock@iol.unh.edu>
This test suite provides testing the support of scattered packets by
Poll Mode Drivers using testpmd. It incorporates 5 different test cases
which test the sending and receiving of packets with lengths that are
less than the mbuf data buffer size, the same as the mbuf data buffer
size, and the mbuf data buffer size plus 1, 4, and 5. The goal of this
test suite is to align with the existing dts test plan for scattered
packets within DTS.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++++++
1 file changed, 105 insertions(+)
create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py
Comments
Should we use the full name (pmd_buffer_scatter) in the subject? I
lean towards the full name.
On Mon, Dec 18, 2023 at 7:13 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> This test suite provides testing the support of scattered packets by
> Poll Mode Drivers using testpmd. It incorporates 5 different test cases
> which test the sending and receiving of packets with lengths that are
> less than the mbuf data buffer size, the same as the mbuf data buffer
> size, and the mbuf data buffer size plus 1, 4, and 5. The goal of this
> test suite is to align with the existing dts test plan for scattered
> packets within DTS.
>
Again, we need to describe why we're adding this commit. In the case
of test suites, the why is obvious, so we should give a good high
level description of what the tests test (something like the test
suite tests the x feature by doing y, y being the salient part of the
tests). The original test plan is actually pretty good, so we can
extract the rationale from it.
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
> create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py
>
> diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py
> new file mode 100644
> index 0000000000..8e2a32a1aa
> --- /dev/null
> +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +"""Multi-segment packet scattering testing suite.
> +
> +Configure the Rx queues to have mbuf data buffers whose sizes are smaller than the maximum packet
> +size (currently set to 2048 to fit a full 1512-byte ethernet frame) and send a total of 5 packets
> +with lengths less than, equal to, and greater than the mbuf size (CRC included).
> +"""
Let's expand this. I'll point to the original test plan again, let's
use some of it here. I think it makes sense to make this docstring a
kind of a test plan with high level description.
> +import struct
> +
> +from scapy.layers.inet import IP # type: ignore[import]
> +from scapy.layers.l2 import Ether # type: ignore[import]
> +from scapy.packet import Raw # type: ignore[import]
> +from scapy.utils import hexstr # type: ignore[import]
> +
> +from framework.remote_session.remote.testpmd_shell import (
> + TestPmdForwardingModes,
> + TestPmdShell,
> +)
> +from framework.test_suite import TestSuite
> +
> +
> +class PmdBufferScatter(TestSuite):
> + """DPDK packet scattering test suite.
> +
And here we could add some more specifics.
I'd like to utilize the original test plans and a split like this
makes sense at a first glance.
> + Attributes:
> + mbsize: The size to se the mbuf to be.
> + """
> +
> + mbsize: int
> +
> + def set_up_suite(self) -> None:
> + self.verify(
> + len(self._port_links) > 1,
> + "Must have at least two port links to run scatter",
> + )
> +
> + self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_egress)
> + self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_ingress)
> +
> + def scatter_pktgen_send_packet(self, pktsize: int) -> str:
> + """Generate and send packet to the SUT.
> +
> + Functional test for scatter packets.
> +
> + Args:
> + pktsize: Size of the packet to generate and send.
> + """
> + packet = Ether() / IP() / Raw()
> + packet.getlayer(2).load = ""
> + payload_len = pktsize - len(packet) - 4
> + payload = ["58"] * payload_len
> + # pack the payload
> + for X_in_hex in payload:
> + packet.load += struct.pack("=B", int("%s%s" % (X_in_hex[0], X_in_hex[1]), 16))
> + load = hexstr(packet.getlayer(2), onlyhex=1)
> + received_packets = self.send_packet_and_capture(packet)
> + self.verify(len(received_packets) > 0, "Did not receive any packets.")
> + load = hexstr(received_packets[0].getlayer(2), onlyhex=1)
> +
> + return load
> +
> + def pmd_scatter(self) -> None:
> + """Testpmd support of receiving and sending scattered multi-segment packets.
> +
> + Support for scattered packets is shown by sending 5 packets of differing length
> + where the length of the packet is calculated by taking mbuf-size + an offset. The
> + offsets used in the test case are -1, 0, 1, 4, 5 respectively.
> +
> + Test:
> + Start testpmd and run functional test with preset mbsize.
> + """
> + testpmd = self.sut_node.create_interactive_shell(
> + TestPmdShell,
> + app_parameters=(
> + "--mbcache=200 "
> + f"--mbuf-size={self.mbsize} "
> + "--max-pkt-len=9000 "
> + "--port-topology=paired "
> + "--tx-offloads=0x00008000"
> + ),
> + privileged=True,
> + )
> + testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> + testpmd.start()
> + link_is_up = testpmd.wait_link_status_up(0) and testpmd.wait_link_status_up(1)
These two calls should probably be inside testpmd.start(). Looks like
we're always going to be doing this.
Also, this assumes there will be two ports. Ideally, the shell itself
will know which ports to check (that should be in EAL parameters), but
that may require a bigger refactor (unless we just parse back the -a
options, which could be permissible).
> + self.verify(link_is_up, "Links never came up in TestPMD.")
> +
> + for offset in [-1, 0, 1, 4, 5]:
> + recv_payload = self.scatter_pktgen_send_packet(self.mbsize + offset)
> + self.verify(
> + ("58 " * 8).strip() in recv_payload,
> + "Received packet had incorrect payload",
> + )
This is still just one test case. We should probably leave it as is
(i.e. not split into 5 test cases), but we should add the information
about the offset/test case into the message so that we know what
actually failed right away.
> + testpmd.stop()
> +
> + def test_scatter_mbuf_2048(self) -> None:
> + """Run :func:`~PmdBufferScatter.pmd_scatter` function after setting the `mbsize` to 2048."""
> + self.mbsize = 2048
> + self.pmd_scatter()
Let's just pass 2048 to the method, I don't see a reason for the
instance variable.
> +
> + def tear_down_suite(self) -> None:
> + self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_egress)
> + self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_ingress)
> --
> 2.43.0
>
On Tue, Dec 19, 2023 at 12:29 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> Should we use the full name (pmd_buffer_scatter) in the subject? I
> lean towards the full name.
>
> On Mon, Dec 18, 2023 at 7:13 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > This test suite provides testing the support of scattered packets by
> > Poll Mode Drivers using testpmd. It incorporates 5 different test cases
> > which test the sending and receiving of packets with lengths that are
> > less than the mbuf data buffer size, the same as the mbuf data buffer
> > size, and the mbuf data buffer size plus 1, 4, and 5. The goal of this
> > test suite is to align with the existing dts test plan for scattered
> > packets within DTS.
> >
>
> Again, we need to describe why we're adding this commit. In the case
> of test suites, the why is obvious, so we should give a good high
> level description of what the tests test (something like the test
> suite tests the x feature by doing y, y being the salient part of the
> tests). The original test plan is actually pretty good, so we can
> extract the rationale from it.
>
This is a good point, I'll pull more from the test plan to improve this.
>
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++++++
> > 1 file changed, 105 insertions(+)
> > create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py
> >
> > diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py
> b/dts/tests/TestSuite_pmd_buffer_scatter.py
> > new file mode 100644
> > index 0000000000..8e2a32a1aa
> > --- /dev/null
> > +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
> > @@ -0,0 +1,105 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 University of New Hampshire
> > +
> > +"""Multi-segment packet scattering testing suite.
> > +
> > +Configure the Rx queues to have mbuf data buffers whose sizes are
> smaller than the maximum packet
> > +size (currently set to 2048 to fit a full 1512-byte ethernet frame) and
> send a total of 5 packets
> > +with lengths less than, equal to, and greater than the mbuf size (CRC
> included).
> > +"""
>
> Let's expand this. I'll point to the original test plan again, let's
> use some of it here. I think it makes sense to make this docstring a
> kind of a test plan with high level description.
>
Sounds good, I'll expand this too.
>
> > +import struct
> > +
> > +from scapy.layers.inet import IP # type: ignore[import]
> > +from scapy.layers.l2 import Ether # type: ignore[import]
> > +from scapy.packet import Raw # type: ignore[import]
> > +from scapy.utils import hexstr # type: ignore[import]
> > +
> > +from framework.remote_session.remote.testpmd_shell import (
> > + TestPmdForwardingModes,
> > + TestPmdShell,
> > +)
> > +from framework.test_suite import TestSuite
> > +
> > +
> > +class PmdBufferScatter(TestSuite):
> > + """DPDK packet scattering test suite.
> > +
>
> And here we could add some more specifics.
>
> I'd like to utilize the original test plans and a split like this
> makes sense at a first glance.
>
I'll add more here in the next version as well. I agree that using the
original test plans will help make this more descriptive and better for the
documentation.
> > + testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> > + testpmd.start()
> > + link_is_up = testpmd.wait_link_status_up(0) and
> testpmd.wait_link_status_up(1)
>
> These two calls should probably be inside testpmd.start(). Looks like
> we're always going to be doing this.
>
> Also, this assumes there will be two ports. Ideally, the shell itself
> will know which ports to check (that should be in EAL parameters), but
> that may require a bigger refactor (unless we just parse back the -a
> options, which could be permissible).
>
Collecting the number of ports should be easy enough from the original args
and then I can just verify ports 0-num are up so I agree that this is a
simple way to change this that adds a good amount of value.
While I don't believe the link status is actually directly related to
starting testpmd, I think that if we are going to start forwarding we still
are also always going to want to be sure that the links are up and we can
properly forward the packets. I'll add this to the verification check in
the start method.
>
> > + self.verify(link_is_up, "Links never came up in TestPMD.")
> > +
> > + for offset in [-1, 0, 1, 4, 5]:
> > + recv_payload = self.scatter_pktgen_send_packet(self.mbsize
> + offset)
> > + self.verify(
> > + ("58 " * 8).strip() in recv_payload,
> > + "Received packet had incorrect payload",
> > + )
>
> This is still just one test case. We should probably leave it as is
> (i.e. not split into 5 test cases), but we should add the information
> about the offset/test case into the message so that we know what
> actually failed right away.
>
Good catch, never hurts to be more descriptive.
>
> > + testpmd.stop()
> > +
> > + def test_scatter_mbuf_2048(self) -> None:
> > + """Run :func:`~PmdBufferScatter.pmd_scatter` function after
> setting the `mbsize` to 2048."""
> > + self.mbsize = 2048
> > + self.pmd_scatter()
>
> Let's just pass 2048 to the method, I don't see a reason for the
> instance variable.
>
Sure, that makes sense to me as well, I'll change this in the next version.
>
> > +
> > + def tear_down_suite(self) -> None:
> > + self.tg_node.main_session.configure_port_mtu(1500,
> self._tg_port_egress)
> > + self.tg_node.main_session.configure_port_mtu(1500,
> self._tg_port_ingress)
> > --
> > 2.43.0
> >
>
Thank you for the feedback!
On Thu, Dec 21, 2023 at 10:47 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
>
>
> On Tue, Dec 19, 2023 at 12:29 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> Should we use the full name (pmd_buffer_scatter) in the subject? I
>> lean towards the full name.
>>
>> On Mon, Dec 18, 2023 at 7:13 PM <jspewock@iol.unh.edu> wrote:
>> >
>> > From: Jeremy Spewock <jspewock@iol.unh.edu>
>> >
>> > This test suite provides testing the support of scattered packets by
>> > Poll Mode Drivers using testpmd. It incorporates 5 different test cases
>> > which test the sending and receiving of packets with lengths that are
>> > less than the mbuf data buffer size, the same as the mbuf data buffer
>> > size, and the mbuf data buffer size plus 1, 4, and 5. The goal of this
>> > test suite is to align with the existing dts test plan for scattered
>> > packets within DTS.
>> >
>>
>> Again, we need to describe why we're adding this commit. In the case
>> of test suites, the why is obvious, so we should give a good high
>> level description of what the tests test (something like the test
>> suite tests the x feature by doing y, y being the salient part of the
>> tests). The original test plan is actually pretty good, so we can
>> extract the rationale from it.
>
>
> This is a good point, I'll pull more from the test plan to improve this.
>
>>
>>
>> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
>> > ---
>> > dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++++++
>> > 1 file changed, 105 insertions(+)
>> > create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py
>> >
>> > diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py
>> > new file mode 100644
>> > index 0000000000..8e2a32a1aa
>> > --- /dev/null
>> > +++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
>> > @@ -0,0 +1,105 @@
>> > +# SPDX-License-Identifier: BSD-3-Clause
>> > +# Copyright(c) 2023 University of New Hampshire
>> > +
>> > +"""Multi-segment packet scattering testing suite.
>> > +
>> > +Configure the Rx queues to have mbuf data buffers whose sizes are smaller than the maximum packet
>> > +size (currently set to 2048 to fit a full 1512-byte ethernet frame) and send a total of 5 packets
>> > +with lengths less than, equal to, and greater than the mbuf size (CRC included).
>> > +"""
>>
>> Let's expand this. I'll point to the original test plan again, let's
>> use some of it here. I think it makes sense to make this docstring a
>> kind of a test plan with high level description.
>
>
> Sounds good, I'll expand this too.
>
>>
>>
>> > +import struct
>> > +
>> > +from scapy.layers.inet import IP # type: ignore[import]
>> > +from scapy.layers.l2 import Ether # type: ignore[import]
>> > +from scapy.packet import Raw # type: ignore[import]
>> > +from scapy.utils import hexstr # type: ignore[import]
>> > +
>> > +from framework.remote_session.remote.testpmd_shell import (
>> > + TestPmdForwardingModes,
>> > + TestPmdShell,
>> > +)
>> > +from framework.test_suite import TestSuite
>> > +
>> > +
>> > +class PmdBufferScatter(TestSuite):
>> > + """DPDK packet scattering test suite.
>> > +
>>
>> And here we could add some more specifics.
>>
>>
>> I'd like to utilize the original test plans and a split like this
>> makes sense at a first glance.
>
>
> I'll add more here in the next version as well. I agree that using the original test plans will help make this more descriptive and better for the documentation.
>
>>
>> > + testpmd.set_forward_mode(TestPmdForwardingModes.mac)
>> > + testpmd.start()
>> > + link_is_up = testpmd.wait_link_status_up(0) and testpmd.wait_link_status_up(1)
>>
>> These two calls should probably be inside testpmd.start(). Looks like
>> we're always going to be doing this.
>>
>> Also, this assumes there will be two ports. Ideally, the shell itself
>> will know which ports to check (that should be in EAL parameters), but
>> that may require a bigger refactor (unless we just parse back the -a
>> options, which could be permissible).
>
>
> Collecting the number of ports should be easy enough from the original args and then I can just verify ports 0-num are up so I agree that this is a simple way to change this that adds a good amount of value.
>
> While I don't believe the link status is actually directly related to starting testpmd, I think that if we are going to start forwarding we still are also always going to want to be sure that the links are up and we can properly forward the packets. I'll add this to the verification check in the start method.
>
Right, we don't necessarily need to verify port status when starting
testpmd (that could be optional though, we could use that in a smoke
test). We should always check it when enabling forwarding (and if we
ever need to not do that we can add an option for that later).
>>
>>
>> > + self.verify(link_is_up, "Links never came up in TestPMD.")
>> > +
>> > + for offset in [-1, 0, 1, 4, 5]:
>> > + recv_payload = self.scatter_pktgen_send_packet(self.mbsize + offset)
>> > + self.verify(
>> > + ("58 " * 8).strip() in recv_payload,
>> > + "Received packet had incorrect payload",
>> > + )
>>
>> This is still just one test case. We should probably leave it as is
>> (i.e. not split into 5 test cases), but we should add the information
>> about the offset/test case into the message so that we know what
>> actually failed right away.
>
>
> Good catch, never hurts to be more descriptive.
>
>>
>>
>> > + testpmd.stop()
>> > +
>> > + def test_scatter_mbuf_2048(self) -> None:
>> > + """Run :func:`~PmdBufferScatter.pmd_scatter` function after setting the `mbsize` to 2048."""
>> > + self.mbsize = 2048
>> > + self.pmd_scatter()
>>
>> Let's just pass 2048 to the method, I don't see a reason for the
>> instance variable.
>
>
> Sure, that makes sense to me as well, I'll change this in the next version.
>
>>
>>
>> > +
>> > + def tear_down_suite(self) -> None:
>> > + self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_egress)
>> > + self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_ingress)
>> > --
>> > 2.43.0
>> >
>
>
> Thank you for the feedback!
new file mode 100644
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023 University of New Hampshire
+
+"""Multi-segment packet scattering testing suite.
+
+Configure the Rx queues to have mbuf data buffers whose sizes are smaller than the maximum packet
+size (currently set to 2048 to fit a full 1512-byte ethernet frame) and send a total of 5 packets
+with lengths less than, equal to, and greater than the mbuf size (CRC included).
+"""
+import struct
+
+from scapy.layers.inet import IP # type: ignore[import]
+from scapy.layers.l2 import Ether # type: ignore[import]
+from scapy.packet import Raw # type: ignore[import]
+from scapy.utils import hexstr # type: ignore[import]
+
+from framework.remote_session.remote.testpmd_shell import (
+ TestPmdForwardingModes,
+ TestPmdShell,
+)
+from framework.test_suite import TestSuite
+
+
+class PmdBufferScatter(TestSuite):
+ """DPDK packet scattering test suite.
+
+ Attributes:
+ mbsize: The size to se the mbuf to be.
+ """
+
+ mbsize: int
+
+ def set_up_suite(self) -> None:
+ self.verify(
+ len(self._port_links) > 1,
+ "Must have at least two port links to run scatter",
+ )
+
+ self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_egress)
+ self.tg_node.main_session.configure_port_mtu(9000, self._tg_port_ingress)
+
+ def scatter_pktgen_send_packet(self, pktsize: int) -> str:
+ """Generate and send packet to the SUT.
+
+ Functional test for scatter packets.
+
+ Args:
+ pktsize: Size of the packet to generate and send.
+ """
+ packet = Ether() / IP() / Raw()
+ packet.getlayer(2).load = ""
+ payload_len = pktsize - len(packet) - 4
+ payload = ["58"] * payload_len
+ # pack the payload
+ for X_in_hex in payload:
+ packet.load += struct.pack("=B", int("%s%s" % (X_in_hex[0], X_in_hex[1]), 16))
+ load = hexstr(packet.getlayer(2), onlyhex=1)
+ received_packets = self.send_packet_and_capture(packet)
+ self.verify(len(received_packets) > 0, "Did not receive any packets.")
+ load = hexstr(received_packets[0].getlayer(2), onlyhex=1)
+
+ return load
+
+ def pmd_scatter(self) -> None:
+ """Testpmd support of receiving and sending scattered multi-segment packets.
+
+ Support for scattered packets is shown by sending 5 packets of differing length
+ where the length of the packet is calculated by taking mbuf-size + an offset. The
+ offsets used in the test case are -1, 0, 1, 4, 5 respectively.
+
+ Test:
+ Start testpmd and run functional test with preset mbsize.
+ """
+ testpmd = self.sut_node.create_interactive_shell(
+ TestPmdShell,
+ app_parameters=(
+ "--mbcache=200 "
+ f"--mbuf-size={self.mbsize} "
+ "--max-pkt-len=9000 "
+ "--port-topology=paired "
+ "--tx-offloads=0x00008000"
+ ),
+ privileged=True,
+ )
+ testpmd.set_forward_mode(TestPmdForwardingModes.mac)
+ testpmd.start()
+ link_is_up = testpmd.wait_link_status_up(0) and testpmd.wait_link_status_up(1)
+ self.verify(link_is_up, "Links never came up in TestPMD.")
+
+ for offset in [-1, 0, 1, 4, 5]:
+ recv_payload = self.scatter_pktgen_send_packet(self.mbsize + offset)
+ self.verify(
+ ("58 " * 8).strip() in recv_payload,
+ "Received packet had incorrect payload",
+ )
+ testpmd.stop()
+
+ def test_scatter_mbuf_2048(self) -> None:
+ """Run :func:`~PmdBufferScatter.pmd_scatter` function after setting the `mbsize` to 2048."""
+ self.mbsize = 2048
+ self.pmd_scatter()
+
+ def tear_down_suite(self) -> None:
+ self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_egress)
+ self.tg_node.main_session.configure_port_mtu(1500, self._tg_port_ingress)