northd: Refactor lflow management into a separate module.

ovn_lflow_add() and other related functions/macros are now moved
into a separate module - lflow-mgr.c.  This module maintains a
table 'struct lflow_table' for the logical flows.  lflow table
maintains a hmap to store the logical flows.

It also maintains the logical switch and router dp groups.

Previous commits which added lflow incremental processing for
the VIF logical ports, stored the references to
the logical ports' lflows using 'struct lflow_ref_list'.  This
struct is renamed to 'struct lflow_ref' and is part of lflow-mgr.c.
It is  modified a bit to store the resource to lflow references.

Example usage of 'struct lflow_ref'.

'struct ovn_port' maintains 2 instances of lflow_ref.  i,e

struct ovn_port {
   ...
   ...
   struct lflow_ref *lflow_ref;
   struct lflow_ref *stateful_lflow_ref;
};

All the logical flows generated by
build_lswitch_and_lrouter_iterate_by_lsp() uses the ovn_port->lflow_ref.

All the logical flows generated by build_lsp_lflows_for_lbnats()
uses the ovn_port->stateful_lflow_ref.

When handling the ovn_port changes incrementally, the lflows referenced
in 'struct ovn_port' are cleared and regenerated and synced to the
SB logical flows.

eg.

lflow_ref_clear_lflows(op->lflow_ref);
build_lswitch_and_lrouter_iterate_by_lsp(op, ...);
lflow_ref_sync_lflows_to_sb(op->lflow_ref, ...);

This patch does few more changes:
  -  Logical flows are now hashed without the logical
     datapaths.  If a logical flow is referenced by just one
     datapath, we don't rehash it.

  -  The synthetic 'hash' column of sbrec_logical_flow now
     doesn't use the logical datapath.  This means that
     when ovn-northd is updated/upgraded and has this commit,
     all the logical flows with 'logical_datapath' column
     set will get deleted and re-added causing some disruptions.

  -  With the commit [1] which added I-P support for logical
     port changes, multiple logical flows with same match 'M'
     and actions 'A' are generated and stored without the
     dp groups, which was not the case prior to
     that patch.
     One example to generate these lflows is:
             ovn-nbctl lsp-set-addresses sw0p1 "MAC1 IP1"
             ovn-nbctl lsp-set-addresses sw1p1 "MAC1 IP1"
	     ovn-nbctl lsp-set-addresses sw2p1 "MAC1 IP1"

     Now with this patch we go back to the earlier way.  i.e
     one logical flow with logical_dp_groups set.

  -  With this patch any updates to a logical port which
     doesn't result in new logical flows will not result in
     deletion and addition of same logical flows.
     Eg.
     ovn-nbctl set logical_switch_port sw0p1 external_ids:foo=bar
     will be a no-op to the SB logical flow table.

[1] - 8bbd678("northd: Incremental processing of VIF additions in 'lflow' node.")

Acked-by: Dumitru Ceara <[email protected]>
Signed-off-by: Numan Siddique <[email protected]>

TODO.rst

@@ -124,3 +124,9 @@ OVN To-do List
   * Improve sub-ports (with parent_port set) conditional monitoring; these
     are currently unconditionally monitored, even if ovn-monitor-all is
     set to false.
+
+* ovn-northd parallel build
+
+  * Move the lflow build parallel processing from northd.c to lflow-mgr.c
+    This would also ensure that the variables declared in northd.c
+    (eg. thread_lflow_counter) are not externed in lflow-mgr.c.

lib/ovn-util.c

@@ -622,13 +622,10 @@ ovn_pipeline_from_name(const char *pipeline)
 uint32_t
 sbrec_logical_flow_hash(const struct sbrec_logical_flow *lf)
 {
-    const struct sbrec_datapath_binding *ld = lf->logical_datapath;
-    uint32_t hash = ovn_logical_flow_hash(lf->table_id,
-                                          ovn_pipeline_from_name(lf->pipeline),
-                                          lf->priority, lf->match,
-                                          lf->actions);
-
-    return ld ? ovn_logical_flow_hash_datapath(&ld->header_.uuid, hash) : hash;
+    return ovn_logical_flow_hash(lf->table_id,
+                                 ovn_pipeline_from_name(lf->pipeline),
+                                 lf->priority, lf->match,
+                                 lf->actions);
 }
 
 uint32_t
@@ -641,13 +638,6 @@ ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline,
     return hash_string(actions, hash);
 }
 
-uint32_t
-ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath,
-                               uint32_t hash)
-{
-    return hash_add(hash, uuid_hash(logical_datapath));
-}
-
 
 struct tnlid_node {
     struct hmap_node hmap_node;

lib/ovn-util.h

@@ -146,8 +146,6 @@ uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *);
 uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline,
                                uint16_t priority,
                                const char *match, const char *actions);
-uint32_t ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath,
-                                        uint32_t hash);
 void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
                    const char *argv[] OVS_UNUSED, void *idl_);
 

northd/automake.mk

@@ -33,7 +33,9 @@ northd_ovn_northd_SOURCES = \
 	northd/inc-proc-northd.c \
 	northd/inc-proc-northd.h \
 	northd/ipam.c \
-	northd/ipam.h
+	northd/ipam.h \
+	northd/lflow-mgr.c \
+	northd/lflow-mgr.h
 northd_ovn_northd_LDADD = \
 	lib/libovn.la \
 	$(OVSDB_LIBDIR)/libovsdb.la \

northd/en-lflow.c

@@ -24,6 +24,7 @@
 #include "en-ls-stateful.h"
 #include "en-northd.h"
 #include "en-meters.h"
+#include "lflow-mgr.h"
 
 #include "lib/inc-proc-eng.h"
 #include "northd.h"
@@ -58,6 +59,8 @@ lflow_get_input_data(struct engine_node *node,
         EN_OVSDB_GET(engine_get_input("SB_multicast_group", node));
     lflow_input->sbrec_igmp_group_table =
         EN_OVSDB_GET(engine_get_input("SB_igmp_group", node));
+    lflow_input->sbrec_logical_dp_group_table =
+        EN_OVSDB_GET(engine_get_input("SB_logical_dp_group", node));
 
     lflow_input->sbrec_mcast_group_by_name_dp =
            engine_ovsdb_node_get_index(
@@ -90,17 +93,19 @@ void en_lflow_run(struct engine_node *node, void *data)
     struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections);
     lflow_input.bfd_connections = &bfd_connections;
 
+    stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
+
     struct lflow_data *lflow_data = data;
-    lflow_data_destroy(lflow_data);
-    lflow_data_init(lflow_data);
+    lflow_table_clear(lflow_data->lflow_table);
+    lflow_reset_northd_refs(&lflow_input);
 
-    stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
     build_bfd_table(eng_ctx->ovnsb_idl_txn,
                     lflow_input.nbrec_bfd_table,
                     lflow_input.sbrec_bfd_table,
                     lflow_input.lr_ports,
                     &bfd_connections);
-    build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input, &lflow_data->lflows);
+    build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input,
+                 lflow_data->lflow_table);
     bfd_cleanup_connections(lflow_input.nbrec_bfd_table,
                             &bfd_connections);
     hmap_destroy(&bfd_connections);
@@ -131,7 +136,8 @@ lflow_northd_handler(struct engine_node *node,
 
     if (!lflow_handle_northd_port_changes(eng_ctx->ovnsb_idl_txn,
                                           &northd_data->trk_data.trk_lsps,
-                                          &lflow_input, &lflow_data->lflows)) {
+                                          &lflow_input,
+                                          lflow_data->lflow_table)) {
         return false;
     }
 
@@ -160,11 +166,13 @@ void *en_lflow_init(struct engine_node *node OVS_UNUSED,
                      struct engine_arg *arg OVS_UNUSED)
 {
     struct lflow_data *data = xmalloc(sizeof *data);
-    lflow_data_init(data);
+    data->lflow_table = lflow_table_alloc();
+    lflow_table_init(data->lflow_table);
     return data;
 }
 
-void en_lflow_cleanup(void *data)
+void en_lflow_cleanup(void *data_)
 {
-    lflow_data_destroy(data);
+    struct lflow_data *data = data_;
+    lflow_table_destroy(data->lflow_table);
 }

northd/en-lflow.h

@@ -9,6 +9,12 @@
 
 #include "lib/inc-proc-eng.h"
 
+struct lflow_table;
+
+struct lflow_data {
+    struct lflow_table *lflow_table;
+};
+
 void en_lflow_run(struct engine_node *node, void *data);
 void *en_lflow_init(struct engine_node *node, struct engine_arg *arg);
 void en_lflow_cleanup(void *data);

northd/inc-proc-northd.c

@@ -99,7 +99,8 @@ static unixctl_cb_func chassis_features_list;
     SB_NODE(bfd, "bfd") \
     SB_NODE(fdb, "fdb") \
     SB_NODE(static_mac_binding, "static_mac_binding") \
-    SB_NODE(chassis_template_var, "chassis_template_var")
+    SB_NODE(chassis_template_var, "chassis_template_var") \
+    SB_NODE(logical_dp_group, "logical_dp_group")
 
 enum sb_engine_node {
 #define SB_NODE(NAME, NAME_STR) SB_##NAME,
@@ -229,6 +230,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_lflow, &en_sb_igmp_group, NULL);
     engine_add_input(&en_lflow, &en_lr_stateful, NULL);
     engine_add_input(&en_lflow, &en_ls_stateful, NULL);
+    engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL);
     engine_add_input(&en_lflow, &en_northd, lflow_northd_handler);
     engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler);