On 19/05/2026 18:08, Eelco Chaudron wrote:
External email: Use caution opening links or attachments
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.
Ack
+ 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.
See below
+ 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.
Ack
+ 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.
I change the order to have operations on the devices only inside the
namespace so altname is not added at all, thus removed entirely.
That said, I read that it might be dependent on environment (udev maybe)
to have altname implicitly added. For this, I moved it to be patch #3 of v2.
I didn't see it's necessary in my system, but maybe in other ones.
+ fi"
]
)
m4_define([ADD_VETH], [ADD_VF($@)])
--
2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
