On 10/9/25 2:29 PM, Xavier Simonart wrote: > Hi Dumitru > Hi Xavier,
> Thanks for the patch Thanks for the review! > Just one nit: ADD_OVS_TUNNEL suffers from the same, but is never used, > and ADD_NATIVE_TUNNEL, unused as well, is also missing on_exit. > So I think we might either fix them or delete them. > ADD_NATIVE_TUNNEL is actually worse, it doesn't cleanup at all. But you're right, we don't use those. I'll post a new version removing these and some more. > Acked-by: Xavier Simonart <[email protected]> > I'll include your ack on the patch in the new version. Regards, Dumitru > Thanks > Xavier > > On Thu, Oct 9, 2025 at 2:05 PM Dumitru Ceara via dev < > [email protected]> wrote: > >> The ADD_VETH() / ADD_VETH_BOND() macros didn't schedule deletion of the >> test interfaces (by calling 'on_exit') immediately after the interfaces >> were created. That means that in the eventuality of a command failure >> in between creation and the call to 'on_exit' the test interfaces >> wouldn't be cleaned up. >> >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> tests/system-common-macros.at | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >> index 1f2b2b4ebe..0bf35754bd 100644 >> --- a/tests/system-common-macros.at >> +++ b/tests/system-common-macros.at >> @@ -60,6 +60,7 @@ m4_define([ADD_BR], [ovs-vsctl _ADD_BR([$1]) -- $2]) >> # >> m4_define([ADD_VETH], >> [ AT_CHECK([ip link add $1 type veth peer name ovs-$1]) >> + on_exit "ip link del ovs-$1" >> CONFIGURE_VETH_OFFLOADS([$1]) >> AT_CHECK([ip link set $1 netns $2]) >> AT_CHECK([ip link set dev ovs-$1 up]) >> @@ -80,7 +81,6 @@ m4_define([ADD_VETH], >> if test -n "$9"; then >> NS_CHECK_EXEC([$2], [ip route add default via $9]) >> fi >> - on_exit "ip link del ovs-$1" >> ] >> ) >> >> @@ -98,13 +98,14 @@ m4_define([ADD_VETH_BOND], >> BONDPORTS="" >> for port in $1; do >> AT_CHECK([ip link add $port type veth peer name ovs-$port]) >> + on_exit 'ip link del ovs-$port' >> CONFIGURE_VETH_OFFLOADS([$port]) >> AT_CHECK([ip link set $port netns $2]) >> AT_CHECK([ip link set dev ovs-$port up]) >> BONDPORTS="$BONDPORTS ovs-$port" >> - on_exit 'ip link del ovs-$port' >> done >> NS_CHECK_EXEC([$2], [ip link add name $4 type bond]) >> + on_exit 'ip link del ovs-$4' >> case "$(echo $5 | sed 's/.*lacp=//' | sed 's/ .*//')" in >> active|passive) >> NS_CHECK_EXEC([$2], [sh -c "echo 802.3ad > >> /sys/class/net/$4/bonding/mode"]) >> @@ -117,7 +118,6 @@ m4_define([ADD_VETH_BOND], >> NS_CHECK_EXEC([$2], [ip addr add $6 dev $4]) >> NS_CHECK_EXEC([$2], [ip link set dev $4 up]) >> AT_CHECK([ovs-vsctl add-bond $3 ovs-$4 $BONDPORTS $5]) >> - on_exit 'ip link del ovs-$4' >> ] >> ) >> >> -- >> 2.51.0 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
