OVN checks whether ovn-installed is already present (in OVS) before updating it.
This might cause ovn-installed related issues in the following case:
- (1) ovn-installed is present
- (2) we claim the interface
- (3) we update ovs, removing ovn-installed and start installing flows
- (4) (next loop), after flows installed, we check if ovn-installed is absent,
and if yes, we update OVS with ovn-installed.
However, in step4, if OVS is still busy from step 3, ovn-installed is read as
present; hence OVN controller does not update it and move to INSTALLED state.
ovn-installed was also not properly deleted on port or binding removal.
Note that port->up is not properly removed on external_ids:iface-id removal when
sb is read-only. This will be fixed in a further patch.
Fixes: a7c7d4519e50 ("controller: avoid recomputes triggered by SBDB
Port_Binding updates.")
Signed-off-by: Xavier Simonart <[email protected]>
---
v2: - handled Dumitru's comments
- rebased on main
- updated state machine diagram as commented in v1 commit message
- remove ovn-installed on port deletion or external_ids:iface-id removal.
- added test case
v3: - handled more comment from Dumitru
- fixed ovn-install not removed when ovs is read-only
- moved test case from unit (ovn.at) to system (system-ovn).
- test case connects to OVS db through tcp and uses iptables to momentarly
block the idl update
to simulate read-only ovsdb
v4: - handled comments from Ales: avoid using is_deleted outside of tracked
loops.
---
controller/binding.c | 68 ++++++-
controller/binding.h | 11 ++
controller/if-status.c | 204 ++++++++++++++++++---
controller/if-status.h | 7 +
controller/ovn-controller.c | 5 +
tests/system-ovn.at | 349 ++++++++++++++++++++++++++++++++++++
6 files changed, 612 insertions(+), 32 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index 5df62baef..357e77609 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -746,6 +746,19 @@ local_binding_get_lport_ofport(const struct shash
*local_bindings,
u16_to_ofp(lbinding->iface->ofport[0]) : 0;
}
+bool
+local_binding_is_ovn_installed(struct shash *local_bindings,
+ const char *pb_name)
+{
+ struct local_binding *lbinding =
+ local_binding_find(local_bindings, pb_name);
+ if (lbinding && lbinding->iface) {
+ return smap_get_bool(&lbinding->iface->external_ids,
+ OVN_INSTALLED_EXT_ID, false);
+ }
+ return false;
+}
+
bool
local_binding_is_up(struct shash *local_bindings, const char *pb_name,
const struct sbrec_chassis *chassis_rec)
@@ -783,6 +796,7 @@ local_binding_is_down(struct shash *local_bindings, const
char *pb_name,
} else if (b_lport->pb->chassis) {
VLOG_DBG("lport %s already claimed by other chassis",
b_lport->pb->logical_port);
+ return true;
}
}
@@ -834,6 +848,38 @@ local_binding_set_up(struct shash *local_bindings, const
char *pb_name,
}
}
+void
+local_binding_remove_ovn_installed(
+ struct shash *local_bindings,
+ const struct ovsrec_interface_table *iface_table,
+ const char *pb_name, bool ovs_readonly)
+{
+ if (ovs_readonly) {
+ return;
+ }
+ struct local_binding *lbinding =
+ local_binding_find(local_bindings, pb_name);
+ if (lbinding && lbinding->iface) {
+ const struct uuid *iface_uuid = &lbinding->iface->header_.uuid;
+ remove_ovn_installed_for_uuid(iface_table, iface_uuid);
+ }
+}
+
+void
+remove_ovn_installed_for_uuid(const struct ovsrec_interface_table *iface_table,
+ const struct uuid *iface_uuid)
+{
+ const struct ovsrec_interface *iface_rec =
+ ovsrec_interface_table_get_for_uuid(iface_table, iface_uuid);
+ if (iface_rec && smap_get_bool(&iface_rec->external_ids,
+ OVN_INSTALLED_EXT_ID, false)) {
+ VLOG_INFO("Removing iface %s ovn-installed in OVS",
+ iface_rec->name);
+ ovsrec_interface_update_external_ids_delkey(iface_rec,
+ OVN_INSTALLED_EXT_ID);
+ }
+}
+
void
local_binding_set_down(struct shash *local_bindings, const char *pb_name,
const struct sbrec_chassis *chassis_rec,
@@ -1239,7 +1285,9 @@ claim_lport(const struct sbrec_port_binding *pb,
return false;
}
} else {
- if (pb->n_up && !pb->up[0]) {
+ if ((pb->n_up && !pb->up[0]) ||
+ !smap_get_bool(&iface_rec->external_ids,
+ OVN_INSTALLED_EXT_ID, false)) {
if_status_mgr_claim_iface(if_mgr, pb, chassis_rec,
sb_readonly);
}
@@ -1464,9 +1512,11 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
const char *requested_chassis_option = smap_get(
&pb->options, "requested-chassis");
VLOG_INFO_RL(&rl,
- "Not claiming lport %s, chassis %s requested-chassis %s",
+ "Not claiming lport %s, chassis %s requested-chassis %s "
+ "pb->chassis %s",
pb->logical_port, b_ctx_in->chassis_rec->name,
- requested_chassis_option ? requested_chassis_option : "[]");
+ requested_chassis_option ? requested_chassis_option : "[]",
+ pb->chassis ? pb->chassis->name: "");
}
}
@@ -2288,6 +2338,11 @@ consider_iface_release(const struct ovsrec_interface
*iface_rec,
return false;
}
}
+ if (lbinding->iface && lbinding->iface->name) {
+ if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
+ lbinding->iface->name,
+ &lbinding->iface->header_.uuid);
+ }
} else if (lbinding && b_lport && b_lport->type == LP_LOCALPORT) {
/* lbinding is associated with a localport. Remove it from the
@@ -2558,6 +2613,7 @@ handle_deleted_lport(const struct sbrec_port_binding *pb,
if (ld) {
remove_pb_from_local_datapath(pb,
b_ctx_out, ld);
+ if_status_mgr_release_iface(b_ctx_out->if_mgr, pb->logical_port);
return;
}
@@ -2581,6 +2637,7 @@ handle_deleted_lport(const struct sbrec_port_binding *pb,
remove_pb_from_local_datapath(pb, b_ctx_out,
ld);
}
+ if_status_mgr_release_iface(b_ctx_out->if_mgr, pb->logical_port);
}
}
@@ -2627,6 +2684,11 @@ 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);
+ }
return true;
}
diff --git a/controller/binding.h b/controller/binding.h
index 6c3a98b02..0b9c6994f 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -159,6 +159,14 @@ bool local_binding_is_up(struct shash *local_bindings,
const char *pb_name,
bool local_binding_is_down(struct shash *local_bindings, const char *pb_name,
const struct sbrec_chassis *);
+bool local_binding_is_ovn_installed(struct shash *local_bindings,
+ const char *pb_name);
+void local_binding_remove_ovn_installed(
+ struct shash *local_bindings,
+ const struct ovsrec_interface_table *iface_table,
+ const char *pb_name,
+ bool ovs_readonly);
+
void local_binding_set_up(struct shash *local_bindings, const char *pb_name,
const struct sbrec_chassis *chassis_rec,
const char *ts_now_str, bool sb_readonly,
@@ -195,6 +203,9 @@ void set_pb_chassis_in_sbrec(const struct
sbrec_port_binding *pb,
const struct sbrec_chassis *chassis_rec,
bool is_set);
+void remove_ovn_installed_for_uuid(const struct ovsrec_interface_table *,
+ const struct uuid *);
+
/* Corresponds to each Port_Binding.type. */
enum en_lport_type {
LP_UNKNOWN,
diff --git a/controller/if-status.c b/controller/if-status.c
index d1c14ac30..ac36a9fb9 100644
--- a/controller/if-status.c
+++ b/controller/if-status.c
@@ -54,32 +54,37 @@ VLOG_DEFINE_THIS_MODULE(if_status);
*/
enum if_state {
- OIF_CLAIMED, /* Newly claimed interface. pb->chassis update not yet
- initiated. */
- OIF_INSTALL_FLOWS, /* Claimed interface with pb->chassis update sent to
- * SB (but update notification not confirmed, so the
- * update may be resent in any of the following states)
- * and for which flows are still being installed.
- */
- OIF_MARK_UP, /* Interface with flows successfully installed in OVS
- * but not yet marked "up" in the binding module (in
- * SB and OVS databases).
- */
- OIF_MARK_DOWN, /* Released interface but not yet marked "down" in the
- * binding module (in SB and/or OVS databases).
- */
- OIF_INSTALLED, /* Interface flows programmed in OVS and binding marked
- * "up" in the binding module.
- */
+ OIF_CLAIMED, /* Newly claimed interface. pb->chassis update not
+ yet initiated. */
+ OIF_INSTALL_FLOWS, /* Claimed interface with pb->chassis update sent to
+ * SB (but update notification not confirmed, so the
+ * update may be resent in any of the following
+ * states and for which flows are still being
+ * installed.
+ */
+ OIF_REM_OLD_OVN_INST, /* Interface with flows successfully installed in OVS
+ * but with ovn-installed still in OVSDB.
+ */
+ OIF_MARK_UP, /* Interface with flows successfully installed in OVS
+ * but not yet marked "up" in the binding module (in
+ * SB and OVS databases).
+ */
+ OIF_MARK_DOWN, /* Released interface but not yet marked "down" in
+ * the binding module (in SB and/or OVS databases).
+ */
+ OIF_INSTALLED, /* Interface flows programmed in OVS and binding
+ * marked "up" in the binding module.
+ */
OIF_MAX,
};
static const char *if_state_names[] = {
- [OIF_CLAIMED] = "CLAIMED",
- [OIF_INSTALL_FLOWS] = "INSTALL_FLOWS",
- [OIF_MARK_UP] = "MARK_UP",
- [OIF_MARK_DOWN] = "MARK_DOWN",
- [OIF_INSTALLED] = "INSTALLED",
+ [OIF_CLAIMED] = "CLAIMED",
+ [OIF_INSTALL_FLOWS] = "INSTALL_FLOWS",
+ [OIF_REM_OLD_OVN_INST] = "REM_OLD_OVN_INST",
+ [OIF_MARK_UP] = "MARK_UP",
+ [OIF_MARK_DOWN] = "MARK_DOWN",
+ [OIF_INSTALLED] = "INSTALLED",
};
/*
@@ -109,11 +114,26 @@ static const char *if_state_names[] = {
* | | | - remove ovn-installed from ovsdb | | |
* | | | mgr_update() | | |
* | +----------------------+ - sbrec_update_chassis if needed | | |
- * | | | | |
- * | | mgr_run(seqno rcvd) | | |
- * | | - set port up in sb | | |
- * | release_iface | - set ovn-installed in ovs | | |
- * | V | | |
+ * | | | | | |
+ * | | +----------------------------------------+ | | |
+ * | | | | | |
+ * | | mgr_run(seqno rcvd, ovn-installed present) | | | |
+ * | V | | | |
+ * | +--------------------+ | | | |
+ * | | | mgr_run() | | | |
+ * +--- | REM_OLD_OVN_INST | - remove ovn-installed in ovs | | | |
+ * | +--------------------+ | | | |
+ * | | | | | |
+ * | | | | | |
+ * | | mgr_update( ovn_installed not present) | | | |
+ * | | | | | |
+ * | | +-------------------------------------------+ | | |
+ * | | | | | |
+ * | | | mgr_run(seqno rcvd, ovn-installed not present) | | |
+ * | | | - set port up in sb | | |
+ * | | | - set ovn-installed in ovs | | |
+ * |release_iface | | | | |
+ * | V V | | |
* | +----------------------+ | | |
* | | | mgr_run() | | |
* +-- | MARK_UP | - set port up in sb | | |
@@ -155,6 +175,9 @@ struct if_status_mgr {
/* All local interfaces, mapping from 'iface-id' to 'struct ovs_iface'. */
struct shash ifaces;
+ /* local interfaces which need ovn-install removal */
+ struct shash ovn_uninstall_hash;
+
/* All local interfaces, stored per state. */
struct hmapx ifaces_per_state[OIF_MAX];
@@ -170,15 +193,20 @@ struct if_status_mgr {
static struct ovs_iface *ovs_iface_create(struct if_status_mgr *,
const char *iface_id,
enum if_state );
+static void add_to_ovn_uninstall_hash(struct if_status_mgr *, const char *,
+ const struct uuid *);
static void ovs_iface_destroy(struct if_status_mgr *, struct ovs_iface *);
+static void ovn_uninstall_hash_destroy(struct if_status_mgr *mgr, char *name);
static void ovs_iface_set_state(struct if_status_mgr *, struct ovs_iface *,
enum if_state);
static void if_status_mgr_update_bindings(
struct if_status_mgr *mgr, struct local_binding_data *binding_data,
const struct sbrec_chassis *,
+ const struct ovsrec_interface_table *iface_table,
bool sb_readonly, bool ovs_readonly);
+static void ovn_uninstall_hash_account_mem(const char *name, bool erase);
struct if_status_mgr *
if_status_mgr_create(void)
{
@@ -189,6 +217,7 @@ if_status_mgr_create(void)
hmapx_init(&mgr->ifaces_per_state[i]);
}
shash_init(&mgr->ifaces);
+ shash_init(&mgr->ovn_uninstall_hash);
return mgr;
}
@@ -202,6 +231,11 @@ if_status_mgr_clear(struct if_status_mgr *mgr)
}
ovs_assert(shash_is_empty(&mgr->ifaces));
+ SHASH_FOR_EACH_SAFE (node, &mgr->ovn_uninstall_hash) {
+ ovn_uninstall_hash_destroy(mgr, node->data);
+ }
+ ovs_assert(shash_is_empty(&mgr->ovn_uninstall_hash));
+
for (size_t i = 0; i < ARRAY_SIZE(mgr->ifaces_per_state); i++) {
ovs_assert(hmapx_is_empty(&mgr->ifaces_per_state[i]));
}
@@ -212,6 +246,7 @@ if_status_mgr_destroy(struct if_status_mgr *mgr)
{
if_status_mgr_clear(mgr);
shash_destroy(&mgr->ifaces);
+ shash_destroy(&mgr->ovn_uninstall_hash);
for (size_t i = 0; i < ARRAY_SIZE(mgr->ifaces_per_state); i++) {
hmapx_destroy(&mgr->ifaces_per_state[i]);
}
@@ -238,6 +273,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr,
switch (iface->state) {
case OIF_CLAIMED:
case OIF_INSTALL_FLOWS:
+ case OIF_REM_OLD_OVN_INST:
case OIF_MARK_UP:
/* Nothing to do here. */
break;
@@ -274,6 +310,7 @@ if_status_mgr_release_iface(struct if_status_mgr *mgr,
const char *iface_id)
/* Not yet fully installed interfaces can be safely deleted. */
ovs_iface_destroy(mgr, iface);
break;
+ case OIF_REM_OLD_OVN_INST:
case OIF_MARK_UP:
case OIF_INSTALLED:
/* Properly mark interfaces "down" if their flows were already
@@ -305,6 +342,7 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr, const
char *iface_id)
/* Not yet fully installed interfaces can be safely deleted. */
ovs_iface_destroy(mgr, iface);
break;
+ case OIF_REM_OLD_OVN_INST:
case OIF_MARK_UP:
case OIF_INSTALLED:
/* Properly mark interfaces "down" if their flows were already
@@ -346,12 +384,33 @@ if_status_handle_claims(struct if_status_mgr *mgr,
return rc;
}
+static void
+clean_ovn_installed(struct if_status_mgr *mgr,
+ const struct ovsrec_interface_table *iface_table)
+{
+ struct shash_node *node;
+
+ SHASH_FOR_EACH_SAFE (node, &mgr->ovn_uninstall_hash) {
+ const struct uuid *iface_uuid = node->data;
+ remove_ovn_installed_for_uuid(iface_table, iface_uuid);
+ free(node->data);
+ char *node_name = shash_steal(&mgr->ovn_uninstall_hash, node);
+ ovn_uninstall_hash_account_mem(node_name, true);
+ free(node_name);
+ }
+}
+
void
if_status_mgr_update(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
+ const struct ovsrec_interface_table *iface_table,
+ bool ovs_readonly,
bool sb_readonly)
{
+ if (!ovs_readonly) {
+ clean_ovn_installed(mgr, iface_table);
+ }
if (!binding_data) {
return;
}
@@ -359,6 +418,17 @@ if_status_mgr_update(struct if_status_mgr *mgr,
struct shash *bindings = &binding_data->bindings;
struct hmapx_node *node;
+ /* Move all interfaces that have been confirmed without ovn-installed,
+ * from OIF_REM_OLD_OVN_INST to OIF_MARK_UP.
+ */
+ HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_REM_OLD_OVN_INST]) {
+ struct ovs_iface *iface = node->data;
+
+ if (!local_binding_is_ovn_installed(bindings, iface->id)) {
+ ovs_iface_set_state(mgr, iface, OIF_MARK_UP);
+ }
+ }
+
/* Interfaces in OIF_MARK_UP/INSTALL_FLOWS state have already set their
* pb->chassis. However, the update might still be in fly (confirmation
* not received yet) or pb->chassis was overwitten by another chassis.
@@ -450,10 +520,23 @@ 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)
+{
+ 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);
+ }
+}
+
void
if_status_mgr_run(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
+ const struct ovsrec_interface_table *iface_table,
bool sb_readonly, bool ovs_readonly)
{
struct ofctrl_acked_seqnos *acked_seqnos =
@@ -471,12 +554,25 @@ if_status_mgr_run(struct if_status_mgr *mgr,
iface->install_seqno)) {
continue;
}
- ovs_iface_set_state(mgr, iface, OIF_MARK_UP);
+ /* Wait for ovn-installed to be absent before moving to MARK_UP state.
+ * Most of the times ovn-installed is already absent and hence we will
+ * not have to wait.
+ * If there is no binding_data, we can't determine if ovn-installed is
+ * present or not; hence also go to the OIF_REM_OLD_OVN_INST state.
+ */
+ if (!binding_data ||
+ local_binding_is_ovn_installed(&binding_data->bindings,
+ iface->id)) {
+ ovs_iface_set_state(mgr, iface, OIF_REM_OLD_OVN_INST);
+ } else {
+ ovs_iface_set_state(mgr, iface, OIF_MARK_UP);
+ }
}
ofctrl_acked_seqnos_destroy(acked_seqnos);
/* Update binding states. */
if_status_mgr_update_bindings(mgr, binding_data, chassis_rec,
+ iface_table,
sb_readonly, ovs_readonly);
}
@@ -492,6 +588,18 @@ ovs_iface_account_mem(const char *iface_id, bool erase)
}
}
+static void
+ovn_uninstall_hash_account_mem(const char *name, bool erase)
+{
+ uint32_t size = (strlen(name) + sizeof(struct uuid) +
+ sizeof(struct shash_node));
+ if (erase) {
+ ifaces_usage -= size;
+ } else {
+ ifaces_usage += size;
+ }
+}
+
static struct ovs_iface *
ovs_iface_create(struct if_status_mgr *mgr, const char *iface_id,
enum if_state state)
@@ -506,6 +614,16 @@ ovs_iface_create(struct if_status_mgr *mgr, const char
*iface_id,
return iface;
}
+static void
+add_to_ovn_uninstall_hash(struct if_status_mgr *mgr, const char *name,
+ const struct uuid *uuid)
+{
+ struct uuid *new_uuid = xzalloc(sizeof *new_uuid);
+ memcpy(new_uuid, uuid, sizeof(*new_uuid));
+ shash_add(&mgr->ovn_uninstall_hash, name, new_uuid);
+ ovn_uninstall_hash_account_mem(name, false);
+}
+
static void
ovs_iface_destroy(struct if_status_mgr *mgr, struct ovs_iface *iface)
{
@@ -521,6 +639,23 @@ ovs_iface_destroy(struct if_status_mgr *mgr, struct
ovs_iface *iface)
free(iface);
}
+static void
+ovn_uninstall_hash_destroy(struct if_status_mgr *mgr, char *name)
+{
+ struct shash_node *node = shash_find(&mgr->ovn_uninstall_hash, name);
+ char *node_name = NULL;
+ if (node) {
+ free(node->data);
+ VLOG_DBG("Interface name %s destroy", name);
+ node_name = shash_steal(&mgr->ovn_uninstall_hash, node);
+ ovn_uninstall_hash_account_mem(name, true);
+ free(node_name);
+ } else {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "Interface name %s not found", name);
+ }
+}
+
static void
ovs_iface_set_state(struct if_status_mgr *mgr, struct ovs_iface *iface,
enum if_state state)
@@ -539,6 +674,7 @@ static void
if_status_mgr_update_bindings(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
+ const struct ovsrec_interface_table *iface_table,
bool sb_readonly, bool ovs_readonly)
{
if (!binding_data) {
@@ -558,7 +694,17 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
sb_readonly, ovs_readonly);
}
- /* Notifiy the binding module to set "up" all bindings that have had
+ /* Notify the binding module to remove "ovn-installed" for all bindings
+ * in the OIF_REM_OLD_OVN_INST state.
+ */
+ HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_REM_OLD_OVN_INST]) {
+ struct ovs_iface *iface = node->data;
+
+ local_binding_remove_ovn_installed(bindings, iface_table, iface->id,
+ ovs_readonly);
+ }
+
+ /* Notify the binding module to set "up" all bindings that have had
* their flows installed but are not yet marked "up" in the binding
* module.
*/
diff --git a/controller/if-status.h b/controller/if-status.h
index 5bd187a25..203764946 100644
--- a/controller/if-status.h
+++ b/controller/if-status.h
@@ -17,6 +17,7 @@
#define IF_STATUS_H 1
#include "openvswitch/shash.h"
+#include "lib/vswitch-idl.h"
#include "binding.h"
@@ -35,9 +36,12 @@ void if_status_mgr_delete_iface(struct if_status_mgr *,
const char *iface_id);
void if_status_mgr_update(struct if_status_mgr *, struct local_binding_data *,
const struct sbrec_chassis *chassis,
+ const struct ovsrec_interface_table *iface_table,
+ bool ovs_readonly,
bool sb_readonly);
void if_status_mgr_run(struct if_status_mgr *mgr, struct local_binding_data *,
const struct sbrec_chassis *,
+ const struct ovsrec_interface_table *iface_table,
bool sb_readonly, bool ovs_readonly);
void if_status_mgr_get_memory_usage(struct if_status_mgr *mgr,
struct simap *usage);
@@ -48,5 +52,8 @@ bool if_status_handle_claims(struct if_status_mgr *mgr,
const struct sbrec_chassis *chassis_rec,
struct hmap *tracked_datapath,
bool sb_readonly);
+void if_status_mgr_remove_ovn_installed(struct if_status_mgr *mgr,
+ const char *name,
+ const struct uuid *uuid);
# endif /* controller/if-status.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index c094cb74d..2d8fee4d6 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -5225,6 +5225,9 @@ main(int argc, char *argv[])
stopwatch_start(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME,
time_msec());
if_status_mgr_update(if_mgr, binding_data, chassis,
+ ovsrec_interface_table_get(
+ ovs_idl_loop.idl),
+ !ovs_idl_txn,
!ovnsb_idl_txn);
stopwatch_stop(IF_STATUS_MGR_UPDATE_STOPWATCH_NAME,
time_msec());
@@ -5254,6 +5257,8 @@ main(int argc, char *argv[])
stopwatch_start(IF_STATUS_MGR_RUN_STOPWATCH_NAME,
time_msec());
if_status_mgr_run(if_mgr, binding_data, chassis,
+ ovsrec_interface_table_get(
+ ovs_idl_loop.idl),
!ovnsb_idl_txn, !ovs_idl_txn);
stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME,
time_msec());
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index b46f67636..cae44edee 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -10667,3 +10667,352 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
patch-.*/d
/connection dropped.*/d"])
AT_CLEANUP
])
+
+# This tests port->up/down and ovn-installed after adding and removing Ports
and Interfaces.
+# 3 Conditions x 3 tests:
+# - 3 Conditions:
+# - In normal conditions
+# - Remove interface while starting and stopping SB and Controller
+# - Remove and add back interface while starting and stopping SB and
Controller
+# - 3 tests:
+# - Add/Remove Logical Port
+# - Add/Remove iface-id
+# - Add/Remove Interface
+# Each tests/conditions checks for
+# - Port_binding->chassis
+# - Port up or down
+# - ovn-installed
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-install on slow ovsdb])
+AT_KEYWORDS([ovn-install])
+
+OVS_TRAFFIC_VSWITCHD_START()
+# Restart ovsdb-server, this time with tcp
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+start_daemon ovsdb-server --remote=punix:"$OVS_RUNDIR"/db.sock
--remote=ptcp:0:127.0.0.1
+
+ovn_start
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+check ovs-vsctl \
+ -- set Open_vSwitch . external-ids:system-id=hv1 \
+ -- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+ -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+ -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+ -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+PARSE_LISTENING_PORT([$ovs_base/ovsdb-server.log], [TCP_PORT])
+start_daemon ovn-controller tcp:127.0.0.1:$TCP_PORT
+
+check ovn-nbctl ls-add ls1
+check ovn-nbctl set Logical_Switch ls1 other_config:subnet=10.1.0.0/16
+
+check ovn-nbctl --wait=hv sync
+
+add_logical_ports() {
+ echo Adding logical ports
+ check ovn-nbctl lsp-add ls1 lsp1
+ check ovn-nbctl lsp-add ls1 lsp2
+}
+
+remove_logical_ports() {
+ echo Removing logical ports
+ check ovn-nbctl lsp-del lsp1
+ check ovn-nbctl lsp-del lsp2
+}
+
+add_ovs_interface() {
+ echo Adding interface $1 $2
+ ovs-vsctl --no-wait -- add-port br-int $1 \
+ -- set Interface $1 external_ids:iface-id=$2 \
+ -- set Interface $1 type=internal
+}
+add_ovs_interfaces() {
+ add_ovs_interface vif1 lsp1
+ add_ovs_interface vif2 lsp2
+}
+remove_ovs_interface() {
+ echo Removing interface $1
+ check ovs-vsctl --no-wait -- del-port $1
+}
+remove_ovs_interfaces() {
+ remove_ovs_interface vif1
+ remove_ovs_interface vif2
+}
+add_iface_ids() {
+ echo Adding iface-id vif1 lsp1
+ ovs-vsctl --no-wait -- set Interface vif1 external_ids:iface-id=lsp1
+ echo Adding iface-id vif2 lsp2
+ ovs-vsctl --no-wait -- set Interface vif2 external_ids:iface-id=lsp2
+}
+remove_iface_id() {
+ echo Removing iface-id $1
+ check ovs-vsctl remove Interface $1 external_ids iface-id
+}
+remove_iface_ids() {
+ remove_iface_id vif1
+ remove_iface_id vif2
+}
+wait_for_local_bindings() {
+ OVS_WAIT_UNTIL(
+ [test `ovs-appctl -t ovn-controller debug/dump-local-bindings | grep
interface | wc -l` -eq 2],
+ [kill -CONT $(cat ovn-sb/ovsdb-server.pid)]
+ )
+}
+sleep_sb() {
+ echo SB going to sleep
+ AT_CHECK([kill -STOP $(cat ovn-sb/ovsdb-server.pid)])
+}
+wake_up_sb() {
+ echo SB waking up
+ AT_CHECK([kill -CONT $(cat ovn-sb/ovsdb-server.pid)])
+}
+sleep_controller() {
+ echo Controller going to sleep
+ ovn-appctl debug/pause
+ OVS_WAIT_UNTIL([test x$(ovn-appctl -t ovn-controller debug/status) =
"xpaused"])
+}
+
+stop_ovsdb_controller_updates() {
+ TCP_PORT=$1
+ echo Stopping updates from ovn-controller to ovsdb using port $TCP_PORT
+ on_exit 'iptables -C INPUT -p tcp --destination-port $TCP_PORT -j DROP
2>/dev/null && iptables -D INPUT -p tcp --destination-port $TCP_PORT -j DROP'
+ iptables -A INPUT -p tcp --destination-port $TCP_PORT -j DROP
+}
+restart_ovsdb_controller_updates() {
+ TCP_PORT=$1
+ echo Restarting updates from ovn-controller to ovsdb
+ iptables -D INPUT -p tcp --destination-port $TCP_PORT -j DROP
+}
+wake_up_controller() {
+ echo Controller waking up
+ ovn-appctl debug/resume
+}
+ensure_controller_run() {
+# We want to make sure controller could run at least one full loop.
+# We can't use wait=hv as sb might be sleeping.
+# Use 2 ovn-appctl to guarentee that ovn-controller run the full loop, and not
just the unixctl handling
+ OVS_WAIT_UNTIL([test x$(ovn-appctl -t ovn-controller debug/status) =
"xrunning"])
+ OVS_WAIT_UNTIL([test x$(ovn-appctl -t ovn-controller debug/status) =
"xrunning"])
+}
+sleep_ovsdb() {
+ echo OVSDB going to sleep
+ AT_CHECK([kill -STOP $(cat ovsdb-server.pid)])
+}
+wake_up_ovsdb() {
+ echo OVSDB waking up
+ AT_CHECK([kill -CONT $(cat ovsdb-server.pid)])
+}
+check_ovn_installed() {
+ OVS_WAIT_UNTIL([test `ovs-vsctl get Interface vif1
external_ids:ovn-installed` = '"true"'])
+ OVS_WAIT_UNTIL([test `ovs-vsctl get Interface vif2
external_ids:ovn-installed` = '"true"'])
+}
+check_ovn_uninstalled() {
+ OVS_WAIT_UNTIL([test x`ovs-vsctl get Interface vif2
external_ids:ovn-installed` = x])
+ OVS_WAIT_UNTIL([test x`ovs-vsctl get Interface vif1
external_ids:ovn-installed` = x])
+}
+check_ports_up() {
+ OVS_WAIT_UNTIL([test `ovn-sbctl get Port_Binding lsp1 up` = 'true'])
+ OVS_WAIT_UNTIL([test `ovn-sbctl get Port_Binding lsp2 up` = 'true'])
+}
+check_ports_down() {
+ OVS_WAIT_UNTIL([test `ovn-sbctl get Port_Binding lsp1 up` = 'false'])
+ OVS_WAIT_UNTIL([test `ovn-sbctl get Port_Binding lsp2 up` = 'false'])
+}
+
+check_ports_bound() {
+ ch=$(fetch_column Chassis _uuid name=hv1)
+ wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch
+ wait_row_count Port_Binding 1 logical_port=lsp2 chassis=$ch
+}
+check_ports_unbound() {
+ wait_column "" Port_Binding chassis logical_port=lsp1
+ wait_column "" Port_Binding chassis logical_port=lsp2
+}
+add_logical_ports
+add_ovs_interfaces
+wait_for_local_bindings
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+############################################################
+########## Remove interface while removing iface-id ########
+############################################################
+AS_BOX(["Remove interface while removing iface-id"])
+stop_ovsdb_controller_updates $TCP_PORT
+remove_iface_id vif1
+ensure_controller_run
+# OVSDB should be seen as ro now
+remove_iface_id vif2
+ensure_controller_run
+# Controller delaying ovn-install removal for vif2 as ovsdb ro
+sleep_controller
+restart_ovsdb_controller_updates $TCP_PORT
+remove_ovs_interface vif2
+# vif2, for which we want to remove ovn-install, is deleted
+wake_up_controller
+check_ovn_uninstalled
+check_ports_down
+check_ports_unbound
+add_ovs_interface vif2 lsp2
+add_iface_ids
+check_ovn_installed
+check_ports_up
+check_ports_bound
+############################################################
+################### Add/Remove iface-id ####################
+############################################################
+AS_BOX(["iface-id removal and added back (no sleeping sb or controller)"])
+remove_iface_ids
+check_ovn_uninstalled
+check_ports_down
+check_ports_unbound
+add_iface_ids
+check_ovn_installed
+check_ports_up
+check_ports_bound
+
+AS_BOX(["iface-id removal"])
+sleep_sb
+remove_iface_ids
+ensure_controller_run
+sleep_controller
+wake_up_sb
+wake_up_controller
+check_ovn_uninstalled
+# Port_down not always set on iface-id removal
+# check_ports_down
+# Port_Binding(chassis) not always removed on iface-id removal
+# check_ports_unbound
+add_iface_ids
+check ovn-nbctl --wait=hv sync
+
+AS_BOX(["iface-id removal 2"])
+# Block IDL from ovn-controller to OVSDB
+stop_ovsdb_controller_updates $TCP_PORT
+remove_iface_id vif2
+ensure_controller_run
+
+# OVSDB should now be seen as read-only by ovn-controller
+remove_iface_id vif1
+ensure_controller_run
+
+# Restart connection from ovn-controller to OVSDB
+restart_ovsdb_controller_updates $TCP_PORT
+check_ovn_uninstalled
+check_ports_down
+check_ports_unbound
+
+add_iface_ids
+check ovn-nbctl --wait=hv sync
+
+AS_BOX(["iface-id removal and added back"])
+sleep_sb
+remove_iface_ids
+ensure_controller_run
+sleep_controller
+add_iface_ids
+wake_up_sb
+wake_up_controller
+check_ovn_installed
+check_ports_up
+check_ports_bound
+############################################################
+###################### Add/Remove Interface ################
+############################################################
+AS_BOX(["Interface removal and added back (no sleeping sb or controller)"])
+remove_ovs_interfaces
+check_ovn_uninstalled
+check_ports_down
+check_ports_unbound
+add_ovs_interfaces
+check_ovn_installed
+check_ports_up
+check_ports_bound
+check ovn-nbctl --wait=hv sync
+
+AS_BOX(["Interface removal"])
+sleep_sb
+remove_ovs_interfaces
+ensure_controller_run
+sleep_controller
+wake_up_sb
+wake_up_controller
+check_ovn_uninstalled
+# Port_down not always set on Interface removal
+# check_ports_down
+# Port_Binding(chassis) not always removed on Interface removal
+# check_ports_unbound
+add_ovs_interfaces
+check ovn-nbctl --wait=hv sync
+
+AS_BOX(["Interface removal and added back"])
+sleep_sb
+remove_ovs_interfaces
+ensure_controller_run
+sleep_controller
+add_ovs_interfaces
+wake_up_sb
+wake_up_controller
+check_ovn_installed
+check_ports_up
+check_ports_bound
+check ovn-nbctl --wait=hv sync
+############################################################
+###################### Add/Remove Logical Port #############
+############################################################
+AS_BOX(["Logical port removal and added back (no sleeping sb or controller)"])
+remove_logical_ports
+check_ovn_uninstalled
+check_ports_unbound
+sleep_ovsdb
+add_logical_ports
+ensure_controller_run
+wake_up_ovsdb
+check_ovn_installed
+check_ports_up
+check_ports_bound
+check ovn-nbctl --wait=hv sync
+
+AS_BOX(["Logical port removal"])
+sleep_sb
+remove_logical_ports
+ensure_controller_run
+sleep_controller
+wake_up_sb
+wake_up_controller
+check_ovn_uninstalled
+check_ports_unbound
+add_logical_ports
+check ovn-nbctl --wait=hv sync
+
+AS_BOX(["Logical port removal and added back"])
+sleep_sb
+remove_logical_ports
+ensure_controller_run
+sleep_controller
+add_logical_ports
+wake_up_sb
+wake_up_controller
+check_ovn_installed
+check_ports_up
+check_ports_bound
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])
+
--
2.31.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
