When a container (or virtual/mirror) child port is deleted,
handle_deleted_vif_lport() unconditionally calls
if_status_mgr_remove_ovn_installed() on the associated lbinding's OVS
interface. For child port types, lbinding points to the parent's
local_binding, so this incorrectly strips ovn-installed from the
parent's OVS interface.
This causes a cascade of problems:
1. ovn-installed is removed from the parent's OVS interface.
2. On the next full recompute (triggered by any SB commit failure,
IDL reconnection, etc.), claim_lport() notices the missing
ovn-installed and re-claims the parent port.
3. The parent port re-enters OIF_CLAIMED -> OIF_INSTALL_FLOWS, during
which local_binding_set_down() marks the parent AND all remaining
children as down in the Southbound database.
4. After flows are re-installed, all ports are set back up.
This results in a disruptive and unnecessary down/up bounce of the
parent port and all sibling container ports whenever a single child
port is deleted.
Fix this by gating the if_status_mgr_remove_ovn_installed() call on
lport_type == LP_VIF, so that only deletion of the primary VIF port
triggers ovn-installed removal from its OVS interface.
Fixes: ec1db7ae09fa ("ovn-controller: fixed ovn-installed not always properly
added or removed.")
Signed-off-by: Aditya Mehakare <[email protected]>
Acked-by: Naveen Yerramneni <[email protected]>
---
controller/binding.c | 3 +-
tests/ovn.at | 71 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/controller/binding.c b/controller/binding.c
index fa1984ee7..63c168886 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2988,7 +2988,8 @@ handle_deleted_vif_lport(const struct sbrec_port_binding
*pb,
}
handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
- if (lbinding && lbinding->iface && lbinding->iface->name) {
+ if (lport_type == LP_VIF && lbinding && lbinding->iface &&
+ lbinding->iface->name) {
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
lbinding->iface);
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 9b328df01..efacdbb57 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -41665,6 +41665,77 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Delete container child port does not bounce parent])
+ovn_start
+
+net_add n1
+
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+as hv1 check ovs-vsctl -- add-port br-int vif \
+ -- set Interface vif external-ids:iface-id=lsp0
+
+check ovn-nbctl ls-add ls0
+check ovn-nbctl lsp-add ls0 lsp0
+for i in 1 2 3; do
+ check ovn-nbctl lsp-add ls0 lsp-cont$i lsp0 $i
+done
+
+wait_for_ports_up
+ch=$(fetch_column Chassis _uuid name=hv1)
+for port in lsp0 lsp-cont1 lsp-cont2 lsp-cont3; do
+ wait_row_count Port_Binding 1 logical_port=$port chassis=$ch
+done
+
+OVS_WAIT_UNTIL([test `as hv1 ovs-vsctl get Interface vif
external_ids:ovn-installed` = '"true"'])
+
+# Case 1: delete one child (lsp-cont3); two siblings remain.
+check ovn-nbctl --wait=hv lsp-del lsp-cont3
+for port in lsp0 lsp-cont1 lsp-cont2; do
+ wait_column "true" sb:Port_Binding up logical_port=$port
+done
+AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed`
= '"true"'])
+
+# Force a full recompute and re-check.
+as hv1 ovn-appctl -t ovn-controller recompute
+check ovn-nbctl --wait=hv sync
+for port in lsp0 lsp-cont1 lsp-cont2; do
+ wait_column "true" sb:Port_Binding up logical_port=$port
+done
+AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed`
= '"true"'])
+
+# Case 2: delete remaining children; parent has zero children.
+for i in 1 2; do
+ check ovn-nbctl --wait=hv lsp-del lsp-cont$i
+done
+wait_column "true" sb:Port_Binding up logical_port=lsp0
+AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed`
= '"true"'])
+
+# Force recompute with no children and verify parent is still stable.
+as hv1 ovn-appctl -t ovn-controller recompute
+check ovn-nbctl --wait=hv sync
+wait_column "true" sb:Port_Binding up logical_port=lsp0
+AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed`
= '"true"'])
+
+# Throughout the whole test, the parent's vif must never have its
+# ovn-installed external-id cleared, and neither the parent nor any
+# child should have been bounced down in the Southbound DB.
+AT_CHECK([grep -q "Removing iface vif ovn-installed in OVS"
hv1/ovn-controller.log], [1], [])
+for port in lsp0 lsp-cont1 lsp-cont2 lsp-cont3; do
+ AT_CHECK([grep -q "Setting lport $port down in Southbound"
hv1/ovn-controller.log], [1], [])
+done
+
+# Make sure that ovn-controller has not crashed.
+AT_CHECK([kill -0 $(cat hv1/ovn-controller.pid)])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD([
AT_SETUP([Deleting vif while controller fight for port claim])
ovn_start
--
2.43.5
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
