On 1 Apr 2026, at 13:21, Eli Britstein wrote:
> On 31/03/2026 16:55, Eelco Chaudron wrote:
>> This patch refactors the DPDK offload test infrastructure to simplify
>> the interface configuration. Instead of using OVS_DPDK_VF_PCI_ADDRS
>> with a space-separated list of PCI addresses and VF indices (e.g.,
>> "0000:17:00.0,0 0000:17:00.0,1"), the tests now use OVS_DPDK_PF_PORT
>> with a single PF interface name (e.g., "ens2f0np0") and derive VF
>> information from sysfs.
>>
>> Signed-off-by: Eelco Chaudron <[email protected]>
>> ---
>> Documentation/topics/testing.rst | 21 ++--
>> tests/system-dpdk-offloads-macros.at | 140 +++++++++++++++------------
>> 2 files changed, 90 insertions(+), 71 deletions(-)
>>
>> diff --git a/Documentation/topics/testing.rst
>> b/Documentation/topics/testing.rst
>> index deb6088d7..019661b97 100644
>> --- a/Documentation/topics/testing.rst
>> +++ b/Documentation/topics/testing.rst
>> @@ -306,19 +306,22 @@ Userspace datapath with DPDK offload
>> To invoke the userspace datapath tests with DPDK and its rte_flow offload,
>> the same prerequisites apply as above. In addition, six Virtual Function
>> (VF)
> I still didn't understand why do we need 6 VFs. Why 2 (for example) is not
> enough?
The offload tests also run the general system-traffic tests (same for
AF_XDP, DPDK, TC, and RTE_FLOW). To support this we need at least five veth
pairs (or a simulation thereof). I chose 6 to make sure that if someone
adds a test to add one more veth pair we are safe, but if it matters somehow
we can move it to 5.
The system tests also dictated the specific naming of the interfaces. The
system tests would become a mess if we somehow allow for dynamic interface
names, that's why the rename has to happen in the macros.
So just to be clear, offload provider specific tests happen in
system-doca-offloads.at, and the general system tests, which run for
kernel, userspace, dpdk, tc, rte_flow, are defined in system-traffic.at.
>> -interfaces must be preconfigured and capable of hardware offloading traffic
>> -between each other.
>> +interfaces must be preconfigured on a single Physical Function (PF) that
>> +supports rte_flow hardware offload.
>> -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::
>> +This is an example on how to set this up for an NVIDIA blade on port
>> +``ens2f0np0``::
>
> As far as I know, this will work *ONLY* on mlx5 ports, because it's the only
> driver that is bifurcated (Linux interfaces are kept).
>
> I don't know how to configure other vendors, if there is an intention to do
> it.
>
> If we do, need to start from the PCI address of the PF, then derive the
> VFs/REPs and not to rely Linux interfaces for the PF/REPs (renames for
> example).
>
> If we don't, maybe note it's not "an example" but it can work only on NVIDIA.
I did try an E810 this week, but failed as it has no support for
representors. I think the E710 should support some of it, I'll try to get
my hands on one next week (or the week after). Based on this I will update
the documentation.
>> - 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"
>> + OVS_DPDK_PF_PORT=ens2f0np0
>> + PF_PCI=$(basename $(readlink /sys/class/net/$OVS_DPDK_PF_PORT/device))
>> + echo 0 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs
>> + devlink dev eswitch set pci/$PF_PCI mode switchdev
>> + echo 6 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs
>> -To invoke the dpdk offloads testsuite with the userspace datapath, run::
>> +This PF's interface name needs to be passed with the OVS_DPDK_PF_PORT
>> variable.
>> +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"
>> + make check-dpdk-offloads OVS_DPDK_PF_PORT=ens2f0np0
>> 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
>
> The content of the file has nothing to do with offloads. Those are just
> macros for dpdk (maybe shared later with doca) for port configurations.
>
> I suggest to rename.
I agree, I will remove the dpdk_ reference, and see if I can move them to
another more common macros file.
>> index ff7a6e095..a90d17ae3 100644
>> --- a/tests/system-dpdk-offloads-macros.at
>> +++ b/tests/system-dpdk-offloads-macros.at
>> @@ -54,10 +54,8 @@ m4_define([CHECK_NO_DPDK_OFFLOAD],
>> #
>> m4_define([OVS_DPDK_OFFLOAD_PRE_CHECK], [
>> OVS_DPDK_PRE_CHECK()
>> - AT_SKIP_IF(
>> - [test "$(printf '%s' "$OVS_DPDK_VF_PCI_ADDRS" | wc -w)" -ne 6])
>> -
>> - AT_SKIP_IF([! ovs_dpdk_verify_vf_cfg "$OVS_DPDK_VF_PCI_ADDRS"])
>> + AT_SKIP_IF([test -z "$OVS_DPDK_PF_PORT"])
>> + AT_SKIP_IF([! ovs_dpdk_verify_pf_cfg "$OVS_DPDK_PF_PORT"])
>
> This, and the below (at least big parts of it) can be later shared with doca.
>
> - Instead of OVS_DPDK{DOCA}_PF_PORT we can have OVS_PF_PCI, that is
> applicable for both.
>
> - Refactor those parts out from this "dpdk" specific file (or let doca use
> functions from it?)
I was reasoning that keeping both with their explicit names, might be
easier when you run the tests in parallel, no need to reset the variable,
but I'm fine either way.
>> ])
>> # OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output],
>> [dbinit-aux-args])
>> @@ -114,19 +112,20 @@ $1";/mlx5_net: Failed to update link status: /d"])
>> m4_define([ADD_VF],
>> [ USER_PORT=$1
>> case "$USER_PORT" in
>> - client) PORT_NO=0 ;;
>> - server) PORT_NO=1 ;;
>> - *) PORT_NO=${USER_PORT##*[!0-9]} ;;
>> + client) VF_IDX=0 ;;
>> + server) VF_IDX=1 ;;
>> + *) VF_IDX=${USER_PORT##*[!0-9]} ;;
>> esac
>> - AT_CHECK([[[ "$PORT_NO" -ge 0 ]] && [[ "$PORT_NO" -le 6 ]] || return
>> 66])
>> - PORT_CFG=$(echo $OVS_DPDK_VF_PCI_ADDRS | cut -d' ' -f$((PORT_NO+1)))
>> - PF_PCI=$(ovs_dpdk_get_pci_id "$PORT_CFG")
>> - VF_IDX=$(ovs_dpdk_get_vf_idx "$PORT_CFG")
>> - REP=$(ovs_dpdk_get_representor_netdev $PF_PCI $VF_IDX)
>> - AT_CHECK([test $? -eq 0])
>> + AT_CHECK([[[ "$VF_IDX" -ge 0 ]] && [[ "$VF_IDX" -le 5 ]] || return
>> 66])
>> + PF_PCI=$(basename $(readlink /sys/class/net/$OVS_DPDK_PF_PORT/device))
>> + VF=$(ovs_dpdk_get_vf_netdev $OVS_DPDK_PF_PORT $VF_IDX)
>> + REP=$(ovs_dpdk_get_representor_netdev $OVS_DPDK_PF_PORT $VF_IDX)
>> + AT_CHECK([test -n "$VF"])
>> + AT_CHECK([test -n "$REP"])
>> - AT_CHECK([ip link set $REP name $1])
>> + AT_CHECK([ip link set $VF down])
>> + AT_CHECK([ip link set $VF name $1])
>> AT_CHECK([ip link set $1 netns $2])
>> AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \
>> set interface ovs-$1 external-ids:iface-id="$1" -- \
>> @@ -139,14 +138,14 @@ m4_define([ADD_VF],
>> NS_CHECK_EXEC([$2], [ip link set dev $1 address $5])
>> else
>> NS_CHECK_EXEC([$2],
>> - [ip link set dev $1 address 02:00:00:00:EC:0$PORT_NO])
>> + [ip link set dev $1 address 02:00:00:00:EC:0$VF_IDX])
>> fi
>> if test -n "$6"; then
>> NS_CHECK_EXEC([$2], [ip route add default via $6])
>> fi
>> on_exit "ip netns exec $2 ip link set $1 netns 1; \
>> - ip link property del dev $1 altname $REP; \
>> - ip link set $1 name $REP"
>> + ip link property del dev $1 altname $VF; \
>> + ip link set $1 name $VF"
>
> For VFs (not attached to OVS) - why do we have to rename the interfaces at
> all? maybe we can just add an altname for them so addresses can be configured
> etc.
It makes it hard/confusing for debugging as now names are not the same
across runs on different dpifs/offload providers. We still need to remove
the alt names, so the code will not change much. So I keep it as is for now.
> For the REP, I see you changed to check those (--names) in the doca commit
> (should be in a separated commit).
Not sure what you mean here. For DOCA I need to rename them as the system
tests assume the representor name is in the ovs-px format. For DPDK we can
use this name, and assign the PCI_ID, this does not work for DOCA.
> Alternatively to the renames we can detect the names from the devargs
> (ovs-appctl dpctl/show), though I'm not sure it worths the effort.
>
> WDYT?
>
>> ]
>> )
>> m4_define([ADD_VETH], [ADD_VF($@)])
>> @@ -162,74 +161,91 @@ m4_define([DUMP_DP_IP_CLEAN_SORTED], [dnl
>> OVS_START_SHELL_HELPERS
>> -# ovs_dpdk_is_valid_pci_vf_addr()
>> +# ovs_dpdk_get_vf_netdev(<PF_PORT_NAME>, <VF_INDEX>)
>> #
>> -# Check if the given PF PCI address and the VF number are valid.
>> +# This function tries to find the VF's netdev for the given PF's VF index.
>> #
>> -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
>> -}
>> +ovs_dpdk_get_vf_netdev() {
>> + PF_PORT_NAME=$1
>> + VF_IDX=$2
>> + PF_PCI=$(basename $(readlink /sys/class/net/$PF_PORT_NAME/device))
>> -# ovs_dpdk_get_pci_id()
>> -#
>> -ovs_dpdk_get_pci_id() {
>> - printf '%s\n' "${1%%,*}"
>> -}
>> + vf_net_dir="/sys/bus/pci/devices/$PF_PCI/virtfn$VF_IDX/net"
>> + if test ! -d "$vf_net_dir"; then
>> + return 1
>> + fi
>> -# ovs_dpdk_get_vf_idx()
>> -#
>> -ovs_dpdk_get_vf_idx() {
>> - printf '%s\n' "${1##*,}"
>> + vf_name=$(ls "$vf_net_dir" 2>/dev/null | head -1)
>> + if test -z "$vf_name"; then
>> + return 2
>> + fi
>> +
>> + echo "$vf_name"
>> + return 0
>> }
>> -# ovs_dpdk_get_representor_netdev(<PF_PCI>, <VF_INDEX>)
>> +# ovs_dpdk_get_representor_netdev(<PF_PORT_NAME>, <VF_INDEX>)
>> #
>> -# This function tries to find the representor netdev for the given PF's VF.
>> +# This function tries to find the representor's netdev for the given PF's VF
>> +# index.
>> #
>> ovs_dpdk_get_representor_netdev() {
>> - PF_PCI=$1
>> + PF_PORT_NAME=$1
>> VF_IDX=$2
>> + PF_PCI=$(basename $(readlink /sys/class/net/$PF_PORT_NAME/device))
>> - 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
>> + representor=$(grep -l "vf$VF_IDX$" \
>> + /sys/bus/pci/devices/$PF_PCI/net/*/phys_port_name \
>> + 2>/dev/null | head -1)
>> + if test -z "$representor"; then
>> + return 2
>> 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
>> + basename $(dirname "$representor")
>> + return 0
>> }
>> -# ovs_dpdk_verify_vf_cfg()
>> +# ovs_dpdk_verify_pf_cfg(<PF_PORT_NAME>)
>> #
>> -# Verify that the given PF PCI addresses and corresponding VF IDs in
>> -# OVS_DPDK_VF_PCI_ADDRS are valid, exist, and have corresponding
>> -# representor ports.
>> +# Verify that the given PF port exists and that the required VFs are
>> properly
>> +# configured, i.e. have representor ports.
>> #
>> -ovs_dpdk_verify_vf_cfg() {
>> - i=0
>> +ovs_dpdk_verify_pf_cfg() {
>> + PF_PORT_NAME=$1
>> - for addr in $1; do
>> - ovs_dpdk_is_valid_pci_vf_addr "$addr" || return 1
>> + test -d "/sys/class/net/$PF_PORT_NAME" || return 1
>> - PCI_ID=$(ovs_dpdk_get_pci_id "$addr")
>> - VF_IDX=$(ovs_dpdk_get_vf_idx "$addr")
>> + PF_PCI=$(basename $(readlink /sys/class/net/$PF_PORT_NAME/device))
>> + VF_NAMES=()
>> + REPRESENTOR_NAMES=()
>> - REP=$(ovs_dpdk_get_representor_netdev $PCI_ID $VF_IDX) || return 1
>> + for i in {0..5}; do
>> + # Get VF netdev using PCI path
>> + vf_net_dir="/sys/bus/pci/devices/$PF_PCI/virtfn$i/net"
>> + if test ! -d "$vf_net_dir"; then
>> + return 1
>> + fi
>> - echo "ovs-p$i: PF PCI $PCI_ID with VF $VF_IDX has representor $REP"
>> - i=$((i + 1))
>> + vf_name=$(ls "$vf_net_dir" 2>/dev/null | head -1)
>> + if test -z "$vf_name"; then
>> + return 2
>> + fi
>> + VF_NAMES+=($vf_name)
>> +
>> + # Find representor port for this VF
>> + representor=$(grep -l "vf$i$" \
>> + /sys/bus/pci/devices/$PF_PCI/net/*/phys_port_name \
>> + 2>/dev/null | head -1)
>> + if test -z "$representor"; then
>> + return 3
>> + fi
>> + REPRESENTOR_NAMES+=($(basename $(dirname "$representor")))
>> done
>> + if test "${#VF_NAMES[*]}" -lt 6 -o "${#REPRESENTOR_NAMES[*]}" -lt 6;
>> then
>> + return 4
>> + fi
>> +
>> return 0
>> }
>>
> As tests/system-dpdk-offloads.at / tests/system-doca-offloads.at are mostly
> duplicated, can we rename and share later?
The goal here is to add offload provider specific test cases here, so I
would like to keep them separate for now.
I will send a new revision after I get some time with an E710 card.
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
