[v3,2/3] Initial implementation for VLAN test suite

Message ID 20240614150238.26374-3-dmarx@iol.unh.edu (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series VLAN Test Suite |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Dean Marx June 14, 2024, 3:02 p.m. UTC
  Test suite for ensuring Poll Mode Driver can enable or disable
vlan filtering, stripping, and header insertion of packets sent on
a port.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/tests/TestSuite_vlan.py | 172 ++++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 dts/tests/TestSuite_vlan.py
  

Comments

Patrick Robb June 14, 2024, 4:19 p.m. UTC | #1
Looks promising thanks - some comments below.

On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:
> +class TestVlan(TestSuite):
> +    """DPDK VLAN test suite.
> +
> +    Ensures VLAN packet reception on the Poll Mode Driver when certain conditions are met.
> +    If one or more of these conditions are not met, the packet reception should be unsuccessful.
> +    """
> +
> +    def set_up_suite(self) -> None:
> +        """Set up the test suite.
> +
> +        Setup:
> +            Create a testpmd session and set up tg nodes
> +            verify that at least two ports are open for session
> +        """
> +        self.verify(len(self._port_links) > 1, "Not enough ports")
> +
> +    def send_vlan_packet_and_verify(
> +        self, should_receive: bool = True, strip: bool = False, vlan_id: int = -1
> +    ) -> None:
> +        """Generate a vlan packet, send and verify on the dut.
> +
> +        Args:
> +            should_receive: indicate whether the packet should be successfully received
> +            vlan_id: expected vlan ID
> +            strip: indicates whether stripping is on or off,
> +            and when the vlan tag is checked for a match
> +        """
> +        data = "P" * 10

packing payload with "X" was some kind of convention with old DTS,
which we have adopted in other suites in new DTS too. Unless you have
a specific reason to not do this, we should probably use "X". Juraj
has also suggested that this be made a standard practice in new DTS
and document that (which is outside of the scope of this patch, just
figured I'd mention it).

> +        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load=data)
> +        received_packets = self.send_packet_and_capture(packet)
> +        received_packets = [
> +            packets
> +            for packets in received_packets
> +            if hasattr(packets, "load") and data in str((packets.load))
> +        ]
> +        if should_receive:
> +            self.verify(
> +                len(received_packets) == 1, "Packet was dropped when it should have been received"
> +            )

We do not rely on a quiet wire (LLDP packets and such may be
happening) and we have no need for relying on a quiet wire.

I think:
If should_receive == true, we validate with "do any of the packets in
received have the expected load/data"
If should_receive == false we validate with "do none of the packets in
received have the expected load/data / do we have no packets at all"

> +            received = received_packets[0]

We aren't going to use this assumption (which came from old dts) that
the packet we want to validate is the one at index 0. The scatter
suite (which is where I'm guessing this idea comes from as it is a
kind of reference testsuite currently) is being refactored to check
all the packets in received, instead of just received[0].

> +    def test_vlan_receipt_no_stripping(self) -> None:
> +        """Ensure vlan packet is dropped when receipts are enabled and header stripping is disabled.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")
> +        testpmd.vlan_filter_set_on(0)
> +        testpmd.rx_vlan_add(1, 0)
> +        testpmd.start()

Why does the testsuite add testpmd methods for setting vlan ids, but
not for setting promisc mode, verbose mode, etc? I think chatting with
Jeremy about this makes sense.

> +
> +        filtered_vlan = 1
> +        self.send_vlan_packet_and_verify(True, vlan_id=filtered_vlan)
> +        testpmd.close()
> +
> +    def test_vlan_receipt_stripping(self) -> None:
> +        """Ensure vlan packet received with no tag when receipts and header stripping are enabled.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")
> +        testpmd.vlan_filter_set_on(0)
> +        testpmd.rx_vlan_add(1, 0)
> +        testpmd.vlan_strip_set_on(0)
> +        testpmd.start()
> +
> +        self.send_vlan_packet_and_verify(should_receive=True, strip=True, vlan_id=1)
> +        testpmd.close()
> +
> +    def test_vlan_no_receipt(self) -> None:
> +        """Ensure vlan packet dropped when filter is on and sent tag not in the filter list.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")
> +        testpmd.vlan_filter_set_on(0)
> +        testpmd.rx_vlan_add(1, 0)
> +        testpmd.start()
> +
> +        filtered_vlan = 1
> +        self.send_vlan_packet_and_verify(should_receive=False, vlan_id=filtered_vlan + 1)
> +        testpmd.close()
> +
> +    def test_vlan_header_insertion(self) -> None:
> +        """Ensure that vlan packet is received with the correct inserted vlan tag.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a non-vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")
> +        testpmd.port_stop_all()
> +        testpmd.tx_vlan_set(1, 51)
> +        testpmd.port_start_all()
> +        testpmd.start()
> +
> +        self.send_packet_and_verify_insertion(expected_id=51)
> +        testpmd.close()
> +
> +    def tear_down_suite(self) -> None:
> +        """Tear down the suite."""
> --
> 2.44.0
>
  
Jeremy Spewock June 17, 2024, 2:56 p.m. UTC | #2
On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Test suite for ensuring Poll Mode Driver can enable or disable
> vlan filtering, stripping, and header insertion of packets sent on
> a port.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
<snip>
> +
> +class TestVlan(TestSuite):
> +    """DPDK VLAN test suite.
> +
> +    Ensures VLAN packet reception on the Poll Mode Driver when certain conditions are met.
> +    If one or more of these conditions are not met, the packet reception should be unsuccessful.
> +    """
> +
> +    def set_up_suite(self) -> None:
> +        """Set up the test suite.
> +
> +        Setup:
> +            Create a testpmd session and set up tg nodes

This isn't part of the setup of this suite, so you can probably leave
this sentence out.

> +            verify that at least two ports are open for session
> +        """
> +        self.verify(len(self._port_links) > 1, "Not enough ports")
> +
> +    def send_vlan_packet_and_verify(
> +        self, should_receive: bool = True, strip: bool = False, vlan_id: int = -1
> +    ) -> None:

I'm not sure it makes sense to default these parameters either. If we
never use the default value of -1 for vlan_id, then we might as well
make the argument positional instead of a keyword argument.

> +        """Generate a vlan packet, send and verify on the dut.
> +

Maybe you could add some more description about what is being verified
in this method to the body of doc-string just to make things more
clear.

> +        Args:
> +            should_receive: indicate whether the packet should be successfully received
> +            vlan_id: expected vlan ID
> +            strip: indicates whether stripping is on or off,
> +            and when the vlan tag is checked for a match

All of the individual arg definitions should end with periods and
start with capitalized words (the descriptions should be capitalized,
not the variable names). Additionally, the second line of the `strip`
description should be indented just to show it's a continuation line.
You can probably fit some more of this continuation on the line on the
one above as well if that's easier.

> +        """
> +        data = "P" * 10
> +        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load=data)
> +        received_packets = self.send_packet_and_capture(packet)
> +        received_packets = [
> +            packets
> +            for packets in received_packets
> +            if hasattr(packets, "load") and data in str((packets.load))
> +        ]

This might be easier to do with the built-in `filter` function in python:
list(filter(lambda p: hasattr(p, "load") and data in str(p.load),
received_packets))

Although what you have now is also intuitive and does the same thing
so it doesn't really matter either way.

> +        if should_receive:
> +            self.verify(
> +                len(received_packets) == 1, "Packet was dropped when it should have been received"
> +            )
<snip>
 +
> +    def send_packet_and_verify_insertion(self, expected_id: int = -1) -> None:

This also uses an invalid ID but I think you could just make this
positional/required instead.

> +        """Generate a packet with no vlan tag, send and verify on the dut.
> +
> +        Args:
> +            expected_id: the vlan id that is being inserted through tx_offload configuration
> +            should_receive: indicate whether the packet should be successfully received

Same comment here about the args.

> +        """
> +        data = "P" * 10
> +        packet = Ether() / Raw(load=data)
> +        received_packets = self.send_packet_and_capture(packet)
> +        received_packets = [
> +            packets
> +            for packets in received_packets
> +            if hasattr(packets, "load") and data in str((packets.load))
> +        ]

The start of this method is almost exactly the same as the start of
the one for sending a VLAN packet. Maybe a different approach could be
sending the packets in the test method, and then making this method
instead just take in a list of packets and verifying them. Or, maybe
you could instead make a different method for sending packets, and
pass what that method returns into this one. Just to make sure there
is as little duplicated code as possible.

> +        self.verify(
> +            len(received_packets) == 1, "Packet was dropped when it should have been received"
> +        )
> +        received = received_packets[0]
> +        self.verify(Dot1Q in received, "The received packet did not have a vlan tag")
> +        self.verify(received.vlan == expected_id, "The received tag did not match the expected tag")
> +
> +    def test_vlan_receipt_no_stripping(self) -> None:
> +        """Ensure vlan packet is dropped when receipts are enabled and header stripping is disabled.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")

I saw Patrick also mentioned this, but I agree, we shouldn't need to
be calling the send_command method anywhere. Maybe what we should do
just to make sure people don't use it is override the method in
TestpmdShell so that it throws an exception. That way we would enforce
the rule a little more and it'd be less confusing.

> +        testpmd.vlan_filter_set_on(0)

This is also the default value for this method, so if we were going to
keep the defaults, this parameter would not be needed. However, we
still should probably make those arguments positional and leave this
as is.

> +        testpmd.rx_vlan_add(1, 0)

This could also be testpmd.rx_vlan_add(1) with the defaults I believe,
but that would be more ambiguous.

> +        testpmd.start()
> +
> +        filtered_vlan = 1

It might make more sense to define this variable above the call to
testpmd.rx_vlan_add and pass it into that function call as well. That
way it isn't hard-coded in some places and used as this variable in
others.

> +        self.send_vlan_packet_and_verify(True, vlan_id=filtered_vlan)
> +        testpmd.close()
> +
> +    def test_vlan_receipt_stripping(self) -> None:
> +        """Ensure vlan packet received with no tag when receipts and header stripping are enabled.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")
> +        testpmd.vlan_filter_set_on(0)
> +        testpmd.rx_vlan_add(1, 0)

This part of the test method is identical to
test_vlan_receipt_no_stripping, we could instead make one method for
testing called vlan_receipt_testing(self, stripping: bool) and then
set the vlan stripping to on if the boolean is true.

> +        testpmd.vlan_strip_set_on(0)
> +        testpmd.start()
> +
> +        self.send_vlan_packet_and_verify(should_receive=True, strip=True, vlan_id=1)
> +        testpmd.close()
> +
> +    def test_vlan_no_receipt(self) -> None:
> +        """Ensure vlan packet dropped when filter is on and sent tag not in the filter list.
> +
> +        Test:
> +            Create an interactive testpmd shell and verify a vlan packet.
> +        """
> +        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
> +        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
> +        testpmd.send_command("set verbose 1", "testpmd>")
> +        testpmd.send_command("set promisc 0 off", "testpmd>")
> +        testpmd.vlan_filter_set_on(0)
> +        testpmd.rx_vlan_add(1, 0)
> +        testpmd.start()
> +
> +        filtered_vlan = 1

This code is also identical to what is in
test_vlan_receipt_no_stripping. Maybe in that additional test method
you could also add a parameter for should_receive: bool and use that
to decide what to pass into the send_vlan_pacet_and_verify function:

def vlan_receipt_testig(self, stripping: bool, should_receive: bool)

^ As opposed to what I mentioned above with the vlan_receipt_testing function.

> +        self.send_vlan_packet_and_verify(should_receive=False, vlan_id=filtered_vlan + 1)
> +        testpmd.close()
<snip>
> +    def tear_down_suite(self) -> None:
> +        """Tear down the suite."""

This method declaration isn't needed since we don't require any tear
down steps to actually clean up after the test suite.




> --
> 2.44.0
>
  

Patch

diff --git a/dts/tests/TestSuite_vlan.py b/dts/tests/TestSuite_vlan.py
new file mode 100644
index 0000000000..8b6827ec4b
--- /dev/null
+++ b/dts/tests/TestSuite_vlan.py
@@ -0,0 +1,172 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 University of New Hampshire
+
+"""Test the support of VLAN Offload Features by Poll Mode Drivers.
+
+The test suite ensures that with the correct configuration, a port
+will not drop a VLAN tagged packet. In order for this to be successful,
+packet header stripping and packet receipts must be enabled on the Poll Mode Driver.
+The test suite checks that when these conditions are met, the packet is received without issue.
+The suite also checks to ensure that when these conditions are not met, as in the cases where
+stripping is disabled, or VLAN packet receipts are disabled, the packet is not received.
+Additionally, it checks the case where VLAN header insertion is enabled in transmitted packets,
+which should be successful if the previous cases pass.
+
+"""
+
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import]
+from scapy.packet import Raw  # type: ignore[import]
+
+from framework.remote_session.testpmd_shell import TestPmdForwardingModes, TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestVlan(TestSuite):
+    """DPDK VLAN test suite.
+
+    Ensures VLAN packet reception on the Poll Mode Driver when certain conditions are met.
+    If one or more of these conditions are not met, the packet reception should be unsuccessful.
+    """
+
+    def set_up_suite(self) -> None:
+        """Set up the test suite.
+
+        Setup:
+            Create a testpmd session and set up tg nodes
+            verify that at least two ports are open for session
+        """
+        self.verify(len(self._port_links) > 1, "Not enough ports")
+
+    def send_vlan_packet_and_verify(
+        self, should_receive: bool = True, strip: bool = False, vlan_id: int = -1
+    ) -> None:
+        """Generate a vlan packet, send and verify on the dut.
+
+        Args:
+            should_receive: indicate whether the packet should be successfully received
+            vlan_id: expected vlan ID
+            strip: indicates whether stripping is on or off,
+            and when the vlan tag is checked for a match
+        """
+        data = "P" * 10
+        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load=data)
+        received_packets = self.send_packet_and_capture(packet)
+        received_packets = [
+            packets
+            for packets in received_packets
+            if hasattr(packets, "load") and data in str((packets.load))
+        ]
+        if should_receive:
+            self.verify(
+                len(received_packets) == 1, "Packet was dropped when it should have been received"
+            )
+            received = received_packets[0]
+            if strip:
+                self.verify(Dot1Q not in received, "Vlan tag was not stripped successfully")
+            else:
+                if len(received_packets) == 1:
+                    self.verify(
+                        received.vlan == vlan_id, "The received tag did not match the expected tag"
+                    )
+        else:
+            self.verify(
+                not len(received_packets) == 1,
+                "Packet was received when it should have been dropped",
+            )
+
+    def send_packet_and_verify_insertion(self, expected_id: int = -1) -> None:
+        """Generate a packet with no vlan tag, send and verify on the dut.
+
+        Args:
+            expected_id: the vlan id that is being inserted through tx_offload configuration
+            should_receive: indicate whether the packet should be successfully received
+        """
+        data = "P" * 10
+        packet = Ether() / Raw(load=data)
+        received_packets = self.send_packet_and_capture(packet)
+        received_packets = [
+            packets
+            for packets in received_packets
+            if hasattr(packets, "load") and data in str((packets.load))
+        ]
+        self.verify(
+            len(received_packets) == 1, "Packet was dropped when it should have been received"
+        )
+        received = received_packets[0]
+        self.verify(Dot1Q in received, "The received packet did not have a vlan tag")
+        self.verify(received.vlan == expected_id, "The received tag did not match the expected tag")
+
+    def test_vlan_receipt_no_stripping(self) -> None:
+        """Ensure vlan packet is dropped when receipts are enabled and header stripping is disabled.
+
+        Test:
+            Create an interactive testpmd shell and verify a vlan packet.
+        """
+        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
+        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
+        testpmd.send_command("set verbose 1", "testpmd>")
+        testpmd.send_command("set promisc 0 off", "testpmd>")
+        testpmd.vlan_filter_set_on(0)
+        testpmd.rx_vlan_add(1, 0)
+        testpmd.start()
+
+        filtered_vlan = 1
+        self.send_vlan_packet_and_verify(True, vlan_id=filtered_vlan)
+        testpmd.close()
+
+    def test_vlan_receipt_stripping(self) -> None:
+        """Ensure vlan packet received with no tag when receipts and header stripping are enabled.
+
+        Test:
+            Create an interactive testpmd shell and verify a vlan packet.
+        """
+        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
+        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
+        testpmd.send_command("set verbose 1", "testpmd>")
+        testpmd.send_command("set promisc 0 off", "testpmd>")
+        testpmd.vlan_filter_set_on(0)
+        testpmd.rx_vlan_add(1, 0)
+        testpmd.vlan_strip_set_on(0)
+        testpmd.start()
+
+        self.send_vlan_packet_and_verify(should_receive=True, strip=True, vlan_id=1)
+        testpmd.close()
+
+    def test_vlan_no_receipt(self) -> None:
+        """Ensure vlan packet dropped when filter is on and sent tag not in the filter list.
+
+        Test:
+            Create an interactive testpmd shell and verify a vlan packet.
+        """
+        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
+        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
+        testpmd.send_command("set verbose 1", "testpmd>")
+        testpmd.send_command("set promisc 0 off", "testpmd>")
+        testpmd.vlan_filter_set_on(0)
+        testpmd.rx_vlan_add(1, 0)
+        testpmd.start()
+
+        filtered_vlan = 1
+        self.send_vlan_packet_and_verify(should_receive=False, vlan_id=filtered_vlan + 1)
+        testpmd.close()
+
+    def test_vlan_header_insertion(self) -> None:
+        """Ensure that vlan packet is received with the correct inserted vlan tag.
+
+        Test:
+            Create an interactive testpmd shell and verify a non-vlan packet.
+        """
+        testpmd = self.sut_node.create_interactive_shell(TestPmdShell, privileged=True)
+        testpmd.set_forward_mode(TestPmdForwardingModes.mac)
+        testpmd.send_command("set verbose 1", "testpmd>")
+        testpmd.send_command("set promisc 0 off", "testpmd>")
+        testpmd.port_stop_all()
+        testpmd.tx_vlan_set(1, 51)
+        testpmd.port_start_all()
+        testpmd.start()
+
+        self.send_packet_and_verify_insertion(expected_id=51)
+        testpmd.close()
+
+    def tear_down_suite(self) -> None:
+        """Tear down the suite."""