@@ -1608,8 +1608,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) {
@@ -1626,8 +1625,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;
@@ -2062,8 +2060,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);
}
}
}
@@ -2491,8 +2488,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) {
@@ -2824,8 +2820,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;
}
@@ -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.
*/
@@ -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",
};
@@ -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);
@@ -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.
*/
@@ -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.
*/
@@ -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);
+ }
}
}
@@ -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,
@@ -16552,6 +16552,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])
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> --- controller/binding.c | 15 +++++---------- controller/if-status.c | 27 +++++++++++++++++++++------ controller/if-status.h | 3 +-- tests/ovn.at | 3 +++ 4 files changed, 30 insertions(+), 18 deletions(-)