[ovs-dev] [PATCH ovn] ovn-northd: Don't generate identical flows for same LBs with different protocol.
Ilya Maximets
i.maximets at ovn.org
Fri Aug 27 15:24:52 UTC 2021
It's common for CMS (e.g., ovn-kubernetes) to create 3 identical load
balancers, one per protocol (tcp, udp, sctp). However, if VIPs of
these load balancers has no ports specified (!vip_port), northd will
generate identical logical flows for them. 2 of 3 such flows will be
just discarded, so it's better to not build them form the beginning.
For example, in an ovn-heater's 120 node density-heavy scenario we
have 3 load balancers with 15K VIPs in each. One for tcp, one for
udp and one for sctp. In this case, ovn-northd generates 26M of
logical flows in total. ~7.5M of them are flows for a single load
balancer. 2 * 7.5M = 15M are identical to the first 7.5M and just
discarded.
Let's find all these identical load balancers and skip while building
logical flows. With this change, 15M of redundant logical flows are
not generated saving ~1.5 seconds of the CPU time per run.
Comparison function and the loop looks heavy, but in testing it takes
only a few milliseconds on these large load balancers.
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
---
The better option might be to allow multiple protocols being configured
per load balancer, but this will require big API/schema/etc changes
and may reuire re-desing of certain northd/ovn-controller logic.
lib/lb.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
lib/lb.h | 4 ++++
northd/ovn-northd.c | 35 ++++++++++++++++++++++++++++
3 files changed, 95 insertions(+)
diff --git a/lib/lb.c b/lib/lb.c
index 7b0ed1abe..fb12c3457 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -168,6 +168,7 @@ ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb)
bool is_sctp = nullable_string_is_equal(nbrec_lb->protocol, "sctp");
struct ovn_northd_lb *lb = xzalloc(sizeof *lb);
+ lb->skip_lflow_build = false;
lb->nlb = nbrec_lb;
lb->proto = is_udp ? "udp" : is_sctp ? "sctp" : "tcp";
lb->n_vips = smap_count(&nbrec_lb->vips);
@@ -238,6 +239,61 @@ ovn_northd_lb_find(struct hmap *lbs, const struct uuid *uuid)
return NULL;
}
+/* Compares two load balancers for equality ignoring the 'protocol'. */
+bool
+ovn_northd_lb_equal_except_for_proto(const struct ovn_northd_lb *lb1,
+ const struct ovn_northd_lb *lb2)
+{
+ /* It's much more convenient to compare Northbound DB configuration. */
+ const struct nbrec_load_balancer *lb_a = lb1->nlb;
+ const struct nbrec_load_balancer *lb_b = lb2->nlb;
+
+ if (!smap_equal(&lb_a->external_ids, &lb_b->external_ids)) {
+ return false;
+ }
+ if (lb_a->n_health_check != lb_b->n_health_check) {
+ return false;
+ }
+ if (lb_a->n_health_check
+ && !memcmp(lb_a->health_check, lb_b->health_check,
+ lb_a->n_health_check * sizeof *lb_a->health_check)) {
+ return false;
+ }
+ if (!smap_equal(&lb_a->ip_port_mappings, &lb_b->ip_port_mappings)) {
+ return false;
+ }
+ if (!smap_equal(&lb_a->options, &lb_b->options)) {
+ return false;
+ }
+ if (lb_a->n_selection_fields != lb_b->n_selection_fields) {
+ return false;
+ }
+ if (lb_a->n_selection_fields &&
+ memcmp(lb_a->selection_fields, lb_b->selection_fields,
+ lb_a->n_selection_fields * sizeof *lb_a->selection_fields)) {
+ return false;
+ }
+ if (!smap_equal(&lb_a->vips, &lb_b->vips)) {
+ return false;
+ }
+
+ /* Below fields are not easily accessible from the Nb DB entry, so
+ * comparing parsed versions. */
+ if (lb1->n_nb_ls != lb2->n_nb_ls || lb1->n_nb_lr != lb2->n_nb_lr) {
+ return false;
+ }
+ if (lb1->n_nb_ls
+ && memcmp(lb1->nb_ls, lb1->nb_ls, lb1->n_nb_ls * sizeof *lb1->nb_ls)) {
+ return false;
+ }
+ if (lb1->n_nb_lr
+ && memcmp(lb1->nb_lr, lb1->nb_lr, lb1->n_nb_lr * sizeof *lb1->nb_lr)) {
+ return false;
+ }
+
+ return true;
+}
+
void
ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od)
{
diff --git a/lib/lb.h b/lib/lb.h
index 832ed31fb..8e92338a3 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -50,6 +50,8 @@ struct ovn_northd_lb {
size_t n_nb_lr;
size_t n_allocated_nb_lr;
struct ovn_datapath **nb_lr;
+
+ bool skip_lflow_build;
};
struct ovn_lb_vip {
@@ -87,6 +89,8 @@ struct ovn_northd_lb_backend {
struct ovn_northd_lb *ovn_northd_lb_create(const struct nbrec_load_balancer *);
struct ovn_northd_lb * ovn_northd_lb_find(struct hmap *, const struct uuid *);
+bool ovn_northd_lb_equal_except_for_proto(const struct ovn_northd_lb *,
+ const struct ovn_northd_lb *);
void ovn_northd_lb_destroy(struct ovn_northd_lb *);
void
ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index af413aba4..d1efc28f4 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3732,6 +3732,35 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *datapaths,
sbrec_datapath_binding_set_load_balancers(od->sb, NULL, 0);
}
}
+
+ /* Northd doesn't use protocol in case vips has no ports specified.
+ * And it's also common that user configures several identical load
+ * balancers, one per protocol. We need to find them and use only one
+ * for logical flow construction. Logical flows will be identical,
+ * so it's faster to just not build them. */
+ struct ovn_northd_lb *lb2;
+ HMAP_FOR_EACH (lb, hmap_node, lbs) {
+ if (lb->skip_lflow_build) {
+ continue;
+ }
+ for (size_t i = 0; i < lb->n_vips; i++) {
+ if (lb->vips[i].vip_port) {
+ goto next;
+ }
+ }
+ HMAP_FOR_EACH (lb2, hmap_node, lbs) {
+ if (lb2 == lb || lb2->skip_lflow_build) {
+ continue;
+ }
+ if (ovn_northd_lb_equal_except_for_proto(lb, lb2)) {
+ /* Load balancer still referenced from logical switch or
+ * router, so we can't destroy it here. */
+ lb2->skip_lflow_build = true;
+ }
+ }
+next:;
+ }
+
}
static void
@@ -12772,6 +12801,9 @@ build_lflows_thread(void *arg)
if (stop_parallel_processing()) {
return NULL;
}
+ if (lb->skip_lflow_build) {
+ continue;
+ }
build_lswitch_arp_nd_service_monitor(lb, lsi->lflows,
&lsi->match,
&lsi->actions);
@@ -12943,6 +12975,9 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
build_lswitch_and_lrouter_iterate_by_op(op, &lsi);
}
HMAP_FOR_EACH (lb, hmap_node, lbs) {
+ if (lb->skip_lflow_build) {
+ continue;
+ }
build_lswitch_arp_nd_service_monitor(lb, lsi.lflows,
&lsi.actions,
&lsi.match);
--
2.31.1
More information about the dev
mailing list