When interface was unbound, the port was not always set down and the
port_binding->chassis not always removed.
Fixes: a7c7d4519e50 ("controller: avoid recomputes triggered by SBDB
Port_Binding updates.")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2150905
Signed-off-by: Xavier Simonart <[email protected]>
---
controller/binding.c | 2 +-
controller/if-status.c | 44 +++++++++++++++++++++++++++++++++++++
controller/if-status.h | 4 ++++
controller/ovn-controller.c | 12 ++++++++++
tests/ovn.at | 12 ++++------
5 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index d85a17730..eb8d580c8 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -894,7 +894,6 @@ local_binding_set_down(struct shash *local_bindings, const
char *pb_name,
if (!sb_readonly && b_lport && b_lport->pb->n_up && b_lport->pb->up[0] &&
(!b_lport->pb->chassis || b_lport->pb->chassis == chassis_rec)) {
- VLOG_INFO("Setting lport %s down in Southbound", pb_name);
binding_lport_set_down(b_lport, sb_readonly);
LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
binding_lport_set_down(b_lport, sb_readonly);
@@ -3384,6 +3383,7 @@ binding_lport_set_down(struct binding_lport *b_lport,
bool sb_readonly)
if (sb_readonly || !b_lport || !b_lport->pb->n_up || !b_lport->pb->up[0]) {
return;
}
+ VLOG_INFO("Setting lport %s down in Southbound", b_lport->name);
bool up = false;
sbrec_port_binding_set_up(b_lport->pb, &up, 1);
diff --git a/controller/if-status.c b/controller/if-status.c
index c008aa79e..26535c9e8 100644
--- a/controller/if-status.c
+++ b/controller/if-status.c
@@ -16,6 +16,7 @@
#include <config.h>
#include "binding.h"
+#include "lport.h"
#include "if-status.h"
#include "ofctrl-seqno.h"
#include "simap.h"
@@ -75,6 +76,9 @@ enum if_state {
OIF_INSTALLED, /* Interface flows programmed in OVS and binding
* marked "up" in the binding module.
*/
+ OIF_UPDATE_PORT, /* Logical ports need to be set down, and pb->chassis
+ * removed.
+ */
OIF_MAX,
};
@@ -85,6 +89,7 @@ static const char *if_state_names[] = {
[OIF_MARK_UP] = "MARK_UP",
[OIF_MARK_DOWN] = "MARK_DOWN",
[OIF_INSTALLED] = "INSTALLED",
+ [OIF_UPDATE_PORT] = "UPDATE_PORT",
};
/*
@@ -264,6 +269,7 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr,
break;
case OIF_INSTALLED:
case OIF_MARK_DOWN:
+ case OIF_UPDATE_PORT:
ovs_iface_set_state(mgr, iface, OIF_CLAIMED);
break;
case OIF_MAX:
@@ -304,6 +310,7 @@ if_status_mgr_release_iface(struct if_status_mgr *mgr,
const char *iface_id)
ovs_iface_set_state(mgr, iface, OIF_MARK_DOWN);
break;
case OIF_MARK_DOWN:
+ case OIF_UPDATE_PORT:
/* Nothing to do here. */
break;
case OIF_MAX:
@@ -336,6 +343,7 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr, const
char *iface_id)
ovs_iface_set_state(mgr, iface, OIF_MARK_DOWN);
break;
case OIF_MARK_DOWN:
+ case OIF_UPDATE_PORT:
/* Nothing to do here. */
break;
case OIF_MAX:
@@ -344,6 +352,36 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr,
const char *iface_id)
}
}
+void
+if_status_handle_lports(struct if_status_mgr *mgr,
+ const struct sbrec_chassis *chassis_rec,
+ struct ovsdb_idl_index *sbrec_port_binding_by_name,
+ bool sb_readonly)
+{
+ if (sb_readonly) {
+ return;
+ }
+
+ struct hmapx_node *node;
+
+ HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_UPDATE_PORT]) {
+ struct ovs_iface *iface = node->data;
+ const struct sbrec_port_binding *pb = lport_lookup_by_name(
+ sbrec_port_binding_by_name, iface->id);
+
+ if ((pb == NULL) || sbrec_port_binding_is_deleted(pb)) {
+ VLOG_DBG("Removing lport %s from list of ports to set down",
+ iface->id);
+ } else {
+ bool up = false;
+ sbrec_port_binding_set_up(pb, &up, 1);
+ VLOG_INFO("Setting lport %s down in Southbound", iface->id);
+ set_pb_chassis_in_sbrec(pb, chassis_rec, false);
+ }
+ ovs_iface_destroy(mgr, node->data);
+ }
+}
+
bool
if_status_handle_claims(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
@@ -424,6 +462,12 @@ if_status_mgr_update(struct if_status_mgr *mgr,
HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_MARK_DOWN]) {
struct ovs_iface *iface = node->data;
+ if (!local_binding_find(bindings, iface->id) && sb_readonly) {
+ VLOG_DBG("%s not found in local_bindings and sb read only => "
+ "port down delayed", iface->id);
+ ovs_iface_set_state(mgr, iface, OIF_UPDATE_PORT);
+ continue;
+ }
if (!sb_readonly) {
local_binding_set_pb(bindings, iface->id, chassis_rec,
NULL, false);
diff --git a/controller/if-status.h b/controller/if-status.h
index 5bd187a25..593597af7 100644
--- a/controller/if-status.h
+++ b/controller/if-status.h
@@ -48,5 +48,9 @@ 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_handle_lports(struct if_status_mgr *mgr,
+ const struct sbrec_chassis *chassis_rec,
+ struct ovsdb_idl_index *sbrec_port_binding_by_name,
+ bool sb_readonly);
# endif /* controller/if-status.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 814a88117..4094eb56e 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1603,6 +1603,11 @@ runtime_data_sb_ro_handler(struct engine_node *node,
void *data)
engine_get_input("SB_chassis", node),
"name");
+ struct ovsdb_idl_index *sbrec_port_binding_by_name =
+ engine_ovsdb_node_get_index(
+ engine_get_input("SB_port_binding", node),
+ "name");
+
if (chassis_id) {
chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
}
@@ -1620,6 +1625,13 @@ runtime_data_sb_ro_handler(struct engine_node *node,
void *data)
engine_set_node_state(node, EN_UPDATED);
rt_data->tracked = true;
}
+
+ if (sbrec_port_binding_by_name) {
+ if_status_handle_lports(ctrl_ctx->if_mgr,
+ chassis,
+ sbrec_port_binding_by_name,
+ sb_readonly);
+ }
}
return true;
}
diff --git a/tests/ovn.at b/tests/ovn.at
index a0378a728..a849a483d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -34403,10 +34403,8 @@ 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
+check_ports_down
+check_ports_unbound
add_iface_ids
check ovn-nbctl --wait=hv sync
@@ -34443,10 +34441,8 @@ 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
+check_ports_down
+check_ports_unbound
add_ovs_interfaces
check ovn-nbctl --wait=hv sync
--
2.31.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
