diff mbox series

[ovs-dev,v1,2/2] Reapply "northd: Add and delete logical routers in en-lflow engine node.".

Message ID 20260209162312.408465-3-jtanenba@redhat.com
State Changes Requested
Headers show
Series Reintroduce incremental processing for | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Jacob Tanenbaum Feb. 9, 2026, 4:23 p.m. UTC
This reverts commit 75dd3b02f1afc3af5d3f99878f8399812f6c2669.

Additionally this includes a few small rebased changes that are required
to compile and run the tests successfully

Reported-by: Mark Michelson <mmichels@redhat.com>
Reported-at: https://issues.redhat.com/browse/FDP-2747
Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>

Comments

Jacob Tanenbaum Feb. 10, 2026, 6:03 p.m. UTC | #1
Recheck-request: github-robot-_Build_and_Test



On Mon, Feb 9, 2026 at 11:23 AM Jacob Tanenbaum <jtanenba@redhat.com> wrote:

> This reverts commit 75dd3b02f1afc3af5d3f99878f8399812f6c2669.
>
> Additionally this includes a few small rebased changes that are required
> to compile and run the tests successfully
>
> Reported-by: Mark Michelson <mmichels@redhat.com>
> Reported-at: https://issues.redhat.com/browse/FDP-2747
> Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
>
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 41a5a97ff..971876785 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -157,16 +157,19 @@ 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;
>
>      struct lflow_input lflow_input;
>      lflow_get_input_data(node, &lflow_input);
>
> +    if (!lflow_handle_northd_lr_changes(eng_ctx->ovnsb_idl_txn,
> +
> &northd_data->trk_data.trk_routers,
> +                                        &lflow_input,
> +                                        lflow_data->lflow_table)) {
> +        return EN_UNHANDLED;
> +    }
> +
>      if (!lflow_handle_northd_port_changes(eng_ctx->ovnsb_idl_txn,
>                                            &northd_data->trk_data.trk_lsps,
>                                            &lflow_input,
> diff --git a/northd/northd.c b/northd/northd.c
> index b4bb4ba6d..aefb0b12a 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -568,6 +568,7 @@ ovn_datapath_create(struct hmap *datapaths, const
> struct uuid *key,
>      od->ipam_info_initialized = false;
>      od->tunnel_key = sdp->sb_dp->tunnel_key;
>      init_mcast_info_for_datapath(od);
> +    od->datapath_lflows = lflow_ref_create();
>      return od;
>  }
>
> @@ -598,6 +599,7 @@ ovn_datapath_destroy(struct ovn_datapath *od)
>          destroy_ports_for_datapath(od);
>          hmapx_destroy(&od->phys_ports);
>          sset_destroy(&od->router_ips);
> +        lflow_ref_destroy(od->datapath_lflows);
>          free(od);
>      }
>  }
> @@ -19611,6 +19613,7 @@ lflow_reset_northd_refs(struct lflow_input
> *lflow_input)
>      struct ls_stateful_record *ls_stateful_rec;
>      struct ovn_lb_datapaths *lb_dps;
>      struct ovn_port *op;
> +    const struct ovn_datapath *od;
>
>      LR_STATEFUL_TABLE_FOR_EACH (lr_stateful_rec,
>                                  lflow_input->lr_stateful_table) {
> @@ -19635,6 +19638,72 @@ lflow_reset_northd_refs(struct lflow_input
> *lflow_input)
>      HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
>          lflow_ref_clear(lb_dps->lflow_ref);
>      }
> +
> +    HMAP_FOR_EACH (od, key_node, &lflow_input->lr_datapaths->datapaths) {
> +        lflow_ref_clear(od->datapath_lflows);
> +    }
> +
> +    HMAP_FOR_EACH (od, key_node, &lflow_input->ls_datapaths->datapaths) {
> +        lflow_ref_clear(od->datapath_lflows);
> +    }
> +}
> +
> +bool
> +lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsb_txn,
> +                                struct tracked_dps *tracked_lrs,
> +                                struct lflow_input *lflow_input,
> +                                struct lflow_table *lflows)
> +{
> +    bool handled = true;
> +    struct hmapx_node *hmapx_node;
> +    HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->deleted) {
> +        struct ovn_datapath *od = hmapx_node->data;
> +        handled = lflow_ref_resync_flows(
> +            od->datapath_lflows, lflows, ovnsb_txn, lflow_input->dps,
> +            lflow_input->ovn_internal_version_changed,
> +            lflow_input->sbrec_logical_flow_table,
> +            lflow_input->sbrec_logical_dp_group_table);
> +        if (!handled) {
> +            return handled;
> +        }
> +    }
> +
> +    struct lswitch_flow_build_info lsi = {
> +        .lr_datapaths = lflow_input->lr_datapaths,
> +        .lr_stateful_table = lflow_input->lr_stateful_table,
> +        .lflows =lflows,
> +        .route_data = lflow_input->route_data,
> +        .route_tables = lflow_input->route_tables,
> +        .route_policies = lflow_input->route_policies,
> +        .match = DS_EMPTY_INITIALIZER,
> +        .actions = DS_EMPTY_INITIALIZER,
> +    };
> +    HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->crupdated) {
> +        struct ovn_datapath *od = hmapx_node->data;
> +
> +        lflow_ref_unlink_lflows(od->datapath_lflows);
> +        build_lswitch_and_lrouter_iterate_by_lr(od, &lsi);
> +    }
> +
> +    /* We need to make sure that all datapath groups are allocated before
> +     * trying to sync logical flows. Otherwise, we would need to recompute
> +     * those datapath groups within those flows over and over again. */
> +    HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->crupdated) {
> +        struct ovn_datapath *od = hmapx_node->data;
> +
> +        handled = lflow_ref_sync_lflows(
> +            od->datapath_lflows, lflows, ovnsb_txn, lflow_input->dps,
> +            lflow_input->ovn_internal_version_changed,
> +            lflow_input->sbrec_logical_flow_table,
> +            lflow_input->sbrec_logical_dp_group_table);
> +        if (!handled) {
> +            break;
> +        }
> +    }
> +
> +    ds_destroy(&lsi.actions);
> +    ds_destroy(&lsi.match);
> +    return handled;
>  }
>
>  bool
> diff --git a/northd/northd.h b/northd/northd.h
> index eb5c15f34..acda62bc6 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -477,11 +477,8 @@ struct ovn_datapath {
>      /* Map of ovn_port objects belonging to this datapath.
>       * This map doesn't include derived ports. */
>      struct hmap ports;
> -
> -    /* XXX Reference to the lflows belonging to this datapath currently
> router
> -     * only lflows. This is NULL for now just to not waste any space
> -     * as it's not used by anything right now, but it wasn't worth
> reverting
> -     * all the related changes. */
> +    /* Reference to the lflows belonging to this datapath currently router
> +     * only lflows. */
>      struct lflow_ref *datapath_lflows;
>  };
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 512e42036..a5fa1c6a9 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -12652,7 +12652,7 @@ check_engine_stats lr_nat norecompute compute
>  check_engine_stats lr_stateful norecompute compute
>  check_engine_stats sync_to_sb_pb norecompute compute
>  check_engine_stats sync_to_sb_lb norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> @@ -12663,7 +12663,7 @@ check_engine_stats lr_nat norecompute compute
>  check_engine_stats lr_stateful norecompute compute
>  check_engine_stats sync_to_sb_pb norecompute compute
>  check_engine_stats sync_to_sb_lb norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check ovn-nbctl --wait=sb lr-add lr0
> --
> 2.52.0
>
>
diff mbox series

Patch

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 41a5a97ff..971876785 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -157,16 +157,19 @@  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;
 
     struct lflow_input lflow_input;
     lflow_get_input_data(node, &lflow_input);
 
+    if (!lflow_handle_northd_lr_changes(eng_ctx->ovnsb_idl_txn,
+                                        &northd_data->trk_data.trk_routers,
+                                        &lflow_input,
+                                        lflow_data->lflow_table)) {
+        return EN_UNHANDLED;
+    }
+
     if (!lflow_handle_northd_port_changes(eng_ctx->ovnsb_idl_txn,
                                           &northd_data->trk_data.trk_lsps,
                                           &lflow_input,
diff --git a/northd/northd.c b/northd/northd.c
index b4bb4ba6d..aefb0b12a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -568,6 +568,7 @@  ovn_datapath_create(struct hmap *datapaths, const struct uuid *key,
     od->ipam_info_initialized = false;
     od->tunnel_key = sdp->sb_dp->tunnel_key;
     init_mcast_info_for_datapath(od);
+    od->datapath_lflows = lflow_ref_create();
     return od;
 }
 
@@ -598,6 +599,7 @@  ovn_datapath_destroy(struct ovn_datapath *od)
         destroy_ports_for_datapath(od);
         hmapx_destroy(&od->phys_ports);
         sset_destroy(&od->router_ips);
+        lflow_ref_destroy(od->datapath_lflows);
         free(od);
     }
 }
@@ -19611,6 +19613,7 @@  lflow_reset_northd_refs(struct lflow_input *lflow_input)
     struct ls_stateful_record *ls_stateful_rec;
     struct ovn_lb_datapaths *lb_dps;
     struct ovn_port *op;
+    const struct ovn_datapath *od;
 
     LR_STATEFUL_TABLE_FOR_EACH (lr_stateful_rec,
                                 lflow_input->lr_stateful_table) {
@@ -19635,6 +19638,72 @@  lflow_reset_northd_refs(struct lflow_input *lflow_input)
     HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
         lflow_ref_clear(lb_dps->lflow_ref);
     }
+
+    HMAP_FOR_EACH (od, key_node, &lflow_input->lr_datapaths->datapaths) {
+        lflow_ref_clear(od->datapath_lflows);
+    }
+
+    HMAP_FOR_EACH (od, key_node, &lflow_input->ls_datapaths->datapaths) {
+        lflow_ref_clear(od->datapath_lflows);
+    }
+}
+
+bool
+lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsb_txn,
+                                struct tracked_dps *tracked_lrs,
+                                struct lflow_input *lflow_input,
+                                struct lflow_table *lflows)
+{
+    bool handled = true;
+    struct hmapx_node *hmapx_node;
+    HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->deleted) {
+        struct ovn_datapath *od = hmapx_node->data;
+        handled = lflow_ref_resync_flows(
+            od->datapath_lflows, lflows, ovnsb_txn, lflow_input->dps,
+            lflow_input->ovn_internal_version_changed,
+            lflow_input->sbrec_logical_flow_table,
+            lflow_input->sbrec_logical_dp_group_table);
+        if (!handled) {
+            return handled;
+        }
+    }
+
+    struct lswitch_flow_build_info lsi = {
+        .lr_datapaths = lflow_input->lr_datapaths,
+        .lr_stateful_table = lflow_input->lr_stateful_table,
+        .lflows =lflows,
+        .route_data = lflow_input->route_data,
+        .route_tables = lflow_input->route_tables,
+        .route_policies = lflow_input->route_policies,
+        .match = DS_EMPTY_INITIALIZER,
+        .actions = DS_EMPTY_INITIALIZER,
+    };
+    HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->crupdated) {
+        struct ovn_datapath *od = hmapx_node->data;
+
+        lflow_ref_unlink_lflows(od->datapath_lflows);
+        build_lswitch_and_lrouter_iterate_by_lr(od, &lsi);
+    }
+
+    /* We need to make sure that all datapath groups are allocated before
+     * trying to sync logical flows. Otherwise, we would need to recompute
+     * those datapath groups within those flows over and over again. */
+    HMAPX_FOR_EACH (hmapx_node, &tracked_lrs->crupdated) {
+        struct ovn_datapath *od = hmapx_node->data;
+
+        handled = lflow_ref_sync_lflows(
+            od->datapath_lflows, lflows, ovnsb_txn, lflow_input->dps,
+            lflow_input->ovn_internal_version_changed,
+            lflow_input->sbrec_logical_flow_table,
+            lflow_input->sbrec_logical_dp_group_table);
+        if (!handled) {
+            break;
+        }
+    }
+
+    ds_destroy(&lsi.actions);
+    ds_destroy(&lsi.match);
+    return handled;
 }
 
 bool
diff --git a/northd/northd.h b/northd/northd.h
index eb5c15f34..acda62bc6 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -477,11 +477,8 @@  struct ovn_datapath {
     /* Map of ovn_port objects belonging to this datapath.
      * This map doesn't include derived ports. */
     struct hmap ports;
-
-    /* XXX Reference to the lflows belonging to this datapath currently router
-     * only lflows. This is NULL for now just to not waste any space
-     * as it's not used by anything right now, but it wasn't worth reverting
-     * all the related changes. */
+    /* Reference to the lflows belonging to this datapath currently router
+     * only lflows. */
     struct lflow_ref *datapath_lflows;
 };
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 512e42036..a5fa1c6a9 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12652,7 +12652,7 @@  check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
 check_engine_stats sync_to_sb_pb norecompute compute
 check_engine_stats sync_to_sb_lb norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
@@ -12663,7 +12663,7 @@  check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
 check_engine_stats sync_to_sb_pb norecompute compute
 check_engine_stats sync_to_sb_lb norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check ovn-nbctl --wait=sb lr-add lr0