When a router is created or destroyed add the ability to en-lr-stateful
engine node to compute the new state of the lr_stateful table instead of
recalculating for the whole database.

Acked-by: Lorenzo Bianconi <[email protected]>
Reported-by: Dumitru Ceara <[email protected]>
Reported-at: https://issues.redhat.com/browse/FDP-757
Signed-off-by: Jacob Tanenbaum <[email protected]>

----

v4: added ack by Lorenzo.

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 50570b611..95c88ecea 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -146,6 +146,10 @@ lflow_northd_handler(struct engine_node *node,
         return EN_UNHANDLED;
     }
 
+    if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
+        return EN_UNHANDLED;
+    }
+
     const struct engine_context *eng_ctx = engine_get_context();
     struct lflow_data *lflow_data = data;
 
diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
index 2b6c0d0ff..cfb8fd1a1 100644
--- a/northd/en-lr-stateful.c
+++ b/northd/en-lr-stateful.c
@@ -93,6 +93,7 @@ en_lr_stateful_init(struct engine_node *node OVS_UNUSED,
     struct ed_type_lr_stateful *data = xzalloc(sizeof *data);
     lr_stateful_table_init(&data->table);
     hmapx_init(&data->trk_data.crupdated);
+    hmapx_init(&data->trk_data.deleted);
     return data;
 }
 
@@ -102,6 +103,7 @@ en_lr_stateful_cleanup(void *data_)
     struct ed_type_lr_stateful *data = data_;
     lr_stateful_table_destroy(&data->table);
     hmapx_destroy(&data->trk_data.crupdated);
+    hmapx_destroy(&data->trk_data.deleted);
 }
 
 void
@@ -110,6 +112,11 @@ en_lr_stateful_clear_tracked_data(void *data_)
     struct ed_type_lr_stateful *data = data_;
 
     hmapx_clear(&data->trk_data.crupdated);
+    struct hmapx_node *hmapx_node;
+    HMAPX_FOR_EACH_SAFE (hmapx_node, &data->trk_data.deleted) {
+        lr_stateful_record_destroy(hmapx_node->data);
+        hmapx_delete(&data->trk_data.deleted, hmapx_node);
+    }
     data->trk_data.vip_nats_changed = false;
 }
 
@@ -132,11 +139,10 @@ en_lr_stateful_run(struct engine_node *node, void *data_)
 }
 
 enum engine_input_handler_result
-lr_stateful_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
+lr_stateful_northd_handler(struct engine_node *node, void *data_)
 {
     struct northd_data *northd_data = engine_get_input_data("northd", node);
-    if (!northd_has_tracked_data(&northd_data->trk_data) ||
-        northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
+    if (!northd_has_tracked_data(&northd_data->trk_data)) {
         return EN_UNHANDLED;
     }
 
@@ -144,10 +150,6 @@ lr_stateful_northd_handler(struct engine_node *node, void 
*data OVS_UNUSED)
      * See (lr_stateful_get_input_data())
      *   1. northd_data->lr_datapaths
      *      This data gets updated when a logical router is created or deleted.
-     *      northd engine node presently falls back to full recompute when
-     *      this happens and so does this node.
-     *      Note: When we add I-P to the created/deleted logical routers, we
-     *      need to revisit this handler.
      *
      *      This node also accesses the router ports of the logical router
      *      (od->ports).  When these logical router ports gets updated,
@@ -164,6 +166,19 @@ lr_stateful_northd_handler(struct engine_node *node, void 
*data OVS_UNUSED)
      * and (3) if any.
      *
      * */
+    if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
+        struct ed_type_lr_stateful *data = data_;
+        struct lr_stateful_table  *table = &data->table;
+
+        if (hmap_count(&northd_data->lr_datapaths.datapaths) >
+            hmap_count(&table->entries)) {
+            table->array = xrealloc(table->array,
+                                    ods_size(&northd_data->lr_datapaths) *
+                                    sizeof *table->array);
+            return EN_HANDLED_UPDATED;
+        }
+    }
+
     return EN_HANDLED_UNCHANGED;
 }
 
@@ -348,6 +363,17 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void 
*data_)
     struct ed_type_lr_stateful *data = data_;
     struct hmapx_node *hmapx_node;
 
+    HMAPX_FOR_EACH (hmapx_node, &lr_nat_data->trk_data.deleted) {
+        struct lr_nat_record *lr_nat_rec = hmapx_node->data;
+        struct lr_stateful_record *lr_stateful_rec =
+            lr_stateful_table_find_by_index_(&data->table,
+                                             lr_nat_rec->lr_index);
+        if (lr_stateful_rec) {
+            hmap_remove(&data->table.entries, &lr_stateful_rec->key_node);
+            hmapx_add(&data->trk_data.deleted, lr_stateful_rec);
+        }
+    }
+
     HMAPX_FOR_EACH (hmapx_node, &lr_nat_data->trk_data.crupdated) {
         const struct lr_nat_record *lrnat_rec = hmapx_node->data;
         const struct ovn_datapath *od =
diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h
index 75975c935..3c953d569 100644
--- a/northd/en-lr-stateful.h
+++ b/northd/en-lr-stateful.h
@@ -107,6 +107,8 @@ struct lr_stateful_table {
 struct lr_stateful_tracked_data {
     /* Created or updated logical router with LB and/or NAT data. */
     struct hmapx crupdated; /* Stores 'struct lr_stateful_record'. */
+    /* Deleted logical router with LB and/or NAT data. */
+    struct hmapx deleted;
 
     /* Indicates if any router's NATs changed which were also LB vips
      * or vice versa. */
@@ -145,7 +147,9 @@ const struct lr_stateful_record 
*lr_stateful_table_find_by_index(
 static inline bool
 lr_stateful_has_tracked_data(struct lr_stateful_tracked_data *trk_data)
 {
-    return !hmapx_is_empty(&trk_data->crupdated) || trk_data->vip_nats_changed;
+    return !hmapx_is_empty(&trk_data->crupdated) ||
+           !hmapx_is_empty(&trk_data->deleted) ||
+           trk_data->vip_nats_changed;
 }
 
 static inline bool
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 3c358c232..4c8a6aca2 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12522,7 +12522,7 @@ check as northd ovn-appctl -t ovn-northd 
inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-add lr0
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
-check_engine_stats lr_stateful recompute nocompute
+check_engine_stats lr_stateful norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 check_engine_stats sync_to_sb_lb norecompute compute
 check_engine_stats lflow recompute nocompute
@@ -12533,7 +12533,7 @@ check ovn-nbctl --wait=sb lr-del lr0
 
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
-check_engine_stats lr_stateful recompute nocompute
+check_engine_stats lr_stateful norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 check_engine_stats sync_to_sb_lb norecompute compute
 check_engine_stats lflow recompute nocompute
-- 
2.51.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to