From: Numan Siddique
Since northd tracked data has the changed lb information,
the lflow change handler for northd inputs can now handle
lb updates incrementally. All the lflows generated for
each lb is stored in the ovn_lb_datapaths->lflow_ref and
this is used similar to how we handle ovn_port changes.
Acked-by: Han Zhou
Signed-off-by: Numan Siddique
---
northd/en-lflow.c | 11 +++--
northd/lb.c | 5 +-
northd/lb.h | 28 +++
northd/lflow-mgr.c | 51
northd/northd.c | 111 +---
northd/northd.h | 4 ++
tests/ovn-northd.at | 30
7 files changed, 198 insertions(+), 42 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index fafdc24465..c49d24d54b 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -123,11 +123,6 @@ lflow_northd_handler(struct engine_node *node,
return false;
}
-/* Fall back to recompute if load balancers have changed. */
-if (northd_has_lbs_in_tracked_data(_data->trk_data)) {
-return false;
-}
-
const struct engine_context *eng_ctx = engine_get_context();
struct lflow_data *lflow_data = data;
@@ -141,6 +136,12 @@ lflow_northd_handler(struct engine_node *node,
return false;
}
+if (!lflow_handle_northd_lb_changes(
+eng_ctx->ovnsb_idl_txn, _data->trk_data.trk_lbs,
+_input, lflow_data->lflow_table)) {
+return false;
+}
+
engine_set_node_state(node, EN_UPDATED);
return true;
}
diff --git a/northd/lb.c b/northd/lb.c
index e35569cb70..af0c92954c 100644
--- a/northd/lb.c
+++ b/northd/lb.c
@@ -23,7 +23,8 @@
/* OVN includes */
#include "lb.h"
-#include "lib/ovn-nb-idl.h"
+#include "lflow-mgr.h"
+#include "lib/lb.h"
#include "northd.h"
#include "ovn/lex.h"
@@ -563,6 +564,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb,
size_t n_ls_datapaths,
lb_dps->lb = lb;
lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
+lb_dps->lflow_ref = lflow_ref_create();
return lb_dps;
}
@@ -572,6 +574,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps)
{
bitmap_free(lb_dps->nb_lr_map);
bitmap_free(lb_dps->nb_ls_map);
+lflow_ref_destroy(lb_dps->lflow_ref);
free(lb_dps);
}
diff --git a/northd/lb.h b/northd/lb.h
index 00f81c3533..aa6616af41 100644
--- a/northd/lb.h
+++ b/northd/lb.h
@@ -128,6 +128,7 @@ void ovn_lb_group_reinit(
const struct nbrec_load_balancer_group *,
const struct hmap *lbs);
+struct lflow_ref;
struct ovn_lb_datapaths {
struct hmap_node hmap_node;
@@ -137,6 +138,33 @@ struct ovn_lb_datapaths {
size_t n_nb_lr;
unsigned long *nb_lr_map;
+
+/* Reference of lflows generated for this load balancer.
+ *
+ * This data is initialized and destroyed by the en_northd node, but
+ * populated and used only by the en_lflow node. Ideally this data should
+ * be maintained as part of en_lflow's data (struct lflow_data): a hash
+ * index from ovn_port key to lflows. However, it would be less efficient
+ * and more complex:
+ *
+ * 1. It would require an extra search (using the index) to find the
+ * lflows.
+ *
+ * 2. Building the index needs to be thread-safe, using either a global
+ * lock which is obviously less efficient, or hash-based lock array which
+ * is more complex.
+ *
+ * Maintaining the lflow_ref here is more straightforward. The drawback is
+ * that we need to keep in mind that this data belongs to en_lflow node,
+ * so never access it from any other nodes.
+ *
+ * 'lflow_ref' is used to reference logical flows generated for this
+ * load balancer.
+ *
+ * Note: lflow_ref is not thread safe. Only one thread should
+ * access ovn_lb_datapaths->lflow_ref at any given time.
+ */
+struct lflow_ref *lflow_ref;
};
struct ovn_lb_datapaths *ovn_lb_datapaths_create(const struct ovn_northd_lb *,
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index 3b423192bb..c45c124f64 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -535,7 +535,15 @@ struct lflow_ref_node {
/* The lflow. */
struct ovn_lflow *lflow;
-/* Index id of the datapath this lflow_ref_node belongs to. */
+/* Indicates whether the lflow was added with a dp_group using the
+ * ovn_lflow_add_with_dp_group() macro. */
+bool dpgrp_lflow;
+/* dpgrp bitmap and bitmap length. Valid only of dpgrp_lflow is true. */
+unsigned long *dpgrp_bitmap;
+size_t dpgrp_bitmap_len;
+
+/* Index id of the datapath this lflow_ref_node belongs to.
+ * Valid only if dpgrp_lflow is false. */
size_t dp_index;
/* Indicates if the lflow_ref_node for an lflow - L(M, A) is linked
@@ -573,7 +581,9 @@ lflow_ref_destroy(struct lflow_ref *lflow_ref)
/* Unlinks the lflows referenced by the
