Improve the efficiency of the requested-chassis feature by using
the new Southbound Port_Binding:requested_chassis column instead
of each chassis performing string comparison for every
Port_Binding record processed.
Signed-off-by: Frode Nordahl <frode.nord...@canonical.com>
---
controller/binding.c | 36 +++++++++++++++++++-----------------
controller/ovn-controller.c | 3 +++
controller/physical.c | 12 +++++++-----
3 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index c037b2352..a2eb86c89 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1056,11 +1056,15 @@ is_binding_lport_this_chassis(struct binding_lport
*b_lport,
static bool
can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec,
- const char *requested_chassis)
+ const struct sbrec_port_binding *pb)
{
- return !requested_chassis || !requested_chassis[0]
- || !strcmp(requested_chassis, chassis_rec->name)
- || !strcmp(requested_chassis, chassis_rec->hostname);
+ /* We need to check for presence of the requested-chassis option in
+ * addittion to checking the pb->requested_chassis column because this
+ * column will be set to NULL whenever the option points to a non-existent
+ * chassis. As the controller routinely clears its own chassis record this
+ * might occur more often than one might think. */
+ return !smap_get(&pb->options, "requested-chassis")
+ || chassis_rec == pb->requested_chassis;
}
/* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER,
@@ -1098,7 +1102,7 @@ release_binding_lport(const struct sbrec_chassis
*chassis_rec,
static bool
consider_vif_lport_(const struct sbrec_port_binding *pb,
- bool can_bind, const char *vif_chassis,
+ bool can_bind,
struct binding_ctx_in *b_ctx_in,
struct binding_ctx_out *b_ctx_out,
struct binding_lport *b_lport,
@@ -1139,7 +1143,10 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
"requested-chassis %s",
pb->logical_port,
b_ctx_in->chassis_rec->name,
- vif_chassis);
+ pb->requested_chassis ?
+ pb->requested_chassis->name : "(option points at "
+ "non-existent "
+ "chassis)");
}
}
@@ -1162,9 +1169,7 @@ consider_vif_lport(const struct sbrec_port_binding *pb,
struct local_binding *lbinding,
struct hmap *qos_map)
{
- const char *vif_chassis = smap_get(&pb->options, "requested-chassis");
- bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
- vif_chassis);
+ bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb);
if (!lbinding) {
lbinding = local_binding_find(&b_ctx_out->lbinding_data->bindings,
@@ -1194,8 +1199,8 @@ consider_vif_lport(const struct sbrec_port_binding *pb,
}
}
- return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
- b_ctx_out, b_lport, qos_map);
+ return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
+ b_lport, qos_map);
}
static bool
@@ -1279,12 +1284,9 @@ consider_container_lport(const struct sbrec_port_binding
*pb,
}
ovs_assert(parent_b_lport && parent_b_lport->pb);
- const char *vif_chassis = smap_get(&parent_b_lport->pb->options,
- "requested-chassis");
- bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
- vif_chassis);
+ bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb);
- return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out,
+ return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
container_b_lport, qos_map);
}
@@ -1333,7 +1335,7 @@ consider_virtual_lport(const struct sbrec_port_binding
*pb,
}
}
- if (!consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out,
+ if (!consider_vif_lport_(pb, true, b_ctx_in, b_ctx_out,
virtual_b_lport, qos_map)) {
return false;
}
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index a719beb0e..b0e4174aa 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -232,6 +232,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
sbrec_port_binding_add_clause_chassis(&pb, OVSDB_F_EQ,
&chassis->header_.uuid);
+ sbrec_port_binding_add_clause_requested_chassis(
+ &pb, OVSDB_F_EQ, &chassis->header_.uuid);
+
/* Ensure that we find out about l2gateway and l3gateway ports that
* should be present on this chassis. Otherwise, we might never find
* out about those ports, if their datapaths don't otherwise have a VIF
diff --git a/controller/physical.c b/controller/physical.c
index 0cfb158c8..780093638 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1066,11 +1066,13 @@ consider_port_binding(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
} else {
ofport = local_binding_get_lport_ofport(local_bindings,
binding->logical_port);
- const char *requested_chassis = smap_get(&binding->options,
- "requested-chassis");
- if (ofport && requested_chassis && requested_chassis[0] &&
- strcmp(requested_chassis, chassis->name) &&
- strcmp(requested_chassis, chassis->hostname)) {
+ /* We need to check for presence of the requested-chassis option in
+ * addittion to checking the pb->requested_chassis column because this
+ * column will be set to NULL whenever the option points to a
+ * non-existent chassis. As the controller routinely clears its own
+ * chassis record this might occur more often than one might think. */
+ if (ofport && smap_get(&binding->options, "requested-chassis")
+ && binding->requested_chassis != chassis) {
/* Even though there is an ofport for this port_binding, it is
* requested on a different chassis. So ignore this ofport.
*/
--
2.32.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
