When a container (or virtual/mirror) child port is deleted,
handle_deleted_vif_lport() 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 skipping ovn-installed removal for port types that are
bound through a parent's local_binding rather than their own, so
that deleting a child port no longer disturbs the parent's interface
state.
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]>
---
V2:
- Replace explicit LP_VIF check with is_lport_type_child() helper
and relocate test.
---
controller/binding.c | 12 ++++++-
tests/ovn-controller.at | 71 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/controller/binding.c b/controller/binding.c
index fa1984ee7..de51be823 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1612,6 +1612,15 @@ is_binding_lport_this_chassis(struct binding_lport
*b_lport,
|| is_postponed_port(b_lport->pb->logical_port)));
}
+/* Returns 'true' if an lport of this type is bound through a parent's
+ * local_binding rather than its own. Container, virtual and mirror ports
+ * share the parent VIF's OVS interface and local_binding. */
+static bool
+is_lport_type_child(enum en_lport_type type)
+{
+ return type == LP_CONTAINER || type == LP_VIRTUAL || type == LP_MIRROR;
+}
+
/* Returns 'true' if the 'lbinding' has binding lports of type
* LP_CONTAINER/LP_MIRROR, 'false' otherwise. */
static bool
@@ -2988,7 +2997,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 (!is_lport_type_child(lport_type) &&
+ lbinding && lbinding->iface && lbinding->iface->name) {
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
lbinding->iface);
}
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index fd63399f1..c006f3219 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -4108,3 +4108,74 @@ wait_for_ports_up
OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - 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
+])
--
2.43.5
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
