diff mbox series

[ovs-dev,v6,09/13] northd: Add lr_stateful handler for lflow engine node.

Message ID 20240130212258.1483365-1-numans@ovn.org
State Accepted
Headers show
Series northd lflow incremental processing | expand

Checks

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

Commit Message

Numan Siddique Jan. 30, 2024, 9:22 p.m. UTC
From: Numan Siddique <numans@ovn.org>

Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/en-lflow.c        |  27 +++
 northd/en-lflow.h        |   1 +
 northd/en-lr-stateful.c  |  33 +++-
 northd/en-lr-stateful.h  |  31 +++-
 northd/inc-proc-northd.c |   2 +-
 northd/northd.c          | 369 ++++++++++++++++++++++-----------------
 northd/northd.h          |  11 +-
 tests/ovn-northd.at      |  48 ++---
 8 files changed, 333 insertions(+), 189 deletions(-)

Comments

Dumitru Ceara Feb. 2, 2024, 12:13 p.m. UTC | #1
On 1/30/24 22:22, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Acked-by: Han Zhou <hzhou@ovn.org>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks!
diff mbox series

Patch

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index c49d24d54b..41b1779539 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -163,6 +163,33 @@  lflow_port_group_handler(struct engine_node *node, void *data OVS_UNUSED)
     return true;
 }
 
+bool
+lflow_lr_stateful_handler(struct engine_node *node, void *data)
+{
+    struct ed_type_lr_stateful *lr_sful_data =
+        engine_get_input_data("lr_stateful", node);
+
+    if (!lr_stateful_has_tracked_data(&lr_sful_data->trk_data)
+        || lr_sful_data->trk_data.vip_nats_changed) {
+        return false;
+    }
+
+    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_lr_stateful_changes(eng_ctx->ovnsb_idl_txn,
+                                          &lr_sful_data->trk_data,
+                                          &lflow_input,
+                                          lflow_data->lflow_table)) {
+        return false;
+    }
+
+    engine_set_node_state(node, EN_UPDATED);
+    return true;
+}
+
 void *en_lflow_init(struct engine_node *node OVS_UNUSED,
                      struct engine_arg *arg OVS_UNUSED)
 {
diff --git a/northd/en-lflow.h b/northd/en-lflow.h
index f7325c56b1..1d813a2a29 100644
--- a/northd/en-lflow.h
+++ b/northd/en-lflow.h
@@ -20,5 +20,6 @@  void *en_lflow_init(struct engine_node *node, struct engine_arg *arg);
 void en_lflow_cleanup(void *data);
 bool lflow_northd_handler(struct engine_node *, void *data);
 bool lflow_port_group_handler(struct engine_node *, void *data);
+bool lflow_lr_stateful_handler(struct engine_node *, void *data);
 
 #endif /* EN_LFLOW_H */
diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
index 3e2a6c254e..5f3d677131 100644
--- a/northd/en-lr-stateful.c
+++ b/northd/en-lr-stateful.c
@@ -40,6 +40,7 @@ 
 #include "lib/ovn-sb-idl.h"
 #include "lib/ovn-util.h"
 #include "lib/stopwatch-names.h"
+#include "lflow-mgr.h"
 #include "northd.h"
 
 VLOG_DEFINE_THIS_MODULE(en_lr_stateful);
@@ -81,7 +82,7 @@  static void remove_lrouter_lb_reachable_ips(struct lr_stateful_record *,
                                             enum lb_neighbor_responder_mode,
                                             const struct sset *lb_ips_v4,
                                             const struct sset *lb_ips_v6);
-static void lr_stateful_rebuild_vip_nats(struct lr_stateful_record *);
+static bool lr_stateful_rebuild_vip_nats(struct lr_stateful_record *);
 
 /* 'lr_stateful' engine node manages the NB logical router LB data.
  */
@@ -109,6 +110,7 @@  en_lr_stateful_clear_tracked_data(void *data_)
     struct ed_type_lr_stateful *data = data_;
 
     hmapx_clear(&data->trk_data.crupdated);
+    data->trk_data.vip_nats_changed = false;
 }
 
 void
@@ -196,6 +198,10 @@  lr_stateful_lb_data_handler(struct engine_node *node, void *data_)
 
             /* Add the lr_stateful_rec rec to the tracking data. */
             hmapx_add(&data->trk_data.crupdated, lr_stateful_rec);
+
+            if (!sset_is_empty(&lr_stateful_rec->vip_nats)) {
+                data->trk_data.vip_nats_changed = true;
+            }
             continue;
         }
 
@@ -313,7 +319,9 @@  lr_stateful_lb_data_handler(struct engine_node *node, void *data_)
 
         HMAPX_FOR_EACH (hmapx_node, &data->trk_data.crupdated) {
             struct lr_stateful_record *lr_stateful_rec = hmapx_node->data;
-            lr_stateful_rebuild_vip_nats(lr_stateful_rec);
+            if (lr_stateful_rebuild_vip_nats(lr_stateful_rec)) {
+                data->trk_data.vip_nats_changed = true;
+            }
             const struct ovn_datapath *od =
                 ovn_datapaths_find_by_index(input_data.lr_datapaths,
                                             lr_stateful_rec->lr_index);
@@ -352,8 +360,13 @@  lr_stateful_lr_nat_handler(struct engine_node *node, void *data_)
                 lr_stateful_record_create(&data->table, lrnat_rec, od,
                                           input_data.lb_datapaths_map,
                                           input_data.lbgrp_datapaths_map);
+            if (!sset_is_empty(&lr_stateful_rec->vip_nats)) {
+                data->trk_data.vip_nats_changed = true;
+            }
         } else {
-            lr_stateful_rebuild_vip_nats(lr_stateful_rec);
+            if (lr_stateful_rebuild_vip_nats(lr_stateful_rec)) {
+                data->trk_data.vip_nats_changed = true;
+            }
         }
 
         /* Add the lr_stateful_rec rec to the tracking data. */
@@ -455,6 +468,7 @@  lr_stateful_record_create(struct lr_stateful_table *table,
     lr_stateful_rec->nbr_uuid = od->nbr->header_.uuid;
     lr_stateful_rec->lrnat_rec = lrnat_rec;
     lr_stateful_rec->lr_index = od->index;
+    lr_stateful_rec->lflow_ref = lflow_ref_create();
 
 
     /* Initialize the record.
@@ -543,6 +557,7 @@  lr_stateful_record_destroy(struct lr_stateful_record *lr_stateful_rec)
 {
     ovn_lb_ip_set_destroy(lr_stateful_rec->lb_ips);
     sset_destroy(&lr_stateful_rec->vip_nats);
+    lflow_ref_destroy(lr_stateful_rec->lflow_ref);
     free(lr_stateful_rec);
 }
 
@@ -657,10 +672,12 @@  remove_lrouter_lb_reachable_ips(struct lr_stateful_record *lr_stateful_rec,
     }
 }
 
-static void
+static bool
 lr_stateful_rebuild_vip_nats(struct lr_stateful_record *lr_stateful_rec)
 {
-    sset_clear(&lr_stateful_rec->vip_nats);
+    struct sset old_vip_nats = SSET_INITIALIZER(&old_vip_nats);
+    sset_swap(&lr_stateful_rec->vip_nats, &old_vip_nats);
+
     const char *external_ip;
     SSET_FOR_EACH (external_ip, &lr_stateful_rec->lrnat_rec->external_ips) {
         bool is_vip_nat = false;
@@ -676,4 +693,10 @@  lr_stateful_rebuild_vip_nats(struct lr_stateful_record *lr_stateful_rec)
             sset_add(&lr_stateful_rec->vip_nats, external_ip);
         }
     }
+
+    bool vip_nats_changed = !sset_equals(&lr_stateful_rec->vip_nats,
+                                         &old_vip_nats);
+    sset_destroy(&old_vip_nats);
+
+    return vip_nats_changed;
 }
diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h
index 7529a61f33..ecff2238d4 100644
--- a/northd/en-lr-stateful.h
+++ b/northd/en-lr-stateful.h
@@ -32,6 +32,7 @@ 
 
 struct ovn_datapath;
 struct lr_nat_record;
+struct lflow_ref;
 
 /* lr_stateful_table:  This represents a table of logical routers with
  *                     stateful related data.
@@ -63,6 +64,30 @@  struct lr_stateful_record {
 
     /* sset of vips which are also part of lr nats. */
     struct sset vip_nats;
+
+    /* 'lflow_ref' is used to reference logical flows generated for
+     * this lr_stateful_record.
+     *
+     * This data is initialized and destroyed by the en_lr_stateful node,
+     * but populated and used only by the en_lflow node. Ideally this data
+     * should be maintained as part of en_lflow's data.  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.
+     *
+     * Adding 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.
+     *
+     * Note: lflow_ref is not thread safe.  Only one thread should
+     * access lr_stateful_record->lflow_ref at any given time.
+     */
+    struct lflow_ref *lflow_ref;
 };
 
 struct lr_stateful_table {
@@ -82,6 +107,10 @@  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'. */
+
+    /* Indicates if any router's NATs changed which were also LB vips
+     * or vice versa. */
+    bool vip_nats_changed;
 };
 
 struct ed_type_lr_stateful {
@@ -112,7 +141,7 @@  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);
+    return !hmapx_is_empty(&trk_data->crupdated) || trk_data->vip_nats_changed;
 }
 
 static inline bool
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 0e17bfe2e6..dcce79510a 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -228,11 +228,11 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_lflow, &en_sb_logical_flow, NULL);
     engine_add_input(&en_lflow, &en_sb_multicast_group, NULL);
     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);
+    engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler);
 
     engine_add_input(&en_sync_to_sb_addr_set, &en_nb_address_set,
                      sync_to_sb_addr_set_nb_address_set_handler);
diff --git a/northd/northd.c b/northd/northd.c
index 937ccdf354..e3448be29f 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -12815,62 +12815,6 @@  build_ip_routing_flows_for_lrp(
     }
 }
 
-/* Logical router ingress table IP_ROUTING : IP Routing.
- *
- * For the LSP 'op' of type router, if there are logical router ports other
- * than the LSP's peer connected to the logical switch, then for routable
- * addresses (such as NAT IPs, LB VIPs, etc.) on each of the connected router
- * ports, add routes to the LSP's peer router.
- */
-static void
-build_ip_routing_flows_for_router_type_lsp(
-    struct ovn_port *op, const struct lr_stateful_table *lr_stateful_table,
-    const struct hmap *lr_ports, struct lflow_table *lflows,
-    struct lflow_ref *lflow_ref)
-{
-    ovs_assert(op->nbsp);
-    if (!lsp_is_router(op->nbsp)) {
-        return;
-    }
-
-    struct ovn_port *peer = ovn_port_get_peer(lr_ports, op);
-    if (!peer || !peer->nbrp || !peer->lrp_networks.n_ipv4_addrs
-        || !op->od->n_router_ports) {
-        return;
-    }
-
-    for (size_t i = 0; i < op->od->n_router_ports; i++) {
-        struct ovn_port *router_port = ovn_port_get_peer(
-                lr_ports, op->od->router_ports[i]);
-        if (!router_port || !router_port->nbrp || router_port == peer) {
-            continue;
-        }
-
-        const struct lr_stateful_record *lr_stateful_rec =
-            lr_stateful_table_find_by_index(lr_stateful_table,
-                                            router_port->od->index);
-
-        if (router_port->nbrp->ha_chassis_group ||
-                router_port->nbrp->n_gateway_chassis) {
-            struct ovn_port_routable_addresses ra =
-                get_op_routable_addresses(router_port, lr_stateful_rec);
-            for (size_t j = 0; j < ra.n_addrs; j++) {
-                struct lport_addresses *laddrs = &ra.laddrs[j];
-                for (size_t k = 0; k < laddrs->n_ipv4_addrs; k++) {
-                    add_route(lflows, peer->od, peer,
-                            peer->lrp_networks.ipv4_addrs[0].addr_s,
-                            laddrs->ipv4_addrs[k].network_s,
-                            laddrs->ipv4_addrs[k].plen, NULL, false, 0,
-                            &peer->nbrp->header_, false,
-                            ROUTE_PRIO_OFFSET_CONNECTED,
-                            lflow_ref);
-                }
-            }
-            destroy_routable_addresses(&ra);
-        }
-    }
-}
-
 static void
 build_static_route_flows_for_lrouter(
         struct ovn_datapath *od, const struct chassis_features *features,
@@ -13112,42 +13056,6 @@  build_arp_resolve_flows_for_lrouter(
                                lflow_ref);
 }
 
-static void
-routable_addresses_to_lflows(struct lflow_table *lflows,
-                             struct ovn_port *router_port,
-                             struct ovn_port *peer,
-                             const struct lr_stateful_record *lr_stateful_rec,
-                             struct ds *match, struct ds *actions,
-                             struct lflow_ref *lflow_ref)
-{
-    struct ovn_port_routable_addresses ra =
-        get_op_routable_addresses(router_port, lr_stateful_rec);
-    if (!ra.n_addrs) {
-        return;
-    }
-
-    for (size_t i = 0; i < ra.n_addrs; i++) {
-        ds_clear(match);
-        ds_put_format(match, "outport == %s && "REG_NEXT_HOP_IPV4" == {",
-                      peer->json_key);
-        bool first = true;
-        for (size_t j = 0; j < ra.laddrs[i].n_ipv4_addrs; j++) {
-            if (!first) {
-                ds_put_cstr(match, ", ");
-            }
-            ds_put_cstr(match, ra.laddrs[i].ipv4_addrs[j].addr_s);
-            first = false;
-        }
-        ds_put_cstr(match, "}");
-
-        ds_clear(actions);
-        ds_put_format(actions, "eth.dst = %s; next;", ra.laddrs[i].ea_s);
-        ovn_lflow_add(lflows, peer->od, S_ROUTER_IN_ARP_RESOLVE, 100,
-                      ds_cstr(match), ds_cstr(actions), lflow_ref);
-    }
-    destroy_routable_addresses(&ra);
-}
-
 /* Local router ingress table ARP_RESOLVE: ARP Resolution.
  *
  * Any unicast packet that reaches this table is an IP packet whose
@@ -13390,52 +13298,6 @@  build_arp_resolve_flows_for_lsp(
     }
 }
 
-static void
-build_arp_resolve_flows_for_lsp_routable_addresses(
-    struct ovn_port *op, struct lflow_table *lflows,
-    const struct hmap *lr_ports,
-    const struct lr_stateful_table *lr_stateful_table,
-    struct ds *match, struct ds *actions,
-    struct lflow_ref *lflow_ref)
-{
-    if (!lsp_is_router(op->nbsp)) {
-        return;
-    }
-
-    struct ovn_port *peer = ovn_port_get_peer(lr_ports, op);
-    if (!peer || !peer->nbrp) {
-        return;
-    }
-
-    if (peer->od->nbr &&
-        smap_get_bool(&peer->od->nbr->options,
-                      "dynamic_neigh_routers", false)) {
-        return;
-    }
-
-    for (size_t i = 0; i < op->od->n_router_ports; i++) {
-        struct ovn_port *router_port =
-            ovn_port_get_peer(lr_ports, op->od->router_ports[i]);
-        if (!router_port || !router_port->nbrp) {
-            continue;
-        }
-
-        /* Skip the router port under consideration. */
-        if (router_port == peer) {
-            continue;
-        }
-
-        if (smap_get(&peer->od->nbr->options, "chassis") || peer->cr_port) {
-            const struct lr_stateful_record *lr_stateful_rec;
-            lr_stateful_rec = lr_stateful_table_find_by_index(
-                lr_stateful_table, router_port->od->index);
-            routable_addresses_to_lflows(lflows, router_port, peer,
-                                         lr_stateful_rec, match, actions,
-                                         lflow_ref);
-        }
-    }
-}
-
 static void
 build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu,
                             struct lflow_table *lflows,
@@ -15649,8 +15511,6 @@  build_lrouter_nat_defrag_and_lb(
 static void
 build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
                             const struct lr_stateful_record *lr_stateful_rec,
-                            const struct lr_stateful_table *lr_stateful_table,
-                            const struct hmap *lr_ports,
                             struct lflow_table *lflows,
                             struct ds *match,
                             struct ds *actions,
@@ -15661,20 +15521,108 @@  build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
     build_lswitch_rport_arp_req_flows_for_lbnats(
         lsp->peer, lr_stateful_rec, lsp->od, lsp,
         lflows, &lsp->nbsp->header_, lflow_ref);
-    build_ip_routing_flows_for_router_type_lsp(lsp, lr_stateful_table,
-                                               lr_ports, lflows,
-                                               lflow_ref);
-    build_arp_resolve_flows_for_lsp_routable_addresses(
-        lsp, lflows, lr_ports, lr_stateful_table, match, actions, lflow_ref);
     build_lswitch_ip_unicast_lookup_for_nats(lsp, lr_stateful_rec, lflows,
                                              match, actions, lflow_ref);
 }
 
+/* Logical router ingress table IP_ROUTING : IP Routing.
+ *
+ * Adds the LRP 'lrp's routable addresses (addresses which can be routed via
+ * the LRP's datapath) as routable flows into the other router datapaths
+ * which are connected to the LRP's peer's logical switch.
+ *
+ * i.e If logical switch sw0 is conencted to the routers R0, R1 and R2,
+ * and if LRP of R0 has routable addresses (IP1 and IP2), then it adds
+ * the routes to reach these IPs in the R1 and R2's datapaths.
+ *
+ * This function also adds the ARP resolve flows for these addresses
+ * (IP1 and IP2) in the ARP_RESOLVE table of R1 and R2.
+ * */
+static void
+build_routable_flows_for_router_port(
+    struct ovn_port *lrp, const struct lr_stateful_record *lr_stateful_rec,
+    struct lflow_table *lflows,
+    struct ds *match,
+    struct ds *actions)
+{
+    ovs_assert(lrp->nbrp && uuid_equals(&lrp->od->nbr->header_.uuid,
+                                        &lr_stateful_rec->nbr_uuid));
+
+    struct ovn_port *lsp_peer = lrp->peer;
+    if (!lsp_peer || !lsp_peer->nbsp) {
+        return;
+    }
+
+    struct ovn_datapath *peer_ls = lsp_peer->od;
+    ovs_assert(peer_ls->nbs);
+
+    struct ovn_port_routable_addresses ra =
+        get_op_routable_addresses(lrp, lr_stateful_rec);
+
+    struct ovn_port *router_port;
+
+    for (size_t i = 0; i < peer_ls->n_router_ports; i++) {
+        router_port = peer_ls->router_ports[i]->peer;
+
+        if (router_port == lrp) {
+            continue;
+        }
+
+        if (lrp->nbrp->ha_chassis_group ||
+                lrp->nbrp->n_gateway_chassis) {
+            for (size_t j = 0; j < ra.n_addrs; j++) {
+                struct lport_addresses *laddrs = &ra.laddrs[j];
+                for (size_t k = 0; k < laddrs->n_ipv4_addrs; k++) {
+                    add_route(lflows, router_port->od, router_port,
+                              router_port->lrp_networks.ipv4_addrs[0].addr_s,
+                              laddrs->ipv4_addrs[k].network_s,
+                              laddrs->ipv4_addrs[k].plen, NULL, false, 0,
+                              &router_port->nbrp->header_, false,
+                              ROUTE_PRIO_OFFSET_CONNECTED,
+                              lrp->stateful_lflow_ref);
+                }
+            }
+        }
+
+        bool dynamic_neigh_router =
+            smap_get_bool(&router_port->od->nbr->options,
+                          "dynamic_neigh_routers", false);
+
+        if (!dynamic_neigh_router &&
+            (router_port->od->is_gw_router || router_port->cr_port)) {
+
+            for (size_t k = 0; k < ra.n_addrs; k++) {
+                ds_clear(match);
+                ds_put_format(match, "outport == %s && "
+                              REG_NEXT_HOP_IPV4" == {",
+                              router_port->json_key);
+                bool first = true;
+                for (size_t j = 0; j < ra.laddrs[k].n_ipv4_addrs; j++) {
+                    if (!first) {
+                        ds_put_cstr(match, ", ");
+                    }
+                    ds_put_cstr(match, ra.laddrs[k].ipv4_addrs[j].addr_s);
+                    first = false;
+                }
+                ds_put_cstr(match, "}");
+
+                ds_clear(actions);
+                ds_put_format(actions, "eth.dst = %s; next;",
+                              ra.laddrs[k].ea_s);
+                ovn_lflow_add(lflows, router_port->od, S_ROUTER_IN_ARP_RESOLVE,
+                              100, ds_cstr(match), ds_cstr(actions),
+                              lrp->stateful_lflow_ref);
+            }
+        }
+    }
+
+    destroy_routable_addresses(&ra);
+}
+
 static void
 build_lbnat_lflows_iterate_by_lsp(
     struct ovn_port *op, const struct lr_stateful_table *lr_stateful_table,
-    const struct hmap *lr_ports, struct ds *match, struct ds *actions,
-    struct lflow_table *lflows)
+    struct ds *match, struct ds *actions, struct lflow_table *lflows)
 {
     ovs_assert(op->nbsp);
 
@@ -15688,8 +15636,8 @@  build_lbnat_lflows_iterate_by_lsp(
     ovs_assert(lr_stateful_rec);
 
     build_lsp_lflows_for_lbnats(op, lr_stateful_rec,
-                                lr_stateful_table, lr_ports, lflows,
-                                match, actions, op->stateful_lflow_ref);
+                                lflows,match, actions,
+                                op->stateful_lflow_ref);
 }
 
 static void
@@ -15699,6 +15647,9 @@  build_lrp_lflows_for_lbnats(struct ovn_port *op,
                             struct ds *match, struct ds *actions,
                             struct lflow_table *lflows)
 {
+    ovs_assert(op->nbrp && uuid_equals(&op->od->nbr->header_.uuid,
+                                       &lr_stateful_rec->nbr_uuid));
+
     /* Drop IP traffic destined to router owned IPs except if the IP is
      * also a SNAT IP. Those are dropped later, in stage
      * "lr_in_arp_resolve", if unSNAT was unsuccessful.
@@ -15731,6 +15682,9 @@  build_lrp_lflows_for_lbnats(struct ovn_port *op,
                                       match, actions, op->stateful_lflow_ref);
 }
 
+/* Builds the load balancer and NAT related flows for the router port 'op'.
+ * It uses the op->stateful_lflow_ref for lflow referencing.
+ */
 static void
 build_lbnat_lflows_iterate_by_lrp(
     struct ovn_port *op, const struct lr_stateful_table *lr_stateful_table,
@@ -15746,6 +15700,9 @@  build_lbnat_lflows_iterate_by_lrp(
 
     build_lrp_lflows_for_lbnats(op, lr_stateful_rec, meter_groups, match,
                                 actions, lflows);
+
+    build_routable_flows_for_router_port(op, lr_stateful_rec, lflows, match,
+                                         actions);
 }
 
 static void
@@ -15765,12 +15722,14 @@  build_lr_stateful_flows(const struct lr_stateful_record *lr_stateful_rec,
     ovs_assert(uuid_equals(&od->nbr->header_.uuid,
                            &lr_stateful_rec->nbr_uuid));
     build_lrouter_nat_defrag_and_lb(lr_stateful_rec, od, lflows, ls_ports,
-                                    lr_ports, match, actions,
-                                    meter_groups, features, NULL);
+                                    lr_ports, match, actions, meter_groups,
+                                    features, lr_stateful_rec->lflow_ref);
     build_lr_gateway_redirect_flows_for_nats(od, lr_stateful_rec->lrnat_rec,
-                                             lflows, match, actions, NULL);
-    build_lrouter_arp_nd_for_datapath(od, lr_stateful_rec->lrnat_rec, lflows,
-                                      meter_groups, NULL);
+                                             lflows, match, actions,
+                                             lr_stateful_rec->lflow_ref);
+    build_lrouter_arp_nd_for_datapath(od, lr_stateful_rec->lrnat_rec,
+                                      lflows, meter_groups,
+                                      lr_stateful_rec->lflow_ref);
 }
 
 static void
@@ -15956,6 +15915,7 @@  build_lflows_thread(void *arg)
     /* Note:  lflow_ref is not thread safe.  Ensure that
      *    - op->lflow_ref
      *    - lb_dps->lflow_ref
+     *    - lr_stateful_rec->lflow_ref
      * are not accessed by multiple threads at the same time. */
     while (!stop_parallel_processing()) {
         wait_for_work(control);
@@ -16006,7 +15966,7 @@  build_lflows_thread(void *arg)
                                                              &lsi->actions,
                                                              lsi->lflows);
                     build_lbnat_lflows_iterate_by_lsp(
-                        op, lsi->lr_stateful_table, lsi->lr_ports, &lsi->match,
+                        op, lsi->lr_stateful_table, &lsi->match,
                         &lsi->actions, lsi->lflows);
                 }
             }
@@ -16258,7 +16218,7 @@  build_lswitch_and_lrouter_flows(
                                                      &lsi.actions,
                                                      lsi.lflows);
             build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_stateful_table,
-                                              lsi.lr_ports, &lsi.match,
+                                              &lsi.match,
                                               &lsi.actions, lsi.lflows);
         }
         HMAP_FOR_EACH (op, key_node, lr_ports) {
@@ -16462,9 +16422,15 @@  void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
 void
 lflow_reset_northd_refs(struct lflow_input *lflow_input)
 {
+    const struct lr_stateful_record *lr_stateful_rec;
     struct ovn_lb_datapaths *lb_dps;
     struct ovn_port *op;
 
+    LR_STATEFUL_TABLE_FOR_EACH (lr_stateful_rec,
+                                lflow_input->lr_stateful_table) {
+        lflow_ref_clear(lr_stateful_rec->lflow_ref);
+    }
+
     HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
         lflow_ref_clear(op->lflow_ref);
         lflow_ref_clear(op->stateful_lflow_ref);
@@ -16534,8 +16500,7 @@  lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
             lflow_ref_unlink_lflows(op->stateful_lflow_ref);
             build_lbnat_lflows_iterate_by_lsp(op,
                                               lflow_input->lr_stateful_table,
-                                              lflow_input->lr_ports, &match,
-                                              &actions, lflows);
+                                              &match, &actions, lflows);
             handled = lflow_ref_sync_lflows(
                 op->stateful_lflow_ref, lflows, ovnsb_txn,
                 lflow_input->ls_datapaths,
@@ -16589,8 +16554,7 @@  lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
             /* Now generate the stateful lflows for 'op' */
             build_lbnat_lflows_iterate_by_lsp(op,
                                               lflow_input->lr_stateful_table,
-                                              lflow_input->lr_ports, &match,
-                                              &actions, lflows);
+                                              &match, &actions, lflows);
             handled = lflow_ref_sync_lflows(
                 op->stateful_lflow_ref, lflows, ovnsb_txn,
                 lflow_input->ls_datapaths,
@@ -16701,6 +16665,91 @@  lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
     return true;
 }
 
+bool
+lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
+                                struct lr_stateful_tracked_data *trk_data,
+                                struct lflow_input *lflow_input,
+                                struct lflow_table *lflows)
+{
+    struct lr_stateful_record *lr_stateful_rec;
+    struct ds actions = DS_EMPTY_INITIALIZER;
+    struct ds match = DS_EMPTY_INITIALIZER;
+    struct hmapx_node *hmapx_node;
+    bool handled = true;
+
+    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
+        lr_stateful_rec = hmapx_node->data;
+        /* Unlink old lflows. */
+        lflow_ref_unlink_lflows(lr_stateful_rec->lflow_ref);
+
+        /* Generate new lflows. */
+        build_lr_stateful_flows(lr_stateful_rec, lflow_input->lr_datapaths,
+                                lflows, lflow_input->ls_ports,
+                                lflow_input->lr_ports, &match, &actions,
+                                lflow_input->meter_groups,
+                                lflow_input->features);
+
+        /* Sync the new flows to SB. */
+        handled = lflow_ref_sync_lflows(
+            lr_stateful_rec->lflow_ref, lflows, ovnsb_txn,
+            lflow_input->ls_datapaths, lflow_input->lr_datapaths,
+            lflow_input->ovn_internal_version_changed,
+            lflow_input->sbrec_logical_flow_table,
+            lflow_input->sbrec_logical_dp_group_table);
+        if (!handled) {
+            goto exit;
+        }
+
+        const struct ovn_datapath *od =
+            ovn_datapaths_find_by_index(lflow_input->lr_datapaths,
+                                        lr_stateful_rec->lr_index);
+        struct ovn_port *op;
+        HMAP_FOR_EACH (op, dp_node, &od->ports) {
+            lflow_ref_unlink_lflows(op->stateful_lflow_ref);
+
+            build_lbnat_lflows_iterate_by_lrp(op,
+                                              lflow_input->lr_stateful_table,
+                                              lflow_input->meter_groups,
+                                              &match, &actions,
+                                              lflows);
+
+            handled = lflow_ref_sync_lflows(
+                op->stateful_lflow_ref, lflows, ovnsb_txn,
+                lflow_input->ls_datapaths, lflow_input->lr_datapaths,
+                lflow_input->ovn_internal_version_changed,
+                lflow_input->sbrec_logical_flow_table,
+                lflow_input->sbrec_logical_dp_group_table);
+            if (!handled) {
+                goto exit;
+            }
+
+            if (op->peer && op->peer->nbsp) {
+                lflow_ref_unlink_lflows(op->peer->stateful_lflow_ref);
+
+                build_lbnat_lflows_iterate_by_lsp(
+                    op->peer, lflow_input->lr_stateful_table, &match, &actions,
+                    lflows);
+
+                handled = lflow_ref_sync_lflows(
+                    op->peer->stateful_lflow_ref, lflows, ovnsb_txn,
+                    lflow_input->ls_datapaths, lflow_input->lr_datapaths,
+                    lflow_input->ovn_internal_version_changed,
+                    lflow_input->sbrec_logical_flow_table,
+                    lflow_input->sbrec_logical_dp_group_table);
+                if (!handled) {
+                    goto exit;
+                }
+            }
+        }
+    }
+
+exit:
+    ds_destroy(&match);
+    ds_destroy(&actions);
+
+    return handled;
+}
+
 static bool
 mirror_needs_update(const struct nbrec_mirror *nb_mirror,
                     const struct sbrec_mirror *sb_mirror)
diff --git a/northd/northd.h b/northd/northd.h
index f37d6dac75..224b5d72ba 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -647,8 +647,9 @@  struct ovn_port {
      * 'patch/router' to reference logical flows generated fo this ovn_port
      *  from the 'lr_stateful' record of the peer port's datapath.
      *
-     * Note: lflow_ref is not thread safe.  Only one thread should
-     * access ovn_ports->lflow_ref at any given time.
+     * Note: lflow_ref and stateful_lflow_ref are not thread safe.  Only one
+     * thread should access ovn_ports->lflow_ref/stateful_lflow_ref at any
+     * given time.
      */
     struct lflow_ref *lflow_ref;
     struct lflow_ref *stateful_lflow_ref;
@@ -676,6 +677,8 @@  void northd_indices_create(struct northd_data *data,
                            struct ovsdb_idl *ovnsb_idl);
 
 struct lflow_table;
+struct lr_stateful_tracked_data;
+
 void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
                   struct lflow_input *input_data,
                   struct lflow_table *);
@@ -689,6 +692,10 @@  bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
                                     struct tracked_lbs *,
                                     struct lflow_input *,
                                     struct lflow_table *lflows);
+bool lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *,
+                                      struct lr_stateful_tracked_data *,
+                                      struct lflow_input *,
+                                      struct lflow_table *lflows);
 bool northd_handle_sb_port_binding_changes(
     const struct sbrec_port_binding_table *, struct hmap *ls_ports,
     struct hmap *lr_ports);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 816eded21a..8a20643989 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12,6 +12,7 @@  m4_define([_DUMP_DB_TABLES], [
     ovn-sbctl list meter >> $1
     ovn-sbctl list meter_band >> $1
     ovn-sbctl list port_group >> $1
+    ovn-sbctl dump-flows > lflows_$1
 ])
 
 # CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -10784,7 +10785,7 @@  check ovn-nbctl --wait=sb lr-lb-add lr0 lb1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10794,7 +10795,7 @@  check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10804,7 +10805,7 @@  check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10814,7 +10815,7 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10824,7 +10825,7 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10832,6 +10833,7 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-lb-del lr0 lb1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd recompute nocompute
+check_engine_stats lr_stateful recompute nocompute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -10946,7 +10948,7 @@  check ovn-nbctl add logical_router lr0 load_balancer_group $lbg1_uuid
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10956,7 +10958,7 @@  check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10966,7 +10968,7 @@  check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10976,7 +10978,7 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10986,7 +10988,7 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11067,7 +11069,7 @@  check ovn-nbctl --wait=sb set logical_router lr1 load_balancer_group=$lbg1_uuid
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11095,7 +11097,7 @@  check ovn-nbctl --wait=sb lr-lb-add lr1 lb2
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11665,7 +11667,7 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat  172.168.0.110 10.0.0.4
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11674,7 +11676,7 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb set NAT . options:foo=bar
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11684,7 +11686,7 @@  check ovn-nbctl --wait=sb set NAT . external_ip=172.168.0.120
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11694,7 +11696,7 @@  check ovn-nbctl --wait=sb set NAT . logical_ip=10.0.0.10
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11704,7 +11706,7 @@  check ovn-nbctl --wait=sb set NAT . type=snat
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11714,7 +11716,7 @@  check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.4 sw
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11725,7 +11727,7 @@  check ovn-nbctl --wait=sb set NAT $nat2_uuid external_mac='"30:54:00:00:00:04"'
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11746,6 +11748,8 @@  check_engine_stats sync_to_sb_pb recompute nocompute
 check_engine_stats lflow recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+# lflow engine should recompute since the nat ip 172.168.0.150
+# is a lb vip.
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat 172.168.0.150 10.0.0.41
 check_engine_stats northd norecompute compute
@@ -11755,6 +11759,8 @@  check_engine_stats sync_to_sb_pb recompute nocompute
 check_engine_stats lflow recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+# lflow engine should recompute since the deleted nat ip 172.168.0.150
+# is a lb vip.
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-nat-del lr0 dnat_and_snat 172.168.0.150
 check_engine_stats northd norecompute compute
@@ -11764,6 +11770,8 @@  check_engine_stats sync_to_sb_pb recompute nocompute
 check_engine_stats lflow recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+# lflow engine should recompute since the deleted nat ip 172.168.0.140
+# is a lb vip.
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lr-nat-del lr0 dnat_and_snat 172.168.0.140
 check_engine_stats northd norecompute compute
@@ -11779,7 +11787,7 @@  check ovn-nbctl --wait=sb clear logical_router lr0 nat
 check_engine_stats northd norecompute compute
 check_engine_stats lr_nat norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_pb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE