Skip to content

Commit 1e5fe58

Browse files
aditya-mehakaredceara
authored andcommitted
controller: Do not remove ovn-installed from parent when deleting child port.
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: ec1db7a ("ovn-controller: fixed ovn-installed not always properly added or removed.") Acked-by: Xavier Simonart <xsimonar@redhat.com> Signed-off-by: Aditya Mehakare <aditya.mehakare@nutanix.com> Acked-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
1 parent 93ec294 commit 1e5fe58

2 files changed

Lines changed: 79 additions & 1 deletion

File tree

controller/binding.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1612,6 +1612,15 @@ is_binding_lport_this_chassis(struct binding_lport *b_lport,
16121612
|| is_postponed_port(b_lport->pb->logical_port)));
16131613
}
16141614

1615+
/* Returns 'true' if an lport of this type is bound through a parent's
1616+
* local_binding rather than its own. Container, virtual and mirror ports
1617+
* share the parent VIF's OVS interface and local_binding. */
1618+
static bool
1619+
is_lport_type_child(enum en_lport_type type)
1620+
{
1621+
return type == LP_CONTAINER || type == LP_VIRTUAL || type == LP_MIRROR;
1622+
}
1623+
16151624
/* Returns 'true' if the 'lbinding' has binding lports of type
16161625
* LP_CONTAINER/LP_MIRROR, 'false' otherwise. */
16171626
static bool
@@ -2988,7 +2997,8 @@ handle_deleted_vif_lport(const struct sbrec_port_binding *pb,
29882997
}
29892998

29902999
handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
2991-
if (lbinding && lbinding->iface && lbinding->iface->name) {
3000+
if (!is_lport_type_child(lport_type) &&
3001+
lbinding && lbinding->iface && lbinding->iface->name) {
29923002
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
29933003
lbinding->iface);
29943004
}

tests/ovn-controller.at

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4108,3 +4108,71 @@ wait_for_ports_up
41084108
OVN_CLEANUP([hv1])
41094109
AT_CLEANUP
41104110
])
4111+
4112+
OVN_FOR_EACH_NORTHD([
4113+
AT_SETUP([ovn-controller - Delete container child port does not bounce parent])
4114+
ovn_start
4115+
4116+
net_add n1
4117+
4118+
sim_add hv1
4119+
as hv1
4120+
check ovs-vsctl add-br br-phys
4121+
ovn_attach n1 br-phys 192.168.0.1
4122+
4123+
as hv1 check ovs-vsctl -- add-port br-int vif \
4124+
-- set Interface vif external-ids:iface-id=lsp0
4125+
4126+
check ovn-nbctl ls-add ls0
4127+
check ovn-nbctl lsp-add ls0 lsp0
4128+
for i in 1 2 3; do
4129+
check ovn-nbctl lsp-add ls0 lsp-cont$i lsp0 $i
4130+
done
4131+
4132+
wait_for_ports_up
4133+
ch=$(fetch_column Chassis _uuid name=hv1)
4134+
for port in lsp0 lsp-cont1 lsp-cont2 lsp-cont3; do
4135+
wait_row_count Port_Binding 1 logical_port=$port chassis=$ch
4136+
done
4137+
4138+
OVS_WAIT_UNTIL([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed` = '"true"'])
4139+
4140+
# Case 1: delete one child (lsp-cont3); two siblings remain.
4141+
check ovn-nbctl --wait=hv lsp-del lsp-cont3
4142+
for port in lsp0 lsp-cont1 lsp-cont2; do
4143+
wait_column "true" sb:Port_Binding up logical_port=$port
4144+
done
4145+
AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed` = '"true"'])
4146+
4147+
# Force a full recompute and re-check.
4148+
as hv1 ovn-appctl -t ovn-controller recompute
4149+
check ovn-nbctl --wait=hv sync
4150+
for port in lsp0 lsp-cont1 lsp-cont2; do
4151+
wait_column "true" sb:Port_Binding up logical_port=$port
4152+
done
4153+
AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed` = '"true"'])
4154+
4155+
# Case 2: delete remaining children; parent has zero children.
4156+
for i in 1 2; do
4157+
check ovn-nbctl --wait=hv lsp-del lsp-cont$i
4158+
done
4159+
wait_column "true" sb:Port_Binding up logical_port=lsp0
4160+
AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed` = '"true"'])
4161+
4162+
# Force recompute with no children and verify parent is still stable.
4163+
as hv1 ovn-appctl -t ovn-controller recompute
4164+
check ovn-nbctl --wait=hv sync
4165+
wait_column "true" sb:Port_Binding up logical_port=lsp0
4166+
AT_CHECK([test `as hv1 ovs-vsctl get Interface vif external_ids:ovn-installed` = '"true"'])
4167+
4168+
# Throughout the whole test, the parent's vif must never have its
4169+
# ovn-installed external-id cleared, and neither the parent nor any
4170+
# child should have been bounced down in the Southbound DB.
4171+
AT_CHECK([grep -q "Removing iface vif ovn-installed in OVS" hv1/ovn-controller.log], [1], [])
4172+
for port in lsp0 lsp-cont1 lsp-cont2 lsp-cont3; do
4173+
AT_CHECK([grep -q "Setting lport $port down in Southbound" hv1/ovn-controller.log], [1], [])
4174+
done
4175+
4176+
OVN_CLEANUP([hv1])
4177+
AT_CLEANUP
4178+
])

0 commit comments

Comments
 (0)