* [PATCH v3 1/7] dts: Add scatter test suite
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-15 7:04 ` Patrick Robb
2023-11-16 19:20 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 2/7] dts: add waiting for port up in testpmd jspewock
` (22 subsequent siblings)
23 siblings, 2 replies; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
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_scatter.py | 86 ++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 dts/tests/TestSuite_scatter.py
diff --git a/dts/tests/TestSuite_scatter.py b/dts/tests/TestSuite_scatter.py
new file mode 100644
index 0000000000..217f465e92
--- /dev/null
+++ b/dts/tests/TestSuite_scatter.py
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023 University of New Hampshire
+
+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 import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class Scatter(TestSuite):
+ 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",
+ )
+ if self._sut_port_egress.os_driver in ["i40e", "ixgbe", "ice"]:
+ self.mbsize = 2048
+ else:
+ self.mbsize = 1024
+
+ 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 test_scatter_mbuf_2048(self) -> None:
+ """
+ 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.send_command("set fwd mac")
+ testpmd.send_command("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.send_command("stop")
+
+ 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.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 1/7] dts: Add scatter test suite
2023-11-13 20:28 ` [PATCH v3 1/7] dts: Add scatter test suite jspewock
@ 2023-11-15 7:04 ` Patrick Robb
2023-11-16 19:20 ` Juraj Linkeš
1 sibling, 0 replies; 43+ messages in thread
From: Patrick Robb @ 2023-11-15 7:04 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 5393 bytes --]
On Mon, Nov 13, 2023 at 3:28 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.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/tests/TestSuite_scatter.py | 86 ++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 dts/tests/TestSuite_scatter.py
>
> diff --git a/dts/tests/TestSuite_scatter.py
> b/dts/tests/TestSuite_scatter.py
> new file mode 100644
> index 0000000000..217f465e92
> --- /dev/null
> +++ b/dts/tests/TestSuite_scatter.py
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +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 import TestPmdShell
> +from framework.test_suite import TestSuite
> +
> +
> +class Scatter(TestSuite):
> + 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",
> + )
> + if self._sut_port_egress.os_driver in ["i40e", "ixgbe", "ice"]:
+ self.mbsize = 2048
> + else:
> + self.mbsize = 1024
>
In old DTS there existed a registry of devices, and the scatter suite
defined mbsize based on the specific NIC in use. I agree that using the
os_driver accomplishes the same thing, and is a simpler implementation, but
am just noting this change.
> +
> + 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 test_scatter_mbuf_2048(self) -> None:
> + """
> + 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,
> + )
>
Just noting that when starting testpmd, --port-topology=loop from the
original scatter suite is changed to --port-topology=paired in order to
align with the recommended test topology for the new DTS framework, and
thus is appropriate. Of course, it shouldn't affect the salient aspect of
this test (checking scattered rx support).
> + testpmd.send_command("set fwd mac")
> + testpmd.send_command("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.send_command("stop")
> +
> + 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.42.0
>
> In my opinion it looks like a good port of the scatter suite[1], with some
inconsequential modifications made according to the design of the new DTS
framework.
Acked-by: Patrick Robb <probb@iol.unh.edu>
[1] https://git.dpdk.org/tools/dts/tree/tests/TestSuite_scatter.py
[-- Attachment #2: Type: text/html, Size: 7273 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 1/7] dts: Add scatter test suite
2023-11-13 20:28 ` [PATCH v3 1/7] dts: Add scatter test suite jspewock
2023-11-15 7:04 ` Patrick Robb
@ 2023-11-16 19:20 ` Juraj Linkeš
2023-11-21 19:26 ` Jeremy Spewock
1 sibling, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 19:20 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Mon, Nov 13, 2023 at 9:28 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.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/tests/TestSuite_scatter.py | 86 ++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 dts/tests/TestSuite_scatter.py
>
> diff --git a/dts/tests/TestSuite_scatter.py b/dts/tests/TestSuite_scatter.py
> new file mode 100644
> index 0000000000..217f465e92
> --- /dev/null
> +++ b/dts/tests/TestSuite_scatter.py
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +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 import TestPmdShell
> +from framework.test_suite import TestSuite
> +
> +
> +class Scatter(TestSuite):
> + 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",
> + )
> + if self._sut_port_egress.os_driver in ["i40e", "ixgbe", "ice"]:
> + self.mbsize = 2048
> + else:
> + self.mbsize = 1024
> +
How exactly should mbsize determined? Is os_driver the sole
determinant? Or is the mbsize also somehow specific to the suite?
If it's just about the driver, then let's move the mbsize calculation
to Port and use that.
> + 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
I guess this is where the discussion about packet verification
originates. Once we conclude that discussion, let's revisit this.
> +
> + def test_scatter_mbuf_2048(self) -> None:
This has 2048 in name, but the mbsize doesn't have to be 2048. Do we
have to have mbuf_2048 in the name?
> + """
> + Test:
> + Start testpmd and run functional test with preset mbsize.
> + """
The one-line description is missing.
> + 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.send_command("set fwd mac")
> + testpmd.send_command("start")
> + link_is_up = testpmd.wait_link_status_up(0) and testpmd.wait_link_status_up(1)
Other test suites may use this or something similar to it. I'd move
the logic to the class so that we don't have to call send_command().
I'd also look into making the start/stop cycle a context manager (so
that's usable with the "with" statement), if at all possible.
> + 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.send_command("stop")
> +
> + 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)
We're assuming the original mtu was 1500. That's a fine assumption,
but let's keep it in mind just in case.
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 1/7] dts: Add scatter test suite
2023-11-16 19:20 ` Juraj Linkeš
@ 2023-11-21 19:26 ` Jeremy Spewock
2023-11-22 8:47 ` Juraj Linkeš
0 siblings, 1 reply; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-21 19:26 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 7794 bytes --]
On Thu, Nov 16, 2023 at 2:20 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Mon, Nov 13, 2023 at 9:28 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.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/tests/TestSuite_scatter.py | 86 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 86 insertions(+)
> > create mode 100644 dts/tests/TestSuite_scatter.py
> >
> > diff --git a/dts/tests/TestSuite_scatter.py
> b/dts/tests/TestSuite_scatter.py
> > new file mode 100644
> > index 0000000000..217f465e92
> > --- /dev/null
> > +++ b/dts/tests/TestSuite_scatter.py
> > @@ -0,0 +1,86 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 University of New Hampshire
> > +
> > +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 import TestPmdShell
> > +from framework.test_suite import TestSuite
> > +
> > +
> > +class Scatter(TestSuite):
> > + 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",
> > + )
> > + if self._sut_port_egress.os_driver in ["i40e", "ixgbe", "ice"]:
> > + self.mbsize = 2048
> > + else:
> > + self.mbsize = 1024
> > +
>
> How exactly should mbsize determined? Is os_driver the sole
> determinant? Or is the mbsize also somehow specific to the suite?
>
> If it's just about the driver, then let's move the mbsize calculation
> to Port and use that.
>
In the previous test suite, there is just a list of nic types that get
their mbuf-size set to 2048, and then everything else is just 1024. All of
the NICs in the list of NIC types use either i40e, ixgbe, or ice drivers,
so filtering based on the driver was an attempt to simplify how this
decision is made.
I think this might be something more specific to the test suite though as
it states that the specific reason that it uses a 2048 mbuf-size is to fit
a full 1518-byte packet with the CRC in a mono-segment packet. I would
assume that the default mbuf-size that testpmd provides would be more
fitting in other cases, but 2048 is specifically used here to keep the
mono-segment packet.
>
> > + 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
>
> I guess this is where the discussion about packet verification
> originates. Once we conclude that discussion, let's revisit this.
>
Exactly, we need to read the packet's load here and this could cause issues
if the packet isn't what you expect it to be.
>
> > +
> > + def test_scatter_mbuf_2048(self) -> None:
>
> This has 2048 in name, but the mbsize doesn't have to be 2048. Do we
> have to have mbuf_2048 in the name?
>
I don't think it is required, but the previous test case is named
scatter_mbuf_2048, so it was named this way for consistency. I think it
might make sense to leave it the same so that it is clear that it is the
same test case and rename it in the future if the community would rather. I
doubt anyone is reliant on this name staying the same, but it might help
for clarity sake.
>
> > + """
> > + Test:
> > + Start testpmd and run functional test with preset mbsize.
> > + """
>
> The one-line description is missing.
>
Good catch, I'll add this.
>
> > + 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.send_command("set fwd mac")
> > + testpmd.send_command("start")
> > + link_is_up = testpmd.wait_link_status_up(0) and
> testpmd.wait_link_status_up(1)
>
> Other test suites may use this or something similar to it. I'd move
> the logic to the class so that we don't have to call send_command().
> I'd also look into making the start/stop cycle a context manager (so
> that's usable with the "with" statement), if at all possible.
>
Very good point, I'll at least make it a method in the class but I'll look
int the context manager option as well because i think that would also be
clear as to when exactly you are forwarding.
>
> > + 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.send_command("stop")
> > +
> > + 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)
>
> We're assuming the original mtu was 1500. That's a fine assumption,
> but let's keep it in mind just in case.
>
Something interesting that I was thinking about doing with MTU in the
future is making it one of the capabilities like we were discussing before.
Once we incorporate the idea of filtering based on capability, we could
have something that checks how high the MTU can be set on a NIC and store
that as a capability of the NIC and then one of the filters could be based
on "can the NIC support an MTU of x size?" In recording this maximum MTU
capability we could also potentially record the starting MTU to reset it
back to.
>
> > --
> > 2.42.0
> >
>
[-- Attachment #2: Type: text/html, Size: 11127 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 1/7] dts: Add scatter test suite
2023-11-21 19:26 ` Jeremy Spewock
@ 2023-11-22 8:47 ` Juraj Linkeš
0 siblings, 0 replies; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-22 8:47 UTC (permalink / raw)
To: Jeremy Spewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Tue, Nov 21, 2023 at 8:26 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
>
>
> On Thu, Nov 16, 2023 at 2:20 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> On Mon, Nov 13, 2023 at 9:28 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.
>> >
>> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
>> > ---
>> > dts/tests/TestSuite_scatter.py | 86 ++++++++++++++++++++++++++++++++++
>> > 1 file changed, 86 insertions(+)
>> > create mode 100644 dts/tests/TestSuite_scatter.py
>> >
Now that I think about it, we should rethink all test suite names we
bring from the original DTS. If I understand it correctly, this is
about packet(s) being scattered (or not scattered) around DPDK memory
buffers. I think the test suite name should capture that the
scattering is about the *packet(s)* being scattered on the *DPDK*
level. Maybe something like TestSuite_pmd_buffer_scatter.py.
>> > diff --git a/dts/tests/TestSuite_scatter.py b/dts/tests/TestSuite_scatter.py
>> > new file mode 100644
>> > index 0000000000..217f465e92
>> > --- /dev/null
>> > +++ b/dts/tests/TestSuite_scatter.py
>> > @@ -0,0 +1,86 @@
>> > +# SPDX-License-Identifier: BSD-3-Clause
>> > +# Copyright(c) 2023 University of New Hampshire
>> > +
>> > +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 import TestPmdShell
>> > +from framework.test_suite import TestSuite
>> > +
>> > +
>> > +class Scatter(TestSuite):
>> > + 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",
>> > + )
>> > + if self._sut_port_egress.os_driver in ["i40e", "ixgbe", "ice"]:
>> > + self.mbsize = 2048
>> > + else:
>> > + self.mbsize = 1024
>> > +
>>
>> How exactly should mbsize determined? Is os_driver the sole
>> determinant? Or is the mbsize also somehow specific to the suite?
>>
>> If it's just about the driver, then let's move the mbsize calculation
>> to Port and use that.
>
>
> In the previous test suite, there is just a list of nic types that get their mbuf-size set to 2048, and then everything else is just 1024. All of the NICs in the list of NIC types use either i40e, ixgbe, or ice drivers, so filtering based on the driver was an attempt to simplify how this decision is made.
>
We should think in terms of how it should be done, not how it has been
done in the original DTS. In many cases, the two are going to be the
same, but we should always make sure that we have an understanding of
why we're doing what we're doing. In this case, I don't understand why
the non-Intel NICs are using a smaller buffer.
> I think this might be something more specific to the test suite though as it states that the specific reason that it uses a 2048 mbuf-size is to fit a full 1518-byte packet with the CRC in a mono-segment packet. I would assume that the default mbuf-size that testpmd provides would be more fitting in other cases, but 2048 is specifically used here to keep the mono-segment packet.
>
With non-Intel NICs, we would have the packet in two buffers, thus
making the test suite different. Maybe we should just set the buffer
size to 2048 if we don't know the reason for the difference. I
certainly don't see one, this doesn't seem like anything Intel
specific.
>>
>>
>> > + 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
>>
>> I guess this is where the discussion about packet verification
>> originates. Once we conclude that discussion, let's revisit this.
>
>
> Exactly, we need to read the packet's load here and this could cause issues if the packet isn't what you expect it to be.
>
>>
>>
>> > +
>> > + def test_scatter_mbuf_2048(self) -> None:
>>
>> This has 2048 in name, but the mbsize doesn't have to be 2048. Do we
>> have to have mbuf_2048 in the name?
>
>
> I don't think it is required, but the previous test case is named scatter_mbuf_2048, so it was named this way for consistency. I think it might make sense to leave it the same so that it is clear that it is the same test case and rename it in the future if the community would rather. I doubt anyone is reliant on this name staying the same, but it might help for clarity sake.
>
If we're always going to test with 2048, then the name is fine, as
that is the salient point of the test case. But I'd move the setting
from test suite setup to the test case, as this looks like it's
specific to the test case - we could have other test cases with
different mbuf sizes (and possibly packet sizes/mtus). That depends on
how exactly we're dealing with the mbuf size (per the previous
comments), though.
>>
>>
>> > + """
>> > + Test:
>> > + Start testpmd and run functional test with preset mbsize.
>> > + """
>>
>> The one-line description is missing.
>
>
> Good catch, I'll add this.
>
>>
>>
>> > + 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.send_command("set fwd mac")
>> > + testpmd.send_command("start")
>> > + link_is_up = testpmd.wait_link_status_up(0) and testpmd.wait_link_status_up(1)
>>
>> Other test suites may use this or something similar to it. I'd move
>> the logic to the class so that we don't have to call send_command().
>> I'd also look into making the start/stop cycle a context manager (so
>> that's usable with the "with" statement), if at all possible.
>
>
> Very good point, I'll at least make it a method in the class but I'll look int the context manager option as well because i think that would also be clear as to when exactly you are forwarding.
>
>>
>>
>> > + 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.send_command("stop")
>> > +
>> > + 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)
>>
>> We're assuming the original mtu was 1500. That's a fine assumption,
>> but let's keep it in mind just in case.
>
>
> Something interesting that I was thinking about doing with MTU in the future is making it one of the capabilities like we were discussing before. Once we incorporate the idea of filtering based on capability, we could have something that checks how high the MTU can be set on a NIC and store that as a capability of the NIC and then one of the filters could be based on "can the NIC support an MTU of x size?" In recording this maximum MTU capability we could also potentially record the starting MTU to reset it back to.
>
These are good ideas, I think we should prioritize exploring the
capabilities discovery/filtering now that we're starting to see some
use cases for it.
>>
>>
>> > --
>> > 2.42.0
>> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 2/7] dts: add waiting for port up in testpmd
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
2023-11-13 20:28 ` [PATCH v3 1/7] dts: Add scatter test suite jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-16 19:05 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 3/7] dts: add scatter to the yaml schema jspewock
` (21 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added a method within the testpmd interactive shell that polls the
status of ports and verifies that the link status on a given port is
"up." Polling will continue until either the link comes up, or the
timeout is reached.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
.../remote_session/remote/testpmd_shell.py | 29 +++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
index 1455b5a199..3ea16c7ab3 100644
--- a/dts/framework/remote_session/remote/testpmd_shell.py
+++ b/dts/framework/remote_session/remote/testpmd_shell.py
@@ -1,9 +1,12 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2023 University of New Hampshire
+import time
from pathlib import PurePath
from typing import Callable
+from framework.settings import SETTINGS
+
from .interactive_shell import InteractiveShell
@@ -47,3 +50,29 @@ def get_devices(self) -> list[TestPmdDevice]:
if "device name:" in line.lower():
dev_list.append(TestPmdDevice(line))
return dev_list
+
+ def wait_link_status_up(self, port_id: int, timeout=SETTINGS.timeout) -> bool:
+ """Wait until the link status on the given port is "up".
+
+ Arguments:
+ port_id: Port to check the link status on.
+ timeout: time to wait for the link to come up.
+
+ Returns:
+ If the link came up in time or not.
+ """
+ time_to_stop = time.time() + timeout
+ while time.time() < time_to_stop:
+ port_info = self.send_command(f"show port info {port_id}")
+ if "Link status: up" in port_info:
+ break
+ time.sleep(0.5)
+ else:
+ self._logger.error(
+ f"The link for port {port_id} did not come up in the given timeout."
+ )
+ return "Link status: up" in port_info
+
+ def close(self) -> None:
+ self.send_command("exit", "")
+ return super().close()
--
2.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 2/7] dts: add waiting for port up in testpmd
2023-11-13 20:28 ` [PATCH v3 2/7] dts: add waiting for port up in testpmd jspewock
@ 2023-11-16 19:05 ` Juraj Linkeš
2023-11-17 18:09 ` Jeremy Spewock
0 siblings, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 19:05 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Added a method within the testpmd interactive shell that polls the
> status of ports and verifies that the link status on a given port is
> "up." Polling will continue until either the link comes up, or the
> timeout is reached.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> .../remote_session/remote/testpmd_shell.py | 29 +++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
> index 1455b5a199..3ea16c7ab3 100644
> --- a/dts/framework/remote_session/remote/testpmd_shell.py
> +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> @@ -1,9 +1,12 @@
> # SPDX-License-Identifier: BSD-3-Clause
> # Copyright(c) 2023 University of New Hampshire
>
> +import time
> from pathlib import PurePath
> from typing import Callable
>
> +from framework.settings import SETTINGS
> +
> from .interactive_shell import InteractiveShell
>
>
> @@ -47,3 +50,29 @@ def get_devices(self) -> list[TestPmdDevice]:
> if "device name:" in line.lower():
> dev_list.append(TestPmdDevice(line))
> return dev_list
> +
> + def wait_link_status_up(self, port_id: int, timeout=SETTINGS.timeout) -> bool:
> + """Wait until the link status on the given port is "up".
> +
> + Arguments:
> + port_id: Port to check the link status on.
> + timeout: time to wait for the link to come up.
> +
> + Returns:
> + If the link came up in time or not.
> + """
Again with the docstrings - the new thing here is the usage of
SETTINGS. Here's an example:
The YAML test run configuration file is specified in the
:option:`--config-file` command line
argument or the :envvar:`DTS_CFG_FILE` environment variable.
The rule is to first mention the cmdline arg, then the env var.
> + time_to_stop = time.time() + timeout
> + while time.time() < time_to_stop:
> + port_info = self.send_command(f"show port info {port_id}")
> + if "Link status: up" in port_info:
> + break
> + time.sleep(0.5)
> + else:
> + self._logger.error(
> + f"The link for port {port_id} did not come up in the given timeout."
> + )
> + return "Link status: up" in port_info
> +
> + def close(self) -> None:
> + self.send_command("exit", "")
> + return super().close()
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 2/7] dts: add waiting for port up in testpmd
2023-11-16 19:05 ` Juraj Linkeš
@ 2023-11-17 18:09 ` Jeremy Spewock
0 siblings, 0 replies; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-17 18:09 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 2971 bytes --]
On Thu, Nov 16, 2023 at 2:05 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Added a method within the testpmd interactive shell that polls the
> > status of ports and verifies that the link status on a given port is
> > "up." Polling will continue until either the link comes up, or the
> > timeout is reached.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > .../remote_session/remote/testpmd_shell.py | 29 +++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/dts/framework/remote_session/remote/testpmd_shell.py
> b/dts/framework/remote_session/remote/testpmd_shell.py
> > index 1455b5a199..3ea16c7ab3 100644
> > --- a/dts/framework/remote_session/remote/testpmd_shell.py
> > +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> > @@ -1,9 +1,12 @@
> > # SPDX-License-Identifier: BSD-3-Clause
> > # Copyright(c) 2023 University of New Hampshire
> >
> > +import time
> > from pathlib import PurePath
> > from typing import Callable
> >
> > +from framework.settings import SETTINGS
> > +
> > from .interactive_shell import InteractiveShell
> >
> >
> > @@ -47,3 +50,29 @@ def get_devices(self) -> list[TestPmdDevice]:
> > if "device name:" in line.lower():
> > dev_list.append(TestPmdDevice(line))
> > return dev_list
> > +
> > + def wait_link_status_up(self, port_id: int,
> timeout=SETTINGS.timeout) -> bool:
> > + """Wait until the link status on the given port is "up".
> > +
> > + Arguments:
> > + port_id: Port to check the link status on.
> > + timeout: time to wait for the link to come up.
> > +
> > + Returns:
> > + If the link came up in time or not.
> > + """
>
> Again with the docstrings - the new thing here is the usage of
> SETTINGS. Here's an example:
>
> The YAML test run configuration file is specified in the
> :option:`--config-file` command line
> argument or the :envvar:`DTS_CFG_FILE` environment variable.
>
> The rule is to first mention the cmdline arg, then the env var.
>
>
Good catch, I'll change this.
> > + time_to_stop = time.time() + timeout
> > + while time.time() < time_to_stop:
> > + port_info = self.send_command(f"show port info {port_id}")
> > + if "Link status: up" in port_info:
> > + break
> > + time.sleep(0.5)
> > + else:
> > + self._logger.error(
> > + f"The link for port {port_id} did not come up in the
> given timeout."
> > + )
> > + return "Link status: up" in port_info
> > +
> > + def close(self) -> None:
> > + self.send_command("exit", "")
> > + return super().close()
> > --
> > 2.42.0
> >
>
[-- Attachment #2: Type: text/html, Size: 4340 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 3/7] dts: add scatter to the yaml schema
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
2023-11-13 20:28 ` [PATCH v3 1/7] dts: Add scatter test suite jspewock
2023-11-13 20:28 ` [PATCH v3 2/7] dts: add waiting for port up in testpmd jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-13 20:28 ` [PATCH v3 4/7] dts: allow passing parameters into interactive apps jspewock
` (20 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Allow for scatter to be specified in the configuration file.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/config/conf_yaml_schema.json | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 84e45fe3c2..97ee32f47a 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -186,7 +186,8 @@
"type": "string",
"enum": [
"hello_world",
- "os_udp"
+ "os_udp",
+ "scatter"
]
},
"test_target": {
--
2.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v3 4/7] dts: allow passing parameters into interactive apps
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (2 preceding siblings ...)
2023-11-13 20:28 ` [PATCH v3 3/7] dts: add scatter to the yaml schema jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-16 18:52 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer jspewock
` (19 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Modified interactive applications to allow for the ability to pass
parameters into the app on start up. Also modified the way EAL
parameters are handled so that the trailing "--" separator is added be
default after all EAL parameters.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/remote_session/remote/testpmd_shell.py | 2 +-
dts/framework/testbed_model/sut_node.py | 11 +++++++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
index 3ea16c7ab3..3f6a86aa35 100644
--- a/dts/framework/remote_session/remote/testpmd_shell.py
+++ b/dts/framework/remote_session/remote/testpmd_shell.py
@@ -32,7 +32,7 @@ def _start_application(
self, get_privileged_command: Callable[[str], str] | None
) -> None:
"""See "_start_application" in InteractiveShell."""
- self._app_args += " -- -i"
+ self._app_args += " -i"
super()._start_application(get_privileged_command)
def get_devices(self) -> list[TestPmdDevice]:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 4161d3a4d5..bcac939e72 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -377,7 +377,8 @@ def create_interactive_shell(
shell_cls: Type[InteractiveShellType],
timeout: float = SETTINGS.timeout,
privileged: bool = False,
- eal_parameters: EalParameters | str | None = None,
+ eal_parameters: EalParameters | str = "",
+ app_parameters: str = "",
) -> InteractiveShellType:
"""Factory method for creating a handler for an interactive session.
@@ -392,12 +393,14 @@ def create_interactive_shell(
eal_parameters: List of EAL parameters to use to launch the app. If this
isn't provided or an empty string is passed, it will default to calling
create_eal_parameters().
+ app_parameters: Additional arguments to pass into the application on the
+ command-line.
Returns:
Instance of the desired interactive application.
"""
- if not eal_parameters:
+ if not eal_parameters and shell_cls.dpdk_app:
eal_parameters = self.create_eal_parameters()
-
+ eal_parameters = f"{eal_parameters} --"
# We need to append the build directory for DPDK apps
if shell_cls.dpdk_app:
shell_cls.path = self.main_session.join_remote_path(
@@ -405,7 +408,7 @@ def create_interactive_shell(
)
return super().create_interactive_shell(
- shell_cls, timeout, privileged, str(eal_parameters)
+ shell_cls, timeout, privileged, f"{eal_parameters} {app_parameters}"
)
def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
--
2.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps
2023-11-13 20:28 ` [PATCH v3 4/7] dts: allow passing parameters into interactive apps jspewock
@ 2023-11-16 18:52 ` Juraj Linkeš
2023-11-17 18:08 ` Jeremy Spewock
0 siblings, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 18:52 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Modified interactive applications to allow for the ability to pass
> parameters into the app on start up. Also modified the way EAL
> parameters are handled so that the trailing "--" separator is added be
> default after all EAL parameters.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/remote_session/remote/testpmd_shell.py | 2 +-
> dts/framework/testbed_model/sut_node.py | 11 +++++++----
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
> index 3ea16c7ab3..3f6a86aa35 100644
> --- a/dts/framework/remote_session/remote/testpmd_shell.py
> +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> @@ -32,7 +32,7 @@ def _start_application(
> self, get_privileged_command: Callable[[str], str] | None
> ) -> None:
> """See "_start_application" in InteractiveShell."""
> - self._app_args += " -- -i"
> + self._app_args += " -i"
> super()._start_application(get_privileged_command)
>
> def get_devices(self) -> list[TestPmdDevice]:
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index 4161d3a4d5..bcac939e72 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -377,7 +377,8 @@ def create_interactive_shell(
> shell_cls: Type[InteractiveShellType],
> timeout: float = SETTINGS.timeout,
> privileged: bool = False,
> - eal_parameters: EalParameters | str | None = None,
> + eal_parameters: EalParameters | str = "",
I think it makes more sense if the type is EalParameters | None. With
this change, the str type of eal_parameters moved to app_parameters.
> + app_parameters: str = "",
> ) -> InteractiveShellType:
> """Factory method for creating a handler for an interactive session.
>
> @@ -392,12 +393,14 @@ def create_interactive_shell(
> eal_parameters: List of EAL parameters to use to launch the app. If this
> isn't provided or an empty string is passed, it will default to calling
> create_eal_parameters().
> + app_parameters: Additional arguments to pass into the application on the
> + command-line.
> Returns:
> Instance of the desired interactive application.
> """
> - if not eal_parameters:
> + if not eal_parameters and shell_cls.dpdk_app:
> eal_parameters = self.create_eal_parameters()
> -
> + eal_parameters = f"{eal_parameters} --"
I think this change is more complicated than it seems at first glance.
Before we either passed EalParameters (meant for DPDK apps) or a
string (meant for other apps). There was no additional check for these
assumptions.
Now that we've split it, I see some cases which seem to be not covered.
1. The app is a DPDK app and we pass EalParameters. The two hyphens
are not added.
2. The app is not a DPDK app and we pass EalParameters. Similarly to
current behavior (without the patch), we kinda assume that the caller
won't do this, but now that we're modifying the behavior, let's add a
check that won't allow EalParameters with non-DPDK apps.
> # We need to append the build directory for DPDK apps
> if shell_cls.dpdk_app:
> shell_cls.path = self.main_session.join_remote_path(
> @@ -405,7 +408,7 @@ def create_interactive_shell(
> )
>
> return super().create_interactive_shell(
> - shell_cls, timeout, privileged, str(eal_parameters)
> + shell_cls, timeout, privileged, f"{eal_parameters} {app_parameters}"
> )
>
> def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps
2023-11-16 18:52 ` Juraj Linkeš
@ 2023-11-17 18:08 ` Jeremy Spewock
2023-11-20 14:35 ` Juraj Linkeš
0 siblings, 1 reply; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-17 18:08 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 4618 bytes --]
On Thu, Nov 16, 2023 at 1:53 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Modified interactive applications to allow for the ability to pass
> > parameters into the app on start up. Also modified the way EAL
> > parameters are handled so that the trailing "--" separator is added be
> > default after all EAL parameters.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/framework/remote_session/remote/testpmd_shell.py | 2 +-
> > dts/framework/testbed_model/sut_node.py | 11 +++++++----
> > 2 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/dts/framework/remote_session/remote/testpmd_shell.py
> b/dts/framework/remote_session/remote/testpmd_shell.py
> > index 3ea16c7ab3..3f6a86aa35 100644
> > --- a/dts/framework/remote_session/remote/testpmd_shell.py
> > +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> > @@ -32,7 +32,7 @@ def _start_application(
> > self, get_privileged_command: Callable[[str], str] | None
> > ) -> None:
> > """See "_start_application" in InteractiveShell."""
> > - self._app_args += " -- -i"
> > + self._app_args += " -i"
> > super()._start_application(get_privileged_command)
> >
> > def get_devices(self) -> list[TestPmdDevice]:
> > diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> > index 4161d3a4d5..bcac939e72 100644
> > --- a/dts/framework/testbed_model/sut_node.py
> > +++ b/dts/framework/testbed_model/sut_node.py
> > @@ -377,7 +377,8 @@ def create_interactive_shell(
> > shell_cls: Type[InteractiveShellType],
> > timeout: float = SETTINGS.timeout,
> > privileged: bool = False,
> > - eal_parameters: EalParameters | str | None = None,
> > + eal_parameters: EalParameters | str = "",
>
> I think it makes more sense if the type is EalParameters | None. With
> this change, the str type of eal_parameters moved to app_parameters.
>
> > + app_parameters: str = "",
> > ) -> InteractiveShellType:
> > """Factory method for creating a handler for an interactive
> session.
> >
> > @@ -392,12 +393,14 @@ def create_interactive_shell(
> > eal_parameters: List of EAL parameters to use to launch the
> app. If this
> > isn't provided or an empty string is passed, it will
> default to calling
> > create_eal_parameters().
> > + app_parameters: Additional arguments to pass into the
> application on the
> > + command-line.
> > Returns:
> > Instance of the desired interactive application.
> > """
> > - if not eal_parameters:
> > + if not eal_parameters and shell_cls.dpdk_app:
> > eal_parameters = self.create_eal_parameters()
> > -
> > + eal_parameters = f"{eal_parameters} --"
>
> I think this change is more complicated than it seems at first glance.
>
> Before we either passed EalParameters (meant for DPDK apps) or a
> string (meant for other apps). There was no additional check for these
> assumptions.
> Now that we've split it, I see some cases which seem to be not covered.
>
> 1. The app is a DPDK app and we pass EalParameters. The two hyphens
> are not added.
> 2. The app is not a DPDK app and we pass EalParameters. Similarly to
> current behavior (without the patch), we kinda assume that the caller
> won't do this, but now that we're modifying the behavior, let's add a
> check that won't allow EalParameters with non-DPDK apps.
>
>
That is a good point that not all DPDk apps need the two hyphens, I can
make that just specific to testpmd and change it instead so that we don't
pass EalParameters into DPDK applications and I think that should cover
these cases.
> > # We need to append the build directory for DPDK apps
> > if shell_cls.dpdk_app:
> > shell_cls.path = self.main_session.join_remote_path(
> > @@ -405,7 +408,7 @@ def create_interactive_shell(
> > )
> >
> > return super().create_interactive_shell(
> > - shell_cls, timeout, privileged, str(eal_parameters)
> > + shell_cls, timeout, privileged, f"{eal_parameters}
> {app_parameters}"
> > )
> >
> > def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> > --
> > 2.42.0
> >
>
[-- Attachment #2: Type: text/html, Size: 6132 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps
2023-11-17 18:08 ` Jeremy Spewock
@ 2023-11-20 14:35 ` Juraj Linkeš
2023-11-21 21:55 ` Jeremy Spewock
0 siblings, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-20 14:35 UTC (permalink / raw)
To: Jeremy Spewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Fri, Nov 17, 2023 at 7:08 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
>
>
> On Thu, Nov 16, 2023 at 1:53 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>> >
>> > From: Jeremy Spewock <jspewock@iol.unh.edu>
>> >
>> > Modified interactive applications to allow for the ability to pass
>> > parameters into the app on start up. Also modified the way EAL
>> > parameters are handled so that the trailing "--" separator is added be
>> > default after all EAL parameters.
>> >
>> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
>> > ---
>> > dts/framework/remote_session/remote/testpmd_shell.py | 2 +-
>> > dts/framework/testbed_model/sut_node.py | 11 +++++++----
>> > 2 files changed, 8 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
>> > index 3ea16c7ab3..3f6a86aa35 100644
>> > --- a/dts/framework/remote_session/remote/testpmd_shell.py
>> > +++ b/dts/framework/remote_session/remote/testpmd_shell.py
>> > @@ -32,7 +32,7 @@ def _start_application(
>> > self, get_privileged_command: Callable[[str], str] | None
>> > ) -> None:
>> > """See "_start_application" in InteractiveShell."""
>> > - self._app_args += " -- -i"
>> > + self._app_args += " -i"
>> > super()._start_application(get_privileged_command)
>> >
>> > def get_devices(self) -> list[TestPmdDevice]:
>> > diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
>> > index 4161d3a4d5..bcac939e72 100644
>> > --- a/dts/framework/testbed_model/sut_node.py
>> > +++ b/dts/framework/testbed_model/sut_node.py
>> > @@ -377,7 +377,8 @@ def create_interactive_shell(
>> > shell_cls: Type[InteractiveShellType],
>> > timeout: float = SETTINGS.timeout,
>> > privileged: bool = False,
>> > - eal_parameters: EalParameters | str | None = None,
>> > + eal_parameters: EalParameters | str = "",
>>
>> I think it makes more sense if the type is EalParameters | None. With
>> this change, the str type of eal_parameters moved to app_parameters.
>>
>> > + app_parameters: str = "",
>> > ) -> InteractiveShellType:
>> > """Factory method for creating a handler for an interactive session.
>> >
>> > @@ -392,12 +393,14 @@ def create_interactive_shell(
>> > eal_parameters: List of EAL parameters to use to launch the app. If this
>> > isn't provided or an empty string is passed, it will default to calling
>> > create_eal_parameters().
>> > + app_parameters: Additional arguments to pass into the application on the
>> > + command-line.
>> > Returns:
>> > Instance of the desired interactive application.
>> > """
>> > - if not eal_parameters:
>> > + if not eal_parameters and shell_cls.dpdk_app:
>> > eal_parameters = self.create_eal_parameters()
>> > -
>> > + eal_parameters = f"{eal_parameters} --"
>>
>> I think this change is more complicated than it seems at first glance.
>>
>> Before we either passed EalParameters (meant for DPDK apps) or a
>> string (meant for other apps). There was no additional check for these
>> assumptions.
>> Now that we've split it, I see some cases which seem to be not covered.
>>
>> 1. The app is a DPDK app and we pass EalParameters. The two hyphens
>> are not added.
>> 2. The app is not a DPDK app and we pass EalParameters. Similarly to
>> current behavior (without the patch), we kinda assume that the caller
>> won't do this, but now that we're modifying the behavior, let's add a
>> check that won't allow EalParameters with non-DPDK apps.
>>
>
> That is a good point that not all DPDk apps need the two hyphens, I can make that just specific to testpmd
I don't know whether all DPDK apps need EAL parameters (I thought they
needed them). My point is about the condition: the two hyphens are
only added for DPDK apps when *default* EAL parameters are used. When
non-default EAL parameters are passed (i.e. when eal_parameters ==
True), the hyphens are not added for DPDK apps.
> and change it instead so that we don't pass EalParameters into DPDK applications and I think that should cover these cases.
>
I think you meant non-DPDK applications here, right?
>
>>
>> > # We need to append the build directory for DPDK apps
>> > if shell_cls.dpdk_app:
>> > shell_cls.path = self.main_session.join_remote_path(
>> > @@ -405,7 +408,7 @@ def create_interactive_shell(
>> > )
>> >
>> > return super().create_interactive_shell(
>> > - shell_cls, timeout, privileged, str(eal_parameters)
>> > + shell_cls, timeout, privileged, f"{eal_parameters} {app_parameters}"
>> > )
>> >
>> > def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
>> > --
>> > 2.42.0
>> >
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps
2023-11-20 14:35 ` Juraj Linkeš
@ 2023-11-21 21:55 ` Jeremy Spewock
0 siblings, 0 replies; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-21 21:55 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 5552 bytes --]
On Mon, Nov 20, 2023 at 9:35 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Fri, Nov 17, 2023 at 7:08 PM Jeremy Spewock <jspewock@iol.unh.edu>
> wrote:
> >
> >
> >
> > On Thu, Nov 16, 2023 at 1:53 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
> wrote:
> >>
> >> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >> >
> >> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >> >
> >> > Modified interactive applications to allow for the ability to pass
> >> > parameters into the app on start up. Also modified the way EAL
> >> > parameters are handled so that the trailing "--" separator is added be
> >> > default after all EAL parameters.
> >> >
> >> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> >> > ---
> >> > dts/framework/remote_session/remote/testpmd_shell.py | 2 +-
> >> > dts/framework/testbed_model/sut_node.py | 11 +++++++----
> >> > 2 files changed, 8 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/dts/framework/remote_session/remote/testpmd_shell.py
> b/dts/framework/remote_session/remote/testpmd_shell.py
> >> > index 3ea16c7ab3..3f6a86aa35 100644
> >> > --- a/dts/framework/remote_session/remote/testpmd_shell.py
> >> > +++ b/dts/framework/remote_session/remote/testpmd_shell.py
> >> > @@ -32,7 +32,7 @@ def _start_application(
> >> > self, get_privileged_command: Callable[[str], str] | None
> >> > ) -> None:
> >> > """See "_start_application" in InteractiveShell."""
> >> > - self._app_args += " -- -i"
> >> > + self._app_args += " -i"
> >> > super()._start_application(get_privileged_command)
> >> >
> >> > def get_devices(self) -> list[TestPmdDevice]:
> >> > diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> >> > index 4161d3a4d5..bcac939e72 100644
> >> > --- a/dts/framework/testbed_model/sut_node.py
> >> > +++ b/dts/framework/testbed_model/sut_node.py
> >> > @@ -377,7 +377,8 @@ def create_interactive_shell(
> >> > shell_cls: Type[InteractiveShellType],
> >> > timeout: float = SETTINGS.timeout,
> >> > privileged: bool = False,
> >> > - eal_parameters: EalParameters | str | None = None,
> >> > + eal_parameters: EalParameters | str = "",
> >>
> >> I think it makes more sense if the type is EalParameters | None. With
> >> this change, the str type of eal_parameters moved to app_parameters.
> >>
> >> > + app_parameters: str = "",
> >> > ) -> InteractiveShellType:
> >> > """Factory method for creating a handler for an interactive
> session.
> >> >
> >> > @@ -392,12 +393,14 @@ def create_interactive_shell(
> >> > eal_parameters: List of EAL parameters to use to launch
> the app. If this
> >> > isn't provided or an empty string is passed, it will
> default to calling
> >> > create_eal_parameters().
> >> > + app_parameters: Additional arguments to pass into the
> application on the
> >> > + command-line.
> >> > Returns:
> >> > Instance of the desired interactive application.
> >> > """
> >> > - if not eal_parameters:
> >> > + if not eal_parameters and shell_cls.dpdk_app:
> >> > eal_parameters = self.create_eal_parameters()
> >> > -
> >> > + eal_parameters = f"{eal_parameters} --"
> >>
> >> I think this change is more complicated than it seems at first glance.
> >>
> >> Before we either passed EalParameters (meant for DPDK apps) or a
> >> string (meant for other apps). There was no additional check for these
> >> assumptions.
> >> Now that we've split it, I see some cases which seem to be not covered.
> >>
> >> 1. The app is a DPDK app and we pass EalParameters. The two hyphens
> >> are not added.
> >> 2. The app is not a DPDK app and we pass EalParameters. Similarly to
> >> current behavior (without the patch), we kinda assume that the caller
> >> won't do this, but now that we're modifying the behavior, let's add a
> >> check that won't allow EalParameters with non-DPDK apps.
> >>
> >
> > That is a good point that not all DPDk apps need the two hyphens, I can
> make that just specific to testpmd
>
> I don't know whether all DPDK apps need EAL parameters (I thought they
> needed them). My point is about the condition: the two hyphens are
> only added for DPDK apps when *default* EAL parameters are used. When
> non-default EAL parameters are passed (i.e. when eal_parameters ==
> True), the hyphens are not added for DPDK apps.
>
> > and change it instead so that we don't pass EalParameters into DPDK
> applications and I think that should cover these cases.
> >
>
> I think you meant non-DPDK applications here, right?
>
I did mean non-DPDK, yes.
>
> >
> >>
> >> > # We need to append the build directory for DPDK apps
> >> > if shell_cls.dpdk_app:
> >> > shell_cls.path = self.main_session.join_remote_path(
> >> > @@ -405,7 +408,7 @@ def create_interactive_shell(
> >> > )
> >> >
> >> > return super().create_interactive_shell(
> >> > - shell_cls, timeout, privileged, str(eal_parameters)
> >> > + shell_cls, timeout, privileged, f"{eal_parameters}
> {app_parameters}"
> >> > )
> >> >
> >> > def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> >> > --
> >> > 2.42.0
> >> >
>
[-- Attachment #2: Type: text/html, Size: 7748 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (3 preceding siblings ...)
2023-11-13 20:28 ` [PATCH v3 4/7] dts: allow passing parameters into interactive apps jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-16 18:34 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 6/7] dts: add pci addresses to EAL parameters jspewock
` (18 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added the options to filter out LLDP and ARP packets when
sniffing for packets with scapy. This was done using BPF filters to
ensure that the noise these packets provide does not interfere with test
cases.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/test_suite.py | 13 +++++++++++--
.../testbed_model/capturing_traffic_generator.py | 12 +++++++++++-
dts/framework/testbed_model/scapy.py | 11 ++++++++++-
dts/framework/testbed_model/tg_node.py | 4 +++-
4 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 3b890c0451..3222f172f3 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -152,7 +152,11 @@ def _configure_ipv4_forwarding(self, enable: bool) -> None:
self.sut_node.configure_ipv4_forwarding(enable)
def send_packet_and_capture(
- self, packet: Packet, duration: float = 1
+ self,
+ packet: Packet,
+ duration: float = 1,
+ no_lldp: bool = True,
+ no_arp: bool = True,
) -> list[Packet]:
"""
Send a packet through the appropriate interface and
@@ -162,7 +166,12 @@ def send_packet_and_capture(
"""
packet = self._adjust_addresses(packet)
return self.tg_node.send_packet_and_capture(
- packet, self._tg_port_egress, self._tg_port_ingress, duration
+ packet,
+ self._tg_port_egress,
+ self._tg_port_ingress,
+ duration,
+ no_lldp,
+ no_arp,
)
def get_expected_packet(self, packet: Packet) -> Packet:
diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b/dts/framework/testbed_model/capturing_traffic_generator.py
index ab98987f8e..0a0d0f7d0d 100644
--- a/dts/framework/testbed_model/capturing_traffic_generator.py
+++ b/dts/framework/testbed_model/capturing_traffic_generator.py
@@ -52,6 +52,8 @@ def send_packet_and_capture(
send_port: Port,
receive_port: Port,
duration: float,
+ no_lldp: bool,
+ no_arp: bool,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
"""Send a packet, return received traffic.
@@ -71,7 +73,7 @@ def send_packet_and_capture(
A list of received packets. May be empty if no packets are captured.
"""
return self.send_packets_and_capture(
- [packet], send_port, receive_port, duration, capture_name
+ [packet], send_port, receive_port, duration, no_lldp, no_arp, capture_name
)
def send_packets_and_capture(
@@ -80,6 +82,8 @@ def send_packets_and_capture(
send_port: Port,
receive_port: Port,
duration: float,
+ no_lldp: bool,
+ no_arp: bool,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
"""Send packets, return received traffic.
@@ -93,6 +97,8 @@ def send_packets_and_capture(
send_port: The egress port on the TG node.
receive_port: The ingress port in the TG node.
duration: Capture traffic for this amount of time after sending the packets.
+ no_lldp: Option to disable capturing LLDP packets
+ no_arp: Option to disable capturing ARP packets
capture_name: The name of the .pcap file where to store the capture.
Returns:
@@ -108,6 +114,8 @@ def send_packets_and_capture(
send_port,
receive_port,
duration,
+ no_lldp,
+ no_arp,
)
self._logger.debug(
@@ -123,6 +131,8 @@ def _send_packets_and_capture(
send_port: Port,
receive_port: Port,
duration: float,
+ no_lldp: bool,
+ no_arp: bool,
) -> list[Packet]:
"""
The extended classes must implement this method which
diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed_model/scapy.py
index af0d4dbb25..58f01af21a 100644
--- a/dts/framework/testbed_model/scapy.py
+++ b/dts/framework/testbed_model/scapy.py
@@ -69,6 +69,7 @@ def scapy_send_packets_and_capture(
send_iface: str,
recv_iface: str,
duration: float,
+ sniff_filter: str,
) -> list[bytes]:
"""RPC function to send and capture packets.
@@ -90,6 +91,7 @@ def scapy_send_packets_and_capture(
iface=recv_iface,
store=True,
started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
+ filter=sniff_filter,
)
sniffer.start()
time.sleep(duration)
@@ -264,10 +266,16 @@ def _send_packets_and_capture(
send_port: Port,
receive_port: Port,
duration: float,
+ no_lldp: bool,
+ no_arp: bool,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
binary_packets = [packet.build() for packet in packets]
-
+ sniff_filter = []
+ if no_lldp:
+ sniff_filter.append("ether[12:2] != 0x88cc")
+ if no_arp:
+ sniff_filter.append("ether[12:2] != 0x0806")
xmlrpc_packets: list[
xmlrpc.client.Binary
] = self.rpc_server_proxy.scapy_send_packets_and_capture(
@@ -275,6 +283,7 @@ def _send_packets_and_capture(
send_port.logical_name,
receive_port.logical_name,
duration,
+ " && ".join(sniff_filter),
) # type: ignore[assignment]
scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 27025cfa31..98e55b7831 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -56,6 +56,8 @@ def send_packet_and_capture(
send_port: Port,
receive_port: Port,
duration: float = 1,
+ no_lldp: bool = True,
+ no_arp: bool = True,
) -> list[Packet]:
"""Send a packet, return received traffic.
@@ -73,7 +75,7 @@ def send_packet_and_capture(
A list of received packets. May be empty if no packets are captured.
"""
return self.traffic_generator.send_packet_and_capture(
- packet, send_port, receive_port, duration
+ packet, send_port, receive_port, duration, no_lldp, no_arp
)
def close(self) -> None:
--
2.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer
2023-11-13 20:28 ` [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer jspewock
@ 2023-11-16 18:34 ` Juraj Linkeš
2023-11-17 18:05 ` Jeremy Spewock
0 siblings, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 18:34 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
As I'm thinking about the filtering, it's not a trivial manner. For
now, I'd like to pass a class instead of multiple parameters, as that
will be easier to extend if we need to add to the filter. The class
could be a dataclass holding the various booleans. Then the capturing
traffic generators would need to implement a method that would
translate the dataclass into a TG-specific filters. I'm not sure we
want to introduce an abstract method for this, as the return value may
differ for different TGs - this needs more thought.
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Added the options to filter out LLDP and ARP packets when
> sniffing for packets with scapy. This was done using BPF filters to
> ensure that the noise these packets provide does not interfere with test
> cases.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/test_suite.py | 13 +++++++++++--
> .../testbed_model/capturing_traffic_generator.py | 12 +++++++++++-
> dts/framework/testbed_model/scapy.py | 11 ++++++++++-
> dts/framework/testbed_model/tg_node.py | 4 +++-
> 4 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 3b890c0451..3222f172f3 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -152,7 +152,11 @@ def _configure_ipv4_forwarding(self, enable: bool) -> None:
> self.sut_node.configure_ipv4_forwarding(enable)
>
> def send_packet_and_capture(
> - self, packet: Packet, duration: float = 1
> + self,
> + packet: Packet,
> + duration: float = 1,
> + no_lldp: bool = True,
> + no_arp: bool = True,
The parameters of this method are not documented in the docstring, but
we should add these new parameters to the docstring. The same goes for
the other methods (and other parameters of other methods) if they're
not overriding an abstract method.
The default should be False if these are meant to be optional, but I
like these defaulting to True, so I'd change the wording from optional
to configurable.
> ) -> list[Packet]:
> """
> Send a packet through the appropriate interface and
> @@ -162,7 +166,12 @@ def send_packet_and_capture(
> """
> packet = self._adjust_addresses(packet)
> return self.tg_node.send_packet_and_capture(
> - packet, self._tg_port_egress, self._tg_port_ingress, duration
> + packet,
> + self._tg_port_egress,
> + self._tg_port_ingress,
> + duration,
> + no_lldp,
> + no_arp,
> )
>
> def get_expected_packet(self, packet: Packet) -> Packet:
> diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b/dts/framework/testbed_model/capturing_traffic_generator.py
> index ab98987f8e..0a0d0f7d0d 100644
> --- a/dts/framework/testbed_model/capturing_traffic_generator.py
> +++ b/dts/framework/testbed_model/capturing_traffic_generator.py
> @@ -52,6 +52,8 @@ def send_packet_and_capture(
> send_port: Port,
> receive_port: Port,
> duration: float,
> + no_lldp: bool,
> + no_arp: bool,
> capture_name: str = _get_default_capture_name(),
> ) -> list[Packet]:
> """Send a packet, return received traffic.
> @@ -71,7 +73,7 @@ def send_packet_and_capture(
> A list of received packets. May be empty if no packets are captured.
> """
> return self.send_packets_and_capture(
> - [packet], send_port, receive_port, duration, capture_name
> + [packet], send_port, receive_port, duration, no_lldp, no_arp, capture_name
> )
>
> def send_packets_and_capture(
> @@ -80,6 +82,8 @@ def send_packets_and_capture(
> send_port: Port,
> receive_port: Port,
> duration: float,
> + no_lldp: bool,
> + no_arp: bool,
> capture_name: str = _get_default_capture_name(),
> ) -> list[Packet]:
> """Send packets, return received traffic.
> @@ -93,6 +97,8 @@ def send_packets_and_capture(
> send_port: The egress port on the TG node.
> receive_port: The ingress port in the TG node.
> duration: Capture traffic for this amount of time after sending the packets.
> + no_lldp: Option to disable capturing LLDP packets
> + no_arp: Option to disable capturing ARP packets
> capture_name: The name of the .pcap file where to store the capture.
>
> Returns:
> @@ -108,6 +114,8 @@ def send_packets_and_capture(
> send_port,
> receive_port,
> duration,
> + no_lldp,
> + no_arp,
> )
>
> self._logger.debug(
> @@ -123,6 +131,8 @@ def _send_packets_and_capture(
> send_port: Port,
> receive_port: Port,
> duration: float,
> + no_lldp: bool,
> + no_arp: bool,
> ) -> list[Packet]:
> """
> The extended classes must implement this method which
> diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed_model/scapy.py
> index af0d4dbb25..58f01af21a 100644
> --- a/dts/framework/testbed_model/scapy.py
> +++ b/dts/framework/testbed_model/scapy.py
> @@ -69,6 +69,7 @@ def scapy_send_packets_and_capture(
> send_iface: str,
> recv_iface: str,
> duration: float,
> + sniff_filter: str,
> ) -> list[bytes]:
> """RPC function to send and capture packets.
>
> @@ -90,6 +91,7 @@ def scapy_send_packets_and_capture(
> iface=recv_iface,
> store=True,
> started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
> + filter=sniff_filter,
> )
> sniffer.start()
> time.sleep(duration)
> @@ -264,10 +266,16 @@ def _send_packets_and_capture(
> send_port: Port,
> receive_port: Port,
> duration: float,
> + no_lldp: bool,
> + no_arp: bool,
> capture_name: str = _get_default_capture_name(),
> ) -> list[Packet]:
> binary_packets = [packet.build() for packet in packets]
> -
> + sniff_filter = []
> + if no_lldp:
> + sniff_filter.append("ether[12:2] != 0x88cc")
> + if no_arp:
> + sniff_filter.append("ether[12:2] != 0x0806")
> xmlrpc_packets: list[
> xmlrpc.client.Binary
> ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
> @@ -275,6 +283,7 @@ def _send_packets_and_capture(
> send_port.logical_name,
> receive_port.logical_name,
> duration,
> + " && ".join(sniff_filter),
> ) # type: ignore[assignment]
>
> scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
> diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
> index 27025cfa31..98e55b7831 100644
> --- a/dts/framework/testbed_model/tg_node.py
> +++ b/dts/framework/testbed_model/tg_node.py
> @@ -56,6 +56,8 @@ def send_packet_and_capture(
> send_port: Port,
> receive_port: Port,
> duration: float = 1,
> + no_lldp: bool = True,
> + no_arp: bool = True,
> ) -> list[Packet]:
> """Send a packet, return received traffic.
>
> @@ -73,7 +75,7 @@ def send_packet_and_capture(
> A list of received packets. May be empty if no packets are captured.
> """
> return self.traffic_generator.send_packet_and_capture(
> - packet, send_port, receive_port, duration
> + packet, send_port, receive_port, duration, no_lldp, no_arp
> )
>
> def close(self) -> None:
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer
2023-11-16 18:34 ` Juraj Linkeš
@ 2023-11-17 18:05 ` Jeremy Spewock
2023-11-20 14:31 ` Juraj Linkeš
0 siblings, 1 reply; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-17 18:05 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 10629 bytes --]
On Thu, Nov 16, 2023 at 1:34 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> As I'm thinking about the filtering, it's not a trivial manner. For
> now, I'd like to pass a class instead of multiple parameters, as that
> will be easier to extend if we need to add to the filter. The class
> could be a dataclass holding the various booleans. Then the capturing
> traffic generators would need to implement a method that would
> translate the dataclass into a TG-specific filters. I'm not sure we
> want to introduce an abstract method for this, as the return value may
> differ for different TGs - this needs more thought.
>
I agree that making it a dataclass and having a method within Scapy for
interpreting it for now would work. You're right there there are some other
considerations to be had like if the return types could be different. I
would think that for the most part a string is what would be used, but it
could be different depending on what traffic generator you use. I know that
tcpdump supports BPFs as they are shown here, so if we went down the route
of using that for collecting packets with other traffic generators we would
be fine on that front, but that might need more discussion as well when we
add those traffic generators.
If we needed this to be more generic as well, it could be possible to
assign each traffic generator a filter type and then we could make
subclasses of the dataclass for each filter type. This way, in the filter
subclasses, they could specify what each of the parameters meant and how to
create the filter. This would always return the right data type for the
filter and allow us to implement methods for some of the more generic
filters (like the one I use here, a BPF) rather than implementing them on
the traffic generator that uses the filter. Then we could just use a
TypeVar to generically create the filters based on what the capturing
traffic generator class says it would need.
I think another way we could go about this problem however is just not
filtering out packets that are generally just noise like these and instead
just check the packets we receive when capturing and only return ones that
are relevant (like ones that have the same layers as the packet we sent for
example). However, I think because of only having one traffic generator it
would be fine to just create a method i nthe scapy capturing traffic
generator that knows how to create BPF filters and I can do that for the
next version if that is preferred.
>
> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Added the options to filter out LLDP and ARP packets when
> > sniffing for packets with scapy. This was done using BPF filters to
> > ensure that the noise these packets provide does not interfere with test
> > cases.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/framework/test_suite.py | 13 +++++++++++--
> > .../testbed_model/capturing_traffic_generator.py | 12 +++++++++++-
> > dts/framework/testbed_model/scapy.py | 11 ++++++++++-
> > dts/framework/testbed_model/tg_node.py | 4 +++-
> > 4 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> > index 3b890c0451..3222f172f3 100644
> > --- a/dts/framework/test_suite.py
> > +++ b/dts/framework/test_suite.py
> > @@ -152,7 +152,11 @@ def _configure_ipv4_forwarding(self, enable: bool)
> -> None:
> > self.sut_node.configure_ipv4_forwarding(enable)
> >
> > def send_packet_and_capture(
> > - self, packet: Packet, duration: float = 1
> > + self,
> > + packet: Packet,
> > + duration: float = 1,
> > + no_lldp: bool = True,
> > + no_arp: bool = True,
>
> The parameters of this method are not documented in the docstring, but
> we should add these new parameters to the docstring. The same goes for
> the other methods (and other parameters of other methods) if they're
> not overriding an abstract method.
>
> The default should be False if these are meant to be optional, but I
> like these defaulting to True, so I'd change the wording from optional
> to configurable.
>
That is a good catch, I didn't think about the wording that way but that
makes a lot of sense. I will make this change.
>
> > ) -> list[Packet]:
> > """
> > Send a packet through the appropriate interface and
> > @@ -162,7 +166,12 @@ def send_packet_and_capture(
> > """
> > packet = self._adjust_addresses(packet)
> > return self.tg_node.send_packet_and_capture(
> > - packet, self._tg_port_egress, self._tg_port_ingress,
> duration
> > + packet,
> > + self._tg_port_egress,
> > + self._tg_port_ingress,
> > + duration,
> > + no_lldp,
> > + no_arp,
> > )
> >
> > def get_expected_packet(self, packet: Packet) -> Packet:
> > diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py
> b/dts/framework/testbed_model/capturing_traffic_generator.py
> > index ab98987f8e..0a0d0f7d0d 100644
> > --- a/dts/framework/testbed_model/capturing_traffic_generator.py
> > +++ b/dts/framework/testbed_model/capturing_traffic_generator.py
> > @@ -52,6 +52,8 @@ def send_packet_and_capture(
> > send_port: Port,
> > receive_port: Port,
> > duration: float,
> > + no_lldp: bool,
> > + no_arp: bool,
> > capture_name: str = _get_default_capture_name(),
> > ) -> list[Packet]:
> > """Send a packet, return received traffic.
> > @@ -71,7 +73,7 @@ def send_packet_and_capture(
> > A list of received packets. May be empty if no packets are
> captured.
> > """
> > return self.send_packets_and_capture(
> > - [packet], send_port, receive_port, duration, capture_name
> > + [packet], send_port, receive_port, duration, no_lldp,
> no_arp, capture_name
> > )
> >
> > def send_packets_and_capture(
> > @@ -80,6 +82,8 @@ def send_packets_and_capture(
> > send_port: Port,
> > receive_port: Port,
> > duration: float,
> > + no_lldp: bool,
> > + no_arp: bool,
> > capture_name: str = _get_default_capture_name(),
> > ) -> list[Packet]:
> > """Send packets, return received traffic.
> > @@ -93,6 +97,8 @@ def send_packets_and_capture(
> > send_port: The egress port on the TG node.
> > receive_port: The ingress port in the TG node.
> > duration: Capture traffic for this amount of time after
> sending the packets.
> > + no_lldp: Option to disable capturing LLDP packets
> > + no_arp: Option to disable capturing ARP packets
> > capture_name: The name of the .pcap file where to store the
> capture.
> >
> > Returns:
> > @@ -108,6 +114,8 @@ def send_packets_and_capture(
> > send_port,
> > receive_port,
> > duration,
> > + no_lldp,
> > + no_arp,
> > )
> >
> > self._logger.debug(
> > @@ -123,6 +131,8 @@ def _send_packets_and_capture(
> > send_port: Port,
> > receive_port: Port,
> > duration: float,
> > + no_lldp: bool,
> > + no_arp: bool,
> > ) -> list[Packet]:
> > """
> > The extended classes must implement this method which
> > diff --git a/dts/framework/testbed_model/scapy.py
> b/dts/framework/testbed_model/scapy.py
> > index af0d4dbb25..58f01af21a 100644
> > --- a/dts/framework/testbed_model/scapy.py
> > +++ b/dts/framework/testbed_model/scapy.py
> > @@ -69,6 +69,7 @@ def scapy_send_packets_and_capture(
> > send_iface: str,
> > recv_iface: str,
> > duration: float,
> > + sniff_filter: str,
> > ) -> list[bytes]:
> > """RPC function to send and capture packets.
> >
> > @@ -90,6 +91,7 @@ def scapy_send_packets_and_capture(
> > iface=recv_iface,
> > store=True,
> > started_callback=lambda *args: scapy.all.sendp(scapy_packets,
> iface=send_iface),
> > + filter=sniff_filter,
> > )
> > sniffer.start()
> > time.sleep(duration)
> > @@ -264,10 +266,16 @@ def _send_packets_and_capture(
> > send_port: Port,
> > receive_port: Port,
> > duration: float,
> > + no_lldp: bool,
> > + no_arp: bool,
> > capture_name: str = _get_default_capture_name(),
> > ) -> list[Packet]:
> > binary_packets = [packet.build() for packet in packets]
> > -
> > + sniff_filter = []
> > + if no_lldp:
> > + sniff_filter.append("ether[12:2] != 0x88cc")
> > + if no_arp:
> > + sniff_filter.append("ether[12:2] != 0x0806")
> > xmlrpc_packets: list[
> > xmlrpc.client.Binary
> > ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
> > @@ -275,6 +283,7 @@ def _send_packets_and_capture(
> > send_port.logical_name,
> > receive_port.logical_name,
> > duration,
> > + " && ".join(sniff_filter),
> > ) # type: ignore[assignment]
> >
> > scapy_packets = [Ether(packet.data) for packet in
> xmlrpc_packets]
> > diff --git a/dts/framework/testbed_model/tg_node.py
> b/dts/framework/testbed_model/tg_node.py
> > index 27025cfa31..98e55b7831 100644
> > --- a/dts/framework/testbed_model/tg_node.py
> > +++ b/dts/framework/testbed_model/tg_node.py
> > @@ -56,6 +56,8 @@ def send_packet_and_capture(
> > send_port: Port,
> > receive_port: Port,
> > duration: float = 1,
> > + no_lldp: bool = True,
> > + no_arp: bool = True,
> > ) -> list[Packet]:
> > """Send a packet, return received traffic.
> >
> > @@ -73,7 +75,7 @@ def send_packet_and_capture(
> > A list of received packets. May be empty if no packets are
> captured.
> > """
> > return self.traffic_generator.send_packet_and_capture(
> > - packet, send_port, receive_port, duration
> > + packet, send_port, receive_port, duration, no_lldp, no_arp
> > )
> >
> > def close(self) -> None:
> > --
> > 2.42.0
> >
>
[-- Attachment #2: Type: text/html, Size: 13608 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer
2023-11-17 18:05 ` Jeremy Spewock
@ 2023-11-20 14:31 ` Juraj Linkeš
0 siblings, 0 replies; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-20 14:31 UTC (permalink / raw)
To: Jeremy Spewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Fri, Nov 17, 2023 at 7:06 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
>
>
> On Thu, Nov 16, 2023 at 1:34 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> As I'm thinking about the filtering, it's not a trivial manner. For
>> now, I'd like to pass a class instead of multiple parameters, as that
>> will be easier to extend if we need to add to the filter. The class
>> could be a dataclass holding the various booleans. Then the capturing
>> traffic generators would need to implement a method that would
>> translate the dataclass into a TG-specific filters. I'm not sure we
>> want to introduce an abstract method for this, as the return value may
>> differ for different TGs - this needs more thought.
>
>
> I agree that making it a dataclass and having a method within Scapy for interpreting it for now would work. You're right there there are some other considerations to be had like if the return types could be different. I would think that for the most part a string is what would be used, but it could be different depending on what traffic generator you use. I know that tcpdump supports BPFs as they are shown here, so if we went down the route of using that for collecting packets with other traffic generators we would be fine on that front, but that might need more discussion as well when we add those traffic generators.
>
> If we needed this to be more generic as well, it could be possible to assign each traffic generator a filter type and then we could make subclasses of the dataclass for each filter type. This way, in the filter subclasses, they could specify what each of the parameters meant and how to create the filter. This would always return the right data type for the filter and allow us to implement methods for some of the more generic filters (like the one I use here, a BPF) rather than implementing them on the traffic generator that uses the filter. Then we could just use a TypeVar to generically create the filters based on what the capturing traffic generator class says it would need.
>
> I think another way we could go about this problem however is just not filtering out packets that are generally just noise like these and instead just check the packets we receive when capturing and only return ones that are relevant (like ones that have the same layers as the packet we sent for example). However, I think because of only having one traffic generator it would be fine to just create a method i nthe scapy capturing traffic generator that knows how to create BPF filters and I can do that for the next version if that is preferred.
>
Let's just create a method in the scapy tg (and the dataclass,
probably in traffic_generator.py?) in this patch and keep discussing
the issue for the next release(s).
>>
>>
>> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>> >
>> > From: Jeremy Spewock <jspewock@iol.unh.edu>
>> >
>> > Added the options to filter out LLDP and ARP packets when
>> > sniffing for packets with scapy. This was done using BPF filters to
>> > ensure that the noise these packets provide does not interfere with test
>> > cases.
>> >
>> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
>> > ---
>> > dts/framework/test_suite.py | 13 +++++++++++--
>> > .../testbed_model/capturing_traffic_generator.py | 12 +++++++++++-
>> > dts/framework/testbed_model/scapy.py | 11 ++++++++++-
>> > dts/framework/testbed_model/tg_node.py | 4 +++-
>> > 4 files changed, 35 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
>> > index 3b890c0451..3222f172f3 100644
>> > --- a/dts/framework/test_suite.py
>> > +++ b/dts/framework/test_suite.py
>> > @@ -152,7 +152,11 @@ def _configure_ipv4_forwarding(self, enable: bool) -> None:
>> > self.sut_node.configure_ipv4_forwarding(enable)
>> >
>> > def send_packet_and_capture(
>> > - self, packet: Packet, duration: float = 1
>> > + self,
>> > + packet: Packet,
>> > + duration: float = 1,
>> > + no_lldp: bool = True,
>> > + no_arp: bool = True,
>>
>> The parameters of this method are not documented in the docstring, but
>> we should add these new parameters to the docstring. The same goes for
>> the other methods (and other parameters of other methods) if they're
>> not overriding an abstract method.
>>
>> The default should be False if these are meant to be optional, but I
>> like these defaulting to True, so I'd change the wording from optional
>> to configurable.
>
>
> That is a good catch, I didn't think about the wording that way but that makes a lot of sense. I will make this change.
>
>>
>>
>> > ) -> list[Packet]:
>> > """
>> > Send a packet through the appropriate interface and
>> > @@ -162,7 +166,12 @@ def send_packet_and_capture(
>> > """
>> > packet = self._adjust_addresses(packet)
>> > return self.tg_node.send_packet_and_capture(
>> > - packet, self._tg_port_egress, self._tg_port_ingress, duration
>> > + packet,
>> > + self._tg_port_egress,
>> > + self._tg_port_ingress,
>> > + duration,
>> > + no_lldp,
>> > + no_arp,
>> > )
>> >
>> > def get_expected_packet(self, packet: Packet) -> Packet:
>> > diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b/dts/framework/testbed_model/capturing_traffic_generator.py
>> > index ab98987f8e..0a0d0f7d0d 100644
>> > --- a/dts/framework/testbed_model/capturing_traffic_generator.py
>> > +++ b/dts/framework/testbed_model/capturing_traffic_generator.py
>> > @@ -52,6 +52,8 @@ def send_packet_and_capture(
>> > send_port: Port,
>> > receive_port: Port,
>> > duration: float,
>> > + no_lldp: bool,
>> > + no_arp: bool,
>> > capture_name: str = _get_default_capture_name(),
>> > ) -> list[Packet]:
>> > """Send a packet, return received traffic.
>> > @@ -71,7 +73,7 @@ def send_packet_and_capture(
>> > A list of received packets. May be empty if no packets are captured.
>> > """
>> > return self.send_packets_and_capture(
>> > - [packet], send_port, receive_port, duration, capture_name
>> > + [packet], send_port, receive_port, duration, no_lldp, no_arp, capture_name
>> > )
>> >
>> > def send_packets_and_capture(
>> > @@ -80,6 +82,8 @@ def send_packets_and_capture(
>> > send_port: Port,
>> > receive_port: Port,
>> > duration: float,
>> > + no_lldp: bool,
>> > + no_arp: bool,
>> > capture_name: str = _get_default_capture_name(),
>> > ) -> list[Packet]:
>> > """Send packets, return received traffic.
>> > @@ -93,6 +97,8 @@ def send_packets_and_capture(
>> > send_port: The egress port on the TG node.
>> > receive_port: The ingress port in the TG node.
>> > duration: Capture traffic for this amount of time after sending the packets.
>> > + no_lldp: Option to disable capturing LLDP packets
>> > + no_arp: Option to disable capturing ARP packets
>> > capture_name: The name of the .pcap file where to store the capture.
>> >
>> > Returns:
>> > @@ -108,6 +114,8 @@ def send_packets_and_capture(
>> > send_port,
>> > receive_port,
>> > duration,
>> > + no_lldp,
>> > + no_arp,
>> > )
>> >
>> > self._logger.debug(
>> > @@ -123,6 +131,8 @@ def _send_packets_and_capture(
>> > send_port: Port,
>> > receive_port: Port,
>> > duration: float,
>> > + no_lldp: bool,
>> > + no_arp: bool,
>> > ) -> list[Packet]:
>> > """
>> > The extended classes must implement this method which
>> > diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed_model/scapy.py
>> > index af0d4dbb25..58f01af21a 100644
>> > --- a/dts/framework/testbed_model/scapy.py
>> > +++ b/dts/framework/testbed_model/scapy.py
>> > @@ -69,6 +69,7 @@ def scapy_send_packets_and_capture(
>> > send_iface: str,
>> > recv_iface: str,
>> > duration: float,
>> > + sniff_filter: str,
>> > ) -> list[bytes]:
>> > """RPC function to send and capture packets.
>> >
>> > @@ -90,6 +91,7 @@ def scapy_send_packets_and_capture(
>> > iface=recv_iface,
>> > store=True,
>> > started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
>> > + filter=sniff_filter,
>> > )
>> > sniffer.start()
>> > time.sleep(duration)
>> > @@ -264,10 +266,16 @@ def _send_packets_and_capture(
>> > send_port: Port,
>> > receive_port: Port,
>> > duration: float,
>> > + no_lldp: bool,
>> > + no_arp: bool,
>> > capture_name: str = _get_default_capture_name(),
>> > ) -> list[Packet]:
>> > binary_packets = [packet.build() for packet in packets]
>> > -
>> > + sniff_filter = []
>> > + if no_lldp:
>> > + sniff_filter.append("ether[12:2] != 0x88cc")
>> > + if no_arp:
>> > + sniff_filter.append("ether[12:2] != 0x0806")
>> > xmlrpc_packets: list[
>> > xmlrpc.client.Binary
>> > ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
>> > @@ -275,6 +283,7 @@ def _send_packets_and_capture(
>> > send_port.logical_name,
>> > receive_port.logical_name,
>> > duration,
>> > + " && ".join(sniff_filter),
>> > ) # type: ignore[assignment]
>> >
>> > scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
>> > diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
>> > index 27025cfa31..98e55b7831 100644
>> > --- a/dts/framework/testbed_model/tg_node.py
>> > +++ b/dts/framework/testbed_model/tg_node.py
>> > @@ -56,6 +56,8 @@ def send_packet_and_capture(
>> > send_port: Port,
>> > receive_port: Port,
>> > duration: float = 1,
>> > + no_lldp: bool = True,
>> > + no_arp: bool = True,
>> > ) -> list[Packet]:
>> > """Send a packet, return received traffic.
>> >
>> > @@ -73,7 +75,7 @@ def send_packet_and_capture(
>> > A list of received packets. May be empty if no packets are captured.
>> > """
>> > return self.traffic_generator.send_packet_and_capture(
>> > - packet, send_port, receive_port, duration
>> > + packet, send_port, receive_port, duration, no_lldp, no_arp
>> > )
>> >
>> > def close(self) -> None:
>> > --
>> > 2.42.0
>> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 6/7] dts: add pci addresses to EAL parameters
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (4 preceding siblings ...)
2023-11-13 20:28 ` [PATCH v3 5/7] dts: add optional packet filtering to scapy sniffer jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-16 18:10 ` Juraj Linkeš
2023-11-13 20:28 ` [PATCH v3 7/7] dts: allow configuring MTU of ports jspewock
` (17 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added allow list to the EAL parameters created in DTS to ensure that
only the relevant PCI devices are considered when launching DPDK
applications.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/testbed_model/sut_node.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index bcac939e72..f9c7bd9bf3 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -20,6 +20,7 @@
from framework.utils import MesonArgs
from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice
+from .hw.port import Port
from .node import Node
@@ -31,6 +32,7 @@ def __init__(
prefix: str,
no_pci: bool,
vdevs: list[VirtualDevice],
+ ports: list[Port],
other_eal_param: str,
):
"""
@@ -56,6 +58,7 @@ def __init__(
self._prefix = f"--file-prefix={prefix}"
self._no_pci = "--no-pci" if no_pci else ""
self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
+ self._ports = " ".join(f"-a {port.pci}" for port in ports)
self._other_eal_param = other_eal_param
def __str__(self) -> str:
@@ -65,6 +68,7 @@ def __str__(self) -> str:
f"{self._prefix} "
f"{self._no_pci} "
f"{self._vdevs} "
+ f"{self._ports} "
f"{self._other_eal_param}"
)
@@ -308,6 +312,7 @@ def create_eal_parameters(
append_prefix_timestamp: bool = True,
no_pci: bool = False,
vdevs: list[VirtualDevice] = None,
+ ports: list[Port] | None = None,
other_eal_param: str = "",
) -> "EalParameters":
"""
@@ -350,12 +355,16 @@ def create_eal_parameters(
if vdevs is None:
vdevs = []
+ if ports is None:
+ ports = self.ports
+
return EalParameters(
lcore_list=lcore_list,
memory_channels=self.config.memory_channels,
prefix=prefix,
no_pci=no_pci,
vdevs=vdevs,
+ ports=ports,
other_eal_param=other_eal_param,
)
--
2.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 6/7] dts: add pci addresses to EAL parameters
2023-11-13 20:28 ` [PATCH v3 6/7] dts: add pci addresses to EAL parameters jspewock
@ 2023-11-16 18:10 ` Juraj Linkeš
2023-11-17 17:13 ` Jeremy Spewock
0 siblings, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 18:10 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Added allow list to the EAL parameters created in DTS to ensure that
> only the relevant PCI devices are considered when launching DPDK
> applications.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/testbed_model/sut_node.py | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index bcac939e72..f9c7bd9bf3 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -20,6 +20,7 @@
> from framework.utils import MesonArgs
>
> from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice
> +from .hw.port import Port
> from .node import Node
>
>
> @@ -31,6 +32,7 @@ def __init__(
> prefix: str,
> no_pci: bool,
> vdevs: list[VirtualDevice],
> + ports: list[Port],
Please add this to the docstring.
This overlaps with the docstrings patch. I guess we'll need to modify
one of the patches when the other one gets merged.
> other_eal_param: str,
> ):
> """
> @@ -56,6 +58,7 @@ def __init__(
> self._prefix = f"--file-prefix={prefix}"
> self._no_pci = "--no-pci" if no_pci else ""
> self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
> + self._ports = " ".join(f"-a {port.pci}" for port in ports)
> self._other_eal_param = other_eal_param
>
> def __str__(self) -> str:
> @@ -65,6 +68,7 @@ def __str__(self) -> str:
> f"{self._prefix} "
> f"{self._no_pci} "
> f"{self._vdevs} "
> + f"{self._ports} "
> f"{self._other_eal_param}"
> )
>
> @@ -308,6 +312,7 @@ def create_eal_parameters(
> append_prefix_timestamp: bool = True,
> no_pci: bool = False,
> vdevs: list[VirtualDevice] = None,
> + ports: list[Port] | None = None,
Same here.
> other_eal_param: str = "",
> ) -> "EalParameters":
> """
> @@ -350,12 +355,16 @@ def create_eal_parameters(
> if vdevs is None:
> vdevs = []
>
> + if ports is None:
> + ports = self.ports
> +
> return EalParameters(
> lcore_list=lcore_list,
> memory_channels=self.config.memory_channels,
> prefix=prefix,
> no_pci=no_pci,
> vdevs=vdevs,
> + ports=ports,
> other_eal_param=other_eal_param,
> )
>
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 6/7] dts: add pci addresses to EAL parameters
2023-11-16 18:10 ` Juraj Linkeš
@ 2023-11-17 17:13 ` Jeremy Spewock
0 siblings, 0 replies; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-17 17:13 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 3421 bytes --]
On Thu, Nov 16, 2023 at 1:10 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Added allow list to the EAL parameters created in DTS to ensure that
> > only the relevant PCI devices are considered when launching DPDK
> > applications.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/framework/testbed_model/sut_node.py | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> > index bcac939e72..f9c7bd9bf3 100644
> > --- a/dts/framework/testbed_model/sut_node.py
> > +++ b/dts/framework/testbed_model/sut_node.py
> > @@ -20,6 +20,7 @@
> > from framework.utils import MesonArgs
> >
> > from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice
> > +from .hw.port import Port
> > from .node import Node
> >
> >
> > @@ -31,6 +32,7 @@ def __init__(
> > prefix: str,
> > no_pci: bool,
> > vdevs: list[VirtualDevice],
> > + ports: list[Port],
>
> Please add this to the docstring.
>
> This overlaps with the docstrings patch. I guess we'll need to modify
> one of the patches when the other one gets merged.
>
Right, this would probably get a little weird with the conflict if one gets
merged before the other. Another option could be copying your docstring
over into this patch as well which might clean it up, but it would sort of
be out of place in this patch. I'll add in the current format for now and
we can change one or the other depending on the order to make sure it
applies.
> > other_eal_param: str,
> > ):
> > """
> > @@ -56,6 +58,7 @@ def __init__(
> > self._prefix = f"--file-prefix={prefix}"
> > self._no_pci = "--no-pci" if no_pci else ""
> > self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
> > + self._ports = " ".join(f"-a {port.pci}" for port in ports)
> > self._other_eal_param = other_eal_param
> >
> > def __str__(self) -> str:
> > @@ -65,6 +68,7 @@ def __str__(self) -> str:
> > f"{self._prefix} "
> > f"{self._no_pci} "
> > f"{self._vdevs} "
> > + f"{self._ports} "
> > f"{self._other_eal_param}"
> > )
> >
> > @@ -308,6 +312,7 @@ def create_eal_parameters(
> > append_prefix_timestamp: bool = True,
> > no_pci: bool = False,
> > vdevs: list[VirtualDevice] = None,
> > + ports: list[Port] | None = None,
>
> Same here.
>
Good catch, I will do the same here.
>
> > other_eal_param: str = "",
> > ) -> "EalParameters":
> > """
> > @@ -350,12 +355,16 @@ def create_eal_parameters(
> > if vdevs is None:
> > vdevs = []
> >
> > + if ports is None:
> > + ports = self.ports
> > +
> > return EalParameters(
> > lcore_list=lcore_list,
> > memory_channels=self.config.memory_channels,
> > prefix=prefix,
> > no_pci=no_pci,
> > vdevs=vdevs,
> > + ports=ports,
> > other_eal_param=other_eal_param,
> > )
> >
> > --
> > 2.42.0
> >
>
[-- Attachment #2: Type: text/html, Size: 5209 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 7/7] dts: allow configuring MTU of ports
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (5 preceding siblings ...)
2023-11-13 20:28 ` [PATCH v3 6/7] dts: add pci addresses to EAL parameters jspewock
@ 2023-11-13 20:28 ` jspewock
2023-11-16 18:05 ` Juraj Linkeš
2023-11-16 19:23 ` [PATCH v3 0/7] dts: Port scatter suite over Juraj Linkeš
` (16 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: jspewock @ 2023-11-13 20:28 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Adds methods in both os_session and linux session to allow for setting
MTU of port interfaces in an OS agnostic way.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/remote_session/linux_session.py | 7 +++++++
dts/framework/remote_session/os_session.py | 9 +++++++++
2 files changed, 16 insertions(+)
diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
index a3f1a6bf3b..dab68d41b1 100644
--- a/dts/framework/remote_session/linux_session.py
+++ b/dts/framework/remote_session/linux_session.py
@@ -196,6 +196,13 @@ def configure_port_ip_address(
verify=True,
)
+ def configure_port_mtu(self, mtu: int, port: Port) -> None:
+ self.send_command(
+ f"ip link set dev {port.logical_name} mtu {mtu}",
+ privileged=True,
+ verify=True,
+ )
+
def configure_ipv4_forwarding(self, enable: bool) -> None:
state = 1 if enable else 0
self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
index 8a709eac1c..c038f78b79 100644
--- a/dts/framework/remote_session/os_session.py
+++ b/dts/framework/remote_session/os_session.py
@@ -277,6 +277,15 @@ def configure_port_ip_address(
Configure (add or delete) an IP address of the input port.
"""
+ @abstractmethod
+ def configure_port_mtu(self, mtu: int, port: Port) -> None:
+ """Configure MTU on a given port.
+
+ Args:
+ mtu: Desired MTU value.
+ port: Port to set the MTU on.
+ """
+
@abstractmethod
def configure_ipv4_forwarding(self, enable: bool) -> None:
"""
--
2.42.0
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 7/7] dts: allow configuring MTU of ports
2023-11-13 20:28 ` [PATCH v3 7/7] dts: allow configuring MTU of ports jspewock
@ 2023-11-16 18:05 ` Juraj Linkeš
2023-11-17 17:06 ` Jeremy Spewock
0 siblings, 1 reply; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 18:05 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Adds methods in both os_session and linux session to allow for setting
> MTU of port interfaces in an OS agnostic way.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/remote_session/linux_session.py | 7 +++++++
> dts/framework/remote_session/os_session.py | 9 +++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
> index a3f1a6bf3b..dab68d41b1 100644
> --- a/dts/framework/remote_session/linux_session.py
> +++ b/dts/framework/remote_session/linux_session.py
> @@ -196,6 +196,13 @@ def configure_port_ip_address(
> verify=True,
> )
>
> + def configure_port_mtu(self, mtu: int, port: Port) -> None:
This is missing a docstring.
The way I've decided to document these overridden abstract methods is
to just to link to the superclass's method, used heavily in
https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-17-juraj.linkes@pantheon.tech/:
"""Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""
The docstring checker complains if the docstring is missing. There may
be better ways, such as with @typing.override (but that requires
Python 3.12 or typing_extensions, but there's a bug in Pylama that
prevents us from using that solution:
https://github.com/klen/pylama/pull/247).
> + self.send_command(
> + f"ip link set dev {port.logical_name} mtu {mtu}",
> + privileged=True,
> + verify=True,
> + )
> +
> def configure_ipv4_forwarding(self, enable: bool) -> None:
> state = 1 if enable else 0
> self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
> diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
> index 8a709eac1c..c038f78b79 100644
> --- a/dts/framework/remote_session/os_session.py
> +++ b/dts/framework/remote_session/os_session.py
> @@ -277,6 +277,15 @@ def configure_port_ip_address(
> Configure (add or delete) an IP address of the input port.
> """
>
> + @abstractmethod
> + def configure_port_mtu(self, mtu: int, port: Port) -> None:
> + """Configure MTU on a given port.
> +
> + Args:
> + mtu: Desired MTU value.
> + port: Port to set the MTU on.
> + """
I've compiled the rules for composing docstrings here:
https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-4-juraj.linkes@pantheon.tech/.
The relevant part here is:
When referencing a parameter of a function or a method in their
docstring, don't use
any articles and put the parameter into single backticks. This mimics
the style of
`Python's documentation <https://docs.python.org/3/index.html>`_.
Both the mtu and the port parameters are mentioned, so they should be
without articles and in backticks.
> +
> @abstractmethod
> def configure_ipv4_forwarding(self, enable: bool) -> None:
> """
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v3 7/7] dts: allow configuring MTU of ports
2023-11-16 18:05 ` Juraj Linkeš
@ 2023-11-17 17:06 ` Jeremy Spewock
0 siblings, 0 replies; 43+ messages in thread
From: Jeremy Spewock @ 2023-11-17 17:06 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
[-- Attachment #1: Type: text/plain, Size: 3556 bytes --]
Both good points, I'll be sure to add/adjust those docstrings.
On Thu, Nov 16, 2023 at 1:05 PM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
> >
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Adds methods in both os_session and linux session to allow for setting
> > MTU of port interfaces in an OS agnostic way.
> >
> > Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> > ---
> > dts/framework/remote_session/linux_session.py | 7 +++++++
> > dts/framework/remote_session/os_session.py | 9 +++++++++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/dts/framework/remote_session/linux_session.py
> b/dts/framework/remote_session/linux_session.py
> > index a3f1a6bf3b..dab68d41b1 100644
> > --- a/dts/framework/remote_session/linux_session.py
> > +++ b/dts/framework/remote_session/linux_session.py
> > @@ -196,6 +196,13 @@ def configure_port_ip_address(
> > verify=True,
> > )
> >
> > + def configure_port_mtu(self, mtu: int, port: Port) -> None:
>
> This is missing a docstring.
> The way I've decided to document these overridden abstract methods is
> to just to link to the superclass's method, used heavily in
>
> https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-17-juraj.linkes@pantheon.tech/
> :
> """Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""
>
> The docstring checker complains if the docstring is missing. There may
> be better ways, such as with @typing.override (but that requires
> Python 3.12 or typing_extensions, but there's a bug in Pylama that
> prevents us from using that solution:
> https://github.com/klen/pylama/pull/247).
>
> > + self.send_command(
> > + f"ip link set dev {port.logical_name} mtu {mtu}",
> > + privileged=True,
> > + verify=True,
> > + )
> > +
> > def configure_ipv4_forwarding(self, enable: bool) -> None:
> > state = 1 if enable else 0
> > self.send_command(f"sysctl -w net.ipv4.ip_forward={state}",
> privileged=True)
> > diff --git a/dts/framework/remote_session/os_session.py
> b/dts/framework/remote_session/os_session.py
> > index 8a709eac1c..c038f78b79 100644
> > --- a/dts/framework/remote_session/os_session.py
> > +++ b/dts/framework/remote_session/os_session.py
> > @@ -277,6 +277,15 @@ def configure_port_ip_address(
> > Configure (add or delete) an IP address of the input port.
> > """
> >
> > + @abstractmethod
> > + def configure_port_mtu(self, mtu: int, port: Port) -> None:
> > + """Configure MTU on a given port.
> > +
> > + Args:
> > + mtu: Desired MTU value.
> > + port: Port to set the MTU on.
> > + """
>
> I've compiled the rules for composing docstrings here:
>
> https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-4-juraj.linkes@pantheon.tech/
> .
>
> The relevant part here is:
>
> When referencing a parameter of a function or a method in their
> docstring, don't use
> any articles and put the parameter into single backticks. This mimics
> the style of
> `Python's documentation <https://docs.python.org/3/index.html>`_.
>
> Both the mtu and the port parameters are mentioned, so they should be
> without articles and in backticks.
>
> > +
> > @abstractmethod
> > def configure_ipv4_forwarding(self, enable: bool) -> None:
> > """
> > --
> > 2.42.0
> >
>
[-- Attachment #2: Type: text/html, Size: 5079 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3 0/7] dts: Port scatter suite over
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (6 preceding siblings ...)
2023-11-13 20:28 ` [PATCH v3 7/7] dts: allow configuring MTU of ports jspewock
@ 2023-11-16 19:23 ` Juraj Linkeš
2023-12-14 22:10 ` [PATCH v4 " jspewock
` (15 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Juraj Linkeš @ 2023-11-16 19:23 UTC (permalink / raw)
To: jspewock
Cc: Honnappa.Nagarahalli, thomas, wathsala.vithanage, probb,
paul.szczepanek, yoan.picchi, ferruh.yigit, andrew.rybchenko,
dev
On Mon, Nov 13, 2023 at 9:28 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> The only thing this version changes is it fixes typos in the commit
> messages so it will pass checkpatch.
>
> Depends-on: series-30231 ("dts: Add the ability to bind ports to drivers")
>
> Jeremy Spewock (7):
> dts: Add scatter test suite
> dts: add waiting for port up in testpmd
> dts: add scatter to the yaml schema
> dts: allow passing parameters into interactive apps
> dts: add optional packet filtering to scapy sniffer
> dts: add pci addresses to EAL parameters
> dts: allow configuring MTU of ports
>
Just a note: I reviewed this in the reverse order, since the test
suite should be last, as it uses the rest of the code. Ideally, the
commits should not break anything when applied in order (i.e. after
applying the first patch, there should be no regressions and so on).
> dts/framework/config/conf_yaml_schema.json | 3 +-
> dts/framework/remote_session/linux_session.py | 7 ++
> dts/framework/remote_session/os_session.py | 9 ++
> .../remote_session/remote/testpmd_shell.py | 31 ++++++-
> dts/framework/test_suite.py | 13 ++-
> .../capturing_traffic_generator.py | 12 ++-
> dts/framework/testbed_model/scapy.py | 11 ++-
> dts/framework/testbed_model/sut_node.py | 20 ++++-
> dts/framework/testbed_model/tg_node.py | 4 +-
> dts/tests/TestSuite_scatter.py | 86 +++++++++++++++++++
> 10 files changed, 185 insertions(+), 11 deletions(-)
> create mode 100644 dts/tests/TestSuite_scatter.py
>
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 0/7] dts: Port scatter suite over
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (7 preceding siblings ...)
2023-11-16 19:23 ` [PATCH v3 0/7] dts: Port scatter suite over Juraj Linkeš
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
` (14 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Addresses comments on the previous version. Main things which were added
include specific methods within the testpmd shell to avoid the need of
calling send_command directly, as well as docstring comment updates.
Jeremy Spewock (7):
dts: add required methods to testpmd_shell
dts: allow passing parameters into interactive apps
dts: add optional packet filtering to scapy sniffer
dts: add pci addresses to EAL parameters
dts: allow configuring MTU of ports
dts: add scatter to the yaml schema
dts: add scatter test suite
dts/framework/config/conf_yaml_schema.json | 3 +-
dts/framework/exception.py | 4 +
dts/framework/remote_session/linux_session.py | 8 ++
dts/framework/remote_session/os_session.py | 9 ++
.../remote_session/remote/testpmd_shell.py | 94 +++++++++++++++-
dts/framework/test_suite.py | 14 ++-
.../capturing_traffic_generator.py | 22 +++-
dts/framework/testbed_model/scapy.py | 28 ++++-
dts/framework/testbed_model/sut_node.py | 27 ++++-
dts/framework/testbed_model/tg_node.py | 12 +-
dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++
11 files changed, 312 insertions(+), 14 deletions(-)
create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 1/7] dts: add required methods to testpmd_shell
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (8 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 " jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
` (13 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added a method within the testpmd interactive shell that polls the
status of ports and verifies that the link status on a given port is
"up." Polling will continue until either the link comes up, or the
timeout is reached. Also added methods for starting and stopping packet
forwarding in testpmd and a method for setting the forwarding mode on
testpmd. The method for starting packet forwarding will also attempt to
verify that forwarding did indeed start by default.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/exception.py | 4 +
.../remote_session/remote/testpmd_shell.py | 92 +++++++++++++++++++
2 files changed, 96 insertions(+)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index b362e42924..e36db20e32 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -119,6 +119,10 @@ def __str__(self) -> str:
return f"Command {self.command} returned a non-zero exit code: {self.command_return_code}"
+class InteractiveCommandExecutionError(DTSError):
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.REMOTE_CMD_EXEC_ERR
+
+
class RemoteDirectoryExistsError(DTSError):
"""
Raised when a remote directory to be created already exists.
diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
index 08ac311016..b5e4cba9b3 100644
--- a/dts/framework/remote_session/remote/testpmd_shell.py
+++ b/dts/framework/remote_session/remote/testpmd_shell.py
@@ -1,9 +1,15 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2023 University of New Hampshire
+import time
+from enum import auto
from pathlib import PurePath
from typing import Callable
+from framework.exception import InteractiveCommandExecutionError
+from framework.settings import SETTINGS
+from framework.utils import StrEnum
+
from .interactive_shell import InteractiveShell
@@ -17,6 +23,37 @@ def __str__(self) -> str:
return self.pci_address
+class TestPmdForwardingModes(StrEnum):
+ r"""The supported packet forwarding modes for :class:`~TestPmdShell`\s"""
+
+ #:
+ io = auto()
+ #:
+ mac = auto()
+ #:
+ macswap = auto()
+ #:
+ flowgen = auto()
+ #:
+ rxonly = auto()
+ #:
+ txonly = auto()
+ #:
+ csum = auto()
+ #:
+ icmpecho = auto()
+ #:
+ ieee1588 = auto()
+ #:
+ noisy = auto()
+ #:
+ fivetswap = "5tswap"
+ #:
+ shared_rxq = "shared-rxq"
+ #:
+ recycle_mbufs = auto()
+
+
class TestPmdShell(InteractiveShell):
path: PurePath = PurePath("app", "dpdk-testpmd")
dpdk_app: bool = True
@@ -28,6 +65,27 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None
self._app_args += " -- -i"
super()._start_application(get_privileged_command)
+ def start(self, verify: bool = True) -> None:
+ """Start packet forwarding with the current configuration.
+
+ Args:
+ verify: If :data:`True` , a second start command will be sent in an attempt to verify
+ packet forwarding started as expected.
+
+ Raises:
+ InteractiveCommandExecutionError: If `verify` is :data:`True` and forwarding fails to
+ start.
+ """
+ self.send_command("start")
+ if verify:
+ # If forwarding was already started, sending "start" again should tell us
+ if "Packet forwarding already started" not in self.send_command("start"):
+ raise InteractiveCommandExecutionError("Testpmd failed to start packet forwarding.")
+
+ def stop(self) -> None:
+ """Stop packet forwarding."""
+ self.send_command("stop")
+
def get_devices(self) -> list[TestPmdDevice]:
"""Get a list of device names that are known to testpmd
@@ -43,3 +101,37 @@ def get_devices(self) -> list[TestPmdDevice]:
if "device name:" in line.lower():
dev_list.append(TestPmdDevice(line))
return dev_list
+
+ def wait_link_status_up(self, port_id: int, timeout=SETTINGS.timeout) -> bool:
+ """Wait until the link status on the given port is "up".
+
+ Arguments:
+ port_id: Port to check the link status on.
+ timeout: Time to wait for the link to come up. The default value for this
+ argument is set using the :option:`-t, --timeout` command-line argument
+ or the :envvar:`DTS_TIMEOUT` environment variable.
+
+ Returns:
+ If the link came up in time or not.
+ """
+ time_to_stop = time.time() + timeout
+ while time.time() < time_to_stop:
+ port_info = self.send_command(f"show port info {port_id}")
+ if "Link status: up" in port_info:
+ break
+ time.sleep(0.5)
+ else:
+ self._logger.error(f"The link for port {port_id} did not come up in the given timeout.")
+ return "Link status: up" in port_info
+
+ def set_forward_mode(self, mode: TestPmdForwardingModes):
+ """Set packet forwarding mode.
+
+ Args:
+ mode: The forwarding mode to use.
+ """
+ self.send_command(f"set fwd {mode.value}")
+
+ def close(self) -> None:
+ self.send_command("exit", "")
+ return super().close()
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 2/7] dts: allow passing parameters into interactive apps
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (9 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
` (12 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Modified interactive applications to allow for the ability to pass
parameters into the app on start up. Also modified the way EAL
parameters are handled so that the trailing "--" separator is added be
default after all EAL parameters.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
.../remote_session/remote/testpmd_shell.py | 2 +-
dts/framework/testbed_model/sut_node.py | 16 ++++++++++------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
index b5e4cba9b3..369807a33e 100644
--- a/dts/framework/remote_session/remote/testpmd_shell.py
+++ b/dts/framework/remote_session/remote/testpmd_shell.py
@@ -62,7 +62,7 @@ class TestPmdShell(InteractiveShell):
def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
"""See "_start_application" in InteractiveShell."""
- self._app_args += " -- -i"
+ self._app_args += " -i"
super()._start_application(get_privileged_command)
def start(self, verify: bool = True) -> None:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 7f75043bd3..9c92232d9e 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -361,7 +361,8 @@ def create_interactive_shell(
shell_cls: Type[InteractiveShellType],
timeout: float = SETTINGS.timeout,
privileged: bool = False,
- eal_parameters: EalParameters | str | None = None,
+ eal_parameters: EalParameters | None = None,
+ app_parameters: str = "",
) -> InteractiveShellType:
"""Factory method for creating a handler for an interactive session.
@@ -376,19 +377,22 @@ def create_interactive_shell(
eal_parameters: List of EAL parameters to use to launch the app. If this
isn't provided or an empty string is passed, it will default to calling
create_eal_parameters().
+ app_parameters: Additional arguments to pass into the application on the
+ command-line.
Returns:
Instance of the desired interactive application.
"""
- if not eal_parameters:
- eal_parameters = self.create_eal_parameters()
-
- # We need to append the build directory for DPDK apps
+ # We need to append the build directory and add EAL parameters for DPDK apps
if shell_cls.dpdk_app:
+ if not eal_parameters:
+ eal_parameters = self.create_eal_parameters()
+ app_parameters = f"{eal_parameters} -- {app_parameters}"
+
shell_cls.path = self.main_session.join_remote_path(
self.remote_dpdk_build_dir, shell_cls.path
)
- return super().create_interactive_shell(shell_cls, timeout, privileged, str(eal_parameters))
+ return super().create_interactive_shell(shell_cls, timeout, privileged, app_parameters)
def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
"""Bind all ports on the SUT to a driver.
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (10 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
` (11 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added the options to filter out LLDP and ARP packets when
sniffing for packets with scapy. This was done using BPF filters to
ensure that the noise these packets provide does not interfere with test
cases.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/test_suite.py | 14 ++++++++--
.../capturing_traffic_generator.py | 22 ++++++++++++++-
dts/framework/testbed_model/scapy.py | 28 ++++++++++++++++++-
dts/framework/testbed_model/tg_node.py | 12 ++++++--
4 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 4a7907ec33..6dfa570041 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -27,6 +27,7 @@
from .settings import SETTINGS
from .test_result import BuildTargetResult, Result, TestCaseResult, TestSuiteResult
from .testbed_model import SutNode, TGNode
+from .testbed_model.capturing_traffic_generator import PacketFilteringConfig
from .testbed_model.hw.port import Port, PortLink
from .utils import get_packet_summaries
@@ -149,7 +150,12 @@ def configure_testbed_ipv4(self, restore: bool = False) -> None:
def _configure_ipv4_forwarding(self, enable: bool) -> None:
self.sut_node.configure_ipv4_forwarding(enable)
- def send_packet_and_capture(self, packet: Packet, duration: float = 1) -> list[Packet]:
+ def send_packet_and_capture(
+ self,
+ packet: Packet,
+ filter_config: PacketFilteringConfig = PacketFilteringConfig(),
+ duration: float = 1,
+ ) -> list[Packet]:
"""
Send a packet through the appropriate interface and
receive on the appropriate interface.
@@ -158,7 +164,11 @@ def send_packet_and_capture(self, packet: Packet, duration: float = 1) -> list[P
"""
packet = self._adjust_addresses(packet)
return self.tg_node.send_packet_and_capture(
- packet, self._tg_port_egress, self._tg_port_ingress, duration
+ packet,
+ self._tg_port_egress,
+ self._tg_port_ingress,
+ filter_config,
+ duration,
)
def get_expected_packet(self, packet: Packet) -> Packet:
diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b/dts/framework/testbed_model/capturing_traffic_generator.py
index e6512061d7..c40b030fe4 100644
--- a/dts/framework/testbed_model/capturing_traffic_generator.py
+++ b/dts/framework/testbed_model/capturing_traffic_generator.py
@@ -11,6 +11,7 @@
import uuid
from abc import abstractmethod
+from dataclasses import dataclass
import scapy.utils # type: ignore[import]
from scapy.packet import Packet # type: ignore[import]
@@ -29,6 +30,19 @@ def _get_default_capture_name() -> str:
return str(uuid.uuid4())
+@dataclass(slots=True)
+class PacketFilteringConfig:
+ """The supported filtering options for :class:`CapturingTrafficGenerator`.
+
+ Attributes:
+ no_lldp: If :data:`True`, LLDP packets will be filtered out when capturing.
+ no_arp: If :data:`True`, ARP packets will be filtered out when capturing.
+ """
+
+ no_lldp: bool = True
+ no_arp: bool = True
+
+
class CapturingTrafficGenerator(TrafficGenerator):
"""Capture packets after sending traffic.
@@ -51,6 +65,7 @@ def send_packet_and_capture(
packet: Packet,
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
@@ -64,6 +79,7 @@ def send_packet_and_capture(
packet: The packet to send.
send_port: The egress port on the TG node.
receive_port: The ingress port in the TG node.
+ filter_config: Filters to apply when capturing packets.
duration: Capture traffic for this amount of time after sending the packet.
capture_name: The name of the .pcap file where to store the capture.
@@ -71,7 +87,7 @@ def send_packet_and_capture(
A list of received packets. May be empty if no packets are captured.
"""
return self.send_packets_and_capture(
- [packet], send_port, receive_port, duration, capture_name
+ [packet], send_port, receive_port, filter_config, duration, capture_name
)
def send_packets_and_capture(
@@ -79,6 +95,7 @@ def send_packets_and_capture(
packets: list[Packet],
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
@@ -92,6 +109,7 @@ def send_packets_and_capture(
packets: The packets to send.
send_port: The egress port on the TG node.
receive_port: The ingress port in the TG node.
+ filter_config: Filters to apply when capturing packets.
duration: Capture traffic for this amount of time after sending the packets.
capture_name: The name of the .pcap file where to store the capture.
@@ -106,6 +124,7 @@ def send_packets_and_capture(
packets,
send_port,
receive_port,
+ filter_config,
duration,
)
@@ -119,6 +138,7 @@ def _send_packets_and_capture(
packets: list[Packet],
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
) -> list[Packet]:
"""
diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed_model/scapy.py
index 9083e92b3d..94b0af7c6f 100644
--- a/dts/framework/testbed_model/scapy.py
+++ b/dts/framework/testbed_model/scapy.py
@@ -30,6 +30,7 @@
from .capturing_traffic_generator import (
CapturingTrafficGenerator,
+ PacketFilteringConfig,
_get_default_capture_name,
)
from .hw.port import Port
@@ -69,6 +70,7 @@ def scapy_send_packets_and_capture(
send_iface: str,
recv_iface: str,
duration: float,
+ sniff_filter: str,
) -> list[bytes]:
"""RPC function to send and capture packets.
@@ -90,6 +92,7 @@ def scapy_send_packets_and_capture(
iface=recv_iface,
store=True,
started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
+ filter=sniff_filter,
)
sniffer.start()
time.sleep(duration)
@@ -249,16 +252,38 @@ def _send_packets(self, packets: list[Packet], port: Port) -> None:
packets = [packet.build() for packet in packets]
self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
+ def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
+ """Combines filter settings from `filter_config` into a BPF that scapy can use.
+
+ Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
+ collected based on various attributes of the packet.
+
+ Args:
+ filter_config: Config class that specifies which filters should be applied.
+
+ Returns:
+ A string representing the combination of BPF filters to be passed to scapy. For
+ example:
+
+ "ether[12:2] != 0x88cc && ether[12:2] != 0x0806"
+ """
+ bpf_filter: list[str] = []
+ if filter_config.no_arp:
+ bpf_filter.append("ether[12:2] != 0x0806")
+ if filter_config.no_lldp:
+ bpf_filter.append("ether[12:2] != 0x88cc")
+ return " && ".join(bpf_filter)
+
def _send_packets_and_capture(
self,
packets: list[Packet],
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
binary_packets = [packet.build() for packet in packets]
-
xmlrpc_packets: list[
xmlrpc.client.Binary
] = self.rpc_server_proxy.scapy_send_packets_and_capture(
@@ -266,6 +291,7 @@ def _send_packets_and_capture(
send_port.logical_name,
receive_port.logical_name,
duration,
+ self._create_packet_filter(filter_config),
) # type: ignore[assignment]
scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 79a55663b5..475dc2968d 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -23,7 +23,10 @@
)
from framework.exception import ConfigurationError
-from .capturing_traffic_generator import CapturingTrafficGenerator
+from .capturing_traffic_generator import (
+ CapturingTrafficGenerator,
+ PacketFilteringConfig,
+)
from .hw.port import Port
from .node import Node
@@ -53,6 +56,7 @@ def send_packet_and_capture(
packet: Packet,
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig = PacketFilteringConfig(),
duration: float = 1,
) -> list[Packet]:
"""Send a packet, return received traffic.
@@ -71,7 +75,11 @@ def send_packet_and_capture(
A list of received packets. May be empty if no packets are captured.
"""
return self.traffic_generator.send_packet_and_capture(
- packet, send_port, receive_port, duration
+ packet,
+ send_port,
+ receive_port,
+ filter_config,
+ duration,
)
def close(self) -> None:
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 4/7] dts: add pci addresses to EAL parameters
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (11 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
` (10 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added allow list to the EAL parameters created in DTS to ensure that
only the relevant PCI devices are considered when launching DPDK
applications.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/testbed_model/sut_node.py | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 9c92232d9e..77caea2fc9 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -20,6 +20,7 @@
from framework.utils import MesonArgs
from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice
+from .hw.port import Port
from .node import Node
@@ -31,6 +32,7 @@ def __init__(
prefix: str,
no_pci: bool,
vdevs: list[VirtualDevice],
+ ports: list[Port],
other_eal_param: str,
):
"""
@@ -46,6 +48,7 @@ def __init__(
VirtualDevice('net_ring0'),
VirtualDevice('net_ring1')
]
+ :param ports: the list of ports to allow.
:param other_eal_param: user defined DPDK eal parameters, eg:
other_eal_param='--single-file-segments'
"""
@@ -56,6 +59,7 @@ def __init__(
self._prefix = f"--file-prefix={prefix}"
self._no_pci = "--no-pci" if no_pci else ""
self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
+ self._ports = " ".join(f"-a {port.pci}" for port in ports)
self._other_eal_param = other_eal_param
def __str__(self) -> str:
@@ -65,6 +69,7 @@ def __str__(self) -> str:
f"{self._prefix} "
f"{self._no_pci} "
f"{self._vdevs} "
+ f"{self._ports} "
f"{self._other_eal_param}"
)
@@ -294,6 +299,7 @@ def create_eal_parameters(
append_prefix_timestamp: bool = True,
no_pci: bool = False,
vdevs: list[VirtualDevice] = None,
+ ports: list[Port] | None = None,
other_eal_param: str = "",
) -> "EalParameters":
"""
@@ -317,6 +323,7 @@ def create_eal_parameters(
VirtualDevice('net_ring0'),
VirtualDevice('net_ring1')
]
+ :param ports: the list of ports to allow.
:param other_eal_param: user defined DPDK eal parameters, eg:
other_eal_param='--single-file-segments'
:return: eal param string, eg:
@@ -334,12 +341,16 @@ def create_eal_parameters(
if vdevs is None:
vdevs = []
+ if ports is None:
+ ports = self.ports
+
return EalParameters(
lcore_list=lcore_list,
memory_channels=self.config.memory_channels,
prefix=prefix,
no_pci=no_pci,
vdevs=vdevs,
+ ports=ports,
other_eal_param=other_eal_param,
)
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 5/7] dts: allow configuring MTU of ports
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (12 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
` (9 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Adds methods in both os_session and linux session to allow for setting
MTU of port interfaces in an OS agnostic way.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/remote_session/linux_session.py | 8 ++++++++
dts/framework/remote_session/os_session.py | 9 +++++++++
2 files changed, 17 insertions(+)
diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
index fd877fbfae..aaa4d57a36 100644
--- a/dts/framework/remote_session/linux_session.py
+++ b/dts/framework/remote_session/linux_session.py
@@ -177,6 +177,14 @@ def configure_port_ip_address(
verify=True,
)
+ def configure_port_mtu(self, mtu: int, port: Port) -> None:
+ """Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""
+ self.send_command(
+ f"ip link set dev {port.logical_name} mtu {mtu}",
+ privileged=True,
+ verify=True,
+ )
+
def configure_ipv4_forwarding(self, enable: bool) -> None:
state = 1 if enable else 0
self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
index 8a709eac1c..cd073f5774 100644
--- a/dts/framework/remote_session/os_session.py
+++ b/dts/framework/remote_session/os_session.py
@@ -277,6 +277,15 @@ def configure_port_ip_address(
Configure (add or delete) an IP address of the input port.
"""
+ @abstractmethod
+ def configure_port_mtu(self, mtu: int, port: Port) -> None:
+ """Configure `mtu` on `port`.
+
+ Args:
+ mtu: Desired MTU value.
+ port: Port to set `mtu` on.
+ """
+
@abstractmethod
def configure_ipv4_forwarding(self, enable: bool) -> None:
"""
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 6/7] dts: add scatter to the yaml schema
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (13 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-14 22:10 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
` (8 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Allow for scatter to be specified in the configuration file.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/config/conf_yaml_schema.json | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 84e45fe3c2..e6dc50ca7f 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -186,7 +186,8 @@
"type": "string",
"enum": [
"hello_world",
- "os_udp"
+ "os_udp",
+ "pmd_buffer_scatter"
]
},
"test_target": {
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 7/7] dts: add scatter test suite
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (14 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
@ 2023-12-14 22:10 ` jspewock
2023-12-18 17:22 ` [PATCH v4 0/7] dts: Port scatter suite over jspewock
` (7 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-14 22:10 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
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
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).
+"""
+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)
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 0/7] dts: Port scatter suite over
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (15 preceding siblings ...)
2023-12-14 22:10 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
` (6 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
v4:
Addresses comments on the previous version. Main things which were added
include specific methods within the testpmd shell to avoid the need of
calling send_command directly, as well as docstring comment updates.
This series had to get resubmitted so that it would show up as one
series on patchwork. Previously each patch was given its own series by
mistake. Seeing as there was no change otherwise, submitting this as v4
once again.
Jeremy Spewock (7):
dts: add required methods to testpmd_shell
dts: allow passing parameters into interactive apps
dts: add optional packet filtering to scapy sniffer
dts: add pci addresses to EAL parameters
dts: allow configuring MTU of ports
dts: add scatter to the yaml schema
dts: add scatter test suite
dts/framework/config/conf_yaml_schema.json | 3 +-
dts/framework/exception.py | 4 +
dts/framework/remote_session/linux_session.py | 8 ++
dts/framework/remote_session/os_session.py | 9 ++
.../remote_session/remote/testpmd_shell.py | 94 +++++++++++++++-
dts/framework/test_suite.py | 14 ++-
.../capturing_traffic_generator.py | 22 +++-
dts/framework/testbed_model/scapy.py | 28 ++++-
dts/framework/testbed_model/sut_node.py | 27 ++++-
dts/framework/testbed_model/tg_node.py | 12 +-
dts/tests/TestSuite_pmd_buffer_scatter.py | 105 ++++++++++++++++++
11 files changed, 312 insertions(+), 14 deletions(-)
create mode 100644 dts/tests/TestSuite_pmd_buffer_scatter.py
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 1/7] dts: add required methods to testpmd_shell
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (16 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 0/7] dts: Port scatter suite over jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
` (5 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added a method within the testpmd interactive shell that polls the
status of ports and verifies that the link status on a given port is
"up." Polling will continue until either the link comes up, or the
timeout is reached. Also added methods for starting and stopping packet
forwarding in testpmd and a method for setting the forwarding mode on
testpmd. The method for starting packet forwarding will also attempt to
verify that forwarding did indeed start by default.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/exception.py | 4 +
.../remote_session/remote/testpmd_shell.py | 92 +++++++++++++++++++
2 files changed, 96 insertions(+)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index b362e42924..e36db20e32 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -119,6 +119,10 @@ def __str__(self) -> str:
return f"Command {self.command} returned a non-zero exit code: {self.command_return_code}"
+class InteractiveCommandExecutionError(DTSError):
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.REMOTE_CMD_EXEC_ERR
+
+
class RemoteDirectoryExistsError(DTSError):
"""
Raised when a remote directory to be created already exists.
diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
index 08ac311016..b5e4cba9b3 100644
--- a/dts/framework/remote_session/remote/testpmd_shell.py
+++ b/dts/framework/remote_session/remote/testpmd_shell.py
@@ -1,9 +1,15 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2023 University of New Hampshire
+import time
+from enum import auto
from pathlib import PurePath
from typing import Callable
+from framework.exception import InteractiveCommandExecutionError
+from framework.settings import SETTINGS
+from framework.utils import StrEnum
+
from .interactive_shell import InteractiveShell
@@ -17,6 +23,37 @@ def __str__(self) -> str:
return self.pci_address
+class TestPmdForwardingModes(StrEnum):
+ r"""The supported packet forwarding modes for :class:`~TestPmdShell`\s"""
+
+ #:
+ io = auto()
+ #:
+ mac = auto()
+ #:
+ macswap = auto()
+ #:
+ flowgen = auto()
+ #:
+ rxonly = auto()
+ #:
+ txonly = auto()
+ #:
+ csum = auto()
+ #:
+ icmpecho = auto()
+ #:
+ ieee1588 = auto()
+ #:
+ noisy = auto()
+ #:
+ fivetswap = "5tswap"
+ #:
+ shared_rxq = "shared-rxq"
+ #:
+ recycle_mbufs = auto()
+
+
class TestPmdShell(InteractiveShell):
path: PurePath = PurePath("app", "dpdk-testpmd")
dpdk_app: bool = True
@@ -28,6 +65,27 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None
self._app_args += " -- -i"
super()._start_application(get_privileged_command)
+ def start(self, verify: bool = True) -> None:
+ """Start packet forwarding with the current configuration.
+
+ Args:
+ verify: If :data:`True` , a second start command will be sent in an attempt to verify
+ packet forwarding started as expected.
+
+ Raises:
+ InteractiveCommandExecutionError: If `verify` is :data:`True` and forwarding fails to
+ start.
+ """
+ self.send_command("start")
+ if verify:
+ # If forwarding was already started, sending "start" again should tell us
+ if "Packet forwarding already started" not in self.send_command("start"):
+ raise InteractiveCommandExecutionError("Testpmd failed to start packet forwarding.")
+
+ def stop(self) -> None:
+ """Stop packet forwarding."""
+ self.send_command("stop")
+
def get_devices(self) -> list[TestPmdDevice]:
"""Get a list of device names that are known to testpmd
@@ -43,3 +101,37 @@ def get_devices(self) -> list[TestPmdDevice]:
if "device name:" in line.lower():
dev_list.append(TestPmdDevice(line))
return dev_list
+
+ def wait_link_status_up(self, port_id: int, timeout=SETTINGS.timeout) -> bool:
+ """Wait until the link status on the given port is "up".
+
+ Arguments:
+ port_id: Port to check the link status on.
+ timeout: Time to wait for the link to come up. The default value for this
+ argument is set using the :option:`-t, --timeout` command-line argument
+ or the :envvar:`DTS_TIMEOUT` environment variable.
+
+ Returns:
+ If the link came up in time or not.
+ """
+ time_to_stop = time.time() + timeout
+ while time.time() < time_to_stop:
+ port_info = self.send_command(f"show port info {port_id}")
+ if "Link status: up" in port_info:
+ break
+ time.sleep(0.5)
+ else:
+ self._logger.error(f"The link for port {port_id} did not come up in the given timeout.")
+ return "Link status: up" in port_info
+
+ def set_forward_mode(self, mode: TestPmdForwardingModes):
+ """Set packet forwarding mode.
+
+ Args:
+ mode: The forwarding mode to use.
+ """
+ self.send_command(f"set fwd {mode.value}")
+
+ def close(self) -> None:
+ self.send_command("exit", "")
+ return super().close()
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 2/7] dts: allow passing parameters into interactive apps
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (17 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 1/7] dts: add required methods to testpmd_shell jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
` (4 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Modified interactive applications to allow for the ability to pass
parameters into the app on start up. Also modified the way EAL
parameters are handled so that the trailing "--" separator is added be
default after all EAL parameters.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
.../remote_session/remote/testpmd_shell.py | 2 +-
dts/framework/testbed_model/sut_node.py | 16 ++++++++++------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/remote/testpmd_shell.py
index b5e4cba9b3..369807a33e 100644
--- a/dts/framework/remote_session/remote/testpmd_shell.py
+++ b/dts/framework/remote_session/remote/testpmd_shell.py
@@ -62,7 +62,7 @@ class TestPmdShell(InteractiveShell):
def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
"""See "_start_application" in InteractiveShell."""
- self._app_args += " -- -i"
+ self._app_args += " -i"
super()._start_application(get_privileged_command)
def start(self, verify: bool = True) -> None:
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 7f75043bd3..9c92232d9e 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -361,7 +361,8 @@ def create_interactive_shell(
shell_cls: Type[InteractiveShellType],
timeout: float = SETTINGS.timeout,
privileged: bool = False,
- eal_parameters: EalParameters | str | None = None,
+ eal_parameters: EalParameters | None = None,
+ app_parameters: str = "",
) -> InteractiveShellType:
"""Factory method for creating a handler for an interactive session.
@@ -376,19 +377,22 @@ def create_interactive_shell(
eal_parameters: List of EAL parameters to use to launch the app. If this
isn't provided or an empty string is passed, it will default to calling
create_eal_parameters().
+ app_parameters: Additional arguments to pass into the application on the
+ command-line.
Returns:
Instance of the desired interactive application.
"""
- if not eal_parameters:
- eal_parameters = self.create_eal_parameters()
-
- # We need to append the build directory for DPDK apps
+ # We need to append the build directory and add EAL parameters for DPDK apps
if shell_cls.dpdk_app:
+ if not eal_parameters:
+ eal_parameters = self.create_eal_parameters()
+ app_parameters = f"{eal_parameters} -- {app_parameters}"
+
shell_cls.path = self.main_session.join_remote_path(
self.remote_dpdk_build_dir, shell_cls.path
)
- return super().create_interactive_shell(shell_cls, timeout, privileged, str(eal_parameters))
+ return super().create_interactive_shell(shell_cls, timeout, privileged, app_parameters)
def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
"""Bind all ports on the SUT to a driver.
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (18 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 2/7] dts: allow passing parameters into interactive apps jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
` (3 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added the options to filter out LLDP and ARP packets when
sniffing for packets with scapy. This was done using BPF filters to
ensure that the noise these packets provide does not interfere with test
cases.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/test_suite.py | 14 ++++++++--
.../capturing_traffic_generator.py | 22 ++++++++++++++-
dts/framework/testbed_model/scapy.py | 28 ++++++++++++++++++-
dts/framework/testbed_model/tg_node.py | 12 ++++++--
4 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 4a7907ec33..6dfa570041 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -27,6 +27,7 @@
from .settings import SETTINGS
from .test_result import BuildTargetResult, Result, TestCaseResult, TestSuiteResult
from .testbed_model import SutNode, TGNode
+from .testbed_model.capturing_traffic_generator import PacketFilteringConfig
from .testbed_model.hw.port import Port, PortLink
from .utils import get_packet_summaries
@@ -149,7 +150,12 @@ def configure_testbed_ipv4(self, restore: bool = False) -> None:
def _configure_ipv4_forwarding(self, enable: bool) -> None:
self.sut_node.configure_ipv4_forwarding(enable)
- def send_packet_and_capture(self, packet: Packet, duration: float = 1) -> list[Packet]:
+ def send_packet_and_capture(
+ self,
+ packet: Packet,
+ filter_config: PacketFilteringConfig = PacketFilteringConfig(),
+ duration: float = 1,
+ ) -> list[Packet]:
"""
Send a packet through the appropriate interface and
receive on the appropriate interface.
@@ -158,7 +164,11 @@ def send_packet_and_capture(self, packet: Packet, duration: float = 1) -> list[P
"""
packet = self._adjust_addresses(packet)
return self.tg_node.send_packet_and_capture(
- packet, self._tg_port_egress, self._tg_port_ingress, duration
+ packet,
+ self._tg_port_egress,
+ self._tg_port_ingress,
+ filter_config,
+ duration,
)
def get_expected_packet(self, packet: Packet) -> Packet:
diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b/dts/framework/testbed_model/capturing_traffic_generator.py
index e6512061d7..c40b030fe4 100644
--- a/dts/framework/testbed_model/capturing_traffic_generator.py
+++ b/dts/framework/testbed_model/capturing_traffic_generator.py
@@ -11,6 +11,7 @@
import uuid
from abc import abstractmethod
+from dataclasses import dataclass
import scapy.utils # type: ignore[import]
from scapy.packet import Packet # type: ignore[import]
@@ -29,6 +30,19 @@ def _get_default_capture_name() -> str:
return str(uuid.uuid4())
+@dataclass(slots=True)
+class PacketFilteringConfig:
+ """The supported filtering options for :class:`CapturingTrafficGenerator`.
+
+ Attributes:
+ no_lldp: If :data:`True`, LLDP packets will be filtered out when capturing.
+ no_arp: If :data:`True`, ARP packets will be filtered out when capturing.
+ """
+
+ no_lldp: bool = True
+ no_arp: bool = True
+
+
class CapturingTrafficGenerator(TrafficGenerator):
"""Capture packets after sending traffic.
@@ -51,6 +65,7 @@ def send_packet_and_capture(
packet: Packet,
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
@@ -64,6 +79,7 @@ def send_packet_and_capture(
packet: The packet to send.
send_port: The egress port on the TG node.
receive_port: The ingress port in the TG node.
+ filter_config: Filters to apply when capturing packets.
duration: Capture traffic for this amount of time after sending the packet.
capture_name: The name of the .pcap file where to store the capture.
@@ -71,7 +87,7 @@ def send_packet_and_capture(
A list of received packets. May be empty if no packets are captured.
"""
return self.send_packets_and_capture(
- [packet], send_port, receive_port, duration, capture_name
+ [packet], send_port, receive_port, filter_config, duration, capture_name
)
def send_packets_and_capture(
@@ -79,6 +95,7 @@ def send_packets_and_capture(
packets: list[Packet],
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
@@ -92,6 +109,7 @@ def send_packets_and_capture(
packets: The packets to send.
send_port: The egress port on the TG node.
receive_port: The ingress port in the TG node.
+ filter_config: Filters to apply when capturing packets.
duration: Capture traffic for this amount of time after sending the packets.
capture_name: The name of the .pcap file where to store the capture.
@@ -106,6 +124,7 @@ def send_packets_and_capture(
packets,
send_port,
receive_port,
+ filter_config,
duration,
)
@@ -119,6 +138,7 @@ def _send_packets_and_capture(
packets: list[Packet],
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
) -> list[Packet]:
"""
diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed_model/scapy.py
index 9083e92b3d..94b0af7c6f 100644
--- a/dts/framework/testbed_model/scapy.py
+++ b/dts/framework/testbed_model/scapy.py
@@ -30,6 +30,7 @@
from .capturing_traffic_generator import (
CapturingTrafficGenerator,
+ PacketFilteringConfig,
_get_default_capture_name,
)
from .hw.port import Port
@@ -69,6 +70,7 @@ def scapy_send_packets_and_capture(
send_iface: str,
recv_iface: str,
duration: float,
+ sniff_filter: str,
) -> list[bytes]:
"""RPC function to send and capture packets.
@@ -90,6 +92,7 @@ def scapy_send_packets_and_capture(
iface=recv_iface,
store=True,
started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
+ filter=sniff_filter,
)
sniffer.start()
time.sleep(duration)
@@ -249,16 +252,38 @@ def _send_packets(self, packets: list[Packet], port: Port) -> None:
packets = [packet.build() for packet in packets]
self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
+ def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
+ """Combines filter settings from `filter_config` into a BPF that scapy can use.
+
+ Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
+ collected based on various attributes of the packet.
+
+ Args:
+ filter_config: Config class that specifies which filters should be applied.
+
+ Returns:
+ A string representing the combination of BPF filters to be passed to scapy. For
+ example:
+
+ "ether[12:2] != 0x88cc && ether[12:2] != 0x0806"
+ """
+ bpf_filter: list[str] = []
+ if filter_config.no_arp:
+ bpf_filter.append("ether[12:2] != 0x0806")
+ if filter_config.no_lldp:
+ bpf_filter.append("ether[12:2] != 0x88cc")
+ return " && ".join(bpf_filter)
+
def _send_packets_and_capture(
self,
packets: list[Packet],
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig,
duration: float,
capture_name: str = _get_default_capture_name(),
) -> list[Packet]:
binary_packets = [packet.build() for packet in packets]
-
xmlrpc_packets: list[
xmlrpc.client.Binary
] = self.rpc_server_proxy.scapy_send_packets_and_capture(
@@ -266,6 +291,7 @@ def _send_packets_and_capture(
send_port.logical_name,
receive_port.logical_name,
duration,
+ self._create_packet_filter(filter_config),
) # type: ignore[assignment]
scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 79a55663b5..475dc2968d 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -23,7 +23,10 @@
)
from framework.exception import ConfigurationError
-from .capturing_traffic_generator import CapturingTrafficGenerator
+from .capturing_traffic_generator import (
+ CapturingTrafficGenerator,
+ PacketFilteringConfig,
+)
from .hw.port import Port
from .node import Node
@@ -53,6 +56,7 @@ def send_packet_and_capture(
packet: Packet,
send_port: Port,
receive_port: Port,
+ filter_config: PacketFilteringConfig = PacketFilteringConfig(),
duration: float = 1,
) -> list[Packet]:
"""Send a packet, return received traffic.
@@ -71,7 +75,11 @@ def send_packet_and_capture(
A list of received packets. May be empty if no packets are captured.
"""
return self.traffic_generator.send_packet_and_capture(
- packet, send_port, receive_port, duration
+ packet,
+ send_port,
+ receive_port,
+ filter_config,
+ duration,
)
def close(self) -> None:
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 4/7] dts: add pci addresses to EAL parameters
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (19 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 3/7] dts: add optional packet filtering to scapy sniffer jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
` (2 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Added allow list to the EAL parameters created in DTS to ensure that
only the relevant PCI devices are considered when launching DPDK
applications.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/testbed_model/sut_node.py | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 9c92232d9e..77caea2fc9 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -20,6 +20,7 @@
from framework.utils import MesonArgs
from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice
+from .hw.port import Port
from .node import Node
@@ -31,6 +32,7 @@ def __init__(
prefix: str,
no_pci: bool,
vdevs: list[VirtualDevice],
+ ports: list[Port],
other_eal_param: str,
):
"""
@@ -46,6 +48,7 @@ def __init__(
VirtualDevice('net_ring0'),
VirtualDevice('net_ring1')
]
+ :param ports: the list of ports to allow.
:param other_eal_param: user defined DPDK eal parameters, eg:
other_eal_param='--single-file-segments'
"""
@@ -56,6 +59,7 @@ def __init__(
self._prefix = f"--file-prefix={prefix}"
self._no_pci = "--no-pci" if no_pci else ""
self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
+ self._ports = " ".join(f"-a {port.pci}" for port in ports)
self._other_eal_param = other_eal_param
def __str__(self) -> str:
@@ -65,6 +69,7 @@ def __str__(self) -> str:
f"{self._prefix} "
f"{self._no_pci} "
f"{self._vdevs} "
+ f"{self._ports} "
f"{self._other_eal_param}"
)
@@ -294,6 +299,7 @@ def create_eal_parameters(
append_prefix_timestamp: bool = True,
no_pci: bool = False,
vdevs: list[VirtualDevice] = None,
+ ports: list[Port] | None = None,
other_eal_param: str = "",
) -> "EalParameters":
"""
@@ -317,6 +323,7 @@ def create_eal_parameters(
VirtualDevice('net_ring0'),
VirtualDevice('net_ring1')
]
+ :param ports: the list of ports to allow.
:param other_eal_param: user defined DPDK eal parameters, eg:
other_eal_param='--single-file-segments'
:return: eal param string, eg:
@@ -334,12 +341,16 @@ def create_eal_parameters(
if vdevs is None:
vdevs = []
+ if ports is None:
+ ports = self.ports
+
return EalParameters(
lcore_list=lcore_list,
memory_channels=self.config.memory_channels,
prefix=prefix,
no_pci=no_pci,
vdevs=vdevs,
+ ports=ports,
other_eal_param=other_eal_param,
)
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 5/7] dts: allow configuring MTU of ports
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (20 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 4/7] dts: add pci addresses to EAL parameters jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
2023-12-18 17:22 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Adds methods in both os_session and linux session to allow for setting
MTU of port interfaces in an OS agnostic way.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/remote_session/linux_session.py | 8 ++++++++
dts/framework/remote_session/os_session.py | 9 +++++++++
2 files changed, 17 insertions(+)
diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
index fd877fbfae..aaa4d57a36 100644
--- a/dts/framework/remote_session/linux_session.py
+++ b/dts/framework/remote_session/linux_session.py
@@ -177,6 +177,14 @@ def configure_port_ip_address(
verify=True,
)
+ def configure_port_mtu(self, mtu: int, port: Port) -> None:
+ """Overrides :meth:`~.os_session.OSSession.configure_port_mtu`."""
+ self.send_command(
+ f"ip link set dev {port.logical_name} mtu {mtu}",
+ privileged=True,
+ verify=True,
+ )
+
def configure_ipv4_forwarding(self, enable: bool) -> None:
state = 1 if enable else 0
self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
index 8a709eac1c..cd073f5774 100644
--- a/dts/framework/remote_session/os_session.py
+++ b/dts/framework/remote_session/os_session.py
@@ -277,6 +277,15 @@ def configure_port_ip_address(
Configure (add or delete) an IP address of the input port.
"""
+ @abstractmethod
+ def configure_port_mtu(self, mtu: int, port: Port) -> None:
+ """Configure `mtu` on `port`.
+
+ Args:
+ mtu: Desired MTU value.
+ port: Port to set `mtu` on.
+ """
+
@abstractmethod
def configure_ipv4_forwarding(self, enable: bool) -> None:
"""
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 6/7] dts: add scatter to the yaml schema
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (21 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 5/7] dts: allow configuring MTU of ports jspewock
@ 2023-12-18 17:22 ` jspewock
2023-12-18 17:22 ` [PATCH v4 7/7] dts: add scatter test suite jspewock
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
Allow for scatter to be specified in the configuration file.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/config/conf_yaml_schema.json | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 84e45fe3c2..e6dc50ca7f 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -186,7 +186,8 @@
"type": "string",
"enum": [
"hello_world",
- "os_udp"
+ "os_udp",
+ "pmd_buffer_scatter"
]
},
"test_target": {
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [PATCH v4 7/7] dts: add scatter test suite
2023-11-13 20:28 [PATCH v3 0/7] dts: Port scatter suite over jspewock
` (22 preceding siblings ...)
2023-12-18 17:22 ` [PATCH v4 6/7] dts: add scatter to the yaml schema jspewock
@ 2023-12-18 17:22 ` jspewock
23 siblings, 0 replies; 43+ messages in thread
From: jspewock @ 2023-12-18 17:22 UTC (permalink / raw)
To: Honnappa.Nagarahalli, juraj.linkes, thomas, wathsala.vithanage,
probb, paul.szczepanek, yoan.picchi, ferruh.yigit,
andrew.rybchenko
Cc: dev, Jeremy Spewock
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
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).
+"""
+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)
--
2.43.0
^ permalink raw reply [flat|nested] 43+ messages in thread