Looking at this patch, I can see it adds a comprehensive test framework for DPDK rte_flow offload functionality. Let me analyze the commit and identify any issues.
commit 00626b5427f4639dadcf6c902fbe32eec89a3640 Author: Eelco Chaudron <[email protected]> tests: Add a simple DPDK rte_flow test framework. This patch adds userspace DPDK with rte_flow offload unit tests that can be run with 'make check-dpdk-offloads'. It requires six VFs for offloaded ports and includes documentation on setup requirements. > diff --git a/Documentation/topics/testing.rst > b/Documentation/topics/testing.rst > index dbeaf2d93..c5a73aac3 100644 > --- a/Documentation/topics/testing.rst > +++ b/Documentation/topics/testing.rst > @@ -363,6 +363,25 @@ manual to figure out how to `Configure hugepages`_. > All the features documented under `Unit Tests`_ are available for the DPDK > testsuite. > > +Userspace datapath with DPDK offload > +'''''''''''''''''''''''''''''''''''' > + > +To invoke the userspace datapath tests with DPDK and it's rte_flow offload, Does this code have a grammatical error with "it's" instead of "its"? > +the same prerequisites apply as above. In addition, six Virtual Function (VF) > +interfaces must be preconfigured and capable of hardware offloading traffic > +between each other. > + > +These six VFs need to be passed as a list of PF PCI addresses with their > +corresponding VF indexes in the OVS_DPDK_VF_PCI_ADDRS variable. > +For example:: > + > + OVS_DPDK_VF_PCI_ADDRS="0000:17:00.0,0 0000:17:00.0,1 0000:17:00.0,2 > 0000:17:00.0,3 0000:17:00.0,4 0000:17:00.0,5" > + > +To invoke the dpdk offloads testsuite with the userspace datapath, run:: > + > + make check-dpdk-offloads \ > + OVS_DPDK_VF_PCI_ADDRS="0000:17:00.0,0 0000:17:00.0,1 0000:17:00.0,2 > 0000:17:00.0,3 0000:17:00.0,4 0000:17:00.0,5" > + > Userspace datapath: Testing and Validation of CPU-specific Optimizations > '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' > > diff --git a/tests/system-dpdk-offloads-macros.at > b/tests/system-dpdk-offloads-macros.at > new file mode 100644 > index 000000000..fa96af5f6 > --- /dev/null > +++ b/tests/system-dpdk-offloads-macros.at > @@ -0,0 +1,236 @@ > +AT_COPYRIGHT([Copyright (c) 2025 Red Hat, Inc. > + > +Licensed under the Apache License, Version 2.0 (the "License"); > +you may not use this file except in compliance with the License. > +You may obtain a copy of the License at: > + > + http://www.apache.org/licenses/LICENSE-2.0 > + > +Unless required by applicable law or agreed to in writing, software > +distributed under the License is distributed on an "AS IS" BASIS, > +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +See the License for the specific language governing permissions and > +limitations under the License.]) [ ... ] > +# ovs_dpdk_is_valid_pci_vf_addr() > +# > +# Check if the give PF PCI address and the VF number are valid. Does this comment have a typo with "give" instead of "given"? > +# > +ovs_dpdk_is_valid_pci_vf_addr() { > + PCI_ID='[[0-9a-fA-F]]{4}:[[0-9a-fA-F]]{2}:[[0-9a-fA-F]]{2}\.[[0-7]]' > + echo "$1" | grep -E -q "^$PCI_ID,[[0-9]]+$" && return 0 || return 1 > +} [ ... ] > +# ovr_dpdk_get_representor_netdev(<PF_PCI>, <VF_INDEX>) Does this function name have a typo with "ovr_dpdk" instead of "ovs_dpdk"? > +# > +# This function tries to find the representor netdev for the give PF's VF. Does this comment have the same typo with "give" instead of "given"? > +# > +ovs_dpdk_get_representor_netdev() { > + PF_PCI=$1 > + VF_IDX=$2 > + > + VF_NET_DIR="/sys/bus/pci/devices/$PF_PCI/virtfn$VF_IDX/net" > + > + if [[ ! -d "$VF_NET_DIR" ]]; then > + echo "ERROR: VF $VF_IDX for PF $PF_PCI does not exist" >&2 > + return 1 > + fi > + > + for iface in "$VF_NET_DIR"/*; do > + if [[ -e "$iface" ]]; then > + basename "$iface" > + return 0 > + fi > + done > + > + echo "ERROR: No representor netdev found for VF $VF_IDX on PF $PF_PCI" > >&2 > + return 1 > +} [ ... ] > +OVS_END_SHELL_HELPERS \ No newline at end of file Does this file lack a trailing newline? > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > index 8894dd711..dd4f485f6 100644 > --- a/tests/ofproto-macros.at > +++ b/tests/ofproto-macros.at > @@ -9,9 +9,9 @@ s/ duration=[0-9.]*s,// > s/ cookie=0x0,// > s/ table=0,// > s/ n_packets=0,// > -s/ n_offload_packets=0,// > +s/ n_offload_packets=[0-9]*,// > s/ n_bytes=0,// > -s/ n_offload_bytes=0,// > +s/ n_offload_bytes=[0-9]*,// > s/ idle_age=[0-9]*,// > s/ hard_age=[0-9]*,// > s/dp_hash=0x[0-9a-f]*\//dp_hash=0x0\// [ ... ] > +# Changes all 'used:...' to say 'used:0.0', to make output easier to compare. > strip_used () { > - sed 's/used:[[0-9]]\.[[0-9]]*/used:0.0/' > + sed -E 's/used:[[0-9]]+\.[[0-9]]*/used:0.0/' > } [ ... ] > diff --git a/tests/sendpkt.py b/tests/sendpkt.py > index 7cbea5165..b79db1e3f 100755 > --- a/tests/sendpkt.py > +++ b/tests/sendpkt.py > @@ -36,6 +36,9 @@ parser = OptionParser(usage=usage) > parser.add_option("-t", "--type", type="string", dest="packet_type", > help="packet type ('eth' is the default PACKET_TYPE)", > default="eth") > +parser.add_option("-c", "--count", type="int", dest="packet_count", > + help="number of packets to send (default: 1)", > + default=1) [ ... ] > +if options.packet_count < 1: > + parser.error('invalid argument to "-c"/"--count". ' > + 'Allowed value must be 1 or higher.') [ ... ] > + for i in range(options.packet_count): > + sockfd.send(pkt) [ ... ] > diff --git a/tests/system-dpdk-offloads.at b/tests/system-dpdk-offloads.at > new file mode 100644 > index 000000000..ddfdd3c7e > --- /dev/null > +++ b/tests/system-dpdk-offloads.at > @@ -0,0 +1,223 @@ [ ... ] > +# Sent a packet in one direction; this should create a non-offloadable > +# flow between br0 and ovs-p1. Does this comment have a grammatical error with "Sent" instead of "Send"? > +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 \ > + $(ovs-ofctl compose-packet --bare 'ICMP_PKT' '')], [0], [ignore]) [ ... ] > +# Sent the reply packet. A new flow should be inserted in hardware for the Does this comment also have "Sent" instead of "Send"? > +# reverse direction (since the FDB entry for the destination is known). > +# However, the revalidator should update the originating flow, causing it to > +# be fully offloaded. > +NS_CHECK_EXEC([at_ns1], [$PYTHON3 $srcdir/sendpkt.py p1 \ > + $(ovs-ofctl compose-packet --bare 'ICMP_PKT_REPLY' '')], [0], [ignore]) [ ... ] > +# Sent 50 packets in each direction, and verify the packets are handled by Does this comment also have "Sent" instead of "Send"? > +# the hardware rather than by the PMDs. > +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py -c 50 p0 \ > + $(ovs-ofctl compose-packet --bare 'ICMP_PKT' '')], [0], [ignore]) The patch introduces a comprehensive testing framework but has several minor issues including grammatical errors, typos, and a missing newline. The functionality appears sound but these cosmetic issues should be addressed for consistency with project standards. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
