[RFC v1 0/2] dts: initial checksum offload suite

[RFC v1 0/2] dts: initial checksum offload suite

[Dean Marx]

Test suite for verifying checksum hardware offload through the
PMD works as expected. This is done by checking the verbose output in
testpmd while in csum forwarding mode, specifically the ol_flags
section, to ensure they match the flags in the test plan. However, 
there are a few issues I noticed while writing the suite that made
me hesitant to submit a patch:

1. SCTP hardware offload is not supported on any of the NICs I tested
on. I've tried this on mlx5, i40e, and bnxt drivers and none of them
support it. SCTP offload is used as part of almost every test case, so I
removed SCTP packets from the suite entirely. I intend to keep it that
way unless anyone is able to use the command "csum set sctp hw 0"
without an "SCTP not supported" error.
2. There are two Tx checksum test cases, which involve checking the Tx
flags section of verbose output to ensure they match the ones in the
test plan. However, the Tx flags don't appear to change at all
depending on what packet you send to testpmd, which leaves me with no
way to verify correct behavior. I'm considering removing the Tx cases
entirely, but they are a large chunk of the suite so if anyone disagrees
I can look for more of a workaround.

If anyone has any comments or advice about the issues above it is
greatly appreciated.

Dean Marx (2):
  dts: add csum HW offload to testpmd shell
  dts: checksum offload test suite

 dts/framework/config/conf_yaml_schema.json    |   3 +-
 dts/framework/remote_session/testpmd_shell.py |  94 ++++++
 dts/tests/TestSuite_checksum_offload.py       | 288 ++++++++++++++++++
 3 files changed, 384 insertions(+), 1 deletion(-)
 create mode 100644 dts/tests/TestSuite_checksum_offload.py

-- 
2.44.0

[RFC v1 1/2] dts: add csum HW offload to testpmd shell

[Dean Marx]

add csum_set_hw method to testpmd shell class. Port over
set_verbose and port start/stop from queue start/stop suite.

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

diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 43e9f56517..be8fbdc295 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -806,6 +806,100 @@ def show_port_stats(self, port_id: int) -> TestPmdPortStats:
 
         return TestPmdPortStats.parse(output)
 
+    def port_stop(self, port: int, verify: bool = True):
+        """Stop specified port.
+
+        Args:
+            port: Specifies the port number to use, must be between 0-32.
+            verify: If :data:`True`, the output of the command is scanned
+                to ensure specified port is stopped. If not, it is considered
+                an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not stopped."""
+        port_output = self.send_command(f"port stop {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to stop port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to stop port {port}.")
+
+    def port_start(self, port: int, verify: bool = True):
+        """Start specified port.
+
+        Args:
+            port: Specifies the port number to use, must be between 0-32.
+            verify: If :data:`True`, the output of the command is scanned
+                to ensure specified port is started. If not, it is considered
+                an error.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and the port
+                is not started."""
+        port_output = self.send_command(f"port start {port}")
+        if verify:
+            if "Done" not in port_output:
+                self._logger.debug(f"Failed to start port {port}: \n{port_output}")
+                raise InteractiveCommandExecutionError(f"Testpmd failed to start port {port}.")
+
+    def csum_set_hw(self, layer: str, port_id: int, verify: bool = True) -> None:
+        """Enables hardware checksum offloading on the specified layer.
+
+        Args:
+            layer: The layer that checksum offloading should be enabled on.
+                options: tcp, ip, udp, sctp, outer-ip, outer-udp.
+            port_id: The port number to enable checksum offloading on, should be within 0-32.
+            verify: If :data:`True` the output of the command will be scanned in an attempt to
+                verify that checksum offloading was enabled on the port.
+
+        Raises:
+            InteractiveCommandExecutionError: If checksum offload is not enabled successfully.
+        """
+        csum_output = self.send_command(f"csum set {layer} hw {port_id}")
+        if (verify and ("Bad arguments" in csum_output or f"Please stop port {port_id} first")):
+            self._logger.debug(f"Failed to set csum hw mode on port {port_id}:\n{csum_output}")
+            raise InteractiveCommandExecutionError(
+                f"Failed to set csum hw mode on port {port_id}"
+                )
+        if verify and f"checksum offload is not supported by port {port_id}" in csum_output:
+            self._logger.debug(f"Checksum {layer} offload is not supported by port {port_id}:\n{csum_output}")
+
+        success = False
+        if "outer-ip" in layer:
+            if "Outer-Ip checksum offload is hw" in csum_output:
+                success = True
+        if "outer-udp" in layer:
+            if "Outer-Udp checksum offload is hw" in csum_output:
+                success = True
+        else:
+            if f"{layer.upper} checksum offload is hw" in csum_output:
+                success = True
+        if not success and verify:
+            self._logger.debug(f"Failed to set csum hw mode on port {port_id}:\n{csum_output}")
+
+    def set_verbose(self, level: int, verify: bool = True) -> None:
+        """Set debug verbosity level.
+
+        Args:
+            level: 0 - silent except for error
+                1 - fully verbose except for Tx packets
+                2 - fully verbose except for Rx packets
+                >2 - fully verbose
+            verify: if :data:`True` an additional command will be sent to verify that verbose level
+                is properly set. Defaults to :data:`True`.
+
+        Raises:
+            InteractiveCommandExecutionError: If `verify` is :data:`True` and verbose level
+            is not correctly set.
+        """
+        verbose_output = self.send_command(f"set verbose {level}")
+        if verify:
+            if "Change verbose level" not in verbose_output:
+                self._logger.debug(f"Failed to set verbose level to {level}: \n{verbose_output}")
+                raise InteractiveCommandExecutionError(
+                    f"Testpmd failed to set verbose level to {level}."
+                )
+
     def _close(self) -> None:
         """Overrides :meth:`~.interactive_shell.close`."""
         self.stop()
-- 
2.44.0

[RFC v1 2/2] dts: checksum offload test suite

[Dean Marx]

test suite for verifying layer 3/4 checksum offload
features on poll mode driver.

Depends-on: patch-142762
("dts: add text parser for testpmd verbose output")
Depends-on: patch-142691
("dts: add send_packets to test suites and rework packet addressing")

Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
---
 dts/framework/config/conf_yaml_schema.json    |   3 +-
 dts/framework/remote_session/testpmd_shell.py |   2 +-
 dts/tests/TestSuite_checksum_offload.py       | 288 ++++++++++++++++++
 3 files changed, 291 insertions(+), 2 deletions(-)
 create mode 100644 dts/tests/TestSuite_checksum_offload.py

diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..a83a6786df 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",
+        "checksum_offload"
       ]
     },
     "test_target": {
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index be8fbdc295..01f378acc3 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -856,7 +856,7 @@ def csum_set_hw(self, layer: str, port_id: int, verify: bool = True) -> None:
             InteractiveCommandExecutionError: If checksum offload is not enabled successfully.
         """
         csum_output = self.send_command(f"csum set {layer} hw {port_id}")
-        if (verify and ("Bad arguments" in csum_output or f"Please stop port {port_id} first")):
+        if (verify and ("Bad arguments" in csum_output or f"Please stop port {port_id} first" in csum_output)):
             self._logger.debug(f"Failed to set csum hw mode on port {port_id}:\n{csum_output}")
             raise InteractiveCommandExecutionError(
                 f"Failed to set csum hw mode on port {port_id}"
diff --git a/dts/tests/TestSuite_checksum_offload.py b/dts/tests/TestSuite_checksum_offload.py
new file mode 100644
index 0000000000..7ce5897c34
--- /dev/null
+++ b/dts/tests/TestSuite_checksum_offload.py
@@ -0,0 +1,288 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 University of New Hampshire
+
+"""DPDK checksum offload testing suite.
+
+This suite verifies L3/L4 checksum offload features of the Poll Mode Driver.
+On the Rx side, IPv4 and UDP/TCP checksum by hardware is checked to ensure
+checksum flags match expected flags. On the Tx side, IPv4/UDP, IPv4/TCP,
+IPv6/UDP, and IPv6/TCP insertion by hardware is checked to checksum flags
+match expected flags.
+
+"""
+
+from typing import List
+
+from scapy.all import Packet  # type: ignore[import-untyped]
+from scapy.layers.inet import GRE, IP, TCP, UDP  # type: ignore[import-untyped]
+from scapy.layers.inet6 import IPv6  # type: ignore[import-untyped]
+from scapy.layers.l2 import Dot1Q  # type: ignore[import-untyped]
+from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
+from scapy.layers.vxlan import VXLAN  # type: ignore[import-untyped]
+from scapy.packet import Raw  # type: ignore[import-untyped]
+
+from framework.remote_session.testpmd_shell import (
+    SimpleForwardingModes,
+    TestPmdShell,
+    VerboseOLFlag,
+)
+from framework.test_suite import TestSuite
+
+
+class TestChecksumOffload(TestSuite):
+    """Checksum offload test suite.
+
+    This suite consists of 8 test cases:
+    1. Insert checksum on transmit packet
+    2. Do not insert checksum on transmit packet
+    3. Validate Rx checksum valid flags
+    4. Hardware checksum check L4 Rx
+    5. Hardware checksum check L3 Rx
+    6. Hardware checksum check L4 Tx
+    7. Hardware checksum check L3 Tx
+    8. Checksum offload with vlan
+
+    """
+
+    def set_up_suite(self) -> None:
+        """Set up the test suite.
+
+        Setup:
+            Verify that at least two port links are created when the
+            test run is initialized.
+        """
+        self.verify(len(self._port_links) > 1, "Not enough port links.")
+
+    def send_packet_and_verify(
+        self, packet_list: List[Packet], load: str, should_receive: bool
+    ) -> None:
+        """Send and verify packet is received on the traffic generator.
+
+        Args:
+            packet: Scapy packet to send and verify.
+            load: Raw layer load attribute in the sent packet.
+            should_receive: Indicates whether the packet should be received by the traffic generator.
+        """
+        for i in range(0, len(packet_list)):
+            received_packets = self.send_packet_and_capture(packet=packet_list[i])
+            received = any(
+                packet.haslayer(Raw) and load in str(packet.load) for packet in received_packets
+            )
+            self.verify(
+                received == should_receive,
+                f"Packet was {'dropped' if should_receive else 'received'}",
+            )
+
+    def send_packet_and_verify_checksum(
+        self, packet: Packet, goodL4: bool, goodIP: bool, testpmd: TestPmdShell
+    ) -> None:
+        """Send packet and verify verbose output matches expected output.
+
+        Args:
+            packet: Scapy packet to send to DUT.
+            goodL4: Verifies RTE_MBUF_F_RX_L4_CKSUM_GOOD in verbose output
+                if :data:`True`, or RTE_MBUF_F_RX_L4_CKSUM_UNKOWN if :data:`False`.
+            goodIP: Verifies RTE_MBUF_F_RX_IP_CKSUM_GOOD in verbose output
+                if :data:`True`, or RTE_MBUF_F_RX_IP_CKSUM_UNKOWN if :data:`False`.
+            testpmd: Testpmd shell session to analyze verbose output of.
+        """
+        testpmd.start()
+        self.send_packet_and_capture(packet=packet)
+        verbose_output = testpmd.extract_verbose_output(testpmd.stop())
+        isL4 = any(
+            VerboseOLFlag.RTE_MBUF_F_RX_L4_CKSUM_GOOD in packet.ol_flags
+            for index in verbose_output
+            for packet in index.packets
+        )
+        isIP = any(
+            VerboseOLFlag.RTE_MBUF_F_RX_IP_CKSUM_GOOD in packet.ol_flags
+            for index in verbose_output
+            for packet in index.packets
+        )
+        self.verify(isL4 == goodL4, "Layer 4 checksum flag did not match expected checksum flag.")
+        self.verify(isIP == goodIP, "IP checksum flag did not match expected checksum flag.")
+
+    def setup_hw_offload(self, testpmd: TestPmdShell) -> None:
+        """Sets IP, UDP, TCP, and SCTP layers to hardware offload."""
+        testpmd.port_stop(port=0)
+        testpmd.csum_set_hw(layer="ip", port_id=0)
+        testpmd.csum_set_hw(layer="udp", port_id=0)
+        testpmd.csum_set_hw(layer="tcp", port_id=0)
+        testpmd.port_start(port=0)
+
+    def test_insert_checksums(self) -> None:
+        """Enable checksum offload insertion and verify packet reception."""
+        packet_list = [
+            Ether() / IP() / UDP() / Raw("xxxxx"),
+            Ether() / IP() / TCP() / Raw("xxxxx"),
+            Ether() / IPv6(src="::1") / UDP() / Raw("xxxxx"),
+            Ether() / IPv6(src="::1") / TCP() / Raw("xxxxx"),
+        ]
+        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
+            testpmd.set_forward_mode(SimpleForwardingModes.csum)
+            testpmd.set_verbose(level=1)
+            self.setup_hw_offload(testpmd=testpmd)
+            testpmd.start()
+            self.send_packet_and_verify(packet_list=packet_list, load="xxxxx", should_receive=True)
+            for i in range(0, len(packet_list)):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=True, goodIP=True, testpmd=testpmd
+                )
+
+    def test_no_insert_checksums(self) -> None:
+        """Enable checksum offload insertion and verify packet reception."""
+        packet_list = [
+            Ether() / IP() / UDP() / Raw("xxxxx"),
+            Ether() / IP() / TCP() / Raw("xxxxx"),
+            Ether() / IPv6(src="::1") / UDP() / Raw("xxxxx"),
+            Ether() / IPv6(src="::1") / TCP() / Raw("xxxxx"),
+        ]
+        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
+            testpmd.set_forward_mode(SimpleForwardingModes.csum)
+            testpmd.set_verbose(level=1)
+            testpmd.start()
+            self.send_packet_and_verify(packet_list=packet_list, load="xxxxx", should_receive=True)
+            for i in range(0, len(packet_list)):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=True, goodIP=True, testpmd=testpmd
+                )
+
+    def test_validate_rx_checksum(self) -> None:
+        """Verify verbose output of Rx packets matches expected behavior."""
+        packet_list = [
+            Ether() / IP() / UDP(),
+            Ether() / IP() / TCP(),
+            Ether() / IPv6(src="::1") / UDP(),
+            Ether() / IPv6(src="::1") / TCP(),
+            Ether() / IP(chksum=0x0) / UDP(chksum=0xF),
+            Ether() / IP(chksum=0x0) / TCP(chksum=0xF),
+            Ether() / IPv6(src="::1") / UDP(chksum=0xF),
+            Ether() / IPv6(src="::1") / TCP(chksum=0xF),
+        ]
+        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
+            testpmd.set_forward_mode(SimpleForwardingModes.csum)
+            testpmd.set_verbose(level=1)
+            self.setup_hw_offload(testpmd=testpmd)
+            for i in range(0, 4):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=True, goodIP=True, testpmd=testpmd
+                )
+            for i in range(4, 6):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=False, goodIP=False, testpmd=testpmd
+                )
+            for i in range(6, 8):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=False, goodIP=True, testpmd=testpmd
+                )
+
+    def test_l4_rx_checksum(self) -> None:
+        """Tests L4 Rx checksum in a variety of scenarios."""
+        packet_list = [
+            Ether() / IP() / UDP(),
+            Ether() / IP() / TCP(),
+            Ether() / IP() / UDP(chksum=0xF),
+            Ether() / IP() / TCP(chksum=0xF),
+        ]
+        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
+            testpmd.set_forward_mode(SimpleForwardingModes.csum)
+            testpmd.set_verbose(level=1)
+            self.setup_hw_offload(testpmd=testpmd)
+            for i in range(0, 2):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=True, goodIP=True, testpmd=testpmd
+                )
+            for i in range(2, 4):
+                # Originally had False for goodIP in the test plan, but that doesn't logically make sense
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=False, goodIP=True, testpmd=testpmd
+                )
+
+    def test_l3_rx_checksum(self) -> None:
+        """Tests L3 Rx checksum hardware offload."""
+        packet_list = [
+            Ether() / IP() / UDP(),
+            Ether() / IP() / TCP(),
+            Ether() / IP(chksum=0xF) / UDP(),
+            Ether() / IP(chksum=0xF) / TCP(),
+        ]
+        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
+            testpmd.set_forward_mode(SimpleForwardingModes.csum)
+            testpmd.set_verbose(level=1)
+            self.setup_hw_offload(testpmd=testpmd)
+            for i in range(0, 2):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=True, goodIP=True, testpmd=testpmd
+                )
+            for i in range(2, 4):
+                # Originally had False for goodL4 in the test plan, but that doesn't logically make sense
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=True, goodIP=False, testpmd=testpmd
+                )
+
+    def test_l4_tx_checksum(self) -> None:
+        """Tests L4 Tx checksum in a variety of scenarios."""
+        packet_list = [
+            Ether() / IP() / UDP(chksum=0xB161),
+            Ether() / IP() / UDP(chksum=0xF),
+            Ether() / IP() / TCP(chksum=0x4904),
+            Ether() / IP() / TCP(chksum=0xF),
+            Ether() / IPv6() / UDP(chksum=0xAF62),
+            Ether() / IPv6() / UDP(chksum=0xF),
+            Ether() / IPv6() / TCP(chksum=0x4705),
+            Ether() / IPv6() / TCP(chksum=0xF),
+            Ether() / IP() / UDP() / VXLAN() / Ether() / IP() / UDP(chksum=0x9949),
+            Ether() / IP() / UDP() / VXLAN() / Ether() / IP() / UDP(chksum=0xF),
+            Ether() / IP() / UDP(chksum=0xF) / VXLAN() / Ether() / IP() / UDP(),
+            Ether() / IP() / UDP(chksum=0xF) / VXLAN() / Ether() / IP() / UDP(chksum=0xF),
+            Ether() / IP() / UDP() / VXLAN() / Ether() / IP() / TCP(chksum=0x30EC),
+            Ether() / IP() / UDP() / VXLAN() / Ether() / IP() / TCP(chksum=0xF),
+            Ether() / IP() / UDP(chksum=0xF) / VXLAN() / Ether() / IP() / TCP(),
+            Ether() / IP() / UDP(chksum=0xF) / VXLAN() / Ether() / IP() / TCP(chksum=0xF),
+            Ether() / IPv6() / UDP() / VXLAN() / Ether() / IPv6() / UDP(chksum=0x9949),
+            Ether() / IPv6() / UDP() / VXLAN() / Ether() / IPv6() / UDP(chksum=0xF),
+            Ether() / IPv6() / UDP(chksum=0xF) / VXLAN() / Ether() / IPv6() / UDP(),
+            Ether() / IPv6() / UDP(chksum=0xF) / VXLAN() / Ether() / IPv6() / UDP(chksum=0xF),
+            Ether() / IPv6() / UDP() / VXLAN() / Ether() / IPv6() / TCP(chksum=0x30EC),
+            Ether() / IPv6() / UDP() / VXLAN() / Ether() / IPv6() / TCP(chksum=0xF),
+            Ether() / IPv6() / UDP(chksum=0xF) / VXLAN() / Ether() / IPv6() / TCP(),
+            Ether() / IPv6() / UDP(chksum=0xF) / VXLAN() / Ether() / IPv6() / TCP(chksum=0xF),
+            Ether() / IP() / GRE() / IP() / UDP(chksum=0x8131),
+            Ether() / IP() / GRE() / IP() / UDP(chksum=0xF),
+            Ether() / IP() / GRE() / IP() / TCP(chksum=0x18D4),
+            Ether() / IP() / GRE() / IP() / TCP(chksum=0xF),
+            Ether() / IPv6() / GRE() / IPv6() / UDP(chksum=0x8131),
+            Ether() / IPv6() / GRE() / IPv6() / UDP(chksum=0xF),
+            Ether() / IPv6() / GRE() / IPv6() / TCP(chksum=0x18D4),
+            Ether() / IPv6() / GRE() / IPv6() / TCP(chksum=0xF),
+        ]
+        """This test case is based on verifying Tx checksum flags in the testpmd verbose
+        output, but these flags seem to vary depending on the type of NIC being used."""
+
+    def test_l3_tx_checksum(self) -> None:
+        """Tests L3 Tx checksum hardware offload."""
+        packet_list = [Ether() / IP(chksum=0x7CCC) / TCP(), Ether() / IP(chksum=0xF) / TCP()]
+        """Another test based on verifying Tx checksum flags in verbose output."""
+
+    def test_vlan_checksum(self) -> None:
+        """Tests VLAN Rx checksum hardware offload and verify packet reception."""
+        packet_list = [
+            Ether() / Dot1Q(vlan=1) / IP(chksum=0x0) / UDP(chksum=0xF) / Raw("xxxxx"),
+            Ether() / Dot1Q(vlan=1) / IP(chksum=0x0) / TCP(chksum=0xF) / Raw("xxxxx"),
+            Ether() / Dot1Q(vlan=1) / IPv6(src="::1") / UDP(chksum=0xF) / Raw("xxxxx"),
+            Ether() / Dot1Q(vlan=1) / IPv6(src="::1") / TCP(chksum=0xF) / Raw("xxxxx"),
+        ]
+        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
+            testpmd.set_forward_mode(SimpleForwardingModes.csum)
+            testpmd.set_verbose(level=1)
+            self.setup_hw_offload(testpmd=testpmd)
+            testpmd.start()
+            self.send_packet_and_verify(packet_list=packet_list, load="xxxxx", should_receive=True)
+            for i in range(0, 2):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=False, goodIP=False, testpmd=testpmd
+                )
+            for i in range(2, 4):
+                self.send_packet_and_verify_checksum(
+                    packet=packet_list[i], goodL4=False, goodIP=True, testpmd=testpmd
+                )
-- 
2.44.0

Re: [RFC v1 0/2] dts: initial checksum offload suite

[Jeremy Spewock]

Hey Dean,

Thanks for the patch, I left some thoughts on the point you raise
below and some of the larger things I noticed throughout the series.
The logic seems to all be right though and I can understand what you
were going for, so the broader ideas seem good to me.

On Mon, Aug 12, 2024 at 9:41 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> Test suite for verifying checksum hardware offload through the
> PMD works as expected. This is done by checking the verbose output in
> testpmd while in csum forwarding mode, specifically the ol_flags
> section, to ensure they match the flags in the test plan. However,
> there are a few issues I noticed while writing the suite that made
> me hesitant to submit a patch:
>
> 1. SCTP hardware offload is not supported on any of the NICs I tested
> on. I've tried this on mlx5, i40e, and bnxt drivers and none of them
> support it. SCTP offload is used as part of almost every test case, so I
> removed SCTP packets from the suite entirely. I intend to keep it that
> way unless anyone is able to use the command "csum set sctp hw 0"
> without an "SCTP not supported" error.
> 2. There are two Tx checksum test cases, which involve checking the Tx
> flags section of verbose output to ensure they match the ones in the
> test plan. However, the Tx flags don't appear to change at all
> depending on what packet you send to testpmd, which leaves me with no
> way to verify correct behavior. I'm considering removing the Tx cases
> entirely, but they are a large chunk of the suite so if anyone disagrees
> I can look for more of a workaround.
>

As for the Tx issue, I have also noticed that these flags are
inconsistent. It looks like the Tx side of things have flags
representing if there is checksum data for the various layers, but
even then it doesn't show up in the verbose output when there is
verbose information. I think that if it isn't consistent then we can't
write a test suite that is generic enough to be run on the hardware
and it probably makes more sense to just leave it out of the suite.
I'm honestly not sure if it is a bug or just a misunderstanding with
what these flags should be doing since the Tx flags are much less
clear to me than the Rx. Regardless, it's not like it's something we
can search for with a capability or anything so we don't have a great
mechanism for getting this to work if it truly is just something where
different drivers do it differently.


> If anyone has any comments or advice about the issues above it is
> greatly appreciated.
>
> Dean Marx (2):
>   dts: add csum HW offload to testpmd shell
>   dts: checksum offload test suite
>
>  dts/framework/config/conf_yaml_schema.json    |   3 +-
>  dts/framework/remote_session/testpmd_shell.py |  94 ++++++
>  dts/tests/TestSuite_checksum_offload.py       | 288 ++++++++++++++++++
>  3 files changed, 384 insertions(+), 1 deletion(-)
>  create mode 100644 dts/tests/TestSuite_checksum_offload.py
>
> --
> 2.44.0
>

Re: [RFC v1 1/2] dts: add csum HW offload to testpmd shell

[Jeremy Spewock]

On Mon, Aug 12, 2024 at 9:41 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> add csum_set_hw method to testpmd shell class. Port over
> set_verbose and port start/stop from queue start/stop suite.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
<snip>
> +    def csum_set_hw(self, layer: str, port_id: int, verify: bool = True) -> None:
> +        """Enables hardware checksum offloading on the specified layer.
> +
> +        Args:
> +            layer: The layer that checksum offloading should be enabled on.
> +                options: tcp, ip, udp, sctp, outer-ip, outer-udp.

If there are specific options like this, it might be worth making this
parameter into a flag where users can specify one or more of the
options and testpmd can handle setting them all. That way users don't
have to even read the doc-string for the options, it will be apparent
in the class they have to use.

> +            port_id: The port number to enable checksum offloading on, should be within 0-32.
> +            verify: If :data:`True` the output of the command will be scanned in an attempt to
> +                verify that checksum offloading was enabled on the port.
> +
> +        Raises:
> +            InteractiveCommandExecutionError: If checksum offload is not enabled successfully.
> +        """
> +        csum_output = self.send_command(f"csum set {layer} hw {port_id}")
> +        if (verify and ("Bad arguments" in csum_output or f"Please stop port {port_id} first")):
> +            self._logger.debug(f"Failed to set csum hw mode on port {port_id}:\n{csum_output}")
> +            raise InteractiveCommandExecutionError(
> +                f"Failed to set csum hw mode on port {port_id}"
> +                )
> +        if verify and f"checksum offload is not supported by port {port_id}" in csum_output:
> +            self._logger.debug(f"Checksum {layer} offload is not supported by port {port_id}:\n{csum_output}")
> +
> +        success = False
> +        if "outer-ip" in layer:
> +            if "Outer-Ip checksum offload is hw" in csum_output:
> +                success = True
> +        if "outer-udp" in layer:
> +            if "Outer-Udp checksum offload is hw" in csum_output:
> +                success = True
> +        else:
> +            if f"{layer.upper} checksum offload is hw" in csum_output:
> +                success = True
> +        if not success and verify:
> +            self._logger.debug(f"Failed to set csum hw mode on port {port_id}:\n{csum_output}")
> +
<snip>
> --
> 2.44.0
>

Re: [RFC v1 2/2] dts: checksum offload test suite

[Jeremy Spewock]

On Mon, Aug 12, 2024 at 9:41 AM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> test suite for verifying layer 3/4 checksum offload
> features on poll mode driver.
>
> Depends-on: patch-142762
> ("dts: add text parser for testpmd verbose output")
> Depends-on: patch-142691
> ("dts: add send_packets to test suites and rework packet addressing")
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
>  dts/framework/config/conf_yaml_schema.json    |   3 +-
>  dts/framework/remote_session/testpmd_shell.py |   2 +-
>  dts/tests/TestSuite_checksum_offload.py       | 288 ++++++++++++++++++
>  3 files changed, 291 insertions(+), 2 deletions(-)
>  create mode 100644 dts/tests/TestSuite_checksum_offload.py
>
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index f02a310bb5..a83a6786df 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",
> +        "checksum_offload"
>        ]
>      },
>      "test_target": {
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index be8fbdc295..01f378acc3 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -856,7 +856,7 @@ def csum_set_hw(self, layer: str, port_id: int, verify: bool = True) -> None:
>              InteractiveCommandExecutionError: If checksum offload is not enabled successfully.
>          """
>          csum_output = self.send_command(f"csum set {layer} hw {port_id}")
> -        if (verify and ("Bad arguments" in csum_output or f"Please stop port {port_id} first")):
> +        if (verify and ("Bad arguments" in csum_output or f"Please stop port {port_id} first" in csum_output)):
>              self._logger.debug(f"Failed to set csum hw mode on port {port_id}:\n{csum_output}")
>              raise InteractiveCommandExecutionError(
>                  f"Failed to set csum hw mode on port {port_id}"

It looks like some testpmd changes snuck into the git history for your
test suite, these probably belong in the previous commit.

<snip>
> +
> +    def test_no_insert_checksums(self) -> None:
> +        """Enable checksum offload insertion and verify packet reception."""
> +        packet_list = [
> +            Ether() / IP() / UDP() / Raw("xxxxx"),
> +            Ether() / IP() / TCP() / Raw("xxxxx"),

In cases like this where the payload is used so many times, it
probably makes more sense to pull it out into its own variable.

> +            Ether() / IPv6(src="::1") / UDP() / Raw("xxxxx"),
> +            Ether() / IPv6(src="::1") / TCP() / Raw("xxxxx"),
> +        ]
> +        with TestPmdShell(node=self.sut_node, enable_rx_cksum=True) as testpmd:
> +            testpmd.set_forward_mode(SimpleForwardingModes.csum)
> +            testpmd.set_verbose(level=1)
> +            testpmd.start()
> +            self.send_packet_and_verify(packet_list=packet_list, load="xxxxx", should_receive=True)
> +            for i in range(0, len(packet_list)):
> +                self.send_packet_and_verify_checksum(
> +                    packet=packet_list[i], goodL4=True, goodIP=True, testpmd=testpmd
> +                )
> +
<snip>
> 2.44.0
>