When a router is created or destroyed add the ability to the en-northd
engine node to compute the ovn_datapath for the router instead of
recalculating the whole database.

Just like updating a router there are limitations including no router
ports and nothing in the COPP table. Differently from updating a logical
router during creation flags can be set because since there are no ports
there is no way setting those flags will effect other objects.

Reported-by: Dumitru Ceara <dce...@redhat.com>
Reported-at: https://issues.redhat.com/browse/FDP-757
Signed-off-by: Jacob Tanenbaum <jtane...@redhat.com>

----

v4: cleared the vector entry for deleted routers
    changed the smap_get for chassis options for new routers

diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c
index 5a7b857f4..0583e34a5 100644
--- a/northd/en-lr-nat.c
+++ b/northd/en-lr-nat.c
@@ -125,6 +125,10 @@ lr_nat_northd_handler(struct engine_node *node, void 
*data_)
         return EN_UNHANDLED;
     }
 
+    if (northd_has_lrouters_in_tracked_data(&northd_data->trk_data)) {
+        return EN_UNHANDLED;
+    }
+
     if (!northd_has_lr_nats_in_tracked_data(&northd_data->trk_data)) {
         return EN_HANDLED_UNCHANGED;
     }
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 56b25d271..4006d91c8 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -208,7 +208,8 @@ northd_nb_logical_router_handler(struct engine_node *node,
         return EN_UNHANDLED;
     }
 
-    if (northd_has_lr_nats_in_tracked_data(&nd->trk_data)) {
+    if (northd_has_lr_nats_in_tracked_data(&nd->trk_data) ||
+        northd_has_lrouters_in_tracked_data(&nd->trk_data)) {
         return EN_HANDLED_UPDATED;
     }
 
diff --git a/northd/northd.c b/northd/northd.c
index 8b5413ef3..eaecb424b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4051,6 +4051,7 @@ destroy_northd_data_tracked_changes(struct northd_data 
*nd)
     hmapx_clear(&trk_changes->ls_with_changed_acls);
     hmapx_clear(&trk_changes->ls_with_changed_ipam);
     destroy_tracked_dps(&trk_changes->trk_switches);
+    destroy_tracked_dps(&trk_changes->trk_routers);
     trk_changes->type = NORTHD_TRACKED_NONE;
 }
 
@@ -4059,6 +4060,8 @@ init_northd_tracked_data(struct northd_data *nd)
 {
     struct northd_tracked_data *trk_data = &nd->trk_data;
     trk_data->type = NORTHD_TRACKED_NONE;
+    hmapx_init(&trk_data->trk_routers.crupdated);
+    hmapx_init(&trk_data->trk_routers.deleted);
     hmapx_init(&trk_data->trk_switches.crupdated);
     hmapx_init(&trk_data->trk_switches.deleted);
     hmapx_init(&trk_data->trk_lsps.created);
@@ -4089,6 +4092,8 @@ destroy_northd_tracked_data(struct northd_data *nd)
     hmapx_destroy(&trk_data->ls_with_changed_lbs);
     hmapx_destroy(&trk_data->ls_with_changed_acls);
     hmapx_destroy(&trk_data->ls_with_changed_ipam);
+    hmapx_destroy(&trk_data->trk_routers.crupdated);
+    hmapx_destroy(&trk_data->trk_routers.deleted);
 }
 
 /* Check if a changed LSP can be handled incrementally within the I-P engine
@@ -4931,13 +4936,37 @@ northd_handle_lr_changes(const struct northd_input *ni,
 {
     const struct nbrec_logical_router *changed_lr;
 
-    if (!hmapx_is_empty(&ni->synced_lrs->new) ||
-        !hmapx_is_empty(&ni->synced_lrs->deleted) ||
-        hmapx_is_empty(&ni->synced_lrs->updated)) {
+    if (hmapx_is_empty(&ni->synced_lrs->new) &&
+        hmapx_is_empty(&ni->synced_lrs->updated) &&
+        hmapx_is_empty(&ni->synced_lrs->deleted)) {
         goto fail;
     }
 
     struct hmapx_node *node;
+    HMAPX_FOR_EACH (node, &ni->synced_lrs->new) {
+        const struct ovn_synced_logical_router *synced = node->data;
+        const struct nbrec_logical_router *new_lr = synced->nb;
+
+        /* If the logical router is create with the below columns set,
+         * then we can't handle it in the incremental processor goto fail. */
+        if (new_lr->copp || (new_lr->n_ports > 0)) {
+            goto fail;
+        }
+        struct ovn_datapath *od = ovn_datapath_create(
+            &nd->lr_datapaths.datapaths, &new_lr->header_.uuid,
+            NULL, new_lr, synced->sdp);
+        ods_assign_array_index(&nd->lr_datapaths, od);
+
+        od->is_gw_router = !!smap_get(&od->nbr->options, "chassis");
+        od->dynamic_routing = smap_get_bool(&od->nbr->options,
+                                            "dynamic-routing", false);
+        od->dynamic_routing_redistribute =
+            parse_dynamic_routing_redistribute(&od->nbr->options, DRRM_NONE,
+                                               od->nbr->name);
+        hmapx_add(&nd->trk_data.trk_nat_lrs,od);
+        hmapx_add(&nd->trk_data.trk_routers.crupdated, od);
+    }
+
     HMAPX_FOR_EACH (node, &ni->synced_lrs->updated) {
         const struct ovn_synced_logical_router *synced = node->data;
         changed_lr = synced->nb;
@@ -4965,9 +4994,50 @@ northd_handle_lr_changes(const struct northd_input *ni,
         }
     }
 
+    HMAPX_FOR_EACH (node, &ni->synced_lrs->deleted) {
+        const struct ovn_synced_logical_router *synced = node->data;
+        const struct nbrec_logical_router *deleted_lr = synced->nb;
+
+        struct ovn_datapath *od = ovn_datapath_find_(
+                                    &nd->lr_datapaths.datapaths,
+                                    &deleted_lr->header_.uuid);
+        if (!od) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+            VLOG_WARN_RL(&rl, "Internal error: a tracked deleted LR doesn't "
+                         "exist in lr_datapaths: "UUID_FMT,
+                         UUID_ARGS(&deleted_lr->header_.uuid));
+            goto fail;
+        }
+
+        if (deleted_lr->copp ||
+            deleted_lr->n_ports > 0 ||
+            deleted_lr->n_policies > 0 ||
+            deleted_lr->n_static_routes > 0) {
+            goto fail;
+        }
+        /* Since there are no ports the lr_group should be empty. If
+         * a logical router is deleted before the db gets
+         * recalculated nothing will create the lr_group. */
+        if (od->lr_group) {
+            free(od->lr_group->router_dps);
+            free(od->lr_group);
+        }
+
+        hmap_remove(&nd->lr_datapaths.datapaths, &od->key_node);
+        vector_get(&nd->lr_datapaths.dps,
+                   od->index, struct ovn_datapath *) = NULL;
+        dynamic_bitmap_set0(&nd->lr_datapaths.dps_index_map, od->index);
+
+        hmapx_add(&nd->trk_data.trk_routers.deleted, od);
+    }
+
     if (!hmapx_is_empty(&nd->trk_data.trk_nat_lrs)) {
         nd->trk_data.type |= NORTHD_TRACKED_LR_NATS;
     }
+    if (!hmapx_is_empty(&nd->trk_data.trk_routers.crupdated) ||
+        !hmapx_is_empty(&nd->trk_data.trk_routers.deleted)) {
+        nd->trk_data.type |= NORTHD_TRACKED_ROUTERS;
+    }
 
     return true;
 fail:
diff --git a/northd/northd.h b/northd/northd.h
index ca35eb91e..6836831d7 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -155,6 +155,7 @@ enum northd_tracked_data_type {
     NORTHD_TRACKED_LS_LBS   = (1 << 3),
     NORTHD_TRACKED_LS_ACLS  = (1 << 4),
     NORTHD_TRACKED_SWITCHES = (1 << 5),
+    NORTHD_TRACKED_ROUTERS  = (1 << 6),
 };
 
 /* Track what's changed in the northd engine node.
@@ -164,6 +165,7 @@ struct northd_tracked_data {
     /* Indicates the type of data tracked.  One or all of NORTHD_TRACKED_*. */
     enum northd_tracked_data_type type;
     struct tracked_dps trk_switches;
+    struct tracked_dps trk_routers;
     struct tracked_ovn_ports trk_lsps;
     struct tracked_lbs trk_lbs;
 
@@ -1014,6 +1016,13 @@ northd_has_lswitches_in_tracked_data(
     return trk_nd_changes->type & NORTHD_TRACKED_SWITCHES;
 }
 
+static inline bool
+northd_has_lrouters_in_tracked_data(
+        struct northd_tracked_data *trk_nd_changes)
+{
+    return trk_nd_changes->type & NORTHD_TRACKED_ROUTERS;
+}
+
 /* Returns 'true' if the IPv4 'addr' is on the same subnet with one of the
  * IPs configured on the router port.
  */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index c9e998129..2d62a2399 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12520,14 +12520,26 @@ check ovn-nbctl --wait=sb lsp-add sw0 sw0p1 -- 
lsp-set-addresses sw0p1 "00:00:20
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-add lr0
-check_engine_stats northd recompute nocompute
+check_engine_stats northd norecompute compute
 check_engine_stats lr_nat recompute nocompute
 check_engine_stats lr_stateful recompute nocompute
 check_engine_stats sync_to_sb_pb recompute nocompute
-check_engine_stats sync_to_sb_lb recompute nocompute
+check_engine_stats sync_to_sb_lb norecompute compute
+check_engine_stats lflow recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lr-del lr0
+
+check_engine_stats northd norecompute compute
+check_engine_stats lr_nat recompute nocompute
+check_engine_stats lr_stateful recompute nocompute
+check_engine_stats sync_to_sb_pb recompute nocompute
+check_engine_stats sync_to_sb_lb norecompute compute
 check_engine_stats lflow recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+check ovn-nbctl --wait=sb lr-add lr0
 # Adding a logical router port should result in recompute
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
-- 
2.51.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to