DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2 0/2] VLAN test suite
@ 2024-06-11 16:15 Dean Marx
  2024-06-11 16:15 ` [PATCH v2 1/2] Initial implementation for " Dean Marx
                   ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-11 16:15 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

This patch contains the initial implemenation of a VLAN test suite that
ensures the correct functionality of VLAN filtering, stripping, and
header insertion on the poll mode driver. VLAN filtering is a useful
feature for separating one Ethernet network into multiple logical
networks, which enhances network security as it prevents wireless
devices from accessing LAN resources. In order to differentiate these
packets based on which VLAN they originate from, VLAN tags are inserted
in the packet header, and they are either dropped or forwarded based on
a VLAN filter list stored on a given port. If a port does not exhibit
this behavior, it can compromise the security of the network.

There are four test cases this suite implements:
1. Packet reception when tag is in the filter list and stripping is off
2. Packet reception without tag when tag is in filter list and stripping
is on
3. No packet reception when tag is not in the filter list
4. Packet reception with tag when tag insertion is on

If these all pass, the poll mode driver is correctly configured to use
vlan features with no security concerns.


Dean Marx (2):
  Initial implementation for VLAN test suite
  conf schema

 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_vlan.py                | 172 +++++++++++++++++++++
 2 files changed, 174 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_vlan.py

-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 1/2] Initial implementation for VLAN test suite
  2024-06-11 16:15 [PATCH v2 0/2] VLAN test suite Dean Marx
@ 2024-06-11 16:15 ` Dean Marx
  2024-06-11 16:15 ` [PATCH v2 2/2] conf schema Dean Marx
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
  2 siblings, 0 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-11 16:15 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

---
 dts/tests/TestSuite_vlan.py | 172 ++++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 dts/tests/TestSuite_vlan.py

diff --git a/dts/tests/TestSuite_vlan.py b/dts/tests/TestSuite_vlan.py
new file mode 100644
index 0000000000..121766de3b
--- /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.send_command("vlan set filter on 0", "testpmd>")
+        testpmd.send_command("rx_vlan add 1 0", "testpmd>")
+        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.send_command("vlan set filter on 0", "testpmd>")
+        testpmd.send_command("rx_vlan add 1 0", "testpmd>")
+        testpmd.send_command("vlan set strip on 0", "testpmd>")
+        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.send_command("vlan set filter on 0", "testpmd>")
+        testpmd.send_command("rx_vlan add 1 0", "testpmd>")
+        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.send_command("port stop all", "testpmd>")
+        testpmd.send_command("tx_vlan set 1 51", "testpmd>")
+        testpmd.send_command("port start all", "testpmd>")
+        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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 2/2] conf schema
  2024-06-11 16:15 [PATCH v2 0/2] VLAN test suite Dean Marx
  2024-06-11 16:15 ` [PATCH v2 1/2] Initial implementation for " Dean Marx
@ 2024-06-11 16:15 ` Dean Marx
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
  2 siblings, 0 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-11 16:15 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

---
 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 4731f4511d..eca8244f27 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
       "enum": [
         "hello_world",
         "os_udp",
-        "pmd_buffer_scatter"
+        "pmd_buffer_scatter",
+        "vlan"
       ]
     },
     "test_target": {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 0/3] VLAN Test Suite
  2024-06-11 16:15 [PATCH v2 0/2] VLAN test suite Dean Marx
  2024-06-11 16:15 ` [PATCH v2 1/2] Initial implementation for " Dean Marx
  2024-06-11 16:15 ` [PATCH v2 2/2] conf schema Dean Marx
@ 2024-06-14 15:02 ` Dean Marx
  2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
                     ` (8 more replies)
  2 siblings, 9 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-14 15:02 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

VLAN test suite for ensuring VLAN filtering, stripping, and header
insertion is functional on DPDK Poll Mode Drivers. The suite contains
four test cases:

1. Enable VLAN filtering and disable stripping - verifies that packets
with a VLAN tag found in the filter list are received with the tag when
stripping is disabled.
2. Enable VLAN filtering and stripping - verifies that packets with a
VLAN tag found in the filter list are received without the tag when
stripping is enabled.
3. Disable VLAN packet receipt - verifies that packets with a VLAN tag
not found in the filter list are dropped.
4. Enable VLAN header insertion in transmitted packets - verifies that
packets without a VLAN tag are received with a VLAN tag when header
insertion is enabled.

VLAN functions are offloaded by the DPDK driver and use the Ethernet
Device API (rte_ethdev.)

Dean Marx (3):
  Added VLAN commands to testpmd_shell class
  Initial implementation for VLAN test suite
  Config schema

 dts/framework/config/conf_yaml_schema.json    |   3 +-
 dts/framework/remote_session/testpmd_shell.py | 164 +++++++++++++++++
 dts/tests/TestSuite_vlan.py                   | 172 ++++++++++++++++++
 3 files changed, 338 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_vlan.py

-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 1/3] Added VLAN commands to testpmd_shell class
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
@ 2024-06-14 15:02   ` Dean Marx
  2024-06-14 15:59     ` Patrick Robb
  2024-06-17 14:37     ` Jeremy Spewock
  2024-06-14 15:02   ` [PATCH v3 2/3] Initial implementation for VLAN test suite Dean Marx
                     ` (7 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-14 15:02 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

The testpmd_shell class is intended for sending commands to
 a remote testpmd session within a test suite, and as of right now 
it is missing most commands. Added commands for vlan filtering, 
stripping, and insertion, as well as stopping and starting ports 
during testpmd runtime.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 164 ++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index cb2ab6bd00..5bbc7d3b1c 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -225,6 +225,170 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def vlan_filter_set_on(self, port: int = 0, verify: bool = True):
+        """Set vlan filter on.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan filtering was enabled successfully. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output:
+                self._logger.debug(f"Failed to enable vlan filter: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to enable vlan filter.")
+
+    def vlan_filter_set_off(self, port: int = 0, verify: bool = True):
+        """Set vlan filter off.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan filtering was disabled successfully. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter off {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output:
+                self._logger.debug(f"Failed to disable vlan filter: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to disable vlan filter.")
+
+    def rx_vlan_add(self, vlan: int = 0, port: int = 0, verify: bool = True):
+        """Add specified vlan tag to the filter list on a port.
+
+        Args:
+            vlan: The vlan tag to add, should be within 1-4094.
+            port: The port number to add the tag on, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was added to the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        vlan_add_output = self.send_command(f"rx_vlan add {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_add_output or "Invalid vlan_id" in vlan_add_output:
+                self._logger.debug(f"Failed to add vlan tag: \n{vlan_add_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
+
+    def rx_vlan_rm(self, vlan: int = 0, port: int = 0, verify: bool = True):
+        """Remove specified vlan tag from filter list on a port.
+
+        Args:
+            vlan: The vlan tag to remove, should be within 1-4094.
+            port: The port number to remove the tag from, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was removed from the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        vlan_rm_output = self.send_command(f"rx_vlan rm {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_rm_output or "Invalid vlan_id" in vlan_rm_output:
+                self._logger.debug(f"Failed to add vlan tag: \n{vlan_rm_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
+
+    def vlan_strip_set_on(self, port: int = 0, verify: bool = True):
+        """Enable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan stripping was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip on {port}")
+        if verify:
+            if "Invalid port" in vlan_strip_output:
+                self._logger.debug(f"Failed to add vlan tag: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
+
+    def vlan_strip_set_off(self, port: int = 0, verify: bool = True):
+        """Disable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan stripping was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip off {port}")
+        if verify:
+            if "Invalid port" in vlan_strip_output:
+                self._logger.debug(f"Failed to enable stripping: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to enable stripping on port {port}.")
+
+    def port_stop_all(self):
+        """Stop all ports."""
+        self.send_command("port stop all")
+
+    def port_start_all(self):
+        """Start all ports."""
+        self.send_command("port start all")
+
+    def tx_vlan_set(self, port: int = 0, vlan: int = 0, verify: bool = True):
+        """Set hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            vlan: The vlan tag to insert, should be within 1-4094.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
+        if verify:
+            if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
+            or "Invalid port" in vlan_insert_output):
+                self._logger.debug(f"Failed to set vlan insertion tag: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to set vlan insertion tag.")
+
+    def tx_vlan_reset(self, port: int = 0, verify: bool = True):
+        """Disable hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port}")
+        if verify:
+            if "Please stop port" in vlan_insert_output or "Invalid port" in vlan_insert_output:
+                self._logger.debug(f"Failed to reset vlan insertion: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to reset vlan insertion.")
+
     def close(self) -> None:
         """Overrides :meth:`~.interactive_shell.close`."""
         self.send_command("quit", "")
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 2/3] Initial implementation for VLAN test suite
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
  2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
@ 2024-06-14 15:02   ` Dean Marx
  2024-06-14 16:19     ` Patrick Robb
  2024-06-17 14:56     ` Jeremy Spewock
  2024-06-14 15:02   ` [PATCH v3 3/3] Config schema Dean Marx
                     ` (6 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-14 15:02 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

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

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."""
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/3] Config schema
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
  2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
  2024-06-14 15:02   ` [PATCH v3 2/3] Initial implementation for VLAN test suite Dean Marx
@ 2024-06-14 15:02   ` Dean Marx
  2024-06-17 14:59     ` Jeremy Spewock
  2024-06-17 14:35   ` [PATCH v3 0/3] VLAN Test Suite Jeremy Spewock
                     ` (5 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Dean Marx @ 2024-06-14 15:02 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Configuration to run vlan test suite

Signed-off-by: Dean Marx <dmarx@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 4731f4511d..eca8244f27 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
       "enum": [
         "hello_world",
         "os_udp",
-        "pmd_buffer_scatter"
+        "pmd_buffer_scatter",
+        "vlan"
       ]
     },
     "test_target": {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/3] Added VLAN commands to testpmd_shell class
  2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
@ 2024-06-14 15:59     ` Patrick Robb
  2024-06-14 20:29       ` Jeremy Spewock
  2024-06-17 14:37     ` Jeremy Spewock
  1 sibling, 1 reply; 34+ messages in thread
From: Patrick Robb @ 2024-06-14 15:59 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, paul.szczepanek, yoan.picchi,
	jspewock, bruce.richardson, luca.vizzarro, dev

On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:
> +    def vlan_filter_set_on(self, port: int = 0, verify: bool = True):
> +        """Set vlan filter on.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan filtering was enabled successfully. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        filter_cmd_output = self.send_command(f"vlan set filter on {port}")

I wonder whether, when convenient, we want to name the methods more or
less 1:1 according to the actual testpmd text command they send? I.e.
in this case should the method be named vlan_set_filter_on instead of
vlan_filter_set_on (aligns better with "vlan set filter on {port}")?
The intellisense provided by the testpmd methods is indeed a QoL
improvement for folks writing testsuites, but at the same time people
who use testpmd will always have the real commands ingrained in their
thoughts, so if we try to stay as true to those as possible, we get
the stability and intellisense and also the method names are still
intuitive.

Maybe even think tiny things like renaming the method set_forward_mode
to set_fwd_mode to align 1:1 with testpmd is good.

That's just my perspective though - I would be interested to see
whether others feel the same or not.

> +        if verify:
> +            if "Invalid port" in filter_cmd_output:
> +                self._logger.debug(f"Failed to enable vlan filter: \n{filter_cmd_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to enable vlan filter.")
> +
> +    def vlan_filter_set_off(self, port: int = 0, verify: bool = True):
> +        """Set vlan filter off.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan filtering was disabled successfully. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        filter_cmd_output = self.send_command(f"vlan set filter off {port}")
> +        if verify:
> +            if "Invalid port" in filter_cmd_output:
> +                self._logger.debug(f"Failed to disable vlan filter: \n{filter_cmd_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to disable vlan filter.")
> +
> +    def rx_vlan_add(self, vlan: int = 0, port: int = 0, verify: bool = True):
> +        """Add specified vlan tag to the filter list on a port.
> +
> +        Args:
> +            vlan: The vlan tag to add, should be within 1-4094.

Worth clarifying that 4094 is the extended vlan range? Normal range is 1-1005?

> +
> +    def port_stop_all(self):
> +        """Stop all ports."""
> +        self.send_command("port stop all")

So the idea is to have distinct port_stop_all() and port_stop(self,
port: int) methods? I think this is reasonable.

> +
> +    def port_start_all(self):
> +        """Start all ports."""
> +        self.send_command("port start all")
> +
> +    def tx_vlan_set(self, port: int = 0, vlan: int = 0, verify: bool = True):
> +        """Set hardware insertion of vlan tags in packets sent on a port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            vlan: The vlan tag to insert, should be within 1-4094.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan insertion was enabled on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
> +        if verify:
> +            if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
> +            or "Invalid port" in vlan_insert_output):
> +                self._logger.debug(f"Failed to set vlan insertion tag: \n{vlan_insert_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to set vlan insertion tag.")
> +
> +    def tx_vlan_reset(self, port: int = 0, verify: bool = True):
> +        """Disable hardware insertion of vlan tags in packets sent on a port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan insertion was disabled on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        vlan_insert_output = self.send_command(f"tx_vlan set {port}")
> +        if verify:
> +            if "Please stop port" in vlan_insert_output or "Invalid port" in vlan_insert_output:
> +                self._logger.debug(f"Failed to reset vlan insertion: \n{vlan_insert_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to reset vlan insertion.")

Could possibly be combined with tx_vlan_set() as one method which
handles both set and reset, but I think splitting them out is fine.

> +
>      def close(self) -> None:
>          """Overrides :meth:`~.interactive_shell.close`."""
>          self.send_command("quit", "")
> --
> 2.44.0
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/3] Initial implementation for VLAN test suite
  2024-06-14 15:02   ` [PATCH v3 2/3] Initial implementation for VLAN test suite Dean Marx
@ 2024-06-14 16:19     ` Patrick Robb
  2024-06-17 14:56     ` Jeremy Spewock
  1 sibling, 0 replies; 34+ messages in thread
From: Patrick Robb @ 2024-06-14 16:19 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, paul.szczepanek, yoan.picchi,
	jspewock, bruce.richardson, luca.vizzarro, dev

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
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/3] Added VLAN commands to testpmd_shell class
  2024-06-14 15:59     ` Patrick Robb
@ 2024-06-14 20:29       ` Jeremy Spewock
  2024-06-14 21:24         ` Patrick Robb
  0 siblings, 1 reply; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-14 20:29 UTC (permalink / raw)
  To: Patrick Robb
  Cc: Dean Marx, Honnappa.Nagarahalli, juraj.linkes, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

On Fri, Jun 14, 2024 at 12:00 PM Patrick Robb <probb@iol.unh.edu> wrote:
>
> On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:
> > +    def vlan_filter_set_on(self, port: int = 0, verify: bool = True):
> > +        """Set vlan filter on.
> > +
> > +        Args:
> > +            port: The port number to use, should be within 0-32.
> > +            verify: If :data:`True`, the output of the command is scanned to verify that
> > +                vlan filtering was enabled successfully. If not, it is
> > +                considered an error.
> > +
> > +        Raises:
> > +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> > +                fails to update.
> > +        """
> > +        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
>
> I wonder whether, when convenient, we want to name the methods more or
> less 1:1 according to the actual testpmd text command they send? I.e.
> in this case should the method be named vlan_set_filter_on instead of
> vlan_filter_set_on (aligns better with "vlan set filter on {port}")?
> The intellisense provided by the testpmd methods is indeed a QoL
> improvement for folks writing testsuites, but at the same time people
> who use testpmd will always have the real commands ingrained in their
> thoughts, so if we try to stay as true to those as possible, we get
> the stability and intellisense and also the method names are still
> intuitive.

I generally try to name these methods in a more human-readable way, so
my intuition would lean more towards naming it something like
`set_vlan_filter_on` or `set_vlan_strip_on`. I could see how it might
make it easier for some testpmd developers, so I'm not sure which
would be better. Personally, as someone who is less familiar with all
the ins and outs of testpmd, I prefer the human-readable approach.

>
> Maybe even think tiny things like renaming the method set_forward_mode
> to set_fwd_mode to align 1:1 with testpmd is good.
>
> That's just my perspective though - I would be interested to see
> whether others feel the same or not.
>
<snip>
> > 2.44.0
> >

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/3] Added VLAN commands to testpmd_shell class
  2024-06-14 20:29       ` Jeremy Spewock
@ 2024-06-14 21:24         ` Patrick Robb
  0 siblings, 0 replies; 34+ messages in thread
From: Patrick Robb @ 2024-06-14 21:24 UTC (permalink / raw)
  To: Jeremy Spewock
  Cc: Dean Marx, Honnappa.Nagarahalli, juraj.linkes, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

On Fri, Jun 14, 2024 at 4:29 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:

> > I wonder whether, when convenient, we want to name the methods more or
> > less 1:1 according to the actual testpmd text command they send? I.e.
> > in this case should the method be named vlan_set_filter_on instead of
> > vlan_filter_set_on (aligns better with "vlan set filter on {port}")?
> > The intellisense provided by the testpmd methods is indeed a QoL
> > improvement for folks writing testsuites, but at the same time people
> > who use testpmd will always have the real commands ingrained in their
> > thoughts, so if we try to stay as true to those as possible, we get
> > the stability and intellisense and also the method names are still
> > intuitive.
>
> I generally try to name these methods in a more human-readable way, so
> my intuition would lean more towards naming it something like
> `set_vlan_filter_on` or `set_vlan_strip_on`. I could see how it might
> make it easier for some testpmd developers, so I'm not sure which
> would be better. Personally, as someone who is less familiar with all
> the ins and outs of testpmd, I prefer the human-readable approach.

I think this is compelling, but to play devil's advocate, I also think
it's not just testpmd developers who might care. The broad community
of DPDK developers, who are working on DPDK features and might want to
write DTS testsuites in the future, are probably also already pretty
familiar with testpmd params. If they start using the framework, it
may be a benefit for the method names to be intuitive, so they don't
have to in essence relearn how to use testpmd just to write testsuites
for DTS. Probably not a big deal given as you say the human readable
approach is pretty intuitive too... anyways maybe this can be a
discussion point at the DTS meeting next week.

I guess people can always fall back on testpmd.send_command() if they
don't like the human readable methods...? Or would we want to ban
that?

https://doc.dpdk.org/guides/testpmd_app_ug/testpmd_funcs.html

Should testpmd class have a method for each runtime command? It's a
long list. Or just methods for the most common commands? Well, we'll
chat at the DTS meeting.

>
> >
> > Maybe even think tiny things like renaming the method set_forward_mode
> > to set_fwd_mode to align 1:1 with testpmd is good.
> >
> > That's just my perspective though - I would be interested to see
> > whether others feel the same or not.
> >
> <snip>
> > > 2.44.0
> > >

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 0/3] VLAN Test Suite
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
                     ` (2 preceding siblings ...)
  2024-06-14 15:02   ` [PATCH v3 3/3] Config schema Dean Marx
@ 2024-06-17 14:35   ` Jeremy Spewock
  2024-06-17 17:50   ` Patrick Robb
                     ` (4 subsequent siblings)
  8 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-17 14:35 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]

Hey Dean,

Thanks for the series! One thing that caught my eye which I figured I'd
mention here since it applies to all the commits is that generally with
patches submitted to the dev mailing list the subject lines of the commits
should include the component that the commit is relevant to. In our case
with DTS, the component that you'd put at the start of the commit message
would be "dts:". So, for example, your second commit in the series would be
"dts: initial implementation for VLAN test suite". Another guideline is
they should be all lowercase other than acronyms. You can see where I got
this information from here:
https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line
.



On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:

> VLAN test suite for ensuring VLAN filtering, stripping, and header
> insertion is functional on DPDK Poll Mode Drivers. The suite contains
> four test cases:
>
> 1. Enable VLAN filtering and disable stripping - verifies that packets
> with a VLAN tag found in the filter list are received with the tag when
> stripping is disabled.
> 2. Enable VLAN filtering and stripping - verifies that packets with a
> VLAN tag found in the filter list are received without the tag when
> stripping is enabled.
> 3. Disable VLAN packet receipt - verifies that packets with a VLAN tag
> not found in the filter list are dropped.
> 4. Enable VLAN header insertion in transmitted packets - verifies that
> packets without a VLAN tag are received with a VLAN tag when header
> insertion is enabled.
>
> VLAN functions are offloaded by the DPDK driver and use the Ethernet
> Device API (rte_ethdev.)
>
> Dean Marx (3):
>   Added VLAN commands to testpmd_shell class
>   Initial implementation for VLAN test suite
>   Config schema
>
>  dts/framework/config/conf_yaml_schema.json    |   3 +-
>  dts/framework/remote_session/testpmd_shell.py | 164 +++++++++++++++++
>  dts/tests/TestSuite_vlan.py                   | 172 ++++++++++++++++++
>  3 files changed, 338 insertions(+), 1 deletion(-)
>  create mode 100644 dts/tests/TestSuite_vlan.py
>
> --
> 2.44.0
>
>

[-- Attachment #2: Type: text/html, Size: 2822 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/3] Added VLAN commands to testpmd_shell class
  2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
  2024-06-14 15:59     ` Patrick Robb
@ 2024-06-17 14:37     ` Jeremy Spewock
  1 sibling, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-17 14:37 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

In the methods you added here, you made all of the parameters to them
optional with a default. This might make sense to allow developers to
be less verbose when calling the method with the defaults, but in the
case of things like the port ID, vlan IDs, and things of that nature I
think making these positional arguments that are required makes more
sense for readability. For example, if I were to call
testpmd.vlan_filter_set_on(), it is not clear where this vlan filter
is being modified. The default is 0 so it will set it on port 0, but
if you're just reading the test suite you wouldn't know that without
looking at the testpmd class. Verify is left as an optional parameter
because we really want to verify that the commands were successful in
all cases, but there could be an obscure reason in the future that a
developer might not really care if a command to testpmd was successful
or not.

I also noticed in most cases where you were verifying commands that
you had to check if multiple failures messages were present. Would it
be easier to check if the command was successful by querying things
like VLANs on a port or VLAN filtering status of a port instead of
trying to cover all error cases?

On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> The testpmd_shell class is intended for sending commands to
>  a remote testpmd session within a test suite, and as of right now
> it is missing most commands. Added commands for vlan filtering,
> stripping, and insertion, as well as stopping and starting ports
> during testpmd runtime.

While it's true that the testpmd class is missing a lot of commands
that exist in testpmd, that isn't really why you are adding them.
Maybe switching this to say something like "It is missing
functionality required for verifying VLAN features on a PMD." might be
more descriptive.

>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
<snip>
> +    def vlan_filter_set_on(self, port: int = 0, verify: bool = True):
> +        """Set vlan filter on.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan filtering was enabled successfully. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
> +        if verify:
> +            if "Invalid port" in filter_cmd_output:

This doesn't really verify that the filter was set as much as it
verifies that the port exists. Is there anything that testpmd prints
when it is successful, or possibly a way to query existing VLAN
filters on a port to see if it was set?

> +                self._logger.debug(f"Failed to enable vlan filter: \n{filter_cmd_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to enable vlan filter.")
> +
> +    def vlan_filter_set_off(self, port: int = 0, verify: bool = True):
> +        """Set vlan filter off.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan filtering was disabled successfully. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        filter_cmd_output = self.send_command(f"vlan set filter off {port}")
> +        if verify:
> +            if "Invalid port" in filter_cmd_output:

Same thing as above comment about verification.

> +                self._logger.debug(f"Failed to disable vlan filter: \n{filter_cmd_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to disable vlan filter.")
> +
<snip>
> +    def vlan_strip_set_on(self, port: int = 0, verify: bool = True):
> +        """Enable vlan stripping on the specified port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan stripping was enabled on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        vlan_strip_output = self.send_command(f"vlan set strip on {port}")
> +        if verify:
> +            if "Invalid port" in vlan_strip_output:

This is similar to above where it isn't really verifying that the vlan
stripping was enabled.

> +                self._logger.debug(f"Failed to add vlan tag: \n{vlan_strip_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
> +
> +    def vlan_strip_set_off(self, port: int = 0, verify: bool = True):
> +        """Disable vlan stripping on the specified port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan stripping was disabled on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        vlan_strip_output = self.send_command(f"vlan set strip off {port}")
> +        if verify:
> +            if "Invalid port" in vlan_strip_output:

Same here.

> +                self._logger.debug(f"Failed to enable stripping: \n{vlan_strip_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to enable stripping on port {port}.")

On all of the other verification methods the message for the error
doesn't include specifics like what port it was on or what vlan tag
failed. I am generally more in favor of including specific information
like this and many of the other testpmd methods do, so if possible I
think we should add some of this information to the other methods here
as well to make it consistent.

> +
<snip>
> 2.44.0
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 2/3] Initial implementation for VLAN test suite
  2024-06-14 15:02   ` [PATCH v3 2/3] Initial implementation for VLAN test suite Dean Marx
  2024-06-14 16:19     ` Patrick Robb
@ 2024-06-17 14:56     ` Jeremy Spewock
  1 sibling, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-17 14:56 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

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
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/3] Config schema
  2024-06-14 15:02   ` [PATCH v3 3/3] Config schema Dean Marx
@ 2024-06-17 14:59     ` Jeremy Spewock
  0 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-17 14:59 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

I think maybe we could find a more descriptive name than just "vlan",
but I wasn't able to think of any that weren't overly verbose. Also,
we're just testing the basic features of a vlan in a pmd, so maybe it
makes enough sense as is.

On Fri, Jun 14, 2024 at 11:03 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Configuration to run vlan test suite

Super nit-picky, but we should probably add a period at the end of
this description in the next version too.

>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
<snip>
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 0/3] VLAN Test Suite
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
                     ` (3 preceding siblings ...)
  2024-06-17 14:35   ` [PATCH v3 0/3] VLAN Test Suite Jeremy Spewock
@ 2024-06-17 17:50   ` Patrick Robb
  2024-06-18 15:20   ` [PATCH v4 1/3] dts: refactored VLAN test suite Dean Marx
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 34+ messages in thread
From: Patrick Robb @ 2024-06-17 17:50 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, paul.szczepanek, yoan.picchi,
	jspewock, bruce.richardson, luca.vizzarro, dev

For CI Testing: re-applying to main and retesting because main was in
a bad state (now fixed by Maxime), causing a virtio_smoke fail on this
series.

https://git.dpdk.org/dpdk/commit/?id=6bdc14606724bc7fb3834d5ec59b1cccf98adf28

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 1/3] dts: refactored VLAN test suite
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
                     ` (4 preceding siblings ...)
  2024-06-17 17:50   ` Patrick Robb
@ 2024-06-18 15:20   ` Dean Marx
  2024-06-18 15:20     ` [PATCH v4 2/3] dts: updated testpmd shell class Dean Marx
  2024-06-18 15:20     ` [PATCH v4 3/3] dts: config schema Dean Marx
  2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
                     ` (2 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-18 15:20 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Tweaked logic on sending and verifying packets for 
more concise code, added verbose and promisc 
function calls from pmd shell module.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/tests/TestSuite_vlan.py | 59 +++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/dts/tests/TestSuite_vlan.py b/dts/tests/TestSuite_vlan.py
index 8b6827ec4b..b9b2a98588 100644
--- a/dts/tests/TestSuite_vlan.py
+++ b/dts/tests/TestSuite_vlan.py
@@ -48,29 +48,26 @@ def send_vlan_packet_and_verify(
             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)
+        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load='xxxxx')
         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))
-        ]
+        test_packet = None
+        for packet in received_packets:
+            if packet.haslayer(Raw) and packet[Raw].load == b'xxxxx':
+                test_packet = packet
+                break
         if should_receive:
             self.verify(
-                len(received_packets) == 1, "Packet was dropped when it should have been received"
+                test_packet is not None, "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")
+                self.verify(Dot1Q not in test_packet, "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"
+                        test_packet.vlan == vlan_id, "The received tag did not match the expected tag"
                     )
         else:
             self.verify(
-                not len(received_packets) == 1,
+                test_packet is None,
                 "Packet was received when it should have been dropped",
             )
 
@@ -81,20 +78,18 @@ def send_packet_and_verify_insertion(self, expected_id: int = -1) -> None:
             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)
+        packet = Ether() / Raw(load='xxxxx')
         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))
-        ]
+        test_packet = None
+        for packet in received_packets:
+            if packet.haslayer(Raw) and packet[Raw].load == b'xxxxx':
+                test_packet = packet
+                break
         self.verify(
-            len(received_packets) == 1, "Packet was dropped when it should have been received"
+            test_packet is not None, "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")
+        self.verify(Dot1Q in test_packet, "The received packet did not have a vlan tag")
+        self.verify(test_packet.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.
@@ -104,8 +99,8 @@ def test_vlan_receipt_no_stripping(self) -> None:
         """
         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.set_verbose(1)
+        testpmd.set_promisc(0, False)
         testpmd.vlan_filter_set_on(0)
         testpmd.rx_vlan_add(1, 0)
         testpmd.start()
@@ -122,8 +117,8 @@ def test_vlan_receipt_stripping(self) -> None:
         """
         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.set_verbose(1)
+        testpmd.set_promisc(0, False)
         testpmd.vlan_filter_set_on(0)
         testpmd.rx_vlan_add(1, 0)
         testpmd.vlan_strip_set_on(0)
@@ -140,8 +135,8 @@ def test_vlan_no_receipt(self) -> None:
         """
         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.set_verbose(1)
+        testpmd.set_promisc(0, False)
         testpmd.vlan_filter_set_on(0)
         testpmd.rx_vlan_add(1, 0)
         testpmd.start()
@@ -158,8 +153,8 @@ def test_vlan_header_insertion(self) -> None:
         """
         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.set_verbose(1)
+        testpmd.set_promisc(0, False)
         testpmd.port_stop_all()
         testpmd.tx_vlan_set(1, 51)
         testpmd.port_start_all()
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 2/3] dts: updated testpmd shell class
  2024-06-18 15:20   ` [PATCH v4 1/3] dts: refactored VLAN test suite Dean Marx
@ 2024-06-18 15:20     ` Dean Marx
  2024-06-18 15:20     ` [PATCH v4 3/3] dts: config schema Dean Marx
  1 sibling, 0 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-18 15:20 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Ported over the promisc and verbose mode functions from 
v2 of the queue start/stop suite to use for the VLAN suite. 
Tweaked some of the verification methods to be
more concise, changed some docstrings to be more specific.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 186 +++++++++++++-----
 1 file changed, 141 insertions(+), 45 deletions(-)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 5bbc7d3b1c..aad3a3a448 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -225,7 +225,7 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
-    def vlan_filter_set_on(self, port: int = 0, verify: bool = True):
+    def vlan_filter_set_on(self, port: int, verify: bool = True):
         """Set vlan filter on.
 
         Args:
@@ -240,11 +240,11 @@ def vlan_filter_set_on(self, port: int = 0, verify: bool = True):
         """
         filter_cmd_output = self.send_command(f"vlan set filter on {port}")
         if verify:
-            if "Invalid port" in filter_cmd_output:
-                self._logger.debug(f"Failed to enable vlan filter: \n{filter_cmd_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to enable vlan filter.")
+            if "Invalid port" in filter_cmd_output or "filter: on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to enable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to enable vlan filter on port {port}.")
 
-    def vlan_filter_set_off(self, port: int = 0, verify: bool = True):
+    def vlan_filter_set_off(self, port: int, verify: bool = True):
         """Set vlan filter off.
 
         Args:
@@ -259,31 +259,31 @@ def vlan_filter_set_off(self, port: int = 0, verify: bool = True):
         """
         filter_cmd_output = self.send_command(f"vlan set filter off {port}")
         if verify:
-            if "Invalid port" in filter_cmd_output:
-                self._logger.debug(f"Failed to disable vlan filter: \n{filter_cmd_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to disable vlan filter.")
+            if "Invalid port" in filter_cmd_output or "filter: off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan filter on port {port}.")
 
-    def rx_vlan_add(self, vlan: int = 0, port: int = 0, verify: bool = True):
+    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
         """Add specified vlan tag to the filter list on a port.
 
         Args:
-            vlan: The vlan tag to add, should be within 1-4094.
+            vlan: The vlan tag to add, should be within 1-1005, 1-4094 extended.
             port: The port number to add the tag on, should be within 0-32.
             verify: If :data:`True`, the output of the command is scanned to verify that
                 the vlan tag was added to the filter list on the specified port. If not, it is
                 considered an error.
 
         Raises:
-            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
-                fails to update.
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not added.
         """
         vlan_add_output = self.send_command(f"rx_vlan add {vlan} {port}")
         if verify:
             if "VLAN-filtering disabled" in vlan_add_output or "Invalid vlan_id" in vlan_add_output:
-                self._logger.debug(f"Failed to add vlan tag: \n{vlan_add_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
+                self._logger.debug(f"Failed to add vlan tag {vlan} on port {port}: \n{vlan_add_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to add vlan tag {vlan} on port {port}.")
 
-    def rx_vlan_rm(self, vlan: int = 0, port: int = 0, verify: bool = True):
+    def rx_vlan_rm(self, vlan: int, port: int, verify: bool = True):
         """Remove specified vlan tag from filter list on a port.
 
         Args:
@@ -294,16 +294,16 @@ def rx_vlan_rm(self, vlan: int = 0, port: int = 0, verify: bool = True):
                 considered an error.
 
         Raises:
-            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
-                fails to update.
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not removed.
         """
         vlan_rm_output = self.send_command(f"rx_vlan rm {vlan} {port}")
         if verify:
             if "VLAN-filtering disabled" in vlan_rm_output or "Invalid vlan_id" in vlan_rm_output:
-                self._logger.debug(f"Failed to add vlan tag: \n{vlan_rm_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
+                self._logger.debug(f"Failed to remove vlan tag {vlan} on port {port}: \n{vlan_rm_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to remove vlan tag {vlan} on port {port}.")
 
-    def vlan_strip_set_on(self, port: int = 0, verify: bool = True):
+    def vlan_strip_set_on(self, port: int, verify: bool = True):
         """Enable vlan stripping on the specified port.
 
         Args:
@@ -313,16 +313,16 @@ def vlan_strip_set_on(self, port: int = 0, verify: bool = True):
                 considered an error.
 
         Raises:
-            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
                 fails to update.
         """
         vlan_strip_output = self.send_command(f"vlan set strip on {port}")
         if verify:
-            if "Invalid port" in vlan_strip_output:
-                self._logger.debug(f"Failed to add vlan tag: \n{vlan_strip_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to add vlan tag.")
+            if "strip on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to set vlan filter on for port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan filter on for port {port}.")
 
-    def vlan_strip_set_off(self, port: int = 0, verify: bool = True):
+    def vlan_strip_set_off(self, port: int, verify: bool = True):
         """Disable vlan stripping on the specified port.
 
         Args:
@@ -332,24 +332,71 @@ def vlan_strip_set_off(self, port: int = 0, verify: bool = True):
                 considered an error.
 
         Raises:
-            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
                 fails to update.
         """
         vlan_strip_output = self.send_command(f"vlan set strip off {port}")
         if verify:
-            if "Invalid port" in vlan_strip_output:
-                self._logger.debug(f"Failed to enable stripping: \n{vlan_strip_output}")
-                raise InteractiveCommandExecutionError(f"Testpmd failed to enable stripping on port {port}.")
+            if "strip off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan stripping on port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan stripping on port {port}.")
 
-    def port_stop_all(self):
-        """Stop all ports."""
-        self.send_command("port stop all")
+    def port_stop_all(self, verify: bool = True):
+        """Stop all ports.
 
-    def port_start_all(self):
-        """Start all ports."""
-        self.send_command("port start all")
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+                fail to stop."""
+        port_output = self.send_command("port stop all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to stop all ports.")
+
+    def port_stop(self, port: int, verify: bool = True):
+        """Stop all ports.
 
-    def tx_vlan_set(self, port: int = 0, vlan: int = 0, verify: bool = True):
+        Args:
+            port: specifies the port number to use, must be between 0-32
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not stopped."""
+        port_output = self.send_command(f"port stop {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to stop port {port}.")
+
+    def port_start_all(self, verify: bool = True):
+        """Start all ports.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+            fail to start."""
+        port_output = self.send_command("port start all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to start all ports.")
+
+    def port_start(self, port: int, verify: bool = True):
+        """Stop all ports.
+
+        Args:
+            port: specifies the port number to use, must be between 0-32
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not started."""
+        port_output = self.send_command(f"port start {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to start port {port}.")
+
+
+    def tx_vlan_set(self, port: int, vlan: int, verify: bool = True):
         """Set hardware insertion of vlan tags in packets sent on a port.
 
         Args:
@@ -360,17 +407,17 @@ def tx_vlan_set(self, port: int = 0, vlan: int = 0, verify: bool = True):
                 considered an error.
 
         Raises:
-            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
-                fails to update.
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+            tag is not set.
         """
         vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
         if verify:
             if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
             or "Invalid port" in vlan_insert_output):
-                self._logger.debug(f"Failed to set vlan insertion tag: \n{vlan_insert_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to set vlan insertion tag.")
+                self._logger.debug(f"Failed to set vlan insertion tag {vlan} on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan insertion tag {vlan} on port {port}.")
 
-    def tx_vlan_reset(self, port: int = 0, verify: bool = True):
+    def tx_vlan_reset(self, port: int, verify: bool = True):
         """Disable hardware insertion of vlan tags in packets sent on a port.
 
         Args:
@@ -380,14 +427,63 @@ def tx_vlan_reset(self, port: int = 0, verify: bool = True):
                 considered an error.
 
         Raises:
-            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
-                fails to update.
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+                tag is not reset.
         """
         vlan_insert_output = self.send_command(f"tx_vlan set {port}")
         if verify:
             if "Please stop port" in vlan_insert_output or "Invalid port" in vlan_insert_output:
-                self._logger.debug(f"Failed to reset vlan insertion: \n{vlan_insert_output}")
-                raise InteractiveCommandExecutionError("Testpmd failed to reset vlan insertion.")
+                self._logger.debug(f"Failed to reset vlan insertion on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to reset vlan insertion on port {port}.")
+
+    def set_promisc(self, port: int, on: bool, verify: bool = True):
+        """Turns promiscuous mode on/off for the specified port
+
+        Args:
+            port: port number to use, should be within 0-32.
+            on: if :data:`True`, turn promisc mode on, otherwise turn off.
+            verify: if :data:`True` an additional command will be sent to verify that promisc mode
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and promisc mode
+            is not correctly set.
+        """
+        if on:
+            promisc_output = self.send_command(f"set promisc {port} on")
+        else:
+            promisc_output = self.send_command(f"set promisc {port} off")
+        if verify:
+            if (on and "Promiscuous mode: enabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+            elif (not on and "Promiscuous mode: disabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+
+
+    def set_verbose(self, level: int, verify: bool = True):
+        """Set debug verbosity level.
+
+        Args:
+            level: 0 - silent except for error
+            1 - fully verbose except for Tx packets
+            2 - fully verbose except for Rx packets
+            >2 - fully verbose
+            verify: if :data:`True` an additional command will be sent to verify that verbose level
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
+            is not correctly set.
+        """
+        verbose_output = self.send_command(f"set verbose {level}")
+        if verify:
+            if "Change verbose level" not in verbose_output:
+                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
 
     def close(self) -> None:
         """Overrides :meth:`~.interactive_shell.close`."""
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 3/3] dts: config schema
  2024-06-18 15:20   ` [PATCH v4 1/3] dts: refactored VLAN test suite Dean Marx
  2024-06-18 15:20     ` [PATCH v4 2/3] dts: updated testpmd shell class Dean Marx
@ 2024-06-18 15:20     ` Dean Marx
  1 sibling, 0 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-18 15:20 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Configuration to run vlan test suite

Signed-off-by: Dean Marx <dmarx@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 4731f4511d..eca8244f27 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
       "enum": [
         "hello_world",
         "os_udp",
-        "pmd_buffer_scatter"
+        "pmd_buffer_scatter",
+        "vlan"
       ]
     },
     "test_target": {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v5 1/3] dts: updated testpmd shell class
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
                     ` (5 preceding siblings ...)
  2024-06-18 15:20   ` [PATCH v4 1/3] dts: refactored VLAN test suite Dean Marx
@ 2024-06-18 16:29   ` Dean Marx
  2024-06-18 16:29     ` [PATCH v5 2/3] dts: refactored VLAN test suite Dean Marx
                       ` (2 more replies)
  2024-06-24 18:17   ` [PATCH v6 " Dean Marx
  2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
  8 siblings, 3 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-18 16:29 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Ported over the promisc and verbose mode functions 
from v2 of the queue start/stop suite to use for the 
VLAN suite. Tweaked some of the verification methods to be
more concise, changed some docstrings to be more specific.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 260 ++++++++++++++++++
 1 file changed, 260 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index cb2ab6bd00..aad3a3a448 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -225,6 +225,266 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def vlan_filter_set_on(self, port: int, verify: bool = True):
+        """Set vlan filter on.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan filtering was enabled successfully. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output or "filter: on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to enable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to enable vlan filter on port {port}.")
+
+    def vlan_filter_set_off(self, port: int, verify: bool = True):
+        """Set vlan filter off.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan filtering was disabled successfully. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter off {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output or "filter: off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan filter on port {port}.")
+
+    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+        """Add specified vlan tag to the filter list on a port.
+
+        Args:
+            vlan: The vlan tag to add, should be within 1-1005, 1-4094 extended.
+            port: The port number to add the tag on, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was added to the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not added.
+        """
+        vlan_add_output = self.send_command(f"rx_vlan add {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_add_output or "Invalid vlan_id" in vlan_add_output:
+                self._logger.debug(f"Failed to add vlan tag {vlan} on port {port}: \n{vlan_add_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to add vlan tag {vlan} on port {port}.")
+
+    def rx_vlan_rm(self, vlan: int, port: int, verify: bool = True):
+        """Remove specified vlan tag from filter list on a port.
+
+        Args:
+            vlan: The vlan tag to remove, should be within 1-4094.
+            port: The port number to remove the tag from, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was removed from the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not removed.
+        """
+        vlan_rm_output = self.send_command(f"rx_vlan rm {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_rm_output or "Invalid vlan_id" in vlan_rm_output:
+                self._logger.debug(f"Failed to remove vlan tag {vlan} on port {port}: \n{vlan_rm_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to remove vlan tag {vlan} on port {port}.")
+
+    def vlan_strip_set_on(self, port: int, verify: bool = True):
+        """Enable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan stripping was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip on {port}")
+        if verify:
+            if "strip on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to set vlan filter on for port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan filter on for port {port}.")
+
+    def vlan_strip_set_off(self, port: int, verify: bool = True):
+        """Disable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan stripping was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip off {port}")
+        if verify:
+            if "strip off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan stripping on port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan stripping on port {port}.")
+
+    def port_stop_all(self, verify: bool = True):
+        """Stop all ports.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+                fail to stop."""
+        port_output = self.send_command("port stop all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to stop all ports.")
+
+    def port_stop(self, port: int, verify: bool = True):
+        """Stop all ports.
+
+        Args:
+            port: specifies the port number to use, must be between 0-32
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not stopped."""
+        port_output = self.send_command(f"port stop {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to stop port {port}.")
+
+    def port_start_all(self, verify: bool = True):
+        """Start all ports.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+            fail to start."""
+        port_output = self.send_command("port start all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to start all ports.")
+
+    def port_start(self, port: int, verify: bool = True):
+        """Stop all ports.
+
+        Args:
+            port: specifies the port number to use, must be between 0-32
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not started."""
+        port_output = self.send_command(f"port start {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to start port {port}.")
+
+
+    def tx_vlan_set(self, port: int, vlan: int, verify: bool = True):
+        """Set hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            vlan: The vlan tag to insert, should be within 1-4094.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+            tag is not set.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
+        if verify:
+            if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
+            or "Invalid port" in vlan_insert_output):
+                self._logger.debug(f"Failed to set vlan insertion tag {vlan} on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan insertion tag {vlan} on port {port}.")
+
+    def tx_vlan_reset(self, port: int, verify: bool = True):
+        """Disable hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+                tag is not reset.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port}")
+        if verify:
+            if "Please stop port" in vlan_insert_output or "Invalid port" in vlan_insert_output:
+                self._logger.debug(f"Failed to reset vlan insertion on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to reset vlan insertion on port {port}.")
+
+    def set_promisc(self, port: int, on: bool, verify: bool = True):
+        """Turns promiscuous mode on/off for the specified port
+
+        Args:
+            port: port number to use, should be within 0-32.
+            on: if :data:`True`, turn promisc mode on, otherwise turn off.
+            verify: if :data:`True` an additional command will be sent to verify that promisc mode
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and promisc mode
+            is not correctly set.
+        """
+        if on:
+            promisc_output = self.send_command(f"set promisc {port} on")
+        else:
+            promisc_output = self.send_command(f"set promisc {port} off")
+        if verify:
+            if (on and "Promiscuous mode: enabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+            elif (not on and "Promiscuous mode: disabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+
+
+    def set_verbose(self, level: int, verify: bool = True):
+        """Set debug verbosity level.
+
+        Args:
+            level: 0 - silent except for error
+            1 - fully verbose except for Tx packets
+            2 - fully verbose except for Rx packets
+            >2 - fully verbose
+            verify: if :data:`True` an additional command will be sent to verify that verbose level
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
+            is not correctly set.
+        """
+        verbose_output = self.send_command(f"set verbose {level}")
+        if verify:
+            if "Change verbose level" not in verbose_output:
+                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
+
     def close(self) -> None:
         """Overrides :meth:`~.interactive_shell.close`."""
         self.send_command("quit", "")
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v5 2/3] dts: refactored VLAN test suite
  2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
@ 2024-06-18 16:29     ` Dean Marx
  2024-06-21 20:53       ` Jeremy Spewock
  2024-06-18 16:29     ` [PATCH v5 3/3] dts: config schema Dean Marx
  2024-06-21 20:50     ` [PATCH v5 1/3] dts: updated testpmd shell class Jeremy Spewock
  2 siblings, 1 reply; 34+ messages in thread
From: Dean Marx @ 2024-06-18 16:29 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Tweaked logic on sending and verifying packets for 
more concise code, added verbose and promisc 
function calls from pmd shell module.

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

diff --git a/dts/tests/TestSuite_vlan.py b/dts/tests/TestSuite_vlan.py
new file mode 100644
index 0000000000..b9b2a98588
--- /dev/null
+++ b/dts/tests/TestSuite_vlan.py
@@ -0,0 +1,167 @@
+# 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
+        """
+        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load='xxxxx')
+        received_packets = self.send_packet_and_capture(packet)
+        test_packet = None
+        for packet in received_packets:
+            if packet.haslayer(Raw) and packet[Raw].load == b'xxxxx':
+                test_packet = packet
+                break
+        if should_receive:
+            self.verify(
+                test_packet is not None, "Packet was dropped when it should have been received"
+            )
+            if strip:
+                self.verify(Dot1Q not in test_packet, "Vlan tag was not stripped successfully")
+            else:
+                    self.verify(
+                        test_packet.vlan == vlan_id, "The received tag did not match the expected tag"
+                    )
+        else:
+            self.verify(
+                test_packet is None,
+                "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
+        """
+        packet = Ether() / Raw(load='xxxxx')
+        received_packets = self.send_packet_and_capture(packet)
+        test_packet = None
+        for packet in received_packets:
+            if packet.haslayer(Raw) and packet[Raw].load == b'xxxxx':
+                test_packet = packet
+                break
+        self.verify(
+            test_packet is not None, "Packet was dropped when it should have been received"
+        )
+        self.verify(Dot1Q in test_packet, "The received packet did not have a vlan tag")
+        self.verify(test_packet.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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v5 3/3] dts: config schema
  2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
  2024-06-18 16:29     ` [PATCH v5 2/3] dts: refactored VLAN test suite Dean Marx
@ 2024-06-18 16:29     ` Dean Marx
  2024-06-21 20:53       ` Jeremy Spewock
  2024-06-21 20:50     ` [PATCH v5 1/3] dts: updated testpmd shell class Jeremy Spewock
  2 siblings, 1 reply; 34+ messages in thread
From: Dean Marx @ 2024-06-18 16:29 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Configuration to run vlan test suite

Signed-off-by: Dean Marx <dmarx@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 4731f4511d..eca8244f27 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
       "enum": [
         "hello_world",
         "os_udp",
-        "pmd_buffer_scatter"
+        "pmd_buffer_scatter",
+        "vlan"
       ]
     },
     "test_target": {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v5 1/3] dts: updated testpmd shell class
  2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
  2024-06-18 16:29     ` [PATCH v5 2/3] dts: refactored VLAN test suite Dean Marx
  2024-06-18 16:29     ` [PATCH v5 3/3] dts: config schema Dean Marx
@ 2024-06-21 20:50     ` Jeremy Spewock
  2 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-21 20:50 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

Just documentation comments, the structure of the code and the
functionality look good to me.

Maybe a different subject for the commit would be more descriptive.
Something like "add methods required for VLAN test suite to
TestpmdShell". Then you can explain what methods are added in the
description.

On Tue, Jun 18, 2024 at 12:30 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Ported over the promisc and verbose mode functions
> from v2 of the queue start/stop suite to use for the
> VLAN suite. Tweaked some of the verification methods to be
> more concise, changed some docstrings to be more specific.

The descriptions here should be the commit message explaining
everything that is being added, not just the diff from the previous
version. This is generally because this will end up being the
description of the commit once the final version gets merged, so the
people looking at the git history will only see the description of the
final version and have no context for what was added in the previous
versions. The same for the subject line of the commit.

>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---

If you want to have a change log associated with the commit however,
you can put comments under these 3 hyphens that won't get included in
the final commit message.

>  dts/framework/remote_session/testpmd_shell.py | 260 ++++++++++++++++++
>  1 file changed, 260 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index cb2ab6bd00..aad3a3a448 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -225,6 +225,266 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True):
>                  f"Test pmd failed to set fwd mode to {mode.value}"
>              )
>
> +    def vlan_filter_set_on(self, port: int, verify: bool = True):
> +        """Set vlan filter on.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.

A comment on this doc-string and the one in the other vlan filter method:
This might be a little more descriptive if you explained how the port
number is being used. Maybe saying something like "number of the port
to add the filter to". Or, alternatively, you could change the first
line of this comment to say "Enable VLAN filtering for port with id
`port`". If you see the code it's clear how this port is being used,
but if you were only looking at the documentation there isn't much
information about how this port number comes into play.

> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan filtering was enabled successfully. If not, it is
> +                considered an error.

This looks like a case where an additional command is sent rather than
the original output being scanned after the update.

> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
> +        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
> +        if verify:
> +            if "Invalid port" in filter_cmd_output or "filter: on" not in self.send_command(f"show port info {port}"):
> +                self._logger.debug(f"Failed to enable vlan filter on port {port}: \n{filter_cmd_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to enable vlan filter on port {port}.")
> +
> +    def vlan_filter_set_off(self, port: int, verify: bool = True):
> +        """Set vlan filter off.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan filtering was disabled successfully. If not, it is
> +                considered an error.

Here it seems like it's not the output of the command being sent, but
another command being sent to verify.

> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
> +                fails to update.
> +        """
<snip>
> +    def vlan_strip_set_on(self, port: int, verify: bool = True):
> +        """Enable vlan stripping on the specified port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan stripping was enabled on the specified port. If not, it is
> +                considered an error.

Same thing here.

> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
> +                fails to update.
> +        """
> +        vlan_strip_output = self.send_command(f"vlan set strip on {port}")
> +        if verify:
> +            if "strip on" not in self.send_command(f"show port info {port}"):
> +                self._logger.debug(f"Failed to set vlan filter on for port {port}: \n{vlan_strip_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan filter on for port {port}.")
> +
> +    def vlan_strip_set_off(self, port: int, verify: bool = True):
> +        """Disable vlan stripping on the specified port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32

This arg is missing a period at the end of the line.

> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan stripping was disabled on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
> +                fails to update.
> +        """
> +        vlan_strip_output = self.send_command(f"vlan set strip off {port}")
> +        if verify:
> +            if "strip off" not in self.send_command(f"show port info {port}"):
> +                self._logger.debug(f"Failed to disable vlan stripping on port {port}: \n{vlan_strip_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan stripping on port {port}.")
> +
> +    def port_stop_all(self, verify: bool = True):
> +        """Stop all ports.
> +

This doc-string is missing the Args section for the verify parameter.

> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
> +                fail to stop."""
> +        port_output = self.send_command("port stop all")
> +        if verify:
> +            if "Done" not in port_output:
> +                self._logger.debug(f"Failed to stop all ports: \n{port_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to stop all ports.")
> +
> +    def port_stop(self, port: int, verify: bool = True):
> +        """Stop all ports.

This seems like a copy paste error from the previous method.

> +
> +        Args:
> +            port: specifies the port number to use, must be between 0-32

verify is missing from this Args section. There is also a missing
period at the end of the line.

> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
> +                is not stopped."""
> +        port_output = self.send_command(f"port stop {port}")
> +        if verify:
> +            if "Done" not in port_output:
> +                self._logger.debug(f"Failed to stop port {port}: \n{port_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to stop port {port}.")
> +
> +    def port_start_all(self, verify: bool = True):
> +        """Start all ports.
> +

This method is also missing an Args section.

> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
> +            fail to start."""
> +        port_output = self.send_command("port start all")
> +        if verify:
> +            if "Done" not in port_output:
> +                self._logger.debug(f"Failed to start all ports: \n{port_output}")
> +                raise InteractiveCommandExecutionError("Testpmd failed to start all ports.")
> +
> +    def port_start(self, port: int, verify: bool = True):
> +        """Stop all ports.

Seems like a copy-paste error here.

> +
> +        Args:
> +            port: specifies the port number to use, must be between 0-32

Missing period at the end of the line here. The args section also
seems to be missing the verify parameter.

> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
> +                is not started."""
<snip>
> +    def set_promisc(self, port: int, on: bool, verify: bool = True):
> +        """Turns promiscuous mode on/off for the specified port

Missing period at the end of the line here.

> +
> +        Args:
> +            port: port number to use, should be within 0-32.
> +            on: if :data:`True`, turn promisc mode on, otherwise turn off.
> +            verify: if :data:`True` an additional command will be sent to verify that promisc mode
> +                is properly set. Defaults to :data:`True`.
> +
> +        Raises:
<snip>
> +
> +    def set_verbose(self, level: int, verify: bool = True):
> +        """Set debug verbosity level.
> +
> +        Args:
> +            level: 0 - silent except for error
> +            1 - fully verbose except for Tx packets
> +            2 - fully verbose except for Rx packets
> +            >2 - fully verbose

These make sense, but it might help to indent the last 3 just so it's
clear what they apply to.

> +            verify: if :data:`True` an additional command will be sent to verify that verbose level
> +                is properly set. Defaults to :data:`True`.

This method doesn't seem to send another command, it just verifies
from the output of the first.

> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
> +            is not correctly set.
> +        """
> +        verbose_output = self.send_command(f"set verbose {level}")
> +        if verify:
> +            if "Change verbose level" not in verbose_output:
> +                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
> +
>      def close(self) -> None:
>          """Overrides :meth:`~.interactive_shell.close`."""
>          self.send_command("quit", "")
> --
> 2.44.0
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v5 2/3] dts: refactored VLAN test suite
  2024-06-18 16:29     ` [PATCH v5 2/3] dts: refactored VLAN test suite Dean Marx
@ 2024-06-21 20:53       ` Jeremy Spewock
  0 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-21 20:53 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

The subject for this commit should likely explain that this patch adds
the VLAN test suite rather than refacoring it. This sort of falls in
line with what I mentioned  on the previous patch about the commit
messages being about what the entire patch adds rather than the
difference from the last version.

Additionally, I think you will find that a lot of my comments on the
previous version of this commit [1] still apply, but I'll try to point
out anything I didn't mention there in this email.

[1] http://inbox.dpdk.org/dev/CAAA20UTSSNB-t-ty+qpWJaz_hRJ1JX9HtM_kP_utnbynpPB0zw@mail.gmail.com/

On Tue, Jun 18, 2024 at 12:30 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Tweaked logic on sending and verifying packets for
> more concise code, added verbose and promisc
> function calls from pmd shell module.

I mentioned this in more detail in the previous patch, but these
descriptions should be more about what the entire patch adds than a
change log from the previous.

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

The conditions should probably be stated here again briefly, and this
doc-string can go more in depth about how the testing is done than the
module which might help explain the test. You can also touch more here
on what specific cases you are testing.

> +    """
> +
<snip>
> +    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.

It probably makes more sense to call this a SUT rather than DUT since
SUT is what we call it elsewhere in the code.

> +
> +        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
> +        """
> +        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load='xxxxx')

It looks like you are using this payload to filter the received
packets but that might not be immediately obvious to other developers.
A comment about what this is for might help make it more clear.

Additionally, since this same filter is used later to check if you
captured the packet you are looking for, it is probably better to pull
this out into a seperate variable so it is only hard-coded in one
place.

> +        received_packets = self.send_packet_and_capture(packet)
> +        test_packet = None
> +        for packet in received_packets:
> +            if packet.haslayer(Raw) and packet[Raw].load == b'xxxxx':
> +                test_packet = packet
> +                break

I said in the previous version you could use filter() to shrink the
list, if you just want to find one that matches like this you could
instead use the built in function called any() in python to get a
boolean on if the packet exists or not. It does essentially the same
thing you are doing but is a little more concise.

I did something similar with the any function in this patch:
https://patchwork.dpdk.org/project/dpdk/patch/20240613181510.30135-5-jspewock@iol.unh.edu/

> +        if should_receive:
> +            self.verify(
> +                test_packet is not None, "Packet was dropped when it should have been received"
> +            )
> +            if strip:
> +                self.verify(Dot1Q not in test_packet, "Vlan tag was not stripped successfully")
> +            else:
> +                    self.verify(
> +                        test_packet.vlan == vlan_id, "The received tag did not match the expected tag"
> +                    )
> +        else:
> +            self.verify(
> +                test_packet is None,
> +                "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
> +        """
> +        packet = Ether() / Raw(load='xxxxx')

We could do the same thing here with pulling out the filtering into
its own variable.

> +        received_packets = self.send_packet_and_capture(packet)
> +        test_packet = None
> +        for packet in received_packets:
<snip>
> 2.44.0
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v5 3/3] dts: config schema
  2024-06-18 16:29     ` [PATCH v5 3/3] dts: config schema Dean Marx
@ 2024-06-21 20:53       ` Jeremy Spewock
  0 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-21 20:53 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

On Tue, Jun 18, 2024 at 12:30 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Configuration to run vlan test suite
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v6 1/3] dts: updated testpmd shell class
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
                     ` (6 preceding siblings ...)
  2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
@ 2024-06-24 18:17   ` Dean Marx
  2024-06-24 18:17     ` [PATCH v6 2/3] dts: refactored VLAN test suite Dean Marx
  2024-06-24 18:17     ` [PATCH v6 3/3] dts: config schema Dean Marx
  2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
  8 siblings, 2 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-24 18:17 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Ported over the promisc and verbose mode functions
from v2 of the queue start/stop suite to use for the VLAN suite.
Tweaked some of the verification methods to be
more concise, changed some docstrings to be more specific.

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 260 ++++++++++++++++++
 1 file changed, 260 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index cb2ab6bd00..89c9435c17 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -225,6 +225,266 @@ def set_forward_mode(self, mode: TestPmdForwardingModes, verify: bool = True):
                 f"Test pmd failed to set fwd mode to {mode.value}"
             )
 
+    def vlan_filter_set_on(self, port: int, verify: bool = True):
+        """Set vlan filter on.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan filtering was enabled successfully. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output or "filter on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to enable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to enable vlan filter on port {port}.")
+
+    def vlan_filter_set_off(self, port: int, verify: bool = True):
+        """Set vlan filter off.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan filtering was disabled successfully. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter off {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output or "filter off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan filter on port {port}.")
+
+    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+        """Add specified vlan tag to the filter list on a port.
+
+        Args:
+            vlan: The vlan tag to add, should be within 1-1005, 1-4094 extended.
+            port: The port number to add the tag on, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was added to the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not added.
+        """
+        vlan_add_output = self.send_command(f"rx_vlan add {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_add_output or "Invalid vlan_id" in vlan_add_output:
+                self._logger.debug(f"Failed to add vlan tag {vlan} on port {port}: \n{vlan_add_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to add vlan tag {vlan} on port {port}.")
+
+    def rx_vlan_rm(self, vlan: int, port: int, verify: bool = True):
+        """Remove specified vlan tag from filter list on a port.
+
+        Args:
+            vlan: The vlan tag to remove, should be within 1-4094.
+            port: The port number to remove the tag from, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was removed from the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not removed.
+        """
+        vlan_rm_output = self.send_command(f"rx_vlan rm {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_rm_output or "Invalid vlan_id" in vlan_rm_output:
+                self._logger.debug(f"Failed to remove vlan tag {vlan} on port {port}: \n{vlan_rm_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to remove vlan tag {vlan} on port {port}.")
+
+    def vlan_strip_set_on(self, port: int, verify: bool = True):
+        """Enable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan stripping was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip on {port}")
+        if verify:
+            if "strip on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to set vlan filter on for port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan filter on for port {port}.")
+
+    def vlan_strip_set_off(self, port: int, verify: bool = True):
+        """Disable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan stripping was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip off {port}")
+        if verify:
+            if "strip off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan stripping on port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan stripping on port {port}.")
+
+    def port_stop_all(self, verify: bool = True):
+        """Stop all ports.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+                fail to stop."""
+        port_output = self.send_command("port stop all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to stop all ports.")
+
+    def port_stop(self, port: int, verify: bool = True):
+        """Stop all ports.
+
+        Args:
+            port: specifies the port number to use, must be between 0-32
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not stopped."""
+        port_output = self.send_command(f"port stop {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to stop port {port}.")
+
+    def port_start_all(self, verify: bool = True):
+        """Start all ports.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+            fail to start."""
+        port_output = self.send_command("port start all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to start all ports.")
+
+    def port_start(self, port: int, verify: bool = True):
+        """Stop all ports.
+
+        Args:
+            port: specifies the port number to use, must be between 0-32
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not started."""
+        port_output = self.send_command(f"port start {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to start port {port}.")
+
+
+    def tx_vlan_set(self, port: int, vlan: int, verify: bool = True):
+        """Set hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            vlan: The vlan tag to insert, should be within 1-4094.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+            tag is not set.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
+        if verify:
+            if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
+            or "Invalid port" in vlan_insert_output):
+                self._logger.debug(f"Failed to set vlan insertion tag {vlan} on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan insertion tag {vlan} on port {port}.")
+
+    def tx_vlan_reset(self, port: int, verify: bool = True):
+        """Disable hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+                tag is not reset.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port}")
+        if verify:
+            if "Please stop port" in vlan_insert_output or "Invalid port" in vlan_insert_output:
+                self._logger.debug(f"Failed to reset vlan insertion on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to reset vlan insertion on port {port}.")
+
+    def set_promisc(self, port: int, on: bool, verify: bool = True):
+        """Turns promiscuous mode on/off for the specified port
+
+        Args:
+            port: port number to use, should be within 0-32.
+            on: if :data:`True`, turn promisc mode on, otherwise turn off.
+            verify: if :data:`True` an additional command will be sent to verify that promisc mode
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and promisc mode
+            is not correctly set.
+        """
+        if on:
+            promisc_output = self.send_command(f"set promisc {port} on")
+        else:
+            promisc_output = self.send_command(f"set promisc {port} off")
+        if verify:
+            if (on and "Promiscuous mode: enabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+            elif (not on and "Promiscuous mode: disabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+
+
+    def set_verbose(self, level: int, verify: bool = True):
+        """Set debug verbosity level.
+
+        Args:
+            level: 0 - silent except for error
+            1 - fully verbose except for Tx packets
+            2 - fully verbose except for Rx packets
+            >2 - fully verbose
+            verify: if :data:`True` an additional command will be sent to verify that verbose level
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
+            is not correctly set.
+        """
+        verbose_output = self.send_command(f"set verbose {level}")
+        if verify:
+            if "Change verbose level" not in verbose_output:
+                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
+
     def close(self) -> None:
         """Overrides :meth:`~.interactive_shell.close`."""
         self.send_command("quit", "")
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v6 2/3] dts: refactored VLAN test suite
  2024-06-24 18:17   ` [PATCH v6 " Dean Marx
@ 2024-06-24 18:17     ` Dean Marx
  2024-06-24 18:17     ` [PATCH v6 3/3] dts: config schema Dean Marx
  1 sibling, 0 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-24 18:17 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Tweaked logic on sending and verifying packets
 for more concise code, added verbose
and promisc function calls from pmd shell module.

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

diff --git a/dts/tests/TestSuite_vlan.py b/dts/tests/TestSuite_vlan.py
new file mode 100644
index 0000000000..24406d065d
--- /dev/null
+++ b/dts/tests/TestSuite_vlan.py
@@ -0,0 +1,167 @@
+# 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
+        """
+        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load='xxxxx')
+        received_packets = self.send_packet_and_capture(packet)
+        test_packet = None
+        for packet in received_packets:
+            if b'xxxxx' in packet.load:
+                test_packet = packet
+                break
+        if should_receive:
+            self.verify(
+                test_packet is not None, "Packet was dropped when it should have been received"
+            )
+            if strip:
+                self.verify(Dot1Q not in test_packet, "Vlan tag was not stripped successfully")
+            else:
+                    self.verify(
+                        test_packet.vlan == vlan_id, "The received tag did not match the expected tag"
+                    )
+        else:
+            self.verify(
+                test_packet is None,
+                "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
+        """
+        packet = Ether() / Raw(load='xxxxx')
+        received_packets = self.send_packet_and_capture(packet)
+        test_packet = None
+        for packet in received_packets:
+            if b'xxxxx' in packet.load:
+                test_packet = packet
+                break
+        self.verify(
+            test_packet is not None, "Packet was dropped when it should have been received"
+        )
+        self.verify(Dot1Q in test_packet, "The received packet did not have a vlan tag")
+        self.verify(test_packet.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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v6 3/3] dts: config schema
  2024-06-24 18:17   ` [PATCH v6 " Dean Marx
  2024-06-24 18:17     ` [PATCH v6 2/3] dts: refactored VLAN test suite Dean Marx
@ 2024-06-24 18:17     ` Dean Marx
  1 sibling, 0 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-24 18:17 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Configuration to run vlan test suite

Signed-off-by: Dean Marx <dmarx@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 4731f4511d..eca8244f27 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
       "enum": [
         "hello_world",
         "os_udp",
-        "pmd_buffer_scatter"
+        "pmd_buffer_scatter",
+        "vlan"
       ]
     },
     "test_target": {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v7 1/3] dts: VLAN test suite implementation
  2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
                     ` (7 preceding siblings ...)
  2024-06-24 18:17   ` [PATCH v6 " Dean Marx
@ 2024-06-25 15:33   ` Dean Marx
  2024-06-25 15:33     ` [PATCH v7 2/3] dts: add VLAN methods to testpmd shell Dean Marx
                       ` (2 more replies)
  8 siblings, 3 replies; 34+ messages in thread
From: Dean Marx @ 2024-06-25 15:33 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Test suite for verifying VLAN filtering, stripping, and insertion
functionality on Poll Mode Driver.

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

diff --git a/dts/tests/TestSuite_vlan.py b/dts/tests/TestSuite_vlan.py
new file mode 100644
index 0000000000..e3c9d8dcf4
--- /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 time import sleep
+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 TestPmdShell, SimpleForwardingModes
+from framework.test_suite import TestSuite
+
+
+class TestVlan(TestSuite):
+    """DPDK VLAN test suite.
+
+    Ensures VLAN packet reception, stripping, and insertion on the Poll Mode Driver
+    when the appropriate conditions are met. The suite contains four test cases:
+
+    1. VLAN reception no stripping - verifies that a vlan packet with a tag
+    within the filter list is received.
+    2. VLAN reception stripping - verifies that a vlan packet with a tag
+    within the filter list is received without the vlan tag.
+    3. VLAN no reception - verifies that a vlan packet with a tag not within
+    the filter list is dropped.
+    4. VLAN insertion - verifies that a non vlan packet is received with a vlan
+    tag when insertion is enabled.
+    """
+
+    def set_up_suite(self) -> None:
+        """Set up the test suite.
+
+        Setup:
+            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, strip: bool, vlan_id: int
+    ) -> None:
+        """Generate a vlan packet, send and verify a packet with
+        the same payload is received 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.
+        """
+        packet = Ether() / Dot1Q(vlan=vlan_id) / Raw(load='xxxxx')
+        received_packets = self.send_packet_and_capture(packet)
+        test_packet = None
+        for packet in received_packets:
+            if b'xxxxx' in packet.load:
+                test_packet = packet
+                break
+        if should_receive:
+            self.verify(
+                test_packet is not None, "Packet was dropped when it should have been received"
+            )
+            if strip:
+                self.verify(Dot1Q not in test_packet, "Vlan tag was not stripped successfully")
+            else:
+                    self.verify(
+                        test_packet.vlan == vlan_id, "The received tag did not match the expected tag"
+                    )
+        else:
+            self.verify(
+                test_packet is None,
+                "Packet was received when it should have been dropped",
+            )
+
+    def send_packet_and_verify_insertion(self, expected_id: int) -> 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.
+        """
+        packet = Ether() / Raw(load='xxxxx')
+        received_packets = self.send_packet_and_capture(packet)
+        test_packet = None
+        for packet in received_packets:
+            if b'xxxxx' in packet.load:
+                test_packet = packet
+                break
+        self.verify(
+            test_packet is not None, "Packet was dropped when it should have been received"
+        )
+        self.verify(Dot1Q in test_packet, "The received packet did not have a vlan tag")
+        self.verify(test_packet.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 = TestPmdShell(node=self.sut_node)
+        testpmd.set_forward_mode(SimpleForwardingModes.mac)
+        testpmd.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        testpmd.vlan_filter_set_on(0)
+        filtered_vlan = 1
+        testpmd.rx_vlan_add(filtered_vlan, 0)
+        testpmd.start()
+
+        self.send_vlan_packet_and_verify(True, strip=False, 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 = TestPmdShell(node=self.sut_node)
+        testpmd.set_forward_mode(SimpleForwardingModes.mac)
+        testpmd.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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 = TestPmdShell(node=self.sut_node)
+        testpmd.set_forward_mode(SimpleForwardingModes.mac)
+        testpmd.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        testpmd.vlan_filter_set_on(0)
+        filtered_vlan = 1
+        testpmd.rx_vlan_add(filtered_vlan, 0)
+        testpmd.start()
+
+        self.send_vlan_packet_and_verify(should_receive=False, strip=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 = TestPmdShell(node=self.sut_node)
+        testpmd.set_forward_mode(SimpleForwardingModes.mac)
+        testpmd.set_verbose(1)
+        testpmd.set_promisc(0, False)
+        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()
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v7 2/3] dts: add VLAN methods to testpmd shell
  2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
@ 2024-06-25 15:33     ` Dean Marx
  2024-06-26 18:22       ` Jeremy Spewock
  2024-06-25 15:33     ` [PATCH v7 3/3] dts: config schema Dean Marx
  2024-06-26 18:21     ` [PATCH v7 1/3] dts: VLAN test suite implementation Jeremy Spewock
  2 siblings, 1 reply; 34+ messages in thread
From: Dean Marx @ 2024-06-25 15:33 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

added the following methods to testpmd shell class:
vlan set filter on/off, rx vlan add/rm,
vlan set strip on/off, port stop/start all/port,
tx vlan set/reset, set promisc/verbose

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 278 ++++++++++++++++++
 1 file changed, 278 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..d504e92a45 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -806,6 +806,284 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
 
         return TestPmdPortStats.parse(output)
 
+    def vlan_filter_set_on(self, port: int, verify: bool = True):
+        """Set vlan filter on.
+
+        Args:
+            port: The port number to enable VLAN filter on, should be within 0-32.
+            verify: If :data:`True`, the output of the command and show port info
+                is scanned to verify that vlan filtering was enabled successfully.
+                If not, it is considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter on {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output or "filter on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to enable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to enable vlan filter on port {port}.")
+
+    def vlan_filter_set_off(self, port: int, verify: bool = True):
+        """Set vlan filter off.
+
+        Args:
+            port: The port number to disable VLAN filter on, should be within 0-32.
+            verify: If :data:`True`, the output of the command and show port info
+                is scanned to verify that vlan filtering was disabled successfully.
+                If not, it is considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the filter
+                fails to update.
+        """
+        filter_cmd_output = self.send_command(f"vlan set filter off {port}")
+        if verify:
+            if "Invalid port" in filter_cmd_output or "filter off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan filter on port {port}: \n{filter_cmd_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan filter on port {port}.")
+
+    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
+        """Add specified vlan tag to the filter list on a port.
+
+        Args:
+            vlan: The vlan tag to add, should be within 1-1005, 1-4094 extended.
+            port: The port number to add the tag on, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was added to the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not added.
+        """
+        vlan_add_output = self.send_command(f"rx_vlan add {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_add_output or "Invalid vlan_id" in vlan_add_output:
+                self._logger.debug(f"Failed to add vlan tag {vlan} on port {port}: \n{vlan_add_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to add vlan tag {vlan} on port {port}.")
+
+    def rx_vlan_rm(self, vlan: int, port: int, verify: bool = True):
+        """Remove specified vlan tag from filter list on a port.
+
+        Args:
+            vlan: The vlan tag to remove, should be within 1-4094.
+            port: The port number to remove the tag from, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                the vlan tag was removed from the filter list on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
+            is not removed.
+        """
+        vlan_rm_output = self.send_command(f"rx_vlan rm {vlan} {port}")
+        if verify:
+            if "VLAN-filtering disabled" in vlan_rm_output or "Invalid vlan_id" in vlan_rm_output:
+                self._logger.debug(f"Failed to remove vlan tag {vlan} on port {port}: \n{vlan_rm_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to remove vlan tag {vlan} on port {port}.")
+
+    def vlan_strip_set_on(self, port: int, verify: bool = True):
+        """Enable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command and show port info
+                is scanned to verify that vlan stripping was enabled on the specified port.
+                If not, it is considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip on {port}")
+        if verify:
+            if "strip on" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to set vlan filter on for port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan filter on for port {port}.")
+
+    def vlan_strip_set_off(self, port: int, verify: bool = True):
+        """Disable vlan stripping on the specified port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command and show port info
+                is scanned to verify that vlan stripping was disabled on the specified port.
+                If not, it is considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and stripping
+                fails to update.
+        """
+        vlan_strip_output = self.send_command(f"vlan set strip off {port}")
+        if verify:
+            if "strip off" not in self.send_command(f"show port info {port}"):
+                self._logger.debug(f"Failed to disable vlan stripping on port {port}: \n{vlan_strip_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to disable vlan stripping on port {port}.")
+
+    def port_stop_all(self, verify: bool = True):
+        """Stop all ports.
+
+        Args:
+            port: Specifies the port number to use, must be between 0-32.
+            verify: If :data:`True`, the output of the command is scanned
+                to ensure all ports are stopped. If not, it is considered
+                an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+                fail to stop."""
+        port_output = self.send_command("port stop all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to stop all ports.")
+
+    def port_stop(self, port: int, verify: bool = True):
+        """Stop specified port.
+
+        Args:
+            port: Specifies the port number to use, must be between 0-32.
+            verify: If :data:`True`, the output of the command is scanned
+                to ensure specified port is stopped. If not, it is considered
+                an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not stopped."""
+        port_output = self.send_command(f"port stop {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to stop port {port}.")
+
+    def port_start_all(self, verify: bool = True):
+        """Start all ports.
+
+        Args:
+            port: Specifies the port number to use, must be between 0-32.
+            verify: If :data:`True`, the output of the command is scanned
+                to ensure all ports are started. If not, it is considered
+                an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and all ports
+            fail to start."""
+        port_output = self.send_command("port start all")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start all ports: \n{port_output}")
+                raise InteractiveCommandExecutionError("Testpmd failed to start all ports.")
+
+    def port_start(self, port: int, verify: bool = True):
+        """Start specified port.
+
+        Args:
+            port: Specifies the port number to use, must be between 0-32.
+            verify: If :data:`True`, the output of the command is scanned
+                to ensure specified port is started. If not, it is considered
+                an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not started."""
+        port_output = self.send_command(f"port start {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to start port {port}.")
+
+
+    def tx_vlan_set(self, port: int, vlan: int, verify: bool = True):
+        """Set hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            vlan: The vlan tag to insert, should be within 1-4094.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was enabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+            tag is not set.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
+        if verify:
+            if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
+            or "Invalid port" in vlan_insert_output):
+                self._logger.debug(f"Failed to set vlan insertion tag {vlan} on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan insertion tag {vlan} on port {port}.")
+
+    def tx_vlan_reset(self, port: int, verify: bool = True):
+        """Disable hardware insertion of vlan tags in packets sent on a port.
+
+        Args:
+            port: The port number to use, should be within 0-32.
+            verify: If :data:`True`, the output of the command is scanned to verify that
+                vlan insertion was disabled on the specified port. If not, it is
+                considered an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
+                tag is not reset.
+        """
+        vlan_insert_output = self.send_command(f"tx_vlan set {port}")
+        if verify:
+            if "Please stop port" in vlan_insert_output or "Invalid port" in vlan_insert_output:
+                self._logger.debug(f"Failed to reset vlan insertion on port {port}: \n{vlan_insert_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to reset vlan insertion on port {port}.")
+
+    def set_promisc(self, port: int, on: bool, verify: bool = True):
+        """Turns promiscuous mode on/off for the specified port
+
+        Args:
+            port: Port number to use, should be within 0-32.
+            on: If :data:`True`, turn promisc mode on, otherwise turn off.
+            verify: If :data:`True` an additional command will be sent to verify that promisc mode
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and promisc mode
+            is not correctly set.
+        """
+        if on:
+            promisc_output = self.send_command(f"set promisc {port} on")
+        else:
+            promisc_output = self.send_command(f"set promisc {port} off")
+        if verify:
+            if (on and "Promiscuous mode: enabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+            elif (not on and "Promiscuous mode: disabled" not in
+            self.send_command(f"show port info {port}")):
+                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
+
+
+    def set_verbose(self, level: int, verify: bool = True):
+        """Set debug verbosity level.
+
+        Args:
+            level: 0 - silent except for error
+                1 - fully verbose except for Tx packets
+                2 - fully verbose except for Rx packets
+                >2 - fully verbose
+            verify: If :data:`True` the command output will be scanned to verify that verbose level
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
+            is not correctly set.
+        """
+        verbose_output = self.send_command(f"set verbose {level}")
+        if verify:
+            if "Change verbose level" not in verbose_output:
+                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
+
     def close(self) -> None:
         """Overrides :meth:`~.interactive_shell.close`."""
         self.send_command("quit", "")
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v7 3/3] dts: config schema
  2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
  2024-06-25 15:33     ` [PATCH v7 2/3] dts: add VLAN methods to testpmd shell Dean Marx
@ 2024-06-25 15:33     ` Dean Marx
  2024-06-26 18:23       ` Jeremy Spewock
  2024-06-26 18:21     ` [PATCH v7 1/3] dts: VLAN test suite implementation Jeremy Spewock
  2 siblings, 1 reply; 34+ messages in thread
From: Dean Marx @ 2024-06-25 15:33 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, jspewock, bruce.richardson, luca.vizzarro
  Cc: dev, Dean Marx

Configuration to run vlan test suite

Signed-off-by: Dean Marx <dmarx@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 f02a310bb5..cd45902cc4 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -187,7 +187,8 @@
       "enum": [
         "hello_world",
         "os_udp",
-        "pmd_buffer_scatter"
+        "pmd_buffer_scatter",
+        "vlan"
       ]
     },
     "test_target": {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v7 1/3] dts: VLAN test suite implementation
  2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
  2024-06-25 15:33     ` [PATCH v7 2/3] dts: add VLAN methods to testpmd shell Dean Marx
  2024-06-25 15:33     ` [PATCH v7 3/3] dts: config schema Dean Marx
@ 2024-06-26 18:21     ` Jeremy Spewock
  2 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-26 18:21 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

Hey Dean, thanks for the update! I just went through and added my
comments to the patches, but most of my feedback was about
documentation and a few places where I think the classes could be
improved by refactoring a little bit and breaking some things out into
their own methods.

A few comments on the overall series however:
I think this patch should come after the patch that adds the testpmd
functions. Just because the order they appear in the series is the
order they get applied, so this patch without the patch that adds the
testpmd methods would throw a lot of errors because the methods don't
exist yet.

Also, could you run the formatting script
(devtools/dts-check-format.sh in the DPDK directory) on this series
when sending out the next version? I noticed it was throwing some
warnings/errors.

On Tue, Jun 25, 2024 at 11:34 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Test suite for verifying VLAN filtering, stripping, and insertion
> functionality on Poll Mode Driver.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
> +    def send_vlan_packet_and_verify(
> +        self, should_receive: bool, strip: bool, vlan_id: int
> +    ) -> None:
> +        """Generate a vlan packet, send and verify a packet with
> +        the same payload is received 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.

We probably should have these args listed in the order that they
appear in the function (should_receive, strip, vlan_id).

> +        """
<snip>
> +
> +    def send_packet_and_verify_insertion(self, expected_id: int) -> 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.

The should_receive parameter seems like it was removed, we should
probably remove this part of the doc-string as well.

> +        """
> +        packet = Ether() / Raw(load='xxxxx')
> +        received_packets = self.send_packet_and_capture(packet)
> +        test_packet = None
> +        for packet in received_packets:
> +            if b'xxxxx' in packet.load:
> +                test_packet = packet
> +                break
> +        self.verify(
> +            test_packet is not None, "Packet was dropped when it should have been received"
> +        )
> +        self.verify(Dot1Q in test_packet, "The received packet did not have a vlan tag")
> +        self.verify(test_packet.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 = TestPmdShell(node=self.sut_node)
> +        testpmd.set_forward_mode(SimpleForwardingModes.mac)
> +        testpmd.set_verbose(1)
> +        testpmd.set_promisc(0, False)
> +        testpmd.vlan_filter_set_on(0)
> +        filtered_vlan = 1
> +        testpmd.rx_vlan_add(filtered_vlan, 0)
> +        testpmd.start()

All of these test cases have the exact same start sequences (other
than one additional command for the tests that involve stripping, or
the 3 extra in the insertion case), we should find a way to pull this
out into another function that they can all call that does the testpmd
setup. Or, alternatively, you could make a decorator for these test
methods that handles all of the testpmd commands. That way, these
test_ methods only need to concern themselves with how they call the
method for sending and verifying packets and don't need to run any
testpmd commands.

I slightly prefer the decorator approach since I think that would be
cleaner than if you were to make a testpmd shell, pass it into a setup
function, and then close it after running testing, but it might be
less clear what's actually going on. Regardless, the main point of it
is having less code duplication between functions.

> +
> +        self.send_vlan_packet_and_verify(True, strip=False, 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 = TestPmdShell(node=self.sut_node)
> +        testpmd.set_forward_mode(SimpleForwardingModes.mac)
> +        testpmd.set_verbose(1)
> +        testpmd.set_promisc(0, False)
> +        testpmd.vlan_filter_set_on(0)
> +        testpmd.rx_vlan_add(1, 0)

This VLAN tag should probably also be pulled out into a variable like
it is in other test methods, but we really should just do it once in a
separate setup method instead of changing this to match up.

> +        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 = TestPmdShell(node=self.sut_node)
> +        testpmd.set_forward_mode(SimpleForwardingModes.mac)
> +        testpmd.set_verbose(1)
> +        testpmd.set_promisc(0, False)
> +        testpmd.vlan_filter_set_on(0)
> +        filtered_vlan = 1
> +        testpmd.rx_vlan_add(filtered_vlan, 0)
> +        testpmd.start()
> +
> +        self.send_vlan_packet_and_verify(should_receive=False, strip=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 = TestPmdShell(node=self.sut_node)
> +        testpmd.set_forward_mode(SimpleForwardingModes.mac)
> +        testpmd.set_verbose(1)
> +        testpmd.set_promisc(0, False)
> +        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()
> --
> 2.44.0
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v7 2/3] dts: add VLAN methods to testpmd shell
  2024-06-25 15:33     ` [PATCH v7 2/3] dts: add VLAN methods to testpmd shell Dean Marx
@ 2024-06-26 18:22       ` Jeremy Spewock
  0 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-26 18:22 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

The functionality all looks good to me, just a few documentation
nit-picks and a suggestion for something that I think could shorten up
one function.

On Tue, Jun 25, 2024 at 11:34 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> added the following methods to testpmd shell class:
> vlan set filter on/off, rx vlan add/rm,
> vlan set strip on/off, port stop/start all/port,
> tx vlan set/reset, set promisc/verbose
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 278 ++++++++++++++++++
>  1 file changed, 278 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index ec22f72221..d504e92a45 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -806,6 +806,284 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
>
>          return TestPmdPortStats.parse(output)
>
<snip>
> +    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True):
> +        """Add specified vlan tag to the filter list on a port.
> +
> +        Args:
> +            vlan: The vlan tag to add, should be within 1-1005, 1-4094 extended.
> +            port: The port number to add the tag on, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                the vlan tag was added to the filter list on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
> +            is not added.

Just a little documentation nit-pick, I noticed this second line is
indented in most other methods, we should probably keep this
consistent.

> +        """
> +        vlan_add_output = self.send_command(f"rx_vlan add {vlan} {port}")
> +        if verify:
> +            if "VLAN-filtering disabled" in vlan_add_output or "Invalid vlan_id" in vlan_add_output:
> +                self._logger.debug(f"Failed to add vlan tag {vlan} on port {port}: \n{vlan_add_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to add vlan tag {vlan} on port {port}.")
> +
> +    def rx_vlan_rm(self, vlan: int, port: int, verify: bool = True):
> +        """Remove specified vlan tag from filter list on a port.
> +
> +        Args:
> +            vlan: The vlan tag to remove, should be within 1-4094.
> +            port: The port number to remove the tag from, should be within 0-32.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                the vlan tag was removed from the filter list on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the tag
> +            is not removed.

Same thing here.

> +        """
> +        vlan_rm_output = self.send_command(f"rx_vlan rm {vlan} {port}")
> +        if verify:
> +            if "VLAN-filtering disabled" in vlan_rm_output or "Invalid vlan_id" in vlan_rm_output:
> +                self._logger.debug(f"Failed to remove vlan tag {vlan} on port {port}: \n{vlan_rm_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to remove vlan tag {vlan} on port {port}.")
> +
<snip>
> +    def tx_vlan_set(self, port: int, vlan: int, verify: bool = True):
> +        """Set hardware insertion of vlan tags in packets sent on a port.
> +
> +        Args:
> +            port: The port number to use, should be within 0-32.
> +            vlan: The vlan tag to insert, should be within 1-4094.
> +            verify: If :data:`True`, the output of the command is scanned to verify that
> +                vlan insertion was enabled on the specified port. If not, it is
> +                considered an error.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and the insertion
> +            tag is not set.

This should probably also be indented.

> +        """
> +        vlan_insert_output = self.send_command(f"tx_vlan set {port} {vlan}")
> +        if verify:
> +            if ("Please stop port" in vlan_insert_output or "Invalid vlan_id" in vlan_insert_output
> +            or "Invalid port" in vlan_insert_output):
> +                self._logger.debug(f"Failed to set vlan insertion tag {vlan} on port {port}: \n{vlan_insert_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set vlan insertion tag {vlan} on port {port}.")
> +
<snip>
> +    def set_promisc(self, port: int, on: bool, verify: bool = True):
> +        """Turns promiscuous mode on/off for the specified port
> +
> +        Args:
> +            port: Port number to use, should be within 0-32.
> +            on: If :data:`True`, turn promisc mode on, otherwise turn off.
> +            verify: If :data:`True` an additional command will be sent to verify that promisc mode
> +                is properly set. Defaults to :data:`True`.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and promisc mode
> +            is not correctly set.

This should probably also be indented.

> +        """
> +        if on:
> +            promisc_output = self.send_command(f"set promisc {port} on")
> +        else:
> +            promisc_output = self.send_command(f"set promisc {port} off")

You can inline this if-else-statement to shorten it up. Something like this:
promisc_output = self.send_command(f"set promisc {port} {'on' if on
else 'off''}")

> +        if verify:
> +            if (on and "Promiscuous mode: enabled" not in
> +            self.send_command(f"show port info {port}")):
> +                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
> +            elif (not on and "Promiscuous mode: disabled" not in
> +            self.send_command(f"show port info {port}")):
> +                self._logger.debug(f"Failed to set promisc mode on port {port}: \n{promisc_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set promisc mode on port {port}.")
> +

I actually mentioned this on Nick's patch, but I think this could be
shortened. I'll copy what I said there:

The logger output and the error seem to be exactly the same here so we
should avoid duplicating them. There are a few ways to go about
combining these two cases in the conditional but I would probably just
use an f-string to conditionally look for "enabled" vs "disabled".

I wonder if this check would be easier using the dataclass for port
info since it will be a boolean inside of the dataclass rather than
just searching for a string. I think if you had a boolean for if
promisc mode was on you could use an XOR of add and is_promisc_mode
and it would have the same effect. Normally I avoid using the
dataclass if the check is simple without it just because I think it is
slightly faster that way, but if there is a good use-case for it (like
there is here) then I think we might as well use it.

I think using the testpmd.show_port_info method would make for a
cleaner approach, so I slightly favor that one.


> +
> +    def set_verbose(self, level: int, verify: bool = True):
> +        """Set debug verbosity level.
> +
> +        Args:
> +            level: 0 - silent except for error
> +                1 - fully verbose except for Tx packets
> +                2 - fully verbose except for Rx packets
> +                >2 - fully verbose
> +            verify: If :data:`True` the command output will be scanned to verify that verbose level
> +                is properly set. Defaults to :data:`True`.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
> +            is not correctly set.
> +        """
> +        verbose_output = self.send_command(f"set verbose {level}")
> +        if verify:
> +            if "Change verbose level" not in verbose_output:
> +                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
> +                raise InteractiveCommandExecutionError(f"Testpmd failed to set verbose level to {level}.")
> +
>      def close(self) -> None:
>          """Overrides :meth:`~.interactive_shell.close`."""
>          self.send_command("quit", "")
> --
> 2.44.0
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v7 3/3] dts: config schema
  2024-06-25 15:33     ` [PATCH v7 3/3] dts: config schema Dean Marx
@ 2024-06-26 18:23       ` Jeremy Spewock
  0 siblings, 0 replies; 34+ messages in thread
From: Jeremy Spewock @ 2024-06-26 18:23 UTC (permalink / raw)
  To: Dean Marx
  Cc: Honnappa.Nagarahalli, juraj.linkes, probb, paul.szczepanek,
	yoan.picchi, bruce.richardson, luca.vizzarro, dev

On Tue, Jun 25, 2024 at 11:34 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Configuration to run vlan test suite
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2024-06-26 18:23 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-11 16:15 [PATCH v2 0/2] VLAN test suite Dean Marx
2024-06-11 16:15 ` [PATCH v2 1/2] Initial implementation for " Dean Marx
2024-06-11 16:15 ` [PATCH v2 2/2] conf schema Dean Marx
2024-06-14 15:02 ` [PATCH v3 0/3] VLAN Test Suite Dean Marx
2024-06-14 15:02   ` [PATCH v3 1/3] Added VLAN commands to testpmd_shell class Dean Marx
2024-06-14 15:59     ` Patrick Robb
2024-06-14 20:29       ` Jeremy Spewock
2024-06-14 21:24         ` Patrick Robb
2024-06-17 14:37     ` Jeremy Spewock
2024-06-14 15:02   ` [PATCH v3 2/3] Initial implementation for VLAN test suite Dean Marx
2024-06-14 16:19     ` Patrick Robb
2024-06-17 14:56     ` Jeremy Spewock
2024-06-14 15:02   ` [PATCH v3 3/3] Config schema Dean Marx
2024-06-17 14:59     ` Jeremy Spewock
2024-06-17 14:35   ` [PATCH v3 0/3] VLAN Test Suite Jeremy Spewock
2024-06-17 17:50   ` Patrick Robb
2024-06-18 15:20   ` [PATCH v4 1/3] dts: refactored VLAN test suite Dean Marx
2024-06-18 15:20     ` [PATCH v4 2/3] dts: updated testpmd shell class Dean Marx
2024-06-18 15:20     ` [PATCH v4 3/3] dts: config schema Dean Marx
2024-06-18 16:29   ` [PATCH v5 1/3] dts: updated testpmd shell class Dean Marx
2024-06-18 16:29     ` [PATCH v5 2/3] dts: refactored VLAN test suite Dean Marx
2024-06-21 20:53       ` Jeremy Spewock
2024-06-18 16:29     ` [PATCH v5 3/3] dts: config schema Dean Marx
2024-06-21 20:53       ` Jeremy Spewock
2024-06-21 20:50     ` [PATCH v5 1/3] dts: updated testpmd shell class Jeremy Spewock
2024-06-24 18:17   ` [PATCH v6 " Dean Marx
2024-06-24 18:17     ` [PATCH v6 2/3] dts: refactored VLAN test suite Dean Marx
2024-06-24 18:17     ` [PATCH v6 3/3] dts: config schema Dean Marx
2024-06-25 15:33   ` [PATCH v7 1/3] dts: VLAN test suite implementation Dean Marx
2024-06-25 15:33     ` [PATCH v7 2/3] dts: add VLAN methods to testpmd shell Dean Marx
2024-06-26 18:22       ` Jeremy Spewock
2024-06-25 15:33     ` [PATCH v7 3/3] dts: config schema Dean Marx
2024-06-26 18:23       ` Jeremy Spewock
2024-06-26 18:21     ` [PATCH v7 1/3] dts: VLAN test suite implementation Jeremy Spewock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).