On 6 May 2026, at 11:05, Eli Britstein wrote:
> ADD_VF is invoked N times, each time $VF is overwritten.
> The restore of the first N-1 VF devices is incorrect. To fix it, keep
> the original name in ORIG_<port> file, and use it for the restore.
>
> Harden tear-down with stderr discarded and `|| true` so missing objects or
> races
> during cleanup do not fail the suite under `set -e`.
Hi Eli,
Thanks for fixing this, see some comments below.
//Eelco
> Fixes: 289e9f6baa7c ("tests: Add a simple DPDK rte_flow test framework.")
> Signed-off-by: Eli Britstein <[email protected]>
> ---
> tests/system-dpdk-offloads-macros.at | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/tests/system-dpdk-offloads-macros.at
> b/tests/system-dpdk-offloads-macros.at
> index 2a4591ced..c6a9ed053 100644
> --- a/tests/system-dpdk-offloads-macros.at
> +++ b/tests/system-dpdk-offloads-macros.at
> @@ -118,7 +118,7 @@ m4_define([ADD_VF],
> AT_CHECK([ip link set $VF down])
> dnl If a prior run left $1 as an altname, rename fails with "File
> exists".
> AT_CHECK([ip link property del dev $VF altname $1 2>/dev/null || true])
> - AT_CHECK([ip link set $VF name $1])
> + AT_CHECK([ip link set $VF name $1 && printf '%s\n' "$VF" > ORIG_$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" -- \
> @@ -136,9 +136,15 @@ m4_define([ADD_VF],
> 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 $VF; \
> - ip link set $1 name $VF"
> + on_exit "orig=\$(cat ORIG_$1 2>/dev/null); \
> + rm -f ORIG_$1 2>/dev/null || true; \
Here and below, we should not use the '2>/dev/null || true'
pattern. No other on_exit handler in the tree does this, and
we want to know when cleanup fails so we can fix the teardown
code.
> + ip netns exec $2 ip link set $1 netns 1 2>/dev/null || true; \
> + ip link property del dev \"\$orig\" altname $1 2>/dev/null ||
> true; \
This is a no-op. $1 is the primary name, not an altname, so
there is nothing to delete here regardless of how the device
is looked up.
> + ip link property del dev $1 altname \"\$orig\" 2>/dev/null ||
> true; \
> + ip link set $1 down 2>/dev/null || true; \
> + if test \"\$orig\"; then \
This should not be conditional; it should just fail if $orig
is not valid.
> + ip link set $1 name \"\$orig\" 2>/dev/null || true; \
After this rename the kernel adds $1 as an altname on $orig.
That altname is never removed, which is the stale state that
patch 2/4 works around at setup time. Adding a post-rename
ip link property del dev "$orig" altname $1
here would make the setup-time workaround in patch 2/4
unnecessary.
> + fi"
> ]
> )
> m4_define([ADD_VETH], [ADD_VF($@)])
> --
> 2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
