Skip to content

Commit

Permalink
controller: Avoid potential 100% cpu when ports are postponed.
Browse files Browse the repository at this point in the history 
When two chassis claim the same port, if a port is in the MARK_UP
state when the other chassis claims it, then ovn-controller was
sometimes using 100% cpu, as if-status was fighting to add ovn-install
(as in the MARK_UP state) and binding was fighting to remove it (as
the port was bound to the other chassis).

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 3490051)
  • Loading branch information
@simonartxavier @dceara
simonartxavier authored and dceara committed Feb 28, 2025
1 parent 319cc0e commit 7113b87
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 18 deletions.
15 changes: 5 additions & 10 deletions controller/binding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1620,8 +1620,7 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
/* Release the lport if there is no lbinding. */
if (lbinding_set && !can_bind) {
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
b_lport->lbinding->iface->name,
&b_lport->lbinding->iface->header_.uuid);
b_lport->lbinding->iface);
}

if (!lbinding_set || !can_bind) {
Expand All @@ -1645,8 +1644,7 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
pb->logical_port)) {
update_lport_tracking(pb, b_ctx_out->tracked_dp_bindings, false);
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
b_lport->lbinding->iface->name,
&b_lport->lbinding->iface->header_.uuid);
b_lport->lbinding->iface);
}

return true;
Expand Down Expand Up @@ -2116,8 +2114,7 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in,
* This can happen if iface-id was removed as we recompute.
*/
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
iface_rec->name,
&iface_rec->header_.uuid);
iface_rec);
}
}
}
Expand Down Expand Up @@ -2553,8 +2550,7 @@ consider_iface_release(const struct ovsrec_interface *iface_rec,
}
if (lbinding->iface && lbinding->iface->name) {
if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
lbinding->iface->name,
&lbinding->iface->header_.uuid);
lbinding->iface);
}

} else if (b_lport && b_lport->type == LP_LOCALPORT) {
Expand Down Expand Up @@ -2884,8 +2880,7 @@ 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_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
lbinding->iface->name,
&lbinding->iface->header_.uuid);
lbinding->iface);
}
return true;
}
Expand Down
27 changes: 21 additions & 6 deletions controller/if-status.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ enum if_state {
* but not yet marked "up" in the binding module (in
* SB and OVS databases).
*/
OIF_UNINSTALLED, /* Interface from whom ovn-install must be removed
*/
OIF_INSTALLED, /* Interface flows programmed in OVS and binding
* marked "up" in the binding module.
*/
Expand All @@ -89,6 +91,7 @@ static const char *if_state_names[] = {
[OIF_REM_OLD_OVN_INST] = "REM_OLD_OVN_INST",
[OIF_MARK_UP] = "MARK_UP",
[OIF_MARK_DOWN] = "MARK_DOWN",
[OIF_UNINSTALLED] = "UNINSTALLED",
[OIF_INSTALLED] = "INSTALLED",
[OIF_UPDATE_PORT] = "UPDATE_PORT",
};
Expand Down Expand Up @@ -326,6 +329,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr,
/* Nothing to do here. */
break;
case OIF_INSTALLED:
case OIF_UNINSTALLED:
case OIF_MARK_DOWN:
case OIF_UPDATE_PORT:
ovs_iface_set_state(mgr, iface, OIF_CLAIMED);
Expand Down Expand Up @@ -362,6 +366,7 @@ if_status_mgr_release_iface(struct if_status_mgr *mgr, const char *iface_id)
case OIF_REM_OLD_OVN_INST:
case OIF_MARK_UP:
case OIF_INSTALLED:
case OIF_UNINSTALLED:
/* Properly mark interfaces "down" if their flows were already
* programmed in OVS.
*/
Expand Down Expand Up @@ -402,6 +407,7 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id,
case OIF_REM_OLD_OVN_INST:
case OIF_MARK_UP:
case OIF_INSTALLED:
case OIF_UNINSTALLED:
/* Properly mark interfaces "down" if their flows were already
* programmed in OVS.
*/
Expand Down Expand Up @@ -634,13 +640,22 @@ if_status_mgr_update(struct if_status_mgr *mgr,

void
if_status_mgr_remove_ovn_installed(struct if_status_mgr *mgr,
const char *name,
const struct uuid *uuid)
const struct ovsrec_interface *iface_rec)
{
VLOG_DBG("Adding %s to list of interfaces for which to remove "
"ovn-installed", name);
if (!shash_find_data(&mgr->ovn_uninstall_hash, name)) {
add_to_ovn_uninstall_hash(mgr, name, uuid);
if (!shash_find_data(&mgr->ovn_uninstall_hash, iface_rec->name)) {
VLOG_DBG("Adding %s to list of interfaces for which to remove "
"ovn-installed", iface_rec->name);
add_to_ovn_uninstall_hash(mgr, iface_rec->name,
&iface_rec->header_.uuid);
}

/* Move out of MARK_UP state which would add ovn-install back. */
const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id");
if (iface_id) {
struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
if (iface && iface->state == OIF_MARK_UP) {
ovs_iface_set_state(mgr, iface, OIF_UNINSTALLED);
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions controller/if-status.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ bool if_status_handle_claims(struct if_status_mgr *mgr,
const struct sbrec_port_binding_table *pb_table,
bool sb_readonly);
void if_status_mgr_remove_ovn_installed(struct if_status_mgr *mgr,
const char *name,
const struct uuid *uuid);
const struct ovsrec_interface *iface_rec);
uint16_t if_status_mgr_iface_get_mtu(const struct if_status_mgr *mgr,
const char *iface_id);
bool if_status_mgr_iface_update(const struct if_status_mgr *mgr,
Expand Down
3 changes: 3 additions & 0 deletions tests/ovn.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16985,6 +16985,9 @@ max_claims=20
AT_CHECK([test "${hv1_claims}" -le "${max_claims}"], [0], [])
AT_CHECK([test "${hv2_claims}" -le "${max_claims}"], [0], [])

n_uninstalled=$(as hv1 grep -c 'Removing iface vif ovn-installed in' hv1/ovn-controller.log)
echo "$n_uninstalled times 'Removing iface vif ovn-installed'"
AT_CHECK([test "${n_uninstalled}" -le "10"], [0], [])
check ovn-nbctl --wait=hv lsp-del lsp0
CHECK_AFTER_RECOMPUTE([hv1], [hv1])

Expand Down

0 comments on commit 7113b87

Please # to comment.