DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Mac Filter Port to New DTS
@ 2024-07-02 19:24 Nicholas Pratte
  2024-07-02 19:24 ` [PATCH v2 1/3] dts: add boolean to adjust addresses Nicholas Pratte
                   ` (6 more replies)
  0 siblings, 7 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-02 19:24 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, probb, jspewock, juraj.linkes,
	paul.szczepanek, luca.vizzarro, yoan.picchi, dmarx
  Cc: dev, Nicholas Pratte

This series incorporates the test suite itself, in addition to framwork
changes needed to complete the suite's stipulated test cases. The test
suite consists of three test cases: one testing basically mac filtering
functionality, the other testing the PMD's mac address pool behavior
within a TestPMD shell, and the third testing filtering functionality
of multicast mac addresses.

The port from old DTS has removed some potentially important details
regarding EAL and TestPMD parameters, and these can be found within the
test suites corresponding commit messages. Moreover, unlike old DTS,
instead of validating packet behavior using TestPMD, this test suite
validates behavior by sending, and analysing,a list of received packets.

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json    |   3 +-
 dts/framework/remote_session/testpmd_shell.py | 177 ++++++++++++++
 dts/framework/test_suite.py                   |   7 +-
 dts/tests/TestSuite_mac_filter.py             | 220 ++++++++++++++++++
 4 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0


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

* [PATCH v2 1/3] dts: add boolean to adjust addresses
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
@ 2024-07-02 19:24 ` Nicholas Pratte
  2024-07-11 19:31   ` Jeremy Spewock
  2024-07-22 13:09   ` Dean Marx
  2024-07-02 19:24 ` [PATCH v2 2/3] dts: add testpmd methods for test suite Nicholas Pratte
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-02 19:24 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, probb, jspewock, juraj.linkes,
	paul.szczepanek, luca.vizzarro, yoan.picchi, dmarx
  Cc: dev, Nicholas Pratte

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/test_suite.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        adjust_addresses: bool = True,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            adjust_addresses: If :data:'True', adjust addresses of the egressing packet with
+                a default addressing scheme. If :data:'False', do not adjust the addresses of
+                egressing packet.
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if adjust_addresses:
+            packet = self._adjust_addresses(packet)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,
-- 
2.44.0


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

* [PATCH v2 2/3] dts: add testpmd methods for test suite
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
  2024-07-02 19:24 ` [PATCH v2 1/3] dts: add boolean to adjust addresses Nicholas Pratte
@ 2024-07-02 19:24 ` Nicholas Pratte
  2024-07-11 19:33   ` Jeremy Spewock
  2024-07-02 19:24 ` [PATCH v2 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-02 19:24 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, probb, jspewock, juraj.linkes,
	paul.szczepanek, luca.vizzarro, yoan.picchi, dmarx
  Cc: dev, Nicholas Pratte

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a booling 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 177 ++++++++++++++++++
 dts/tests/TestSuite_mac_filter.py             |   0
 2 files changed, 177 insertions(+)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..0be1fb8754 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,183 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
         return TestPmdPort.parse(output)
 
+    def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: bool = True):
+        """Add or remove a mac address on a given port's Allowlist.
+
+        Args:
+            port_id: The port ID the mac address is set on.
+            mac_address: The mac address to be added or removed to the specified port.
+            add: If :data:`True`, add the specified mac address. If :data:`False`, remove specified
+                mac address.
+            verify: If :data:'True', assert that the 'mac_addr' operation was successful. If
+                :data:'False', run the command and skip this assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If the set mac address operation fails.
+        """
+        mac_cmd = "add" if add else "remove"
+        output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid argument provided to mac_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if "mac_addr_cmd error:" in output:
+                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mac_cmd} {mac_address} on port {port_id} \n{output}"
+                )
+
+    def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, verify: bool = True):
+        """Add or remove multicast mac address to a specified port filter.
+
+        Args:
+            port_id: The port ID the multicast address is set on.
+            multi_addr: The multicast address to be added to the filter.
+            add: If :data:'True', add the specified multicast address to the port filter.
+                If :data:'False', remove the specified multicast address from the port filter.
+            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
+                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
+        """
+        mcast_cmd = "add" if add else "remove"
+        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid arguments provided to mcast_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if (
+                "Invalid multicast_addr" in output
+                or f'multicast address {"already" if add else "not"} filtered by port' in output
+            ):
+                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
+                )
+
+    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_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 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.
+        """
+        promisc_output = self.send_command(f"set promisc {port} {'on' if on else 'off'}")
+        if verify:
+            stats = self.show_port_info(port_id=port)
+            if on ^ stats.is_promiscuous_mode_enabled:
+                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 show_port_stats_all(self) -> list[TestPmdPortStats]:
         """Returns the statistics of all the ports.
 
diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
new file mode 100644
index 0000000000..e69de29bb2
-- 
2.44.0


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

* [PATCH v2 3/3] dts: mac filter test suite refactored for new dts
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
  2024-07-02 19:24 ` [PATCH v2 1/3] dts: add boolean to adjust addresses Nicholas Pratte
  2024-07-02 19:24 ` [PATCH v2 2/3] dts: add testpmd methods for test suite Nicholas Pratte
@ 2024-07-02 19:24 ` Nicholas Pratte
  2024-07-11 19:34   ` Jeremy Spewock
  2024-07-18 19:05 ` [PATCH v3 0/3] Mac Filter Port to New DTS Nicholas Pratte
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-02 19:24 UTC (permalink / raw)
  To: Honnappa.Nagarahalli, probb, jspewock, juraj.linkes,
	paul.szczepanek, luca.vizzarro, yoan.picchi, dmarx
  Cc: dev, Nicholas Pratte

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py          | 220 +++++++++++++++++++++
 2 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 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",
+        "mac_filter"
       ]
     },
     "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
index e69de29bb2..c960d6b36c 100644
--- a/dts/tests/TestSuite_mac_filter.py
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a device
+contains a mac address not contained with its mac address pool, the packet should
+be dropped. Alternatively, if a packet is received that contains a destination mac
+within the devices address pool, the packet should be accepted and forwarded. This
+behavior should remain consistent across all packets, namely those containing dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac address
+allow list is behaving as expected.
+"""
+
+from time import sleep
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+    """Mac address allowlist filtering test suite.
+
+    Configure mac address filtering on a given port, and test the port's filtering behavior
+    using both a given port's hardware address as well as dummy addresses. If a port accepts
+    a packet that is not contained within its mac address allowlist, then a given test case
+    fails. Alternatively, if a port drops a packet that is designated within its mac address
+    allowlist, a given test case will fail.
+
+    Moreover, a given port should demonstrate proper behavior when bound to the Poll Mode
+    Driver. A port should not have an mac address allowlist that exceeds its designated size.
+    A port's default hardware address should not be removed from its address pool, and invalid
+    addresses should not be included in the allowlist. If a port abides by the above rules, the
+    test case passes.
+    """
+
+    def send_packet_and_verify(
+        self,
+        mac_address: str,
+        add_vlan: bool = False,
+        should_receive: bool = True,
+    ) -> None:
+        """Generate, send, and verify a packet based on specified parameters.
+
+        Test cases within this suite utilize this method to create, send, and verify
+        packets based on criteria relating to the packet's destination mac address,
+        vlan tag, and whether or not the packet should be received or not. Packets
+        are verified using an inserted payload. If the list of received packets
+        contains this payload within any of its packets, the test case passes. Each
+        call with this method sends exactly one packet.
+
+        Args:
+            mac_address: The destination mac address of the packet being sent.
+            add_vlan: Add a vlan tag to the packet being sent. :data:'2' if the packet
+                should be received, :data:'1' if the packet should not be received but
+                requires a vlan tag, and None for any other condition.
+            should_receive: If :data:'True', assert whether or not the sent packet
+                has been received. If :data:'False', assert that the send packet was not
+                received. :data:'True' by default
+        """
+        if add_vlan:
+            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) / IP() / Raw(load="X" * 22)
+        else:
+            packet = Ether() / IP() / Raw(load="X" * 22)
+        packet.dst = mac_address
+        received_packets = [
+            packets
+            for packets in self.send_packet_and_capture(packet, adjust_addresses=False)
+            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
+        ]
+        if should_receive:
+            self.verify(len(received_packets) == 1, "Expected packet not received")
+        else:
+            self.verify(len(received_packets) == 0, "Expected packet received")
+
+    def test_add_remove_mac_addresses(self) -> None:
+        """Assess basic mac addressing filtering functionalities.
+
+        This test cases validates for proper behavior of mac address filtering with both
+        a port's default, burned-in mac address, as well as additional mac addresses
+        added to the PMD. Packets should either be received or not received depending on
+        the properties applied to the PMD at any given time.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Send a packet with the port's default mac address. (Should receive)
+            Send a packet with fake mac address. (Should not receive)
+            Add fake mac address to the PMD's address pool.
+            Send a packet with the fake mac address to the PMD. (Should receive)
+            Remove the fake mac address from the PMD's address pool.
+            Sent a packet with the fake mac address to the PMD. (Should not receive)
+        """
+        testpmd = TestPmdShell(self.sut_node)
+        testpmd.set_promisc(0, on=False)
+        testpmd.start()
+        mac_address = self._sut_port_ingress.mac_address
+
+        # Send a packet with NIC default mac address
+        self.send_packet_and_verify(mac_address=mac_address, should_receive=True)
+        # Send a packet with different mac address
+        fake_address = "00:00:00:00:00:01"
+        self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+
+        # Add mac address to pool and rerun tests
+        testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
+        self.send_packet_and_verify(mac_address=fake_address, should_receive=True)
+        testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
+        self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+        testpmd.close()
+        sleep(6)
+
+    def test_invalid_address(self) -> None:
+        """Assess the behavior of a NIC mac address pool while bound to the PMD.
+
+        An assessment of a NIC's behavior when mounted to a PMD as it relates to mac addresses
+        and address pooling. Devices should not be able to use invalid mac addresses, remove their
+        built-in hardware address, or exceed their address pools.
+
+        Test:
+            Start TestPMD.
+            Attempt to add an invalid mac address. (Should fail)
+            Attempt to remove the device's hardware address with no additional addresses in the
+                address pool. (Should fail)
+            Add a fake mac address to the pool twice in succession. (Should not create any errors)
+            Attempt to remove the device's hardware address with other addresses in the address
+                pool. (Should fail)
+            Determine the device's mac address pool size, and fill the pool with fake addresses.
+            Attempt to add another fake mac address, overloading the address pool. (Should fail)
+        """
+        testpmd = TestPmdShell(self.sut_node)
+        testpmd.start()
+        mac_address = self._sut_port_ingress.mac_address
+        try:
+            testpmd.set_mac_addr(0, "00:00:00:00:00:00", add=True)
+            self.verify(False, "Invalid mac address added.")
+        except InteractiveCommandExecutionError:
+            pass
+        try:
+            testpmd.set_mac_addr(0, mac_address, add=False)
+            self.verify(False, "Default mac address removed.")
+        except InteractiveCommandExecutionError:
+            pass
+        # Should be no errors adding this twice
+        testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+        testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+        # Double check to see if default mac address can be removed
+        try:
+            testpmd.set_mac_addr(0, mac_address, add=False)
+            self.verify(False, "Default mac address removed.")
+        except InteractiveCommandExecutionError:
+            pass
+
+        for i in range(testpmd.show_port_info(0).max_mac_addresses_num - 1):
+            # A0 fake address based on the index 'i'.
+            fake_address = str(hex(i)[2:].zfill(12))
+            # Insert ':' characters every two indexes to create a fake mac address.
+            fake_address = ":".join(fake_address[x : x + 2] for x in range(0, len(fake_address), 2))
+            testpmd.set_mac_addr(0, fake_address, add=True, verify=False)
+        try:
+            testpmd.set_mac_addr(0, "F" + mac_address[1:], add=True)
+            self.verify(False, "Mac address limit exceeded.")
+        except InteractiveCommandExecutionError:
+            pass
+        testpmd.close()
+        sleep(6)
+
+    def test_multicast_filter(self) -> None:
+        """Assess basic multicast address filtering functionalities.
+
+        Ensure that multicast filtering performs as intended when a given device is bound
+        to the PMD, with and without dot1q vlan tagging.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Add a fake multicast address to the PMD's multicast address pool.
+            Send a packet with the fake multicast address to the PMD. (Should receive)
+            Set vlan filtering on the PMD, and add vlan ID to the PMD.
+            Send a packet with the fake multicast address and vlan ID to the PMD. (Should receive)
+            Send a packet with the fake multicast address and a different vlan ID to the PMD.
+                (Should not receive)
+            Remove the vlan tag from the PMD, and turn vlan filtering off on the PMD.
+            Send a packet with the fake multicast address and no vlan tag to the PMD.
+                (Should receive)
+            Remove the fake multicast address from the PMDs multicast address filter.
+            Send a packet with the fake multicast address to the PMD. (Should not receive)
+        """
+        testpmd = TestPmdShell(self.sut_node)
+        testpmd.start()
+        testpmd.set_promisc(0, on=False)
+        multicast_address = "01:00:5E:00:00:00"
+        vlan_id = 2
+
+        testpmd.set_multicast_mac_addr(0, multi_addr=multicast_address, add=True)
+        self.send_packet_and_verify(multicast_address, should_receive=True)
+
+        # Test vlan filtering on multicast addressing.
+        # Verify vlan functionality for debugging purposes.
+        testpmd.vlan_filter_set_on(port=0)
+        testpmd.rx_vlan_add(vlan_id, 0)
+        self.send_packet_and_verify(multicast_address, should_receive=True, add_vlan=True)
+        self.send_packet_and_verify(multicast_address, should_receive=False, add_vlan=True)
+
+        # Remove vlan tag and filtering and run basic multicast addr test.
+        testpmd.rx_vlan_rm(vlan_id, 0)
+        testpmd.vlan_filter_set_off(port=0)
+        self.send_packet_and_verify(multicast_address, should_receive=True)
+
+        # Remove multicast filter and verify the packet was not received.
+        testpmd.set_multicast_mac_addr(0, multicast_address, add=False)
+        self.send_packet_and_verify(multicast_address, should_receive=False)
+        testpmd.close()
+        sleep(6)
-- 
2.44.0


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

* Re: [PATCH v2 1/3] dts: add boolean to adjust addresses
  2024-07-02 19:24 ` [PATCH v2 1/3] dts: add boolean to adjust addresses Nicholas Pratte
@ 2024-07-11 19:31   ` Jeremy Spewock
  2024-07-17 17:19     ` Nicholas Pratte
  2024-07-19 15:37     ` Jeremy Spewock
  2024-07-22 13:09   ` Dean Marx
  1 sibling, 2 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-07-11 19:31 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: Honnappa.Nagarahalli, probb, juraj.linkes, paul.szczepanek,
	luca.vizzarro, yoan.picchi, dmarx, dev

I think this change makes sense, and I mentioned this on Dean's patch
that has the same change in it but I think we should have more
discussion about which route to take with this addressing problem. It
is definitely something that we have to address since it is required
for a suite like this or any suite that operates with multiple queues
to ensure traffic is handled by the different queues. If we end up
going the boolean parameter route however, then I think this solution
looks great and it passes by my standards.

There was one comment that I had about adding something super small to
the doc-string, but other than that the code all looked good to me.

On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Various test cases in the mac filter test suite called for granular
> manipulation of destination mac addresses to properly test mac address
> filtering functionality. To compensate, there is now an
> adjust_addresses boolean which the user can toggle if they wish to send
> their own addressing; the boolean is true by default.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/test_suite.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 694b2eba65..551a587525 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -185,6 +185,7 @@ def send_packet_and_capture(
>          packet: Packet,
>          filter_config: PacketFilteringConfig = PacketFilteringConfig(),
>          duration: float = 1,
> +        adjust_addresses: bool = True,
>      ) -> list[Packet]:
>          """Send and receive `packet` using the associated TG.
>
> @@ -195,11 +196,15 @@ def send_packet_and_capture(
>              packet: The packet to send.
>              filter_config: The filter to use when capturing packets.
>              duration: Capture traffic for this amount of time after sending `packet`.
> +            adjust_addresses: If :data:'True', adjust addresses of the egressing packet with
> +                a default addressing scheme. If :data:'False', do not adjust the addresses of
> +                egressing packet.

It might also be worth naming what the default is in the doc-string
(by which I mean that the parameter defaults to True, not the default
address scheme).

>
>          Returns:
>              A list of received packets.
>          """
> -        packet = self._adjust_addresses(packet)
> +        if adjust_addresses:
> +            packet = self._adjust_addresses(packet)
>          return self.tg_node.send_packet_and_capture(
>              packet,
>              self._tg_port_egress,
> --
> 2.44.0
>

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

* Re: [PATCH v2 2/3] dts: add testpmd methods for test suite
  2024-07-02 19:24 ` [PATCH v2 2/3] dts: add testpmd methods for test suite Nicholas Pratte
@ 2024-07-11 19:33   ` Jeremy Spewock
  2024-07-17 19:57     ` Nicholas Pratte
  0 siblings, 1 reply; 33+ messages in thread
From: Jeremy Spewock @ 2024-07-11 19:33 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: Honnappa.Nagarahalli, probb, juraj.linkes, paul.szczepanek,
	luca.vizzarro, yoan.picchi, dmarx, dev

There were a few emails that came through for this series but this was
the most recent one so I went with reviewing this one, but it looks
like the more descriptive commit subject got lost in this iteration.
It would probably be worth adding that back.

Additionally, it looks like the functions you added here (including
the ones added from the VLAN suite, but I believe those ones are
already fixed in the latest version ) are all missing their return
types. Of course they don't return anything, but that is still useful
to note by annotating it as "None".

On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Several new methods have been added to TestPMDShell in order to produce
> the mac filter's individual test cases:
>  - set_mac_addr
>  - set_multicast_mac_addr
>  - rx_vlan_add
>  - rx_vlan_rm
>  - vlan_filter_set_on
>  - vlan_filter_set_off
>  - set_promisc
>
> set_mac_addr and set_multicast_addr were created for the mac filter test
> suite, enabling users to both add or remove mac and multicast
> addresses based on a booling 'add or remove' parameter. The success or

I think this is a typo and "booling" should be "boolean" but I could be wrong.

> failure of each call can be verified if a user deems it necessary.
>
> The other methods listed are implemented in other respective test
> suites, and their implementations have been copied, but are subject to
> change; they are not the focus of this patch.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 177 ++++++++++++++++++
>  dts/tests/TestSuite_mac_filter.py             |   0

It looks like creating the file somehow snuck into the diff for this
commit instead of the other one that populates it.

>  2 files changed, 177 insertions(+)
>  create mode 100644 dts/tests/TestSuite_mac_filter.py
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index ec22f72221..0be1fb8754 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
<snip>
> +    def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, verify: bool = True):
> +        """Add or remove multicast mac address to a specified port filter.

Just to make this more clear that you specify the port and not the
port filter, it might be helpful here to show that the port possesses
the filter by saying "a specified port's filter."

> +
> +        Args:
> +            port_id: The port ID the multicast address is set on.
> +            multi_addr: The multicast address to be added to the filter.
> +            add: If :data:'True', add the specified multicast address to the port filter.
> +                If :data:'False', remove the specified multicast address from the port filter.
> +            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
> +                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
> +        """
> +        mcast_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid arguments provided to mcast_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument provided")
> +
> +        if verify:
> +            if (
> +                "Invalid multicast_addr" in output
> +                or f'multicast address {"already" if add else "not"} filtered by port' in output
> +            ):
> +                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
> +                )
<snip>

>      def show_port_stats_all(self) -> list[TestPmdPortStats]:
>          """Returns the statistics of all the ports.
>
> diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
> new file mode 100644
> index 0000000000..e69de29bb2
> --
> 2.44.0
>

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

* Re: [PATCH v2 3/3] dts: mac filter test suite refactored for new dts
  2024-07-02 19:24 ` [PATCH v2 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
@ 2024-07-11 19:34   ` Jeremy Spewock
  0 siblings, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-07-11 19:34 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: Honnappa.Nagarahalli, probb, juraj.linkes, paul.szczepanek,
	luca.vizzarro, yoan.picchi, dmarx, dev

Code all looked good to me, just a couple of documentation comments.

On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
><snip>
> +class TestMacFilter(TestSuite):
> +    """Mac address allowlist filtering test suite.
> +
> +    Configure mac address filtering on a given port, and test the port's filtering behavior
> +    using both a given port's hardware address as well as dummy addresses. If a port accepts
> +    a packet that is not contained within its mac address allowlist, then a given test case
> +    fails. Alternatively, if a port drops a packet that is designated within its mac address
> +    allowlist, a given test case will fail.
> +
> +    Moreover, a given port should demonstrate proper behavior when bound to the Poll Mode
> +    Driver. A port should not have an mac address allowlist that exceeds its designated size.

I think this should just be " a mac address allowlist" rather than "an".

> +    A port's default hardware address should not be removed from its address pool, and invalid
> +    addresses should not be included in the allowlist. If a port abides by the above rules, the
> +    test case passes.
> +    """
> +
> +    def send_packet_and_verify(
> +        self,
> +        mac_address: str,
> +        add_vlan: bool = False,
> +        should_receive: bool = True,
> +    ) -> None:
> +        """Generate, send, and verify a packet based on specified parameters.
> +
> +        Test cases within this suite utilize this method to create, send, and verify
> +        packets based on criteria relating to the packet's destination mac address,
> +        vlan tag, and whether or not the packet should be received or not. Packets
> +        are verified using an inserted payload. If the list of received packets
> +        contains this payload within any of its packets, the test case passes. Each

It might be worth noting here that it passes assuming `should_recieve` is true.

> +        call with this method sends exactly one packet.
> +
> +        Args:
> +            mac_address: The destination mac address of the packet being sent.
> +            add_vlan: Add a vlan tag to the packet being sent. :data:'2' if the packet

I think if this started with "Whether to add..."  it would be more
clear that it is a boolean.

> +                should be received, :data:'1' if the packet should not be received but
> +                requires a vlan tag, and None for any other condition.

This tripped me up at first because I thought you were saying that
add_vlan could be 2, 1, or None. It might be worth just adding to the
start of that second sentence that "The tag will be...". Also, it
might be more clear if you explain that the tag will be omitted in
other cases rather than it being None.

> +            should_receive: If :data:'True', assert whether or not the sent packet
> +                has been received. If :data:'False', assert that the send packet was not
> +                received. :data:'True' by default
> +        """
> +        if add_vlan:
> +            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) / IP() / Raw(load="X" * 22)
> +        else:
> +            packet = Ether() / IP() / Raw(load="X" * 22)
> +        packet.dst = mac_address
> +        received_packets = [
> +            packets
> +            for packets in self.send_packet_and_capture(packet, adjust_addresses=False)
> +            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
> +        ]
> +        if should_receive:
> +            self.verify(len(received_packets) == 1, "Expected packet not received")
> +        else:
> +            self.verify(len(received_packets) == 0, "Expected packet received")
> +
> +    def test_add_remove_mac_addresses(self) -> None:
> +        """Assess basic mac addressing filtering functionalities.
> +
> +        This test cases validates for proper behavior of mac address filtering with both

Small typo, but I think this is meant to be "this test case validates...".

> +        a port's default, burned-in mac address, as well as additional mac addresses
> +        added to the PMD. Packets should either be received or not received depending on
> +        the properties applied to the PMD at any given time.
> +
> +        Test:
> +            Start TestPMD with promiscuous mode.
> +            Send a packet with the port's default mac address. (Should receive)
> +            Send a packet with fake mac address. (Should not receive)
> +            Add fake mac address to the PMD's address pool.
> +            Send a packet with the fake mac address to the PMD. (Should receive)
> +            Remove the fake mac address from the PMD's address pool.
> +            Sent a packet with the fake mac address to the PMD. (Should not receive)
> +        """
> +        testpmd = TestPmdShell(self.sut_node)
> +        testpmd.set_promisc(0, on=False)
> +        testpmd.start()
> +        mac_address = self._sut_port_ingress.mac_address
> +
> +        # Send a packet with NIC default mac address
> +        self.send_packet_and_verify(mac_address=mac_address, should_receive=True)
> +        # Send a packet with different mac address
> +        fake_address = "00:00:00:00:00:01"
> +        self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
> +
> +        # Add mac address to pool and rerun tests
> +        testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
> +        self.send_packet_and_verify(mac_address=fake_address, should_receive=True)
> +        testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
> +        self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
> +        testpmd.close()
> +        sleep(6)
> <snip>

> --
> 2.44.0
>

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

* Re: [PATCH v2 1/3] dts: add boolean to adjust addresses
  2024-07-11 19:31   ` Jeremy Spewock
@ 2024-07-17 17:19     ` Nicholas Pratte
  2024-07-19 15:37     ` Jeremy Spewock
  1 sibling, 0 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-17 17:19 UTC (permalink / raw)
  To: Jeremy Spewock
  Cc: Honnappa.Nagarahalli, probb, juraj.linkes, paul.szczepanek,
	luca.vizzarro, yoan.picchi, dmarx, dev

<snip>
> It might also be worth naming what the default is in the doc-string
> (by which I mean that the parameter defaults to True, not the default
> address scheme).

I can fix that.

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

* Re: [PATCH v2 2/3] dts: add testpmd methods for test suite
  2024-07-11 19:33   ` Jeremy Spewock
@ 2024-07-17 19:57     ` Nicholas Pratte
  0 siblings, 0 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-17 19:57 UTC (permalink / raw)
  To: Jeremy Spewock
  Cc: Honnappa.Nagarahalli, probb, juraj.linkes, paul.szczepanek,
	luca.vizzarro, yoan.picchi, dmarx, dev

On Thu, Jul 11, 2024 at 3:33 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> There were a few emails that came through for this series but this was
> the most recent one so I went with reviewing this one, but it looks
> like the more descriptive commit subject got lost in this iteration.
> It would probably be worth adding that back.
>
> Additionally, it looks like the functions you added here (including
> the ones added from the VLAN suite, but I believe those ones are
> already fixed in the latest version ) are all missing their return
> types. Of course they don't return anything, but that is still useful
> to note by annotating it as "None".

Surprised I never saw that. I'll fix this.


> On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
> >
> > Several new methods have been added to TestPMDShell in order to produce
> > the mac filter's individual test cases:
> >  - set_mac_addr
> >  - set_multicast_mac_addr
> >  - rx_vlan_add
> >  - rx_vlan_rm
> >  - vlan_filter_set_on
> >  - vlan_filter_set_off
> >  - set_promisc
> >
> > set_mac_addr and set_multicast_addr were created for the mac filter test
> > suite, enabling users to both add or remove mac and multicast
> > addresses based on a booling 'add or remove' parameter. The success or
>
> I think this is a typo and "booling" should be "boolean" but I could be wrong.

Good catch!

>
> > failure of each call can be verified if a user deems it necessary.
> >
> > The other methods listed are implemented in other respective test
> > suites, and their implementations have been copied, but are subject to
> > change; they are not the focus of this patch.
> >
> > Bugzilla ID: 1454
> > Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> > ---
> >  dts/framework/remote_session/testpmd_shell.py | 177 ++++++++++++++++++
> >  dts/tests/TestSuite_mac_filter.py             |   0
>
> It looks like creating the file somehow snuck into the diff for this
> commit instead of the other one that populates it.

Strange. I can fix that real quick.

>
> >  2 files changed, 177 insertions(+)
> >  create mode 100644 dts/tests/TestSuite_mac_filter.py
> >
> > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> > index ec22f72221..0be1fb8754 100644
> > --- a/dts/framework/remote_session/testpmd_shell.py
> > +++ b/dts/framework/remote_session/testpmd_shell.py
> <snip>
> > +    def set_multicast_mac_addr(self, port_id: int, multi_addr: str, add: bool, verify: bool = True):
> > +        """Add or remove multicast mac address to a specified port filter.
>
> Just to make this more clear that you specify the port and not the
> port filter, it might be helpful here to show that the port possesses
> the filter by saying "a specified port's filter."

Yes. This was just a typo. What you have here is what I was going for.

>
> > +
> > +        Args:
> > +            port_id: The port ID the multicast address is set on.
> > +            multi_addr: The multicast address to be added to the filter.
> > +            add: If :data:'True', add the specified multicast address to the port filter.
> > +                If :data:'False', remove the specified multicast address from the port filter.
> > +            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
> > +                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
> > +
> > +        Raises:
> > +            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
> > +        """
> > +        mcast_cmd = "add" if add else "remove"
> > +        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
> > +        if "Bad arguments" in output:
> > +            self._logger.debug("Invalid arguments provided to mcast_addr")
> > +            raise InteractiveCommandExecutionError("Invalid argument provided")
> > +
> > +        if verify:
> > +            if (
> > +                "Invalid multicast_addr" in output
> > +                or f'multicast address {"already" if add else "not"} filtered by port' in output
> > +            ):
> > +                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
> > +                raise InteractiveCommandExecutionError(
> > +                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
> > +                )
> <snip>
>
> >      def show_port_stats_all(self) -> list[TestPmdPortStats]:
> >          """Returns the statistics of all the ports.
> >
> > diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > --
> > 2.44.0
> >

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

* [PATCH v3 0/3] Mac Filter Port to New DTS
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
                   ` (2 preceding siblings ...)
  2024-07-02 19:24 ` [PATCH v2 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
@ 2024-07-18 19:05 ` Nicholas Pratte
  2024-07-18 19:05   ` [PATCH v3 1/3] dts: add boolean to adjust addresses Nicholas Pratte
                     ` (2 more replies)
  2024-07-26 16:39 ` [PATCH v4 0/2] Mac Filter Port to New DTS Nicholas Pratte
                   ` (2 subsequent siblings)
  6 siblings, 3 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-18 19:05 UTC (permalink / raw)
  To: probb, paul.szczepanek, luca.vizzarro, jspewock,
	Honnappa.Nagarahalli, dmarx, yoan.picchi, juraj.linkes
  Cc: dev, Nicholas Pratte

v3:
  * Minor adjustments to doc strings
  * Return labels added for testpmd methods

Nicholas Pratte (3):
  dts: add boolean to adjust addresses
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json    |   3 +-
 dts/framework/remote_session/testpmd_shell.py | 179 ++++++++++++++
 dts/framework/test_suite.py                   |   7 +-
 dts/tests/TestSuite_mac_filter.py             | 223 ++++++++++++++++++
 4 files changed, 410 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0


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

* [PATCH v3 1/3] dts: add boolean to adjust addresses
  2024-07-18 19:05 ` [PATCH v3 0/3] Mac Filter Port to New DTS Nicholas Pratte
@ 2024-07-18 19:05   ` Nicholas Pratte
  2024-07-18 19:05   ` [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses Nicholas Pratte
  2024-07-18 19:40   ` [PATCH v3 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
  2 siblings, 0 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-18 19:05 UTC (permalink / raw)
  To: probb, paul.szczepanek, luca.vizzarro, jspewock,
	Honnappa.Nagarahalli, dmarx, yoan.picchi, juraj.linkes
  Cc: dev, Nicholas Pratte

Various test cases in the mac filter test suite called for granular
manipulation of destination mac addresses to properly test mac address
filtering functionality. To compensate, there is now an
adjust_addresses boolean which the user can toggle if they wish to send
their own addressing; the boolean is true by default.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/test_suite.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 694b2eba65..551a587525 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -185,6 +185,7 @@ def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        adjust_addresses: bool = True,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -195,11 +196,15 @@ def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            adjust_addresses: If :data:'True', adjust addresses of the egressing packet with
+                a default addressing scheme. If :data:'False', do not adjust the addresses of
+                egressing packet.
 
         Returns:
             A list of received packets.
         """
-        packet = self._adjust_addresses(packet)
+        if adjust_addresses:
+            packet = self._adjust_addresses(packet)
         return self.tg_node.send_packet_and_capture(
             packet,
             self._tg_port_egress,
-- 
2.44.0


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

* [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses
  2024-07-18 19:05 ` [PATCH v3 0/3] Mac Filter Port to New DTS Nicholas Pratte
  2024-07-18 19:05   ` [PATCH v3 1/3] dts: add boolean to adjust addresses Nicholas Pratte
@ 2024-07-18 19:05   ` Nicholas Pratte
  2024-07-19 15:37     ` Jeremy Spewock
  2024-07-22 13:08     ` Dean Marx
  2024-07-18 19:40   ` [PATCH v3 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
  2 siblings, 2 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-18 19:05 UTC (permalink / raw)
  To: probb, paul.szczepanek, luca.vizzarro, jspewock,
	Honnappa.Nagarahalli, dmarx, yoan.picchi, juraj.linkes
  Cc: dev, Nicholas Pratte

Several new methods have been added to TestPMDShell in order to produce
the mac filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr
 - rx_vlan_add
 - rx_vlan_rm
 - vlan_filter_set_on
 - vlan_filter_set_off
 - set_promisc

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a boolean 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

The other methods listed are implemented in other respective test
suites, and their implementations have been copied, but are subject to
change; they are not the focus of this patch.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 179 ++++++++++++++++++
 1 file changed, 179 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..8122457ad1 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -767,6 +767,185 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
         return TestPmdPort.parse(output)
 
+    def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: bool = True) -> None:
+        """Add or remove a mac address on a given port's Allowlist.
+
+        Args:
+            port_id: The port ID the mac address is set on.
+            mac_address: The mac address to be added or removed to the specified port.
+            add: If :data:`True`, add the specified mac address. If :data:`False`, remove specified
+                mac address.
+            verify: If :data:'True', assert that the 'mac_addr' operation was successful. If
+                :data:'False', run the command and skip this assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If the set mac address operation fails.
+        """
+        mac_cmd = "add" if add else "remove"
+        output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid argument provided to mac_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if "mac_addr_cmd error:" in output:
+                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mac_cmd} {mac_address} on port {port_id} \n{output}"
+                )
+
+    def set_multicast_mac_addr(
+        self, port_id: int, multi_addr: str, add: bool, verify: bool = True
+    ) -> None:
+        """Add or remove multicast mac address to a specified port's filter.
+
+        Args:
+            port_id: The port ID the multicast address is set on.
+            multi_addr: The multicast address to be added to the filter.
+            add: If :data:'True', add the specified multicast address to the port filter.
+                If :data:'False', remove the specified multicast address from the port filter.
+            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
+                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
+        """
+        mcast_cmd = "add" if add else "remove"
+        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid arguments provided to mcast_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if (
+                "Invalid multicast_addr" in output
+                or f'multicast address {"already" if add else "not"} filtered by port' in output
+            ):
+                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
+                )
+
+    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True) -> None:
+        """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) -> None:
+        """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_filter_set_on(self, port: int, verify: bool = True) -> None:
+        """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) -> None:
+        """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 set_promisc(self, port: int, on: bool, verify: bool = True) -> None:
+        """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.
+        """
+        promisc_output = self.send_command(f"set promisc {port} {'on' if on else 'off'}")
+        if verify:
+            stats = self.show_port_info(port_id=port)
+            if on ^ stats.is_promiscuous_mode_enabled:
+                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 show_port_stats_all(self) -> list[TestPmdPortStats]:
         """Returns the statistics of all the ports.
 
-- 
2.44.0


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

* [PATCH v3 3/3] dts: mac filter test suite refactored for new dts
  2024-07-18 19:05 ` [PATCH v3 0/3] Mac Filter Port to New DTS Nicholas Pratte
  2024-07-18 19:05   ` [PATCH v3 1/3] dts: add boolean to adjust addresses Nicholas Pratte
  2024-07-18 19:05   ` [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses Nicholas Pratte
@ 2024-07-18 19:40   ` Nicholas Pratte
  2024-07-19 15:38     ` Jeremy Spewock
  2024-07-22 13:08     ` Dean Marx
  2 siblings, 2 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-18 19:40 UTC (permalink / raw)
  To: probb, paul.szczepanek, luca.vizzarro, jspewock,
	Honnappa.Nagarahalli, dmarx, yoan.picchi, juraj.linkes
  Cc: dev, Nicholas Pratte

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py          | 223 +++++++++++++++++++++
 2 files changed, 225 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 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",
+        "mac_filter"
       ]
     },
     "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
new file mode 100644
index 0000000000..53a3331224
--- /dev/null
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,223 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a device
+contains a mac address not contained with its mac address pool, the packet should
+be dropped. Alternatively, if a packet is received that contains a destination mac
+within the devices address pool, the packet should be accepted and forwarded. This
+behavior should remain consistent across all packets, namely those containing dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac address
+allow list is behaving as expected.
+"""
+
+from time import sleep
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+    """Mac address allowlist filtering test suite.
+
+    Configure mac address filtering on a given port, and test the port's filtering behavior
+    using both a given port's hardware address as well as dummy addresses. If a port accepts
+    a packet that is not contained within its mac address allowlist, then a given test case
+    fails. Alternatively, if a port drops a packet that is designated within its mac address
+    allowlist, a given test case will fail.
+
+    Moreover, a given port should demonstrate proper behavior when bound to the Poll Mode
+    Driver. A port should not have a mac address allowlist that exceeds its designated size.
+    A port's default hardware address should not be removed from its address pool, and invalid
+    addresses should not be included in the allowlist. If a port abides by the above rules, the
+    test case passes.
+    """
+
+    def send_packet_and_verify(
+        self,
+        mac_address: str,
+        add_vlan: bool = False,
+        should_receive: bool = True,
+    ) -> None:
+        """Generate, send, and verify a packet based on specified parameters.
+
+        Test cases within this suite utilize this method to create, send, and verify
+        packets based on criteria relating to the packet's destination mac address,
+        vlan tag, and whether or not the packet should be received or not. Packets
+        are verified using an inserted payload. Assuming the test case expects to
+        receive a specified packet, if the list of received packets contains this
+        payload within any of its packets, the test case passes. Alternatively, if
+        the designed packet should not be received, and the packet payload is not,
+        received, then the test case fails. Each call with this method sends exactly
+        one packet.
+
+        Args:
+            mac_address: The destination mac address of the packet being sent.
+            add_vlan: If :data:'True', add a vlan tag to the packet being sent. The
+                vlan tag will be :data:'2' if the packet should be received and
+                :data:'1' if the packet should not be received but requires a vlan tag.
+            should_receive: If :data:'True', assert whether or not the sent packet
+                has been received. If :data:'False', assert that the send packet was not
+                received. :data:'True' by default
+        """
+        if add_vlan:
+            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) / IP() / Raw(load="X" * 22)
+        else:
+            packet = Ether() / IP() / Raw(load="X" * 22)
+        packet.dst = mac_address
+        received_packets = [
+            packets
+            for packets in self.send_packet_and_capture(packet, adjust_addresses=False)
+            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
+        ]
+        if should_receive:
+            self.verify(len(received_packets) == 1, "Expected packet not received")
+        else:
+            self.verify(len(received_packets) == 0, "Expected packet received")
+
+    def test_add_remove_mac_addresses(self) -> None:
+        """Assess basic mac addressing filtering functionalities.
+
+        This test case validates for proper behavior of mac address filtering with both
+        a port's default, burned-in mac address, as well as additional mac addresses
+        added to the PMD. Packets should either be received or not received depending on
+        the properties applied to the PMD at any given time.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Send a packet with the port's default mac address. (Should receive)
+            Send a packet with fake mac address. (Should not receive)
+            Add fake mac address to the PMD's address pool.
+            Send a packet with the fake mac address to the PMD. (Should receive)
+            Remove the fake mac address from the PMD's address pool.
+            Sent a packet with the fake mac address to the PMD. (Should not receive)
+        """
+        testpmd = TestPmdShell(self.sut_node)
+        testpmd.set_promisc(0, on=False)
+        testpmd.start()
+        mac_address = self._sut_port_ingress.mac_address
+
+        # Send a packet with NIC default mac address
+        self.send_packet_and_verify(mac_address=mac_address, should_receive=True)
+        # Send a packet with different mac address
+        fake_address = "00:00:00:00:00:01"
+        self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+
+        # Add mac address to pool and rerun tests
+        testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
+        self.send_packet_and_verify(mac_address=fake_address, should_receive=True)
+        testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
+        self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+        testpmd.close()
+        sleep(6)
+
+    def test_invalid_address(self) -> None:
+        """Assess the behavior of a NIC mac address pool while bound to the PMD.
+
+        An assessment of a NIC's behavior when mounted to a PMD as it relates to mac addresses
+        and address pooling. Devices should not be able to use invalid mac addresses, remove their
+        built-in hardware address, or exceed their address pools.
+
+        Test:
+            Start TestPMD.
+            Attempt to add an invalid mac address. (Should fail)
+            Attempt to remove the device's hardware address with no additional addresses in the
+                address pool. (Should fail)
+            Add a fake mac address to the pool twice in succession. (Should not create any errors)
+            Attempt to remove the device's hardware address with other addresses in the address
+                pool. (Should fail)
+            Determine the device's mac address pool size, and fill the pool with fake addresses.
+            Attempt to add another fake mac address, overloading the address pool. (Should fail)
+        """
+        testpmd = TestPmdShell(self.sut_node)
+        testpmd.start()
+        mac_address = self._sut_port_ingress.mac_address
+        try:
+            testpmd.set_mac_addr(0, "00:00:00:00:00:00", add=True)
+            self.verify(False, "Invalid mac address added.")
+        except InteractiveCommandExecutionError:
+            pass
+        try:
+            testpmd.set_mac_addr(0, mac_address, add=False)
+            self.verify(False, "Default mac address removed.")
+        except InteractiveCommandExecutionError:
+            pass
+        # Should be no errors adding this twice
+        testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+        testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+        # Double check to see if default mac address can be removed
+        try:
+            testpmd.set_mac_addr(0, mac_address, add=False)
+            self.verify(False, "Default mac address removed.")
+        except InteractiveCommandExecutionError:
+            pass
+
+        for i in range(testpmd.show_port_info(0).max_mac_addresses_num - 1):
+            # A0 fake address based on the index 'i'.
+            fake_address = str(hex(i)[2:].zfill(12))
+            # Insert ':' characters every two indexes to create a fake mac address.
+            fake_address = ":".join(fake_address[x : x + 2] for x in range(0, len(fake_address), 2))
+            testpmd.set_mac_addr(0, fake_address, add=True, verify=False)
+        try:
+            testpmd.set_mac_addr(0, "F" + mac_address[1:], add=True)
+            self.verify(False, "Mac address limit exceeded.")
+        except InteractiveCommandExecutionError:
+            pass
+        testpmd.close()
+        sleep(6)
+
+    def test_multicast_filter(self) -> None:
+        """Assess basic multicast address filtering functionalities.
+
+        Ensure that multicast filtering performs as intended when a given device is bound
+        to the PMD, with and without dot1q vlan tagging.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Add a fake multicast address to the PMD's multicast address pool.
+            Send a packet with the fake multicast address to the PMD. (Should receive)
+            Set vlan filtering on the PMD, and add vlan ID to the PMD.
+            Send a packet with the fake multicast address and vlan ID to the PMD. (Should receive)
+            Send a packet with the fake multicast address and a different vlan ID to the PMD.
+                (Should not receive)
+            Remove the vlan tag from the PMD, and turn vlan filtering off on the PMD.
+            Send a packet with the fake multicast address and no vlan tag to the PMD.
+                (Should receive)
+            Remove the fake multicast address from the PMDs multicast address filter.
+            Send a packet with the fake multicast address to the PMD. (Should not receive)
+        """
+        testpmd = TestPmdShell(self.sut_node)
+        testpmd.start()
+        testpmd.set_promisc(0, on=False)
+        multicast_address = "01:00:5E:00:00:00"
+        vlan_id = 2
+
+        testpmd.set_multicast_mac_addr(0, multi_addr=multicast_address, add=True)
+        self.send_packet_and_verify(multicast_address, should_receive=True)
+
+        # Test vlan filtering on multicast addressing.
+        # Verify vlan functionality for debugging purposes.
+        testpmd.vlan_filter_set_on(port=0)
+        testpmd.rx_vlan_add(vlan_id, 0)
+        self.send_packet_and_verify(multicast_address, should_receive=True, add_vlan=True)
+        self.send_packet_and_verify(multicast_address, should_receive=False, add_vlan=True)
+
+        # Remove vlan tag and filtering and run basic multicast addr test.
+        testpmd.rx_vlan_rm(vlan_id, 0)
+        testpmd.vlan_filter_set_off(port=0)
+        self.send_packet_and_verify(multicast_address, should_receive=True)
+
+        # Remove multicast filter and verify the packet was not received.
+        testpmd.set_multicast_mac_addr(0, multicast_address, add=False)
+        self.send_packet_and_verify(multicast_address, should_receive=False)
+        testpmd.close()
+        sleep(6)
-- 
2.44.0


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

* Re: [PATCH v2 1/3] dts: add boolean to adjust addresses
  2024-07-11 19:31   ` Jeremy Spewock
  2024-07-17 17:19     ` Nicholas Pratte
@ 2024-07-19 15:37     ` Jeremy Spewock
  1 sibling, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-07-19 15:37 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: Honnappa.Nagarahalli, probb, juraj.linkes, paul.szczepanek,
	luca.vizzarro, yoan.picchi, dmarx, dev

On Thu, Jul 11, 2024 at 3:31 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
> I think this change makes sense, and I mentioned this on Dean's patch
> that has the same change in it but I think we should have more
> discussion about which route to take with this addressing problem. It
> is definitely something that we have to address since it is required
> for a suite like this or any suite that operates with multiple queues
> to ensure traffic is handled by the different queues. If we end up
> going the boolean parameter route however, then I think this solution
> looks great and it passes by my standards.
>
> There was one comment that I had about adding something super small to
> the doc-string, but other than that the code all looked good to me.
>

We did end up having a discussion about this on slack and it seemed
there was a slight favor towards using the method of only changing it
if the user hasn't set it themselves. If you have any other thoughts
on it though definitely feel free to share them!

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

* Re: [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses
  2024-07-18 19:05   ` [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses Nicholas Pratte
@ 2024-07-19 15:37     ` Jeremy Spewock
  2024-07-22 13:08     ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-07-19 15:37 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, paul.szczepanek, luca.vizzarro, Honnappa.Nagarahalli,
	dmarx, yoan.picchi, juraj.linkes, dev

On Thu, Jul 18, 2024 at 3:12 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Several new methods have been added to TestPMDShell in order to produce
> the mac filter's individual test cases:
>  - set_mac_addr
>  - set_multicast_mac_addr
>  - rx_vlan_add
>  - rx_vlan_rm
>  - vlan_filter_set_on
>  - vlan_filter_set_off
>  - set_promisc
>
> set_mac_addr and set_multicast_addr were created for the mac filter test
> suite, enabling users to both add or remove mac and multicast
> addresses based on a boolean 'add or remove' parameter. The success or
> failure of each call can be verified if a user deems it necessary.
>
> The other methods listed are implemented in other respective test
> suites, and their implementations have been copied, but are subject to
> change; they are not the focus of this patch.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---

All looks good to me, there were just a few places that I noticed
things had been changed in the VLAN suite that should probably get
moved into here (like the removal of separate methods for turning
filtering on and off, for example). Otherwise:

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

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

* Re: [PATCH v3 3/3] dts: mac filter test suite refactored for new dts
  2024-07-18 19:40   ` [PATCH v3 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
@ 2024-07-19 15:38     ` Jeremy Spewock
  2024-07-22 13:08     ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-07-19 15:38 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, paul.szczepanek, luca.vizzarro, Honnappa.Nagarahalli,
	dmarx, yoan.picchi, juraj.linkes, dev

I just noticed one mistake in a doc-comment. Otherwise however, good work.

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

On Thu, Jul 18, 2024 at 3:40 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The mac address filter test suite, whose test cases are based on old
> DTS's test cases, has been refactored to interface with the new DTS
> framework.
>
> In porting over this test suite into the new framework, some
> adjustments were made, namely in the EAL and TestPMD parameter provided
> before executing the application. While the original test plan was
> referenced, by and large, only for the individual test cases, I'll leave
> the parameters the original test plan was asking for below for the sake
> of discussion:
>
> --burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
> --txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
<snip>
> +    def send_packet_and_verify(
> +        self,
> +        mac_address: str,
> +        add_vlan: bool = False,
> +        should_receive: bool = True,
> +    ) -> None:
> +        """Generate, send, and verify a packet based on specified parameters.
> +
> +        Test cases within this suite utilize this method to create, send, and verify
> +        packets based on criteria relating to the packet's destination mac address,
> +        vlan tag, and whether or not the packet should be received or not. Packets
> +        are verified using an inserted payload. Assuming the test case expects to
> +        receive a specified packet, if the list of received packets contains this
> +        payload within any of its packets, the test case passes. Alternatively, if
> +        the designed packet should not be received, and the packet payload is not,

I like this addition, but I just noticed that an additional comma
snuck in after the "not" at the end of the line here.

> +        received, then the test case fails. Each call with this method sends exactly
> +        one packet.
> +
> +        Args:
> +            mac_address: The destination mac address of the packet being sent.
> +            add_vlan: If :data:'True', add a vlan tag to the packet being sent. The
> +                vlan tag will be :data:'2' if the packet should be received and
> +                :data:'1' if the packet should not be received but requires a vlan tag.
> +            should_receive: If :data:'True', assert whether or not the sent packet
> +                has been received. If :data:'False', assert that the send packet was not
> +                received. :data:'True' by default
> +        """
<snip>
>

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

* Re: [PATCH v3 3/3] dts: mac filter test suite refactored for new dts
  2024-07-18 19:40   ` [PATCH v3 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
  2024-07-19 15:38     ` Jeremy Spewock
@ 2024-07-22 13:08     ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Dean Marx @ 2024-07-22 13:08 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, paul.szczepanek, luca.vizzarro, jspewock,
	Honnappa.Nagarahalli, yoan.picchi, juraj.linkes, dev

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

>
> <snip>

+This test suite ensures proper and expected behavior of Allowlist filtering
>  via mac
> +addresses on devices bound to the Poll Mode Driver. If a packet received
> on a device
> +contains a mac address not contained with its mac address pool, the
> packet should
> +be dropped. Alternatively, if a packet is received that contains a
> destination mac
> +within the devices address pool, the packet should be accepted and
> forwarded. This
> +behavior should remain consistent across all packets, namely those
> containing dot1q
> +tags or otherwise.
>

This is pretty minor but you might want to change "with its mac address
pool" to "within", just
to stay consistent with the wording in the rest of the suite


> <snip>
> +        received_packets = [
> +            packets
> +            for packets in self.send_packet_and_capture(packet,
> adjust_addresses=False)
> +            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
> +        ]
>

Also Jeremy said he mentioned Juraj's opinion on using Jeremy's method of
turning off
adjust addresses, if you end up doing that just make it depend on this
patch:
patch-1142113 ("add send_packets to test suites and rework
packet addressing")

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

On Thu, Jul 18, 2024 at 3:40 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:

> The mac address filter test suite, whose test cases are based on old
> DTS's test cases, has been refactored to interface with the new DTS
> framework.
>
> In porting over this test suite into the new framework, some
> adjustments were made, namely in the EAL and TestPMD parameter provided
> before executing the application. While the original test plan was
> referenced, by and large, only for the individual test cases, I'll leave
> the parameters the original test plan was asking for below for the sake
> of discussion:
>
> --burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
> --txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
> v2:
>  * Refactored the address pool capacity tests to use all available
>    octets in the mac address.
>  * Change the payload to 'X' characters instead of 'P' characters.
> ---
>  dts/framework/config/conf_yaml_schema.json |   3 +-
>  dts/tests/TestSuite_mac_filter.py          | 223 +++++++++++++++++++++
>  2 files changed, 225 insertions(+), 1 deletion(-)
>  create mode 100644 dts/tests/TestSuite_mac_filter.py
>
> diff --git a/dts/framework/config/conf_yaml_schema.json
> b/dts/framework/config/conf_yaml_schema.json
> index f02a310bb5..ad1f3757f7 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",
> +        "mac_filter"
>        ]
>      },
>      "test_target": {
> diff --git a/dts/tests/TestSuite_mac_filter.py
> b/dts/tests/TestSuite_mac_filter.py
> new file mode 100644
> index 0000000000..53a3331224
> --- /dev/null
> +++ b/dts/tests/TestSuite_mac_filter.py
> @@ -0,0 +1,223 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023-2024 University of New Hampshire
> +"""Mac address filtering test suite.
> +
> +This test suite ensures proper and expected behavior of Allowlist
> filtering via mac
> +addresses on devices bound to the Poll Mode Driver. If a packet received
> on a device
> +contains a mac address not contained with its mac address pool, the
> packet should
> +be dropped. Alternatively, if a packet is received that contains a
> destination mac
> +within the devices address pool, the packet should be accepted and
> forwarded. This
> +behavior should remain consistent across all packets, namely those
> containing dot1q
> +tags or otherwise.
> +
> +The following test suite assesses behaviors based on the aforementioned
> logic.
> +Additionally, testing is done within the PMD itself to ensure that the
> mac address
> +allow list is behaving as expected.
> +"""
> +
> +from time import sleep
> +
> +from scapy.layers.inet import IP  # type: ignore[import-untyped]
> +from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
> +from scapy.packet import Raw  # type: ignore[import-untyped]
> +
> +from framework.exception import InteractiveCommandExecutionError
> +from framework.remote_session.testpmd_shell import TestPmdShell
> +from framework.test_suite import TestSuite
> +
> +
> +class TestMacFilter(TestSuite):
> +    """Mac address allowlist filtering test suite.
> +
> +    Configure mac address filtering on a given port, and test the port's
> filtering behavior
> +    using both a given port's hardware address as well as dummy
> addresses. If a port accepts
> +    a packet that is not contained within its mac address allowlist, then
> a given test case
> +    fails. Alternatively, if a port drops a packet that is designated
> within its mac address
> +    allowlist, a given test case will fail.
> +
> +    Moreover, a given port should demonstrate proper behavior when bound
> to the Poll Mode
> +    Driver. A port should not have a mac address allowlist that exceeds
> its designated size.
> +    A port's default hardware address should not be removed from its
> address pool, and invalid
> +    addresses should not be included in the allowlist. If a port abides
> by the above rules, the
> +    test case passes.
> +    """
> +
> +    def send_packet_and_verify(
> +        self,
> +        mac_address: str,
> +        add_vlan: bool = False,
> +        should_receive: bool = True,
> +    ) -> None:
> +        """Generate, send, and verify a packet based on specified
> parameters.
> +
> +        Test cases within this suite utilize this method to create, send,
> and verify
> +        packets based on criteria relating to the packet's destination
> mac address,
> +        vlan tag, and whether or not the packet should be received or
> not. Packets
> +        are verified using an inserted payload. Assuming the test case
> expects to
> +        receive a specified packet, if the list of received packets
> contains this
> +        payload within any of its packets, the test case passes.
> Alternatively, if
> +        the designed packet should not be received, and the packet
> payload is not,
> +        received, then the test case fails. Each call with this method
> sends exactly
> +        one packet.
> +
> +        Args:
> +            mac_address: The destination mac address of the packet being
> sent.
> +            add_vlan: If :data:'True', add a vlan tag to the packet being
> sent. The
> +                vlan tag will be :data:'2' if the packet should be
> received and
> +                :data:'1' if the packet should not be received but
> requires a vlan tag.
> +            should_receive: If :data:'True', assert whether or not the
> sent packet
> +                has been received. If :data:'False', assert that the send
> packet was not
> +                received. :data:'True' by default
> +        """
> +        if add_vlan:
> +            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) /
> IP() / Raw(load="X" * 22)
> +        else:
> +            packet = Ether() / IP() / Raw(load="X" * 22)
> +        packet.dst = mac_address
> +        received_packets = [
> +            packets
> +            for packets in self.send_packet_and_capture(packet,
> adjust_addresses=False)
> +            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
> +        ]
> +        if should_receive:
> +            self.verify(len(received_packets) == 1, "Expected packet not
> received")
> +        else:
> +            self.verify(len(received_packets) == 0, "Expected packet
> received")
> +
> +    def test_add_remove_mac_addresses(self) -> None:
> +        """Assess basic mac addressing filtering functionalities.
> +
> +        This test case validates for proper behavior of mac address
> filtering with both
> +        a port's default, burned-in mac address, as well as additional
> mac addresses
> +        added to the PMD. Packets should either be received or not
> received depending on
> +        the properties applied to the PMD at any given time.
> +
> +        Test:
> +            Start TestPMD with promiscuous mode.
> +            Send a packet with the port's default mac address. (Should
> receive)
> +            Send a packet with fake mac address. (Should not receive)
> +            Add fake mac address to the PMD's address pool.
> +            Send a packet with the fake mac address to the PMD. (Should
> receive)
> +            Remove the fake mac address from the PMD's address pool.
> +            Sent a packet with the fake mac address to the PMD. (Should
> not receive)
> +        """
> +        testpmd = TestPmdShell(self.sut_node)
> +        testpmd.set_promisc(0, on=False)
> +        testpmd.start()
> +        mac_address = self._sut_port_ingress.mac_address
> +
> +        # Send a packet with NIC default mac address
> +        self.send_packet_and_verify(mac_address=mac_address,
> should_receive=True)
> +        # Send a packet with different mac address
> +        fake_address = "00:00:00:00:00:01"
> +        self.send_packet_and_verify(mac_address=fake_address,
> should_receive=False)
> +
> +        # Add mac address to pool and rerun tests
> +        testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
> +        self.send_packet_and_verify(mac_address=fake_address,
> should_receive=True)
> +        testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
> +        self.send_packet_and_verify(mac_address=fake_address,
> should_receive=False)
> +        testpmd.close()
> +        sleep(6)
> +
> +    def test_invalid_address(self) -> None:
> +        """Assess the behavior of a NIC mac address pool while bound to
> the PMD.
> +
> +        An assessment of a NIC's behavior when mounted to a PMD as it
> relates to mac addresses
> +        and address pooling. Devices should not be able to use invalid
> mac addresses, remove their
> +        built-in hardware address, or exceed their address pools.
> +
> +        Test:
> +            Start TestPMD.
> +            Attempt to add an invalid mac address. (Should fail)
> +            Attempt to remove the device's hardware address with no
> additional addresses in the
> +                address pool. (Should fail)
> +            Add a fake mac address to the pool twice in succession.
> (Should not create any errors)
> +            Attempt to remove the device's hardware address with other
> addresses in the address
> +                pool. (Should fail)
> +            Determine the device's mac address pool size, and fill the
> pool with fake addresses.
> +            Attempt to add another fake mac address, overloading the
> address pool. (Should fail)
> +        """
> +        testpmd = TestPmdShell(self.sut_node)
> +        testpmd.start()
> +        mac_address = self._sut_port_ingress.mac_address
> +        try:
> +            testpmd.set_mac_addr(0, "00:00:00:00:00:00", add=True)
> +            self.verify(False, "Invalid mac address added.")
> +        except InteractiveCommandExecutionError:
> +            pass
> +        try:
> +            testpmd.set_mac_addr(0, mac_address, add=False)
> +            self.verify(False, "Default mac address removed.")
> +        except InteractiveCommandExecutionError:
> +            pass
> +        # Should be no errors adding this twice
> +        testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
> +        testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
> +        # Double check to see if default mac address can be removed
> +        try:
> +            testpmd.set_mac_addr(0, mac_address, add=False)
> +            self.verify(False, "Default mac address removed.")
> +        except InteractiveCommandExecutionError:
> +            pass
> +
> +        for i in range(testpmd.show_port_info(0).max_mac_addresses_num -
> 1):
> +            # A0 fake address based on the index 'i'.
> +            fake_address = str(hex(i)[2:].zfill(12))
> +            # Insert ':' characters every two indexes to create a fake
> mac address.
> +            fake_address = ":".join(fake_address[x : x + 2] for x in
> range(0, len(fake_address), 2))
> +            testpmd.set_mac_addr(0, fake_address, add=True, verify=False)
> +        try:
> +            testpmd.set_mac_addr(0, "F" + mac_address[1:], add=True)
> +            self.verify(False, "Mac address limit exceeded.")
> +        except InteractiveCommandExecutionError:
> +            pass
> +        testpmd.close()
> +        sleep(6)
> +
> +    def test_multicast_filter(self) -> None:
> +        """Assess basic multicast address filtering functionalities.
> +
> +        Ensure that multicast filtering performs as intended when a given
> device is bound
> +        to the PMD, with and without dot1q vlan tagging.
> +
> +        Test:
> +            Start TestPMD with promiscuous mode.
> +            Add a fake multicast address to the PMD's multicast address
> pool.
> +            Send a packet with the fake multicast address to the PMD.
> (Should receive)
> +            Set vlan filtering on the PMD, and add vlan ID to the PMD.
> +            Send a packet with the fake multicast address and vlan ID to
> the PMD. (Should receive)
> +            Send a packet with the fake multicast address and a different
> vlan ID to the PMD.
> +                (Should not receive)
> +            Remove the vlan tag from the PMD, and turn vlan filtering off
> on the PMD.
> +            Send a packet with the fake multicast address and no vlan tag
> to the PMD.
> +                (Should receive)
> +            Remove the fake multicast address from the PMDs multicast
> address filter.
> +            Send a packet with the fake multicast address to the PMD.
> (Should not receive)
> +        """
> +        testpmd = TestPmdShell(self.sut_node)
> +        testpmd.start()
> +        testpmd.set_promisc(0, on=False)
> +        multicast_address = "01:00:5E:00:00:00"
> +        vlan_id = 2
> +
> +        testpmd.set_multicast_mac_addr(0, multi_addr=multicast_address,
> add=True)
> +        self.send_packet_and_verify(multicast_address,
> should_receive=True)
> +
> +        # Test vlan filtering on multicast addressing.
> +        # Verify vlan functionality for debugging purposes.
> +        testpmd.vlan_filter_set_on(port=0)
> +        testpmd.rx_vlan_add(vlan_id, 0)
> +        self.send_packet_and_verify(multicast_address,
> should_receive=True, add_vlan=True)
> +        self.send_packet_and_verify(multicast_address,
> should_receive=False, add_vlan=True)
> +
> +        # Remove vlan tag and filtering and run basic multicast addr test.
> +        testpmd.rx_vlan_rm(vlan_id, 0)
> +        testpmd.vlan_filter_set_off(port=0)
> +        self.send_packet_and_verify(multicast_address,
> should_receive=True)
> +
> +        # Remove multicast filter and verify the packet was not received.
> +        testpmd.set_multicast_mac_addr(0, multicast_address, add=False)
> +        self.send_packet_and_verify(multicast_address,
> should_receive=False)
> +        testpmd.close()
> +        sleep(6)
> --
> 2.44.0
>
>

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

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

* Re: [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses
  2024-07-18 19:05   ` [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses Nicholas Pratte
  2024-07-19 15:37     ` Jeremy Spewock
@ 2024-07-22 13:08     ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Dean Marx @ 2024-07-22 13:08 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, paul.szczepanek, luca.vizzarro, jspewock,
	Honnappa.Nagarahalli, yoan.picchi, juraj.linkes, dev

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

I think Jeremy already mentioned this but just make sure you update
vlan_set_filter_on/off to the new version that has an on boolean arg.

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

On Thu, Jul 18, 2024 at 3:12 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:

> Several new methods have been added to TestPMDShell in order to produce
> the mac filter's individual test cases:
>  - set_mac_addr
>  - set_multicast_mac_addr
>  - rx_vlan_add
>  - rx_vlan_rm
>  - vlan_filter_set_on
>  - vlan_filter_set_off
>  - set_promisc
>
> set_mac_addr and set_multicast_addr were created for the mac filter test
> suite, enabling users to both add or remove mac and multicast
> addresses based on a boolean 'add or remove' parameter. The success or
> failure of each call can be verified if a user deems it necessary.
>
> The other methods listed are implemented in other respective test
> suites, and their implementations have been copied, but are subject to
> change; they are not the focus of this patch.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 179 ++++++++++++++++++
>  1 file changed, 179 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py
> b/dts/framework/remote_session/testpmd_shell.py
> index ec22f72221..8122457ad1 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -767,6 +767,185 @@ def show_port_info(self, port_id: int) ->
> TestPmdPort:
>
>          return TestPmdPort.parse(output)
>
> +    def set_mac_addr(self, port_id: int, mac_address: str, add: bool,
> verify: bool = True) -> None:
> +        """Add or remove a mac address on a given port's Allowlist.
> +
> +        Args:
> +            port_id: The port ID the mac address is set on.
> +            mac_address: The mac address to be added or removed to the
> specified port.
> +            add: If :data:`True`, add the specified mac address. If
> :data:`False`, remove specified
> +                mac address.
> +            verify: If :data:'True', assert that the 'mac_addr' operation
> was successful. If
> +                :data:'False', run the command and skip this assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If the set mac address
> operation fails.
> +        """
> +        mac_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mac_addr {mac_cmd} {port_id}
> {mac_address}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid argument provided to mac_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument
> provided")
> +
> +        if verify:
> +            if "mac_addr_cmd error:" in output:
> +                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on
> port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mac_cmd} {mac_address} on port {port_id}
> \n{output}"
> +                )
> +
> +    def set_multicast_mac_addr(
> +        self, port_id: int, multi_addr: str, add: bool, verify: bool =
> True
> +    ) -> None:
> +        """Add or remove multicast mac address to a specified port's
> filter.
> +
> +        Args:
> +            port_id: The port ID the multicast address is set on.
> +            multi_addr: The multicast address to be added to the filter.
> +            add: If :data:'True', add the specified multicast address to
> the port filter.
> +                If :data:'False', remove the specified multicast address
> from the port filter.
> +            verify: If :data:'True', assert that the 'mcast_addr'
> operations was successful.
> +                If :data:'False', execute the 'mcast_addr' operation and
> skip the assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If either the 'add' or
> 'remove' operations fails.
> +        """
> +        mcast_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id}
> {multi_addr}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid arguments provided to mcast_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument
> provided")
> +
> +        if verify:
> +            if (
> +                "Invalid multicast_addr" in output
> +                or f'multicast address {"already" if add else "not"}
> filtered by port' in output
> +            ):
> +                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr}
> on port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mcast_cmd} {multi_addr} on port
> {port_id} \n{output}"
> +                )
> +
> +    def rx_vlan_add(self, vlan: int, port: int, verify: bool = True) ->
> None:
> +        """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) ->
> None:
> +        """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_filter_set_on(self, port: int, verify: bool = True) -> None:
> +        """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) -> None:
> +        """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 set_promisc(self, port: int, on: bool, verify: bool = True) ->
> None:
> +        """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.
> +        """
> +        promisc_output = self.send_command(f"set promisc {port} {'on' if
> on else 'off'}")
> +        if verify:
> +            stats = self.show_port_info(port_id=port)
> +            if on ^ stats.is_promiscuous_mode_enabled:
> +                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 show_port_stats_all(self) -> list[TestPmdPortStats]:
>          """Returns the statistics of all the ports.
>
> --
> 2.44.0
>
>

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

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

* Re: [PATCH v2 1/3] dts: add boolean to adjust addresses
  2024-07-02 19:24 ` [PATCH v2 1/3] dts: add boolean to adjust addresses Nicholas Pratte
  2024-07-11 19:31   ` Jeremy Spewock
@ 2024-07-22 13:09   ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Dean Marx @ 2024-07-22 13:09 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: Honnappa.Nagarahalli, probb, jspewock, juraj.linkes,
	paul.szczepanek, luca.vizzarro, yoan.picchi, dev

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

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

On Tue, Jul 2, 2024 at 3:25 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:

> Various test cases in the mac filter test suite called for granular
> manipulation of destination mac addresses to properly test mac address
> filtering functionality. To compensate, there is now an
> adjust_addresses boolean which the user can toggle if they wish to send
> their own addressing; the boolean is true by default.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/test_suite.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 694b2eba65..551a587525 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -185,6 +185,7 @@ def send_packet_and_capture(
>          packet: Packet,
>          filter_config: PacketFilteringConfig = PacketFilteringConfig(),
>          duration: float = 1,
> +        adjust_addresses: bool = True,
>      ) -> list[Packet]:
>          """Send and receive `packet` using the associated TG.
>
> @@ -195,11 +196,15 @@ def send_packet_and_capture(
>              packet: The packet to send.
>              filter_config: The filter to use when capturing packets.
>              duration: Capture traffic for this amount of time after
> sending `packet`.
> +            adjust_addresses: If :data:'True', adjust addresses of the
> egressing packet with
> +                a default addressing scheme. If :data:'False', do not
> adjust the addresses of
> +                egressing packet.
>
>          Returns:
>              A list of received packets.
>          """
> -        packet = self._adjust_addresses(packet)
> +        if adjust_addresses:
> +            packet = self._adjust_addresses(packet)
>          return self.tg_node.send_packet_and_capture(
>              packet,
>              self._tg_port_egress,
> --
> 2.44.0
>
>

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

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

* [PATCH v4 0/2] Mac Filter Port to New DTS
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
                   ` (3 preceding siblings ...)
  2024-07-18 19:05 ` [PATCH v3 0/3] Mac Filter Port to New DTS Nicholas Pratte
@ 2024-07-26 16:39 ` Nicholas Pratte
  2024-07-26 16:45   ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
  2024-07-26 16:46   ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
  2024-07-26 16:39 ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
  2024-07-26 16:39 ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
  6 siblings, 2 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-26 16:39 UTC (permalink / raw)
  To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
  Cc: dev, Nicholas Pratte

v4:
  * Refactored test suite to use context manager.
  * Added dependencies for vlan testpmd methods and adjust
    addresses.

Nicholas Pratte (2):
  dts: add methods for setting mac and multicast addresses
  dts: mac filter test suite refactored for new dts

 dts/framework/config/conf_yaml_schema.json    |   3 +-
 dts/framework/remote_session/testpmd_shell.py |  59 +++++
 dts/tests/TestSuite_mac_filter.py             | 217 ++++++++++++++++++
 3 files changed, 278 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

-- 
2.44.0


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

* [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
                   ` (4 preceding siblings ...)
  2024-07-26 16:39 ` [PATCH v4 0/2] Mac Filter Port to New DTS Nicholas Pratte
@ 2024-07-26 16:39 ` Nicholas Pratte
  2024-08-02 20:02   ` Jeremy Spewock
  2024-07-26 16:39 ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
  6 siblings, 1 reply; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-26 16:39 UTC (permalink / raw)
  To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
  Cc: dev, Nicholas Pratte

New methods have been added to TestPMDShell in order to produce the mac
filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a boolean 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 8e5a1c084a..64ffb23439 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -765,6 +765,65 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
         return TestPmdPort.parse(output)
 
+    def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: bool = True) -> None:
+        """Add or remove a mac address on a given port's Allowlist.
+
+        Args:
+            port_id: The port ID the mac address is set on.
+            mac_address: The mac address to be added or removed to the specified port.
+            add: If :data:`True`, add the specified mac address. If :data:`False`, remove specified
+                mac address.
+            verify: If :data:'True', assert that the 'mac_addr' operation was successful. If
+                :data:'False', run the command and skip this assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If the set mac address operation fails.
+        """
+        mac_cmd = "add" if add else "remove"
+        output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid argument provided to mac_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if "mac_addr_cmd error:" in output:
+                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mac_cmd} {mac_address} on port {port_id} \n{output}"
+                )
+
+    def set_multicast_mac_addr(
+        self, port_id: int, multi_addr: str, add: bool, verify: bool = True
+    ) -> None:
+        """Add or remove multicast mac address to a specified port's filter.
+
+        Args:
+            port_id: The port ID the multicast address is set on.
+            multi_addr: The multicast address to be added to the filter.
+            add: If :data:'True', add the specified multicast address to the port filter.
+                If :data:'False', remove the specified multicast address from the port filter.
+            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
+                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
+        """
+        mcast_cmd = "add" if add else "remove"
+        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid arguments provided to mcast_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if (
+                "Invalid multicast_addr" in output
+                or f'multicast address {"already" if add else "not"} filtered by port' in output
+            ):
+                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
+                )
+
     def show_port_stats_all(self) -> list[TestPmdPortStats]:
         """Returns the statistics of all the ports.
 
-- 
2.44.0


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

* [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
                   ` (5 preceding siblings ...)
  2024-07-26 16:39 ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
@ 2024-07-26 16:39 ` Nicholas Pratte
  2024-08-02 20:02   ` Jeremy Spewock
  6 siblings, 1 reply; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-26 16:39 UTC (permalink / raw)
  To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
  Cc: dev, Nicholas Pratte

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

depends-on: patch-142691 ("dts: add send_packets to test suites and
rework packet addressing")
depends-on: patch-142696 ("dts: add VLAN methods to testpmd shell")

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
v4:
 * Refactored TestPMD sessions to interface with context manager.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py          | 217 +++++++++++++++++++++
 2 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 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",
+        "mac_filter"
       ]
     },
     "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
new file mode 100644
index 0000000000..9d61eb514d
--- /dev/null
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,217 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a device
+contains a mac address not contained with its mac address pool, the packet should
+be dropped. Alternatively, if a packet is received that contains a destination mac
+within the devices address pool, the packet should be accepted and forwarded. This
+behavior should remain consistent across all packets, namely those containing dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac address
+allow list is behaving as expected.
+"""
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+    """Mac address allowlist filtering test suite.
+
+    Configure mac address filtering on a given port, and test the port's filtering behavior
+    using both a given port's hardware address as well as dummy addresses. If a port accepts
+    a packet that is not contained within its mac address allowlist, then a given test case
+    fails. Alternatively, if a port drops a packet that is designated within its mac address
+    allowlist, a given test case will fail.
+
+    Moreover, a given port should demonstrate proper behavior when bound to the Poll Mode
+    Driver. A port should not have a mac address allowlist that exceeds its designated size.
+    A port's default hardware address should not be removed from its address pool, and invalid
+    addresses should not be included in the allowlist. If a port abides by the above rules, the
+    test case passes.
+    """
+
+    def send_packet_and_verify(
+        self,
+        mac_address: str,
+        add_vlan: bool = False,
+        should_receive: bool = True,
+    ) -> None:
+        """Generate, send, and verify a packet based on specified parameters.
+
+        Test cases within this suite utilize this method to create, send, and verify
+        packets based on criteria relating to the packet's destination mac address,
+        vlan tag, and whether or not the packet should be received or not. Packets
+        are verified using an inserted payload. Assuming the test case expects to
+        receive a specified packet, if the list of received packets contains this
+        payload within any of its packets, the test case passes. Alternatively, if
+        the designed packet should not be received, and the packet payload is not,
+        received, then the test case fails. Each call with this method sends exactly
+        one packet.
+
+        Args:
+            mac_address: The destination mac address of the packet being sent.
+            add_vlan: If :data:'True', add a vlan tag to the packet being sent. The
+                vlan tag will be :data:'2' if the packet should be received and
+                :data:'1' if the packet should not be received but requires a vlan tag.
+            should_receive: If :data:'True', assert whether or not the sent packet
+                has been received. If :data:'False', assert that the send packet was not
+                received. :data:'True' by default
+        """
+        if add_vlan:
+            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) / IP() / Raw(load="X" * 22)
+        else:
+            packet = Ether() / IP() / Raw(load="X" * 22)
+        packet.dst = mac_address
+        received_packets = [
+            packets
+            for packets in self.send_packet_and_capture(packet)
+            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
+        ]
+        if should_receive:
+            self.verify(len(received_packets) == 1, "Expected packet not received")
+        else:
+            self.verify(len(received_packets) == 0, "Expected packet received")
+
+    def test_add_remove_mac_addresses(self) -> None:
+        """Assess basic mac addressing filtering functionalities.
+
+        This test case validates for proper behavior of mac address filtering with both
+        a port's default, burned-in mac address, as well as additional mac addresses
+        added to the PMD. Packets should either be received or not received depending on
+        the properties applied to the PMD at any given time.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Send a packet with the port's default mac address. (Should receive)
+            Send a packet with fake mac address. (Should not receive)
+            Add fake mac address to the PMD's address pool.
+            Send a packet with the fake mac address to the PMD. (Should receive)
+            Remove the fake mac address from the PMD's address pool.
+            Sent a packet with the fake mac address to the PMD. (Should not receive)
+        """
+        with TestPmdShell(self.sut_node) as testpmd:
+            testpmd.set_promisc(0, on=False)
+            testpmd.start()
+            mac_address = self._sut_port_ingress.mac_address
+
+            # Send a packet with NIC default mac address
+            self.send_packet_and_verify(mac_address=mac_address, should_receive=True)
+            # Send a packet with different mac address
+            fake_address = "00:00:00:00:00:01"
+            self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+
+            # Add mac address to pool and rerun tests
+            testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
+            self.send_packet_and_verify(mac_address=fake_address, should_receive=True)
+            testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
+            self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+
+    def test_invalid_address(self) -> None:
+        """Assess the behavior of a NIC mac address pool while bound to the PMD.
+
+        An assessment of a NIC's behavior when mounted to a PMD as it relates to mac addresses
+        and address pooling. Devices should not be able to use invalid mac addresses, remove their
+        built-in hardware address, or exceed their address pools.
+
+        Test:
+            Start TestPMD.
+            Attempt to add an invalid mac address. (Should fail)
+            Attempt to remove the device's hardware address with no additional addresses in the
+                address pool. (Should fail)
+            Add a fake mac address to the pool twice in succession. (Should not create any errors)
+            Attempt to remove the device's hardware address with other addresses in the address
+                pool. (Should fail)
+            Determine the device's mac address pool size, and fill the pool with fake addresses.
+            Attempt to add another fake mac address, overloading the address pool. (Should fail)
+        """
+        with TestPmdShell(self.sut_node) as testpmd:
+            testpmd.start()
+            mac_address = self._sut_port_ingress.mac_address
+            try:
+                testpmd.set_mac_addr(0, "00:00:00:00:00:00", add=True)
+                self.verify(False, "Invalid mac address added.")
+            except InteractiveCommandExecutionError:
+                pass
+            try:
+                testpmd.set_mac_addr(0, mac_address, add=False)
+                self.verify(False, "Default mac address removed.")
+            except InteractiveCommandExecutionError:
+                pass
+            # Should be no errors adding this twice
+            testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+            testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+            # Double check to see if default mac address can be removed
+            try:
+                testpmd.set_mac_addr(0, mac_address, add=False)
+                self.verify(False, "Default mac address removed.")
+            except InteractiveCommandExecutionError:
+                pass
+
+            for i in range(testpmd.show_port_info(0).max_mac_addresses_num - 1):
+                # A0 fake address based on the index 'i'.
+                fake_address = str(hex(i)[2:].zfill(12))
+                # Insert ':' characters every two indexes to create a fake mac address.
+                fake_address = ":".join(
+                    fake_address[x : x + 2] for x in range(0, len(fake_address), 2)
+                )
+                testpmd.set_mac_addr(0, fake_address, add=True, verify=False)
+            try:
+                testpmd.set_mac_addr(0, "F" + mac_address[1:], add=True)
+                self.verify(False, "Mac address limit exceeded.")
+            except InteractiveCommandExecutionError:
+                pass
+
+    def test_multicast_filter(self) -> None:
+        """Assess basic multicast address filtering functionalities.
+
+        Ensure that multicast filtering performs as intended when a given device is bound
+        to the PMD, with and without dot1q vlan tagging.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Add a fake multicast address to the PMD's multicast address pool.
+            Send a packet with the fake multicast address to the PMD. (Should receive)
+            Set vlan filtering on the PMD, and add vlan ID to the PMD.
+            Send a packet with the fake multicast address and vlan ID to the PMD. (Should receive)
+            Send a packet with the fake multicast address and a different vlan ID to the PMD.
+                (Should not receive)
+            Remove the vlan tag from the PMD, and turn vlan filtering off on the PMD.
+            Send a packet with the fake multicast address and no vlan tag to the PMD.
+                (Should receive)
+            Remove the fake multicast address from the PMDs multicast address filter.
+            Send a packet with the fake multicast address to the PMD. (Should not receive)
+        """
+        with TestPmdShell(self.sut_node) as testpmd:
+            testpmd.start()
+            testpmd.set_promisc(0, on=False)
+            multicast_address = "01:00:5E:00:00:00"
+            vlan_id = 2
+
+            testpmd.set_multicast_mac_addr(0, multi_addr=multicast_address, add=True)
+            self.send_packet_and_verify(multicast_address, should_receive=True)
+
+            # Test vlan filtering on multicast addressing.
+            # Verify vlan functionality for debugging purposes.
+            testpmd.vlan_filter_set(0, on=True)
+            testpmd.rx_vlan(vlan_id, 0, add=True)
+            self.send_packet_and_verify(multicast_address, should_receive=True, add_vlan=True)
+            self.send_packet_and_verify(multicast_address, should_receive=False, add_vlan=True)
+
+            # Remove vlan tag and filtering and run basic multicast addr test.
+            testpmd.rx_vlan(vlan_id, 0, add=False)
+            testpmd.vlan_filter_set(0, on=False)
+            self.send_packet_and_verify(multicast_address, should_receive=True)
+
+            # Remove multicast filter and verify the packet was not received.
+            testpmd.set_multicast_mac_addr(0, multicast_address, add=False)
+            self.send_packet_and_verify(multicast_address, should_receive=False)
-- 
2.44.0


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

* [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses
  2024-07-26 16:39 ` [PATCH v4 0/2] Mac Filter Port to New DTS Nicholas Pratte
@ 2024-07-26 16:45   ` Nicholas Pratte
  2024-08-02 20:26     ` Jeremy Spewock
  2024-08-12 18:58     ` Dean Marx
  2024-07-26 16:46   ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
  1 sibling, 2 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-26 16:45 UTC (permalink / raw)
  To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
  Cc: dev, Nicholas Pratte

New methods have been added to TestPMDShell in order to produce the mac
filter's individual test cases:
 - set_mac_addr
 - set_multicast_mac_addr

set_mac_addr and set_multicast_addr were created for the mac filter test
suite, enabling users to both add or remove mac and multicast
addresses based on a boolean 'add or remove' parameter. The success or
failure of each call can be verified if a user deems it necessary.

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/remote_session/testpmd_shell.py | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 8e5a1c084a..64ffb23439 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -765,6 +765,65 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
 
         return TestPmdPort.parse(output)
 
+    def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: bool = True) -> None:
+        """Add or remove a mac address on a given port's Allowlist.
+
+        Args:
+            port_id: The port ID the mac address is set on.
+            mac_address: The mac address to be added or removed to the specified port.
+            add: If :data:`True`, add the specified mac address. If :data:`False`, remove specified
+                mac address.
+            verify: If :data:'True', assert that the 'mac_addr' operation was successful. If
+                :data:'False', run the command and skip this assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If the set mac address operation fails.
+        """
+        mac_cmd = "add" if add else "remove"
+        output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid argument provided to mac_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if "mac_addr_cmd error:" in output:
+                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mac_cmd} {mac_address} on port {port_id} \n{output}"
+                )
+
+    def set_multicast_mac_addr(
+        self, port_id: int, multi_addr: str, add: bool, verify: bool = True
+    ) -> None:
+        """Add or remove multicast mac address to a specified port's filter.
+
+        Args:
+            port_id: The port ID the multicast address is set on.
+            multi_addr: The multicast address to be added to the filter.
+            add: If :data:'True', add the specified multicast address to the port filter.
+                If :data:'False', remove the specified multicast address from the port filter.
+            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
+                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
+
+        Raises:
+            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
+        """
+        mcast_cmd = "add" if add else "remove"
+        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
+        if "Bad arguments" in output:
+            self._logger.debug("Invalid arguments provided to mcast_addr")
+            raise InteractiveCommandExecutionError("Invalid argument provided")
+
+        if verify:
+            if (
+                "Invalid multicast_addr" in output
+                or f'multicast address {"already" if add else "not"} filtered by port' in output
+            ):
+                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
+                raise InteractiveCommandExecutionError(
+                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
+                )
+
     def show_port_stats_all(self) -> list[TestPmdPortStats]:
         """Returns the statistics of all the ports.
 
-- 
2.44.0


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

* [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-07-26 16:39 ` [PATCH v4 0/2] Mac Filter Port to New DTS Nicholas Pratte
  2024-07-26 16:45   ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
@ 2024-07-26 16:46   ` Nicholas Pratte
  2024-08-02 20:25     ` Jeremy Spewock
                       ` (2 more replies)
  1 sibling, 3 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-07-26 16:46 UTC (permalink / raw)
  To: probb, dmarx, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes
  Cc: dev, Nicholas Pratte

The mac address filter test suite, whose test cases are based on old
DTS's test cases, has been refactored to interface with the new DTS
framework.

In porting over this test suite into the new framework, some
adjustments were made, namely in the EAL and TestPMD parameter provided
before executing the application. While the original test plan was
referenced, by and large, only for the individual test cases, I'll leave
the parameters the original test plan was asking for below for the sake
of discussion:

--burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
--txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3

depends-on: patch-142691 ("dts: add send_packets to test suites and
rework packet addressing")
depends-on: patch-142696 ("dts: add VLAN methods to testpmd shell")

Bugzilla ID: 1454
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>

---
v2:
 * Refactored the address pool capacity tests to use all available
   octets in the mac address.
 * Change the payload to 'X' characters instead of 'P' characters.
v4:
 * Refactored TestPMD sessions to interface with context manager.
---
 dts/framework/config/conf_yaml_schema.json |   3 +-
 dts/tests/TestSuite_mac_filter.py          | 217 +++++++++++++++++++++
 2 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_mac_filter.py

diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..ad1f3757f7 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",
+        "mac_filter"
       ]
     },
     "test_target": {
diff --git a/dts/tests/TestSuite_mac_filter.py b/dts/tests/TestSuite_mac_filter.py
new file mode 100644
index 0000000000..9d61eb514d
--- /dev/null
+++ b/dts/tests/TestSuite_mac_filter.py
@@ -0,0 +1,217 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023-2024 University of New Hampshire
+"""Mac address filtering test suite.
+
+This test suite ensures proper and expected behavior of Allowlist filtering via mac
+addresses on devices bound to the Poll Mode Driver. If a packet received on a device
+contains a mac address not contained with its mac address pool, the packet should
+be dropped. Alternatively, if a packet is received that contains a destination mac
+within the devices address pool, the packet should be accepted and forwarded. This
+behavior should remain consistent across all packets, namely those containing dot1q
+tags or otherwise.
+
+The following test suite assesses behaviors based on the aforementioned logic.
+Additionally, testing is done within the PMD itself to ensure that the mac address
+allow list is behaving as expected.
+"""
+
+from scapy.layers.inet import IP  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q, Ether  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.exception import InteractiveCommandExecutionError
+from framework.remote_session.testpmd_shell import TestPmdShell
+from framework.test_suite import TestSuite
+
+
+class TestMacFilter(TestSuite):
+    """Mac address allowlist filtering test suite.
+
+    Configure mac address filtering on a given port, and test the port's filtering behavior
+    using both a given port's hardware address as well as dummy addresses. If a port accepts
+    a packet that is not contained within its mac address allowlist, then a given test case
+    fails. Alternatively, if a port drops a packet that is designated within its mac address
+    allowlist, a given test case will fail.
+
+    Moreover, a given port should demonstrate proper behavior when bound to the Poll Mode
+    Driver. A port should not have a mac address allowlist that exceeds its designated size.
+    A port's default hardware address should not be removed from its address pool, and invalid
+    addresses should not be included in the allowlist. If a port abides by the above rules, the
+    test case passes.
+    """
+
+    def send_packet_and_verify(
+        self,
+        mac_address: str,
+        add_vlan: bool = False,
+        should_receive: bool = True,
+    ) -> None:
+        """Generate, send, and verify a packet based on specified parameters.
+
+        Test cases within this suite utilize this method to create, send, and verify
+        packets based on criteria relating to the packet's destination mac address,
+        vlan tag, and whether or not the packet should be received or not. Packets
+        are verified using an inserted payload. Assuming the test case expects to
+        receive a specified packet, if the list of received packets contains this
+        payload within any of its packets, the test case passes. Alternatively, if
+        the designed packet should not be received, and the packet payload is not,
+        received, then the test case fails. Each call with this method sends exactly
+        one packet.
+
+        Args:
+            mac_address: The destination mac address of the packet being sent.
+            add_vlan: If :data:'True', add a vlan tag to the packet being sent. The
+                vlan tag will be :data:'2' if the packet should be received and
+                :data:'1' if the packet should not be received but requires a vlan tag.
+            should_receive: If :data:'True', assert whether or not the sent packet
+                has been received. If :data:'False', assert that the send packet was not
+                received. :data:'True' by default
+        """
+        if add_vlan:
+            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) / IP() / Raw(load="X" * 22)
+        else:
+            packet = Ether() / IP() / Raw(load="X" * 22)
+        packet.dst = mac_address
+        received_packets = [
+            packets
+            for packets in self.send_packet_and_capture(packet)
+            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
+        ]
+        if should_receive:
+            self.verify(len(received_packets) == 1, "Expected packet not received")
+        else:
+            self.verify(len(received_packets) == 0, "Expected packet received")
+
+    def test_add_remove_mac_addresses(self) -> None:
+        """Assess basic mac addressing filtering functionalities.
+
+        This test case validates for proper behavior of mac address filtering with both
+        a port's default, burned-in mac address, as well as additional mac addresses
+        added to the PMD. Packets should either be received or not received depending on
+        the properties applied to the PMD at any given time.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Send a packet with the port's default mac address. (Should receive)
+            Send a packet with fake mac address. (Should not receive)
+            Add fake mac address to the PMD's address pool.
+            Send a packet with the fake mac address to the PMD. (Should receive)
+            Remove the fake mac address from the PMD's address pool.
+            Sent a packet with the fake mac address to the PMD. (Should not receive)
+        """
+        with TestPmdShell(self.sut_node) as testpmd:
+            testpmd.set_promisc(0, on=False)
+            testpmd.start()
+            mac_address = self._sut_port_ingress.mac_address
+
+            # Send a packet with NIC default mac address
+            self.send_packet_and_verify(mac_address=mac_address, should_receive=True)
+            # Send a packet with different mac address
+            fake_address = "00:00:00:00:00:01"
+            self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+
+            # Add mac address to pool and rerun tests
+            testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
+            self.send_packet_and_verify(mac_address=fake_address, should_receive=True)
+            testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
+            self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
+
+    def test_invalid_address(self) -> None:
+        """Assess the behavior of a NIC mac address pool while bound to the PMD.
+
+        An assessment of a NIC's behavior when mounted to a PMD as it relates to mac addresses
+        and address pooling. Devices should not be able to use invalid mac addresses, remove their
+        built-in hardware address, or exceed their address pools.
+
+        Test:
+            Start TestPMD.
+            Attempt to add an invalid mac address. (Should fail)
+            Attempt to remove the device's hardware address with no additional addresses in the
+                address pool. (Should fail)
+            Add a fake mac address to the pool twice in succession. (Should not create any errors)
+            Attempt to remove the device's hardware address with other addresses in the address
+                pool. (Should fail)
+            Determine the device's mac address pool size, and fill the pool with fake addresses.
+            Attempt to add another fake mac address, overloading the address pool. (Should fail)
+        """
+        with TestPmdShell(self.sut_node) as testpmd:
+            testpmd.start()
+            mac_address = self._sut_port_ingress.mac_address
+            try:
+                testpmd.set_mac_addr(0, "00:00:00:00:00:00", add=True)
+                self.verify(False, "Invalid mac address added.")
+            except InteractiveCommandExecutionError:
+                pass
+            try:
+                testpmd.set_mac_addr(0, mac_address, add=False)
+                self.verify(False, "Default mac address removed.")
+            except InteractiveCommandExecutionError:
+                pass
+            # Should be no errors adding this twice
+            testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+            testpmd.set_mac_addr(0, "1" + mac_address[1:], add=True)
+            # Double check to see if default mac address can be removed
+            try:
+                testpmd.set_mac_addr(0, mac_address, add=False)
+                self.verify(False, "Default mac address removed.")
+            except InteractiveCommandExecutionError:
+                pass
+
+            for i in range(testpmd.show_port_info(0).max_mac_addresses_num - 1):
+                # A0 fake address based on the index 'i'.
+                fake_address = str(hex(i)[2:].zfill(12))
+                # Insert ':' characters every two indexes to create a fake mac address.
+                fake_address = ":".join(
+                    fake_address[x : x + 2] for x in range(0, len(fake_address), 2)
+                )
+                testpmd.set_mac_addr(0, fake_address, add=True, verify=False)
+            try:
+                testpmd.set_mac_addr(0, "F" + mac_address[1:], add=True)
+                self.verify(False, "Mac address limit exceeded.")
+            except InteractiveCommandExecutionError:
+                pass
+
+    def test_multicast_filter(self) -> None:
+        """Assess basic multicast address filtering functionalities.
+
+        Ensure that multicast filtering performs as intended when a given device is bound
+        to the PMD, with and without dot1q vlan tagging.
+
+        Test:
+            Start TestPMD with promiscuous mode.
+            Add a fake multicast address to the PMD's multicast address pool.
+            Send a packet with the fake multicast address to the PMD. (Should receive)
+            Set vlan filtering on the PMD, and add vlan ID to the PMD.
+            Send a packet with the fake multicast address and vlan ID to the PMD. (Should receive)
+            Send a packet with the fake multicast address and a different vlan ID to the PMD.
+                (Should not receive)
+            Remove the vlan tag from the PMD, and turn vlan filtering off on the PMD.
+            Send a packet with the fake multicast address and no vlan tag to the PMD.
+                (Should receive)
+            Remove the fake multicast address from the PMDs multicast address filter.
+            Send a packet with the fake multicast address to the PMD. (Should not receive)
+        """
+        with TestPmdShell(self.sut_node) as testpmd:
+            testpmd.start()
+            testpmd.set_promisc(0, on=False)
+            multicast_address = "01:00:5E:00:00:00"
+            vlan_id = 2
+
+            testpmd.set_multicast_mac_addr(0, multi_addr=multicast_address, add=True)
+            self.send_packet_and_verify(multicast_address, should_receive=True)
+
+            # Test vlan filtering on multicast addressing.
+            # Verify vlan functionality for debugging purposes.
+            testpmd.vlan_filter_set(0, on=True)
+            testpmd.rx_vlan(vlan_id, 0, add=True)
+            self.send_packet_and_verify(multicast_address, should_receive=True, add_vlan=True)
+            self.send_packet_and_verify(multicast_address, should_receive=False, add_vlan=True)
+
+            # Remove vlan tag and filtering and run basic multicast addr test.
+            testpmd.rx_vlan(vlan_id, 0, add=False)
+            testpmd.vlan_filter_set(0, on=False)
+            self.send_packet_and_verify(multicast_address, should_receive=True)
+
+            # Remove multicast filter and verify the packet was not received.
+            testpmd.set_multicast_mac_addr(0, multicast_address, add=False)
+            self.send_packet_and_verify(multicast_address, should_receive=False)
-- 
2.44.0


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

* Re: [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses
  2024-07-26 16:39 ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
@ 2024-08-02 20:02   ` Jeremy Spewock
  0 siblings, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-08-02 20:02 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
	paul.szczepanek, juraj.linkes, dev

On Fri, Jul 26, 2024 at 12:39 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> New methods have been added to TestPMDShell in order to produce the mac
> filter's individual test cases:
>  - set_mac_addr
>  - set_multicast_mac_addr
>
> set_mac_addr and set_multicast_addr were created for the mac filter test
> suite, enabling users to both add or remove mac and multicast
> addresses based on a boolean 'add or remove' parameter. The success or
> failure of each call can be verified if a user deems it necessary.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>

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

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

* Re: [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-07-26 16:39 ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
@ 2024-08-02 20:02   ` Jeremy Spewock
  0 siblings, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-08-02 20:02 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
	paul.szczepanek, juraj.linkes, dev

On Fri, Jul 26, 2024 at 12:39 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The mac address filter test suite, whose test cases are based on old
> DTS's test cases, has been refactored to interface with the new DTS
> framework.
>
> In porting over this test suite into the new framework, some
> adjustments were made, namely in the EAL and TestPMD parameter provided
> before executing the application. While the original test plan was
> referenced, by and large, only for the individual test cases, I'll leave
> the parameters the original test plan was asking for below for the sake
> of discussion:
>
> --burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
> --txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3
>
> depends-on: patch-142691 ("dts: add send_packets to test suites and
> rework packet addressing")
> depends-on: patch-142696 ("dts: add VLAN methods to testpmd shell")
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>

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

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

* Re: [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-07-26 16:46   ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
@ 2024-08-02 20:25     ` Jeremy Spewock
  2024-08-02 20:27       ` Jeremy Spewock
  2024-08-12 18:47     ` Dean Marx
  2024-09-04 21:14     ` Dean Marx
  2 siblings, 1 reply; 33+ messages in thread
From: Jeremy Spewock @ 2024-08-02 20:25 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
	paul.szczepanek, juraj.linkes, dev

Just a few small comments, otherwise:

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

On Fri, Jul 26, 2024 at 12:46 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The mac address filter test suite, whose test cases are based on old
> DTS's test cases, has been refactored to interface with the new DTS
> framework.
>
> In porting over this test suite into the new framework, some
> adjustments were made, namely in the EAL and TestPMD parameter provided
> before executing the application. While the original test plan was
> referenced, by and large, only for the individual test cases, I'll leave
> the parameters the original test plan was asking for below for the sake
> of discussion:
>
> --burst=1 --rxpt=0 --rxht=0 --rxwt=0 --txpt=36 --txht=0 --txwt=0
> --txfreet=32 --rxfreet=64 --mbcache=250 --portmask=0x3
>
> depends-on: patch-142691 ("dts: add send_packets to test suites and
> rework packet addressing")
> depends-on: patch-142696 ("dts: add VLAN methods to testpmd shell")
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
<snip>
> +
> +    def send_packet_and_verify(
> +        self,
> +        mac_address: str,
> +        add_vlan: bool = False,
> +        should_receive: bool = True,
> +    ) -> None:
> +        """Generate, send, and verify a packet based on specified parameters.
> +
> +        Test cases within this suite utilize this method to create, send, and verify
> +        packets based on criteria relating to the packet's destination mac address,
> +        vlan tag, and whether or not the packet should be received or not. Packets
> +        are verified using an inserted payload. Assuming the test case expects to
> +        receive a specified packet, if the list of received packets contains this
> +        payload within any of its packets, the test case passes. Alternatively, if
> +        the designed packet should not be received, and the packet payload is not,

I think there is an extra comma here, but we probably should remove
the "not," all together since the test case really fails here if it is
received.

> +        received, then the test case fails. Each call with this method sends exactly
> +        one packet.
> +
> +        Args:
> +            mac_address: The destination mac address of the packet being sent.
> +            add_vlan: If :data:'True', add a vlan tag to the packet being sent. The
> +                vlan tag will be :data:'2' if the packet should be received and
> +                :data:'1' if the packet should not be received but requires a vlan tag.
> +            should_receive: If :data:'True', assert whether or not the sent packet
> +                has been received. If :data:'False', assert that the send packet was not
> +                received. :data:'True' by default
> +        """
> +        if add_vlan:
> +            packet = Ether() / Dot1Q(vlan=2 if should_receive else 1) / IP() / Raw(load="X" * 22)
> +        else:
> +            packet = Ether() / IP() / Raw(load="X" * 22)
> +        packet.dst = mac_address
> +        received_packets = [
> +            packets
> +            for packets in self.send_packet_and_capture(packet)
> +            if hasattr(packets, "load") and "X" * 22 in str(packets.load)
> +        ]
> +        if should_receive:
> +            self.verify(len(received_packets) == 1, "Expected packet not received")
> +        else:
> +            self.verify(len(received_packets) == 0, "Expected packet received")
> +
> +    def test_add_remove_mac_addresses(self) -> None:
> +        """Assess basic mac addressing filtering functionalities.
> +
> +        This test case validates for proper behavior of mac address filtering with both
> +        a port's default, burned-in mac address, as well as additional mac addresses
> +        added to the PMD. Packets should either be received or not received depending on
> +        the properties applied to the PMD at any given time.
> +
> +        Test:
> +            Start TestPMD with promiscuous mode.
> +            Send a packet with the port's default mac address. (Should receive)
> +            Send a packet with fake mac address. (Should not receive)
> +            Add fake mac address to the PMD's address pool.
> +            Send a packet with the fake mac address to the PMD. (Should receive)
> +            Remove the fake mac address from the PMD's address pool.
> +            Sent a packet with the fake mac address to the PMD. (Should not receive)

Typo: sent should be send.

> +        """
> +        with TestPmdShell(self.sut_node) as testpmd:
> +            testpmd.set_promisc(0, on=False)
> +            testpmd.start()
> +            mac_address = self._sut_port_ingress.mac_address
> +
> +            # Send a packet with NIC default mac address
> +            self.send_packet_and_verify(mac_address=mac_address, should_receive=True)
> +            # Send a packet with different mac address
> +            fake_address = "00:00:00:00:00:01"
> +            self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
> +
> +            # Add mac address to pool and rerun tests
> +            testpmd.set_mac_addr(0, mac_address=fake_address, add=True)
> +            self.send_packet_and_verify(mac_address=fake_address, should_receive=True)
> +            testpmd.set_mac_addr(0, mac_address=fake_address, add=False)
> +            self.send_packet_and_verify(mac_address=fake_address, should_receive=False)
> +<snip>
> 2.44.0
>

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

* Re: [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses
  2024-07-26 16:45   ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
@ 2024-08-02 20:26     ` Jeremy Spewock
  2024-08-12 18:58     ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-08-02 20:26 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
	paul.szczepanek, juraj.linkes, dev

Oops, I just reviewed an older version without realizing this one
existed, haha. Although, seeing the full diff again I saw a few
documentation nit-picks that I put below.

On Fri, Jul 26, 2024 at 12:45 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> New methods have been added to TestPMDShell in order to produce the mac
> filter's individual test cases:
>  - set_mac_addr
>  - set_multicast_mac_addr
>
> set_mac_addr and set_multicast_addr were created for the mac filter test
> suite, enabling users to both add or remove mac and multicast
> addresses based on a boolean 'add or remove' parameter. The success or
> failure of each call can be verified if a user deems it necessary.
>
> Bugzilla ID: 1454
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
>  dts/framework/remote_session/testpmd_shell.py | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index 8e5a1c084a..64ffb23439 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -765,6 +765,65 @@ def show_port_info(self, port_id: int) -> TestPmdPort:
>
>          return TestPmdPort.parse(output)
>
> +    def set_mac_addr(self, port_id: int, mac_address: str, add: bool, verify: bool = True) -> None:
> +        """Add or remove a mac address on a given port's Allowlist.
> +
> +        Args:
> +            port_id: The port ID the mac address is set on.
> +            mac_address: The mac address to be added or removed to the specified port.

This might be more clear if it said the mac address to be "added to or
removed from" since the "removed to" doesn't exactly fit.

> +            add: If :data:`True`, add the specified mac address. If :data:`False`, remove specified
> +                mac address.
> +            verify: If :data:'True', assert that the 'mac_addr' operation was successful. If
> +                :data:'False', run the command and skip this assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If the set mac address operation fails.
> +        """
> +        mac_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mac_addr {mac_cmd} {port_id} {mac_address}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid argument provided to mac_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument provided")
> +
> +        if verify:
> +            if "mac_addr_cmd error:" in output:
> +                self._logger.debug(f"Failed to {mac_cmd} {mac_address} on port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mac_cmd} {mac_address} on port {port_id} \n{output}"
> +                )
> +
> +    def set_multicast_mac_addr(
> +        self, port_id: int, multi_addr: str, add: bool, verify: bool = True
> +    ) -> None:
> +        """Add or remove multicast mac address to a specified port's filter.

Same thing here as above, but it might be a little trickier here.
Maybe this could be something like allow or disallow to make it more
homogeneous? I'm not quite sure.

> +
> +        Args:
> +            port_id: The port ID the multicast address is set on.
> +            multi_addr: The multicast address to be added to the filter.

Since this method also has an `add` parameter, it would probably be
helpful to specify that this also can be either added to or removed
from the filter.


> +            add: If :data:'True', add the specified multicast address to the port filter.
> +                If :data:'False', remove the specified multicast address from the port filter.
> +            verify: If :data:'True', assert that the 'mcast_addr' operations was successful.
> +                If :data:'False', execute the 'mcast_addr' operation and skip the assertion.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If either the 'add' or 'remove' operations fails.
> +        """
> +        mcast_cmd = "add" if add else "remove"
> +        output = self.send_command(f"mcast_addr {mcast_cmd} {port_id} {multi_addr}")
> +        if "Bad arguments" in output:
> +            self._logger.debug("Invalid arguments provided to mcast_addr")
> +            raise InteractiveCommandExecutionError("Invalid argument provided")
> +
> +        if verify:
> +            if (
> +                "Invalid multicast_addr" in output
> +                or f'multicast address {"already" if add else "not"} filtered by port' in output
> +            ):
> +                self._logger.debug(f"Failed to {mcast_cmd} {multi_addr} on port {port_id}")
> +                raise InteractiveCommandExecutionError(
> +                    f"Failed to {mcast_cmd} {multi_addr} on port {port_id} \n{output}"
> +                )
> +
>      def show_port_stats_all(self) -> list[TestPmdPortStats]:
>          """Returns the statistics of all the ports.
>
> --
> 2.44.0
>

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

* Re: [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-08-02 20:25     ` Jeremy Spewock
@ 2024-08-02 20:27       ` Jeremy Spewock
  0 siblings, 0 replies; 33+ messages in thread
From: Jeremy Spewock @ 2024-08-02 20:27 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, dmarx, luca.vizzarro, yoan.picchi, Honnappa.Nagarahalli,
	paul.szczepanek, juraj.linkes, dev

Apologies, sent reviews in the wrong order so I am sending another
reply to this one just to make sure it appears second in people's
inboxes for less confusion.

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

* Re: [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-07-26 16:46   ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
  2024-08-02 20:25     ` Jeremy Spewock
@ 2024-08-12 18:47     ` Dean Marx
  2024-09-04 21:14     ` Dean Marx
  2 siblings, 0 replies; 33+ messages in thread
From: Dean Marx @ 2024-08-12 18:47 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes, dev

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

>
> +"""Mac address filtering test suite.
> +
> +This test suite ensures proper and expected behavior of Allowlist
> filtering via mac
> +addresses on devices bound to the Poll Mode Driver. If a packet received
> on a device
> +contains a mac address not contained with its mac address pool, the
> packet should
> +be dropped. Alternatively, if a packet is received that contains a
> destination mac
> +within the devices address pool, the packet should be accepted and
> forwarded. This
> +behavior should remain consistent across all packets, namely those
> containing dot1q
> +tags or otherwise.
>
<snip>

This should probably say "not contained within its mac address pool"
instead of "with", since that's how you're wording the rest of the
docstrings

<snip>

> +        Test cases within this suite utilize this method to create, send,
> and verify
> +        packets based on criteria relating to the packet's destination
> mac address,
> +        vlan tag, and whether or not the packet should be received or
> not. Packets
> +        are verified using an inserted payload. Assuming the test case
> expects to
> +        receive a specified packet, if the list of received packets
> contains this
> +        payload within any of its packets, the test case passes.
> Alternatively, if
> +        the designed packet should not be received, and the packet
> payload is not,
> +        received, then the test case fails. Each call with this method
> sends exactly
> +        one packet.
>
<snip>

"and whether or not the packet should be received or not" is redundant

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

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

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

* Re: [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses
  2024-07-26 16:45   ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
  2024-08-02 20:26     ` Jeremy Spewock
@ 2024-08-12 18:58     ` Dean Marx
  1 sibling, 0 replies; 33+ messages in thread
From: Dean Marx @ 2024-08-12 18:58 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes, dev

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

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

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

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

* Re: [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-07-26 16:46   ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
  2024-08-02 20:25     ` Jeremy Spewock
  2024-08-12 18:47     ` Dean Marx
@ 2024-09-04 21:14     ` Dean Marx
  2024-09-05 19:11       ` Nicholas Pratte
  2 siblings, 1 reply; 33+ messages in thread
From: Dean Marx @ 2024-09-04 21:14 UTC (permalink / raw)
  To: Nicholas Pratte
  Cc: probb, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes, dev

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

>
> <snip>
> +        if should_receive:
> +            self.verify(len(received_packets) == 1, "Expected packet not
> received")
> +        else:
> +            self.verify(len(received_packets) == 0, "Expected packet
> received")
>

Side note, didn't notice until I tested it but "Expected packet received"
doesn't really make sense as an error message

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

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

* Re: [PATCH v4 2/2] dts: mac filter test suite refactored for new dts
  2024-09-04 21:14     ` Dean Marx
@ 2024-09-05 19:11       ` Nicholas Pratte
  0 siblings, 0 replies; 33+ messages in thread
From: Nicholas Pratte @ 2024-09-05 19:11 UTC (permalink / raw)
  To: Dean Marx
  Cc: probb, jspewock, luca.vizzarro, yoan.picchi,
	Honnappa.Nagarahalli, paul.szczepanek, juraj.linkes, dev,
	Thomas Monjalon, Ferruh Yigit, Stephen Hemminger,
	Morten Brørup, mkashani

I wanted to point out a unique issue I've been experiencing on the
Mellanox/NVIDIA NICs (Connect x5). The mac address pool feature, which
is assessed in the test_invalid_address, inserts 128 (in the case of
Connect_X5) addresses and fails this test case. On the other hand,
Broadcom P225p devices are capped at 127 addresses because it includes
its default, vendor-provided mac address in the 128 mac address pool
total. Basically Mellanox allows 129 addresses total because they do
not include the device's default mac address total, and other devices
do include this address in the total.

This is a minor issue, but a consensus may need to be made since there
is no assertion that I can find anywhere stating which implementation
is correct.

On Wed, Sep 4, 2024 at 5:13 PM Dean Marx <dmarx@iol.unh.edu> wrote:
>>
>> <snip>
>> +        if should_receive:
>> +            self.verify(len(received_packets) == 1, "Expected packet not received")
>> +        else:
>> +            self.verify(len(received_packets) == 0, "Expected packet received")
>
>
> Side note, didn't notice until I tested it but "Expected packet received" doesn't really make sense as an error message

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

end of thread, other threads:[~2024-09-05 19:11 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-02 19:24 [PATCH v2 0/3] Mac Filter Port to New DTS Nicholas Pratte
2024-07-02 19:24 ` [PATCH v2 1/3] dts: add boolean to adjust addresses Nicholas Pratte
2024-07-11 19:31   ` Jeremy Spewock
2024-07-17 17:19     ` Nicholas Pratte
2024-07-19 15:37     ` Jeremy Spewock
2024-07-22 13:09   ` Dean Marx
2024-07-02 19:24 ` [PATCH v2 2/3] dts: add testpmd methods for test suite Nicholas Pratte
2024-07-11 19:33   ` Jeremy Spewock
2024-07-17 19:57     ` Nicholas Pratte
2024-07-02 19:24 ` [PATCH v2 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
2024-07-11 19:34   ` Jeremy Spewock
2024-07-18 19:05 ` [PATCH v3 0/3] Mac Filter Port to New DTS Nicholas Pratte
2024-07-18 19:05   ` [PATCH v3 1/3] dts: add boolean to adjust addresses Nicholas Pratte
2024-07-18 19:05   ` [PATCH v3 2/3] dts: add methods for setting mac and multicast addresses Nicholas Pratte
2024-07-19 15:37     ` Jeremy Spewock
2024-07-22 13:08     ` Dean Marx
2024-07-18 19:40   ` [PATCH v3 3/3] dts: mac filter test suite refactored for new dts Nicholas Pratte
2024-07-19 15:38     ` Jeremy Spewock
2024-07-22 13:08     ` Dean Marx
2024-07-26 16:39 ` [PATCH v4 0/2] Mac Filter Port to New DTS Nicholas Pratte
2024-07-26 16:45   ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
2024-08-02 20:26     ` Jeremy Spewock
2024-08-12 18:58     ` Dean Marx
2024-07-26 16:46   ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
2024-08-02 20:25     ` Jeremy Spewock
2024-08-02 20:27       ` Jeremy Spewock
2024-08-12 18:47     ` Dean Marx
2024-09-04 21:14     ` Dean Marx
2024-09-05 19:11       ` Nicholas Pratte
2024-07-26 16:39 ` [PATCH v4 1/2] dts: add methods for setting mac and multicast addresses Nicholas Pratte
2024-08-02 20:02   ` Jeremy Spewock
2024-07-26 16:39 ` [PATCH v4 2/2] dts: mac filter test suite refactored for new dts Nicholas Pratte
2024-08-02 20:02   ` 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).