[ovs-dev,v3,12/16] northd: Add lr_stateful handler for lflow engine node.

From: Numan Siddique <numans@ovn.org>

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/en-lflow.c        |  29 +++
 northd/en-lflow.h        |   1 +
 northd/en-lr-stateful.c  |  39 ++++-
 northd/en-lr-stateful.h  |  26 +++
 northd/inc-proc-northd.c |   3 +-
 northd/northd.c          | 370 ++++++++++++++++++++++-----------------
 northd/northd.h          |  10 ++
 tests/ovn-northd.at      |  48 ++---
 8 files changed, 336 insertions(+), 190 deletions(-)

On 11/28/23 03:37, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  northd/en-lflow.c        |  29 +++
>  northd/en-lflow.h        |   1 +
>  northd/en-lr-stateful.c  |  39 ++++-
>  northd/en-lr-stateful.h  |  26 +++
>  northd/inc-proc-northd.c |   3 +-
>  northd/northd.c          | 370 ++++++++++++++++++++++-----------------
>  northd/northd.h          |  10 ++
>  tests/ovn-northd.at      |  48 ++---
>  8 files changed, 336 insertions(+), 190 deletions(-)
> 
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 13284b5556..09748f570b 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -164,6 +164,35 @@ 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 =

I don't really like the 'lr_sful_data' abbreviation.  What about
'stateful_data', it's one character longer?  Or even 'lr_stateful_data'?

I now realize that this is used in a lot of places throughout
the series.  I don't want to generate a lot of extra work just to
rename it so I'll leave it up to you.

> +        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;

Nit: I'd add an empty line here and remove the one above (to group
all declarations).

> +    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 a54749ad93..8e025f057e 100644
> --- a/northd/en-lr-stateful.c
> +++ b/northd/en-lr-stateful.c
> @@ -39,6 +39,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_build_vip_nats(struct lr_stateful_record *);
> +static bool lr_stateful_build_vip_nats(struct lr_stateful_record *);
>  
>  /* 'lr_stateful' engine node manages the NB logical router LB data.
>   */
> @@ -110,6 +111,7 @@ en_lr_stateful_clear_tracked_data(void *data_)
>      struct ed_type_lr_stateful *data = (struct ed_type_lr_stateful *) data_;
>  
>      hmapx_clear(&data->trk_data.crupdated);
> +    data->trk_data.vip_nats_changed = false;
>  }
>  
>  void
> @@ -190,6 +192,10 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_)
>  
>              /* Add the lr_sful_rec rec to the tracking data. */
>              hmapx_add(&data->trk_data.crupdated, lr_sful_rec);
> +
> +            if (!sset_is_empty(&lr_sful_rec->vip_nats)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>              continue;
>          }
>  
> @@ -298,7 +304,9 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_)
>           * vip nats. */
>          HMAPX_FOR_EACH (hmapx_node, &data->trk_data.crupdated) {
>              lr_sful_rec = hmapx_node->data;
> -            lr_stateful_build_vip_nats(lr_sful_rec);
> +            if (lr_stateful_build_vip_nats(lr_sful_rec)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>              lr_sful_rec->has_lb_vip = od_has_lb_vip(lr_sful_rec->od);
>          }
>  
> @@ -335,8 +343,13 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void *data_)
>                                              lrnat_rec,
>                                              input_data.lb_datapaths_map,
>                                              input_data.lbgrp_datapaths_map);
> +            if (!sset_is_empty(&lr_sful_rec->vip_nats)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>          } else {
> -            lr_stateful_build_vip_nats(lr_sful_rec);
> +            if (lr_stateful_build_vip_nats(lr_sful_rec)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>          }
>  
>          /* Add the lr_sful_rec rec to the tracking data. */
> @@ -435,6 +448,7 @@ lr_stateful_record_create(struct lr_stateful_table *table,
>      lr_sful_rec->od = lrnat_rec->od;
>      lr_stateful_record_init(lr_sful_rec, lb_datapaths_map,
>                                 lbgrp_datapaths_map);
> +    lr_sful_rec->lflow_ref = lflow_ref_alloc(lr_sful_rec->od->nbr->name);
>  
>      hmap_insert(&table->entries, &lr_sful_rec->key_node,
>                  uuid_hash(&lr_sful_rec->od->nbr->header_.uuid));
> @@ -449,6 +463,7 @@ lr_stateful_record_destroy(struct lr_stateful_record *lr_sful_rec)
>      ovn_lb_ip_set_destroy(lr_sful_rec->lb_ips);
>      lr_sful_rec->lb_ips = NULL;
>      sset_destroy(&lr_sful_rec->vip_nats);
> +    lflow_ref_destroy(lr_sful_rec->lflow_ref);
>      free(lr_sful_rec);
>  }
>  
> @@ -515,7 +530,7 @@ lr_stateful_record_init(struct lr_stateful_record *lr_sful_rec,
>  
>      sset_init(&lr_sful_rec->vip_nats);
>  
> -    if (!nbr->n_nat) {
> +    if (nbr->n_nat) {

Is this a bug fix for a previous patch in the series?  I think this belongs in patch
05/16 ("northd: Add a new engine 'lr_stateful' to manage lr's stateful data.").

>          lr_stateful_build_vip_nats(lr_sful_rec);
>      }
>  
> @@ -629,10 +644,12 @@ remove_lrouter_lb_reachable_ips(struct lr_stateful_record *lr_sful_rec,
>      }
>  }
>  
> -static void
> +static bool
>  lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec)
>  {
> -    sset_clear(&lr_sful_rec->vip_nats);
> +    struct sset old_vip_nats = SSET_INITIALIZER(&old_vip_nats);
> +    sset_swap(&lr_sful_rec->vip_nats, &old_vip_nats);
> +
>      const char *external_ip;
>      SSET_FOR_EACH (external_ip, &lr_sful_rec->lrnat_rec->external_ips) {
>          bool is_vip_nat = false;
> @@ -648,4 +665,14 @@ lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec)
>              sset_add(&lr_sful_rec->vip_nats, external_ip);
>          }
>      }
> +
> +    bool vip_nats_changed =
> +        sset_count(&lr_sful_rec->vip_nats) != sset_count(&old_vip_nats);
> +    if (!vip_nats_changed) {
> +        vip_nats_changed = !sset_equals(&lr_sful_rec->vip_nats,
> +                                        &old_vip_nats);
> +    }

It's enough to write:

bool vip_nats_changed = !sset_equals(&lr_sful_rec->vip_nats,
                                     &old_vip_nats);

Tha'ts because sset_equals() checks internally first if both args
have the same length.

> +    sset_destroy(&old_vip_nats);
> +
> +    return vip_nats_changed;
>  }
> diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h
> index 0571e057f8..077de1bad1 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.
> @@ -56,6 +57,27 @@ 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_lb_nat_data record.
> +     *
> +     * This data is initialized and destroyed by the en_lr_lb_nat_data 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.
> +     */
> +    struct lflow_ref *lflow_ref;
>  };
>  
>  struct lr_stateful_table {
> @@ -75,6 +97,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;

We also need to update the lr_stateful_has_tracked_data() function to
take into account this extra state we're tracking.  I think it should
be:

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;
}

this to account for updated LR/LB-group associations (without new LBs).

Otherwise, in lr_stateful_lb_data_handler() below, we don't call
"engine_set_node_state(node, EN_UPDATED);" if an existing LB is
associated to a router and changes the "vip_nats".

>  };
>  
>  struct ed_type_lr_stateful {
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 0e17bfe2e6..8be413200e 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -228,11 +228,12 @@ 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_ls_stateful, NULL);

This is a duplicate.  Probably a typo.

>      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 f56916b3da..235b9e100f 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1227,7 +1227,7 @@ ovn_port_create(struct hmap *ports, const char *key,
>  
>      op->lflow_ref = lflow_ref_alloc(key);
>      op->stateful_lflow_ref = lflow_ref_alloc(key);
> -
> +    op->routable_lflow_ref = lflow_ref_alloc(key);
>      return op;
>  }
>  
> @@ -1268,6 +1268,7 @@ ovn_port_destroy_orphan(struct ovn_port *port)
>      free(port->key);
>      lflow_ref_destroy(port->lflow_ref);
>      lflow_ref_destroy(port->stateful_lflow_ref);
> +    lflow_ref_destroy(port->routable_lflow_ref);
>  
>      free(port);
>  }
> @@ -12832,63 +12833,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_sful_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 (int 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_sful_rec =
> -            lr_stateful_table_find_by_index(lr_sful_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_sful_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,
> @@ -13130,42 +13074,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_sful_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_sful_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
> @@ -13408,52 +13316,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_sful_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_sful_rec;
> -            lr_sful_rec = lr_stateful_table_find_by_index(lr_sful_table,
> -                                                    router_port->od->index);
> -            routable_addresses_to_lflows(lflows, router_port, peer,
> -                                         lr_sful_rec, match, actions,
> -                                         lflow_ref);
> -        }
> -    }
> -}
> -
>  static void
>  build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu,
>                              struct lflow_table *lflows,
> @@ -15670,8 +15532,6 @@ static void
>  build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
>                              struct ovn_port *lrp_peer,
>                              const struct lr_stateful_record *lr_sful_rec,
> -                            const struct lr_stateful_table *lr_sful_table,
> -                            const struct hmap *lr_ports,
>                              struct lflow_table *lflows,
>                              struct ds *match,
>                              struct ds *actions,
> @@ -15681,19 +15541,101 @@ build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
>      build_lswitch_rport_arp_req_flows_for_lbnats(
>          lrp_peer, lr_sful_rec, lsp->od, lsp,
>          lflows, &lsp->nbsp->header_, lflow_ref);
> -    build_ip_routing_flows_for_router_type_lsp(lsp, lr_sful_table,
> -                                               lr_ports, lflows,
> -                                               lflow_ref);
> -    build_arp_resolve_flows_for_lsp_routable_addresses(
> -        lsp, lflows, lr_ports, lr_sful_table, match, actions, lflow_ref);
>      build_lswitch_ip_unicast_lookup_for_nats(lsp, lr_sful_rec, lflows,
>                                               match, actions, lflow_ref);
>  }
>  
> +/* Logical router ingress table IP_ROUTING : IP Routing.
> + *
> + * For the LRP 'lrp's peer's logical switch, if there are logical router
> + * ports ('peer_lrp's) other than the 'lrp', then for routable addresses
> + * (such as NAT IPs, LB VIPs, etc.) of the 'lrp' add routes to the
> + * peer_lrp's datapath.
> + */
> +static void
> +build_routable_flows_for_router_port(
> +    struct ovn_port *lrp, const struct lr_stateful_record *lr_sful_rec,
> +    struct lflow_table *lflows,
> +    struct ds *match,
> +    struct ds *actions,
> +    struct lflow_ref *lflow_ref)
> +{
> +    ovs_assert(lrp->nbrp && lrp->od == lr_sful_rec->od);
> +
> +    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_sful_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,
> +                            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),
> +                              lflow_ref);
> +            }
> +        }
> +    }
> +
> +    destroy_routable_addresses(&ra);
> +}
> +
>  static void
>  build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op,
>                                    const struct lr_stateful_table *lr_sful_table,
> -                                  const struct hmap *lr_ports,
>                                    struct ds *match,
>                                    struct ds *actions,
>                                    struct lflow_table *lflows,
> @@ -15711,8 +15653,8 @@ build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op,
>      ovs_assert(lr_sful_rec);
>  
>      build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec,
> -                                lr_sful_table, lr_ports, lflows,
> -                                match, actions, lflow_ref);
> +                                lflows,match, actions,
> +                                lflow_ref);
>  }
>  
>  static void
> @@ -15762,7 +15704,8 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port *op,
>                                    struct ds *match,
>                                    struct ds *actions,
>                                    struct lflow_table *lflows,
> -                                  struct lflow_ref *lflow_ref)
> +                                  struct lflow_ref *lflow_ref,
> +                                  struct lflow_ref *routable_lflow_ref)
>  {
>      ovs_assert(op->nbrp);
>  
> @@ -15773,6 +15716,9 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port *op,
>  
>      build_lrp_lflows_for_lbnats(op, lr_sful_rec, meter_groups, match,
>                                  actions, lflows, lflow_ref);
> +
> +    build_routable_flows_for_router_port(op, lr_sful_rec, lflows, match,
> +                                         actions, routable_lflow_ref);
>  }
>  
>  static void
> @@ -15787,15 +15733,15 @@ build_lr_stateful_flows(const struct lr_stateful_record *lr_sful_rec,
>  {
>      build_lrouter_nat_defrag_and_lb(lr_sful_rec, lflows, ls_ports, lr_ports,
>                                      match, actions, meter_groups, features,
> -                                    NULL);
> +                                    lr_sful_rec->lflow_ref);
>      build_lr_gateway_redirect_flows_for_nats(lr_sful_rec->od,
>                                               lr_sful_rec->lrnat_rec, lflows,
>                                               match, actions,
> -                                             NULL);
> +                                             lr_sful_rec->lflow_ref);
>      build_lrouter_arp_nd_for_datapath(lr_sful_rec->od,
>                                        lr_sful_rec->lrnat_rec, lflows,
>                                        meter_groups,
> -                                      NULL);
> +                                      lr_sful_rec->lflow_ref);
>  }
>  
>  static void
> @@ -16026,7 +15972,6 @@ build_lflows_thread(void *arg)
>                                                               &lsi->actions,
>                                                               lsi->lflows);
>                      build_lbnat_lflows_iterate_by_lsp(op, lsi->lr_sful_table,
> -                                                      lsi->lr_ports,
>                                                        &lsi->match,
>                                                        &lsi->actions,
>                                                        lsi->lflows,
> @@ -16048,7 +15993,8 @@ build_lflows_thread(void *arg)
>                                                        &lsi->match,
>                                                        &lsi->actions,
>                                                        lsi->lflows,
> -                                                      op->stateful_lflow_ref);
> +                                                      op->stateful_lflow_ref,
> +                                                      op->routable_lflow_ref);
>                  }
>              }
>              for (bnum = control->id;
> @@ -16280,9 +16226,9 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths,
>                                                       &lsi.match,
>                                                       &lsi.actions,
>                                                       lsi.lflows);
> -            build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table, lsi.lr_ports,
> -                                              &lsi.match, &lsi.actions,
> -                                              lsi.lflows,
> +            build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table,
> +                                              &lsi.match,
> +                                              &lsi.actions, lsi.lflows,
>                                                op->stateful_lflow_ref);
>          }
>          HMAP_FOR_EACH (op, key_node, lr_ports) {
> @@ -16292,7 +16238,8 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths,
>                                                &lsi.match,
>                                                &lsi.actions,
>                                                lsi.lflows,
> -                                              op->stateful_lflow_ref);
> +                                              op->stateful_lflow_ref,
> +                                              op->routable_lflow_ref);
>          }
>          stopwatch_stop(LFLOWS_PORTS_STOPWATCH_NAME, time_msec());
>          stopwatch_start(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
> @@ -16485,12 +16432,24 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
>  void
>  reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
>  {
> +    const struct lr_stateful_record *lr_sful_rec;
>      struct ovn_lb_datapaths *lb_dps;
>      struct ovn_port *op;
>  
> +    LR_STATEFUL_TABLE_FOR_EACH (lr_sful_rec, lflow_input->lr_sful_table) {
> +        lflow_ref_reset(lr_sful_rec->lflow_ref);
> +    }
> +
>      HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
>          lflow_ref_reset(op->lflow_ref);
>          lflow_ref_reset(op->stateful_lflow_ref);
> +        lflow_ref_reset(op->routable_lflow_ref);
> +    }
> +
> +    HMAP_FOR_EACH (op, key_node, lflow_input->lr_ports) {
> +        lflow_ref_reset(op->lflow_ref);
> +        lflow_ref_reset(op->stateful_lflow_ref);
> +        lflow_ref_reset(op->routable_lflow_ref);
>      }
>  
>      HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
> @@ -16547,11 +16506,10 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
>              ovs_assert(lr_sful_rec);
>              lflow_ref_clear_lflows(op->stateful_lflow_ref);
>              build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec,
> -                                        lflow_input->lr_sful_table,
> -                                        lflow_input->lr_ports,
>                                          lflows, &match, &actions,
>                                          op->stateful_lflow_ref);
> -            lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows, ovnsb_txn,
> +            lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows,
> +                             ovnsb_txn,
>                               lflow_input->ls_datapaths,
>                               lflow_input->lr_datapaths,
>                               lflow_input->ovn_internal_version_changed,
> @@ -16705,6 +16663,92 @@ 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_sful_rec;
> +    struct hmapx_node *hmapx_node;
> +
> +    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
> +        lr_sful_rec = hmapx_node->data;
> +
> +        lflow_ref_clear_lflows(lr_sful_rec->lflow_ref);
> +
> +        /* Generate new lflows. */
> +        struct ds match = DS_EMPTY_INITIALIZER;
> +        struct ds actions = DS_EMPTY_INITIALIZER;
> +
> +        build_lr_stateful_flows(lr_sful_rec, lflows, lflow_input->ls_ports,
> +                                lflow_input->lr_ports, &match, &actions,
> +                                lflow_input->meter_groups,
> +                                lflow_input->features);
> +
> +        /* Sync the new flows to SB. */
> +        lflow_ref_sync_lflows_to_sb(lr_sful_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);
> +
> +        struct ovn_port *op;
> +        HMAP_FOR_EACH (op, dp_node, &lr_sful_rec->od->ports) {
> +            lflow_ref_clear_lflows(op->stateful_lflow_ref);
> +            lflow_ref_clear_lflows_for_all_dps(op->routable_lflow_ref,
> +                                        ods_size(lflow_input->ls_datapaths),
> +                                        ods_size(lflow_input->lr_datapaths));
> +
> +            build_lbnat_lflows_iterate_by_lrp(op, lflow_input->lr_sful_table,
> +                                              lflow_input->meter_groups,
> +                                              &match, &actions,
> +                                              lflows,
> +                                              op->stateful_lflow_ref,
> +                                              op->routable_lflow_ref);
> +
> +            lflow_ref_sync_lflows_to_sb(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);
> +
> +            lflow_ref_sync_lflows_to_sb(op->routable_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 (op->peer && op->peer->nbsp) {
> +                lflow_ref_clear_lflows(op->peer->stateful_lflow_ref);
> +
> +                build_lbnat_lflows_iterate_by_lsp(op->peer,
> +                                                lflow_input->lr_sful_table,
> +                                                &match, &actions,
> +                                                lflows,
> +                                                op->peer->stateful_lflow_ref);
> +
> +                lflow_ref_sync_lflows_to_sb(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);
> +            }
> +        }
> +
> +        ds_destroy(&match);
> +        ds_destroy(&actions);
> +    }
> +
> +    return true;
> +}
> +
>  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 863bcce001..6adf06c26f 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -688,9 +688,13 @@ struct ovn_port {
>       * 'stateful_lflow_ref' is used for logical switch ports of type
>       * 'patch/router' to referenece logical flows generated fo this ovn_port
>       *  from the 'lr_stateful' record of the peer port's datapath.
> +     *
> +     * 'routable_lflow_ref' is used to reference logical flows generated for
> +     * the routable ips of a logical router port.
>       */
>      struct lflow_ref *lflow_ref;
>      struct lflow_ref *stateful_lflow_ref;
> +    struct lflow_ref *routable_lflow_ref;
>  };
>  
>  void ovnnb_db_run(struct northd_input *input_data,
> @@ -715,6 +719,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 *);
> @@ -728,6 +734,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 50d88fecd5..f517057534 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
> @@ -10687,7 +10688,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
>  
> @@ -10697,7 +10698,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
>  
> @@ -10707,7 +10708,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
>  
> @@ -10717,7 +10718,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
>  
> @@ -10727,7 +10728,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
>  
> @@ -10735,6 +10736,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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
> @@ -10850,7 +10852,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
>  
> @@ -10860,7 +10862,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
>  
> @@ -10870,7 +10872,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
>  
> @@ -10880,7 +10882,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
>  
> @@ -10890,7 +10892,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
>  
> @@ -10971,7 +10973,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
>  
> @@ -10999,7 +11001,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
>  
> @@ -11177,7 +11179,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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
>  
> @@ -11186,7 +11188,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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
>  
> @@ -11196,7 +11198,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
>  
> @@ -11206,7 +11208,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
>  
> @@ -11216,7 +11218,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
>  
> @@ -11226,7 +11228,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
>  
> @@ -11237,7 +11239,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
>  
> @@ -11258,6 +11260,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 NORTHD_TYPE 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
> @@ -11267,6 +11271,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 NORTHD_TYPE 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
> @@ -11276,6 +11282,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 NORTHD_TYPE 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
> @@ -11291,7 +11299,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
>  

Regards,
Dumitru

On Mon, Nov 27, 2023 at 6:38 PM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  northd/en-lflow.c        |  29 +++
>  northd/en-lflow.h        |   1 +
>  northd/en-lr-stateful.c  |  39 ++++-
>  northd/en-lr-stateful.h  |  26 +++
>  northd/inc-proc-northd.c |   3 +-
>  northd/northd.c          | 370 ++++++++++++++++++++++-----------------
>  northd/northd.h          |  10 ++
>  tests/ovn-northd.at      |  48 ++---
>  8 files changed, 336 insertions(+), 190 deletions(-)
>
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 13284b5556..09748f570b 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -164,6 +164,35 @@ 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 a54749ad93..8e025f057e 100644
> --- a/northd/en-lr-stateful.c
> +++ b/northd/en-lr-stateful.c
> @@ -39,6 +39,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_build_vip_nats(struct lr_stateful_record *);
> +static bool lr_stateful_build_vip_nats(struct lr_stateful_record *);
>
>  /* 'lr_stateful' engine node manages the NB logical router LB data.
>   */
> @@ -110,6 +111,7 @@ en_lr_stateful_clear_tracked_data(void *data_)
>      struct ed_type_lr_stateful *data = (struct ed_type_lr_stateful *)
data_;
>
>      hmapx_clear(&data->trk_data.crupdated);
> +    data->trk_data.vip_nats_changed = false;
>  }
>
>  void
> @@ -190,6 +192,10 @@ lr_stateful_lb_data_handler(struct engine_node
*node, void *data_)
>
>              /* Add the lr_sful_rec rec to the tracking data. */
>              hmapx_add(&data->trk_data.crupdated, lr_sful_rec);
> +
> +            if (!sset_is_empty(&lr_sful_rec->vip_nats)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>              continue;
>          }
>
> @@ -298,7 +304,9 @@ lr_stateful_lb_data_handler(struct engine_node *node,
void *data_)
>           * vip nats. */
>          HMAPX_FOR_EACH (hmapx_node, &data->trk_data.crupdated) {
>              lr_sful_rec = hmapx_node->data;
> -            lr_stateful_build_vip_nats(lr_sful_rec);
> +            if (lr_stateful_build_vip_nats(lr_sful_rec)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>              lr_sful_rec->has_lb_vip = od_has_lb_vip(lr_sful_rec->od);
>          }
>
> @@ -335,8 +343,13 @@ lr_stateful_lr_nat_handler(struct engine_node *node,
void *data_)
>                                              lrnat_rec,
>                                              input_data.lb_datapaths_map,
>
 input_data.lbgrp_datapaths_map);
> +            if (!sset_is_empty(&lr_sful_rec->vip_nats)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>          } else {
> -            lr_stateful_build_vip_nats(lr_sful_rec);
> +            if (lr_stateful_build_vip_nats(lr_sful_rec)) {
> +                data->trk_data.vip_nats_changed = true;
> +            }
>          }
>
>          /* Add the lr_sful_rec rec to the tracking data. */
> @@ -435,6 +448,7 @@ lr_stateful_record_create(struct lr_stateful_table
*table,
>      lr_sful_rec->od = lrnat_rec->od;
>      lr_stateful_record_init(lr_sful_rec, lb_datapaths_map,
>                                 lbgrp_datapaths_map);
> +    lr_sful_rec->lflow_ref = lflow_ref_alloc(lr_sful_rec->od->nbr->name);
>
>      hmap_insert(&table->entries, &lr_sful_rec->key_node,
>                  uuid_hash(&lr_sful_rec->od->nbr->header_.uuid));
> @@ -449,6 +463,7 @@ lr_stateful_record_destroy(struct lr_stateful_record
*lr_sful_rec)
>      ovn_lb_ip_set_destroy(lr_sful_rec->lb_ips);
>      lr_sful_rec->lb_ips = NULL;
>      sset_destroy(&lr_sful_rec->vip_nats);
> +    lflow_ref_destroy(lr_sful_rec->lflow_ref);
>      free(lr_sful_rec);
>  }
>
> @@ -515,7 +530,7 @@ lr_stateful_record_init(struct lr_stateful_record
*lr_sful_rec,
>
>      sset_init(&lr_sful_rec->vip_nats);
>
> -    if (!nbr->n_nat) {
> +    if (nbr->n_nat) {
>          lr_stateful_build_vip_nats(lr_sful_rec);
>      }
>
> @@ -629,10 +644,12 @@ remove_lrouter_lb_reachable_ips(struct
lr_stateful_record *lr_sful_rec,
>      }
>  }
>
> -static void
> +static bool
>  lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec)
>  {
> -    sset_clear(&lr_sful_rec->vip_nats);
> +    struct sset old_vip_nats = SSET_INITIALIZER(&old_vip_nats);
> +    sset_swap(&lr_sful_rec->vip_nats, &old_vip_nats);
> +
>      const char *external_ip;
>      SSET_FOR_EACH (external_ip, &lr_sful_rec->lrnat_rec->external_ips) {
>          bool is_vip_nat = false;
> @@ -648,4 +665,14 @@ lr_stateful_build_vip_nats(struct lr_stateful_record
*lr_sful_rec)
>              sset_add(&lr_sful_rec->vip_nats, external_ip);
>          }
>      }
> +
> +    bool vip_nats_changed =
> +        sset_count(&lr_sful_rec->vip_nats) != sset_count(&old_vip_nats);
> +    if (!vip_nats_changed) {
> +        vip_nats_changed = !sset_equals(&lr_sful_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 0571e057f8..077de1bad1 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.
> @@ -56,6 +57,27 @@ 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_lb_nat_data record.
> +     *
> +     * This data is initialized and destroyed by the en_lr_lb_nat_data
node,

s/en_lr_br_nat_data/en_lr_stateful

In addition, the same/redundant comments should be applied to all the
similar lflow_ref members in other structures (such as lb_datapaths), so it
would be good to have such comments describing the principle in one place
and have a pointer from each place such member is defined.

> +     * 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.
> +     */
> +    struct lflow_ref *lflow_ref;
>  };
>
>  struct lr_stateful_table {
> @@ -75,6 +97,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 {
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 0e17bfe2e6..8be413200e 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -228,11 +228,12 @@ 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_ls_stateful, 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 f56916b3da..235b9e100f 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1227,7 +1227,7 @@ ovn_port_create(struct hmap *ports, const char *key,
>
>      op->lflow_ref = lflow_ref_alloc(key);
>      op->stateful_lflow_ref = lflow_ref_alloc(key);
> -
> +    op->routable_lflow_ref = lflow_ref_alloc(key);
>      return op;
>  }
>
> @@ -1268,6 +1268,7 @@ ovn_port_destroy_orphan(struct ovn_port *port)
>      free(port->key);
>      lflow_ref_destroy(port->lflow_ref);
>      lflow_ref_destroy(port->stateful_lflow_ref);
> +    lflow_ref_destroy(port->routable_lflow_ref);
>
>      free(port);
>  }
> @@ -12832,63 +12833,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_sful_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 (int 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_sful_rec =
> -            lr_stateful_table_find_by_index(lr_sful_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_sful_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,
> @@ -13130,42 +13074,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_sful_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_sful_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
> @@ -13408,52 +13316,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_sful_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_sful_rec;
> -            lr_sful_rec = lr_stateful_table_find_by_index(lr_sful_table,
> -
 router_port->od->index);
> -            routable_addresses_to_lflows(lflows, router_port, peer,
> -                                         lr_sful_rec, match, actions,
> -                                         lflow_ref);
> -        }
> -    }
> -}
> -
>  static void
>  build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu,
>                              struct lflow_table *lflows,
> @@ -15670,8 +15532,6 @@ static void
>  build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
>                              struct ovn_port *lrp_peer,
>                              const struct lr_stateful_record *lr_sful_rec,
> -                            const struct lr_stateful_table
*lr_sful_table,
> -                            const struct hmap *lr_ports,
>                              struct lflow_table *lflows,
>                              struct ds *match,
>                              struct ds *actions,
> @@ -15681,19 +15541,101 @@ build_lsp_lflows_for_lbnats(struct ovn_port
*lsp,
>      build_lswitch_rport_arp_req_flows_for_lbnats(
>          lrp_peer, lr_sful_rec, lsp->od, lsp,
>          lflows, &lsp->nbsp->header_, lflow_ref);
> -    build_ip_routing_flows_for_router_type_lsp(lsp, lr_sful_table,
> -                                               lr_ports, lflows,
> -                                               lflow_ref);
> -    build_arp_resolve_flows_for_lsp_routable_addresses(
> -        lsp, lflows, lr_ports, lr_sful_table, match, actions, lflow_ref);
>      build_lswitch_ip_unicast_lookup_for_nats(lsp, lr_sful_rec, lflows,
>                                               match, actions, lflow_ref);
>  }
>
> +/* Logical router ingress table IP_ROUTING : IP Routing.
> + *
> + * For the LRP 'lrp's peer's logical switch, if there are logical router
> + * ports ('peer_lrp's) other than the 'lrp', then for routable addresses
> + * (such as NAT IPs, LB VIPs, etc.) of the 'lrp' add routes to the
> + * peer_lrp's datapath.
> + */

The comment needs to be updated.
1. It should include ARP_RESOLVE, too
2. There is no peer_lrp any more. It is changed to adding peer LS's router
ports' routable addresses to the lrp's LR datapath lflows.

> +static void
> +build_routable_flows_for_router_port(
> +    struct ovn_port *lrp, const struct lr_stateful_record *lr_sful_rec,
> +    struct lflow_table *lflows,
> +    struct ds *match,
> +    struct ds *actions,
> +    struct lflow_ref *lflow_ref)
> +{
> +    ovs_assert(lrp->nbrp && lrp->od == lr_sful_rec->od);

This assert is good, and it should also be added to
build_lrp_lflows_for_lbnats. This assertion is helpful because we are doing
I-P for two types of inputs (lr_sful_rec and lrp) of the same lflows, while
one of the inputs (lr_sful_rec) is deduced from the other input (lrp). The
assertion helps maintain this assumption.

> +
> +    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_sful_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,
> +                            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),
> +                              lflow_ref);
> +            }
> +        }
> +    }
> +
> +    destroy_routable_addresses(&ra);
> +}
> +
>  static void
>  build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op,
>                                    const struct lr_stateful_table
*lr_sful_table,
> -                                  const struct hmap *lr_ports,
>                                    struct ds *match,
>                                    struct ds *actions,
>                                    struct lflow_table *lflows,
> @@ -15711,8 +15653,8 @@ build_lbnat_lflows_iterate_by_lsp(struct ovn_port
*op,
>      ovs_assert(lr_sful_rec);
>
>      build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec,
> -                                lr_sful_table, lr_ports, lflows,
> -                                match, actions, lflow_ref);
> +                                lflows,match, actions,
> +                                lflow_ref);
>  }
>
>  static void
> @@ -15762,7 +15704,8 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port
*op,
>                                    struct ds *match,
>                                    struct ds *actions,
>                                    struct lflow_table *lflows,
> -                                  struct lflow_ref *lflow_ref)
> +                                  struct lflow_ref *lflow_ref,
> +                                  struct lflow_ref *routable_lflow_ref)
>  {
>      ovs_assert(op->nbrp);
>
> @@ -15773,6 +15716,9 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port
*op,
>
>      build_lrp_lflows_for_lbnats(op, lr_sful_rec, meter_groups, match,
>                                  actions, lflows, lflow_ref);
> +
> +    build_routable_flows_for_router_port(op, lr_sful_rec, lflows, match,
> +                                         actions, routable_lflow_ref);
>  }
>
>  static void
> @@ -15787,15 +15733,15 @@ build_lr_stateful_flows(const struct
lr_stateful_record *lr_sful_rec,
>  {
>      build_lrouter_nat_defrag_and_lb(lr_sful_rec, lflows, ls_ports,
lr_ports,
>                                      match, actions, meter_groups,
features,
> -                                    NULL);
> +                                    lr_sful_rec->lflow_ref);
>      build_lr_gateway_redirect_flows_for_nats(lr_sful_rec->od,
>                                               lr_sful_rec->lrnat_rec,
lflows,
>                                               match, actions,
> -                                             NULL);
> +                                             lr_sful_rec->lflow_ref);
>      build_lrouter_arp_nd_for_datapath(lr_sful_rec->od,
>                                        lr_sful_rec->lrnat_rec, lflows,
>                                        meter_groups,
> -                                      NULL);
> +                                      lr_sful_rec->lflow_ref);
>  }
>
>  static void
> @@ -16026,7 +15972,6 @@ build_lflows_thread(void *arg)
>
&lsi->actions,
>
lsi->lflows);
>                      build_lbnat_lflows_iterate_by_lsp(op,
lsi->lr_sful_table,
> -                                                      lsi->lr_ports,
>                                                        &lsi->match,
>                                                        &lsi->actions,
>                                                        lsi->lflows,
> @@ -16048,7 +15993,8 @@ build_lflows_thread(void *arg)
>                                                        &lsi->match,
>                                                        &lsi->actions,
>                                                        lsi->lflows,
> -
 op->stateful_lflow_ref);
> +
 op->stateful_lflow_ref,
> +
 op->routable_lflow_ref);
>                  }
>              }
>              for (bnum = control->id;
> @@ -16280,9 +16226,9 @@ build_lswitch_and_lrouter_flows(const struct
ovn_datapaths *ls_datapaths,
>                                                       &lsi.match,
>                                                       &lsi.actions,
>                                                       lsi.lflows);
> -            build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table,
lsi.lr_ports,
> -                                              &lsi.match, &lsi.actions,
> -                                              lsi.lflows,
> +            build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table,
> +                                              &lsi.match,
> +                                              &lsi.actions, lsi.lflows,
>                                                op->stateful_lflow_ref);
>          }
>          HMAP_FOR_EACH (op, key_node, lr_ports) {
> @@ -16292,7 +16238,8 @@ build_lswitch_and_lrouter_flows(const struct
ovn_datapaths *ls_datapaths,
>                                                &lsi.match,
>                                                &lsi.actions,
>                                                lsi.lflows,
> -                                              op->stateful_lflow_ref);
> +                                              op->stateful_lflow_ref,
> +                                              op->routable_lflow_ref);
>          }
>          stopwatch_stop(LFLOWS_PORTS_STOPWATCH_NAME, time_msec());
>          stopwatch_start(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
> @@ -16485,12 +16432,24 @@ void build_lflows(struct ovsdb_idl_txn
*ovnsb_txn,
>  void
>  reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
>  {
> +    const struct lr_stateful_record *lr_sful_rec;
>      struct ovn_lb_datapaths *lb_dps;
>      struct ovn_port *op;
>
> +    LR_STATEFUL_TABLE_FOR_EACH (lr_sful_rec, lflow_input->lr_sful_table)
{
> +        lflow_ref_reset(lr_sful_rec->lflow_ref);
> +    }
> +
>      HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
>          lflow_ref_reset(op->lflow_ref);
>          lflow_ref_reset(op->stateful_lflow_ref);
> +        lflow_ref_reset(op->routable_lflow_ref);
> +    }
> +
> +    HMAP_FOR_EACH (op, key_node, lflow_input->lr_ports) {
> +        lflow_ref_reset(op->lflow_ref);
> +        lflow_ref_reset(op->stateful_lflow_ref);
> +        lflow_ref_reset(op->routable_lflow_ref);
>      }
>
>      HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
> @@ -16547,11 +16506,10 @@ lflow_handle_northd_port_changes(struct
ovsdb_idl_txn *ovnsb_txn,
>              ovs_assert(lr_sful_rec);
>              lflow_ref_clear_lflows(op->stateful_lflow_ref);
>              build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec,
> -                                        lflow_input->lr_sful_table,
> -                                        lflow_input->lr_ports,
>                                          lflows, &match, &actions,
>                                          op->stateful_lflow_ref);
> -            lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows,
ovnsb_txn,
> +            lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows,
> +                             ovnsb_txn,
>                               lflow_input->ls_datapaths,
>                               lflow_input->lr_datapaths,
>                               lflow_input->ovn_internal_version_changed,
> @@ -16705,6 +16663,92 @@ 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_sful_rec;
> +    struct hmapx_node *hmapx_node;
> +
> +    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
> +        lr_sful_rec = hmapx_node->data;
> +
> +        lflow_ref_clear_lflows(lr_sful_rec->lflow_ref);
> +
> +        /* Generate new lflows. */
> +        struct ds match = DS_EMPTY_INITIALIZER;
> +        struct ds actions = DS_EMPTY_INITIALIZER;
> +
> +        build_lr_stateful_flows(lr_sful_rec, lflows,
lflow_input->ls_ports,
> +                                lflow_input->lr_ports, &match, &actions,
> +                                lflow_input->meter_groups,
> +                                lflow_input->features);
> +
> +        /* Sync the new flows to SB. */
> +        lflow_ref_sync_lflows_to_sb(lr_sful_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);
> +
> +        struct ovn_port *op;
> +        HMAP_FOR_EACH (op, dp_node, &lr_sful_rec->od->ports) {
> +            lflow_ref_clear_lflows(op->stateful_lflow_ref);
> +            lflow_ref_clear_lflows_for_all_dps(op->routable_lflow_ref,
> +
 ods_size(lflow_input->ls_datapaths),
> +
 ods_size(lflow_input->lr_datapaths));
> +
> +            build_lbnat_lflows_iterate_by_lrp(op,
lflow_input->lr_sful_table,
> +                                              lflow_input->meter_groups,
> +                                              &match, &actions,
> +                                              lflows,
> +                                              op->stateful_lflow_ref,
> +                                              op->routable_lflow_ref);
> +
> +            lflow_ref_sync_lflows_to_sb(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);
> +
> +            lflow_ref_sync_lflows_to_sb(op->routable_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 (op->peer && op->peer->nbsp) {
> +                lflow_ref_clear_lflows(op->peer->stateful_lflow_ref);
> +
> +                build_lbnat_lflows_iterate_by_lsp(op->peer,
> +
 lflow_input->lr_sful_table,
> +                                                &match, &actions,
> +                                                lflows,
> +
 op->peer->stateful_lflow_ref);
> +
> +                lflow_ref_sync_lflows_to_sb(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);
> +            }
> +        }
> +
> +        ds_destroy(&match);
> +        ds_destroy(&actions);
> +    }
> +
> +    return true;
> +}
> +
>  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 863bcce001..6adf06c26f 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -688,9 +688,13 @@ struct ovn_port {
>       * 'stateful_lflow_ref' is used for logical switch ports of type
>       * 'patch/router' to referenece logical flows generated fo this
ovn_port
>       *  from the 'lr_stateful' record of the peer port's datapath.
> +     *
> +     * 'routable_lflow_ref' is used to reference logical flows generated
for
> +     * the routable ips of a logical router port.
>       */
>      struct lflow_ref *lflow_ref;
>      struct lflow_ref *stateful_lflow_ref;
> +    struct lflow_ref *routable_lflow_ref;

In my opinion it is better to avoid adding the extra routable_lflow_ref. In
this patch all the stateful_lflow_ref and routable_lflow_ref are handled
together. Even if it may be separately cleaned and reprocessed, the lflows
associated with a single ovn_port (in this case it is a LRP) is still very
few and it would look simpler (for correctness and maintainability) to
handle them together atomically. (We may separate them some day when they
need to be associated with different objects, if needed in the future.)

Thanks,
Han

>  };
>
>  void ovnnb_db_run(struct northd_input *input_data,
> @@ -715,6 +719,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 *);
> @@ -728,6 +734,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 50d88fecd5..f517057534 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
> @@ -10687,7 +10688,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
>
> @@ -10697,7 +10698,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
>
> @@ -10707,7 +10708,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
>
> @@ -10717,7 +10718,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
>
> @@ -10727,7 +10728,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
>
> @@ -10735,6 +10736,7 @@ check as northd ovn-appctl -t NORTHD_TYPE
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
> @@ -10850,7 +10852,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
>
> @@ -10860,7 +10862,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
>
> @@ -10870,7 +10872,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
>
> @@ -10880,7 +10882,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
>
> @@ -10890,7 +10892,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
>
> @@ -10971,7 +10973,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
>
> @@ -10999,7 +11001,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
>
> @@ -11177,7 +11179,7 @@ check as northd ovn-appctl -t NORTHD_TYPE
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
>
> @@ -11186,7 +11188,7 @@ check as northd ovn-appctl -t NORTHD_TYPE
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
>
> @@ -11196,7 +11198,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
>
> @@ -11206,7 +11208,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
>
> @@ -11216,7 +11218,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
>
> @@ -11226,7 +11228,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
>
> @@ -11237,7 +11239,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
>
> @@ -11258,6 +11260,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 NORTHD_TYPE 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
> @@ -11267,6 +11271,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 NORTHD_TYPE 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
> @@ -11276,6 +11282,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 NORTHD_TYPE 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
> @@ -11291,7 +11299,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
>
> --
> 2.41.0
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

On Wed, Dec 13, 2023 at 8:54 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 11/28/23 03:37, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >  northd/en-lflow.c        |  29 +++
> >  northd/en-lflow.h        |   1 +
> >  northd/en-lr-stateful.c  |  39 ++++-
> >  northd/en-lr-stateful.h  |  26 +++
> >  northd/inc-proc-northd.c |   3 +-
> >  northd/northd.c          | 370 ++++++++++++++++++++++-----------------
> >  northd/northd.h          |  10 ++
> >  tests/ovn-northd.at      |  48 ++---
> >  8 files changed, 336 insertions(+), 190 deletions(-)
> >
> > diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> > index 13284b5556..09748f570b 100644
> > --- a/northd/en-lflow.c
> > +++ b/northd/en-lflow.c
> > @@ -164,6 +164,35 @@ 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 =
>
> I don't really like the 'lr_sful_data' abbreviation.  What about
> 'stateful_data', it's one character longer?  Or even 'lr_stateful_data'?
>
> I now realize that this is used in a lot of places throughout
> the series.  I don't want to generate a lot of extra work just to
> rename it so I'll leave it up to you.
>

In v4,  I've changed  'sful' to 'stateful' where it is used.


> > +        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;
>
> Nit: I'd add an empty line here and remove the one above (to group
> all declarations).

Ack.  Done in v4.


>
> > +    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 a54749ad93..8e025f057e 100644
> > --- a/northd/en-lr-stateful.c
> > +++ b/northd/en-lr-stateful.c
> > @@ -39,6 +39,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_build_vip_nats(struct lr_stateful_record *);
> > +static bool lr_stateful_build_vip_nats(struct lr_stateful_record *);
> >
> >  /* 'lr_stateful' engine node manages the NB logical router LB data.
> >   */
> > @@ -110,6 +111,7 @@ en_lr_stateful_clear_tracked_data(void *data_)
> >      struct ed_type_lr_stateful *data = (struct ed_type_lr_stateful *) data_;
> >
> >      hmapx_clear(&data->trk_data.crupdated);
> > +    data->trk_data.vip_nats_changed = false;
> >  }
> >
> >  void
> > @@ -190,6 +192,10 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_)
> >
> >              /* Add the lr_sful_rec rec to the tracking data. */
> >              hmapx_add(&data->trk_data.crupdated, lr_sful_rec);
> > +
> > +            if (!sset_is_empty(&lr_sful_rec->vip_nats)) {
> > +                data->trk_data.vip_nats_changed = true;
> > +            }
> >              continue;
> >          }
> >
> > @@ -298,7 +304,9 @@ lr_stateful_lb_data_handler(struct engine_node *node, void *data_)
> >           * vip nats. */
> >          HMAPX_FOR_EACH (hmapx_node, &data->trk_data.crupdated) {
> >              lr_sful_rec = hmapx_node->data;
> > -            lr_stateful_build_vip_nats(lr_sful_rec);
> > +            if (lr_stateful_build_vip_nats(lr_sful_rec)) {
> > +                data->trk_data.vip_nats_changed = true;
> > +            }
> >              lr_sful_rec->has_lb_vip = od_has_lb_vip(lr_sful_rec->od);
> >          }
> >
> > @@ -335,8 +343,13 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void *data_)
> >                                              lrnat_rec,
> >                                              input_data.lb_datapaths_map,
> >                                              input_data.lbgrp_datapaths_map);
> > +            if (!sset_is_empty(&lr_sful_rec->vip_nats)) {
> > +                data->trk_data.vip_nats_changed = true;
> > +            }
> >          } else {
> > -            lr_stateful_build_vip_nats(lr_sful_rec);
> > +            if (lr_stateful_build_vip_nats(lr_sful_rec)) {
> > +                data->trk_data.vip_nats_changed = true;
> > +            }
> >          }
> >
> >          /* Add the lr_sful_rec rec to the tracking data. */
> > @@ -435,6 +448,7 @@ lr_stateful_record_create(struct lr_stateful_table *table,
> >      lr_sful_rec->od = lrnat_rec->od;
> >      lr_stateful_record_init(lr_sful_rec, lb_datapaths_map,
> >                                 lbgrp_datapaths_map);
> > +    lr_sful_rec->lflow_ref = lflow_ref_alloc(lr_sful_rec->od->nbr->name);
> >
> >      hmap_insert(&table->entries, &lr_sful_rec->key_node,
> >                  uuid_hash(&lr_sful_rec->od->nbr->header_.uuid));
> > @@ -449,6 +463,7 @@ lr_stateful_record_destroy(struct lr_stateful_record *lr_sful_rec)
> >      ovn_lb_ip_set_destroy(lr_sful_rec->lb_ips);
> >      lr_sful_rec->lb_ips = NULL;
> >      sset_destroy(&lr_sful_rec->vip_nats);
> > +    lflow_ref_destroy(lr_sful_rec->lflow_ref);
> >      free(lr_sful_rec);
> >  }
> >
> > @@ -515,7 +530,7 @@ lr_stateful_record_init(struct lr_stateful_record *lr_sful_rec,
> >
> >      sset_init(&lr_sful_rec->vip_nats);
> >
> > -    if (!nbr->n_nat) {
> > +    if (nbr->n_nat) {
>
> Is this a bug fix for a previous patch in the series?  I think this belongs in patch
> 05/16 ("northd: Add a new engine 'lr_stateful' to manage lr's stateful data.").

Good catch.  Fixed in v4.


>
> >          lr_stateful_build_vip_nats(lr_sful_rec);
> >      }
> >
> > @@ -629,10 +644,12 @@ remove_lrouter_lb_reachable_ips(struct lr_stateful_record *lr_sful_rec,
> >      }
> >  }
> >
> > -static void
> > +static bool
> >  lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec)
> >  {
> > -    sset_clear(&lr_sful_rec->vip_nats);
> > +    struct sset old_vip_nats = SSET_INITIALIZER(&old_vip_nats);
> > +    sset_swap(&lr_sful_rec->vip_nats, &old_vip_nats);
> > +
> >      const char *external_ip;
> >      SSET_FOR_EACH (external_ip, &lr_sful_rec->lrnat_rec->external_ips) {
> >          bool is_vip_nat = false;
> > @@ -648,4 +665,14 @@ lr_stateful_build_vip_nats(struct lr_stateful_record *lr_sful_rec)
> >              sset_add(&lr_sful_rec->vip_nats, external_ip);
> >          }
> >      }
> > +
> > +    bool vip_nats_changed =
> > +        sset_count(&lr_sful_rec->vip_nats) != sset_count(&old_vip_nats);
> > +    if (!vip_nats_changed) {
> > +        vip_nats_changed = !sset_equals(&lr_sful_rec->vip_nats,
> > +                                        &old_vip_nats);
> > +    }
>
> It's enough to write:
>
> bool vip_nats_changed = !sset_equals(&lr_sful_rec->vip_nats,
>                                      &old_vip_nats);
>
> Tha'ts because sset_equals() checks internally first if both args
> have the same length.

Ack.  Done in v4.


>
> > +    sset_destroy(&old_vip_nats);
> > +
> > +    return vip_nats_changed;
> >  }
> > diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h
> > index 0571e057f8..077de1bad1 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.
> > @@ -56,6 +57,27 @@ 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_lb_nat_data record.
> > +     *
> > +     * This data is initialized and destroyed by the en_lr_lb_nat_data 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.
> > +     */
> > +    struct lflow_ref *lflow_ref;
> >  };
> >
> >  struct lr_stateful_table {
> > @@ -75,6 +97,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;
>
> We also need to update the lr_stateful_has_tracked_data() function to
> take into account this extra state we're tracking.  I think it should
> be:
>
> 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;
> }
>
> this to account for updated LR/LB-group associations (without new LBs).
>
> Otherwise, in lr_stateful_lb_data_handler() below, we don't call
> "engine_set_node_state(node, EN_UPDATED);" if an existing LB is
> associated to a router and changes the "vip_nats".

Good catch.  Addressed in v4.


>
> >  };
> >
> >  struct ed_type_lr_stateful {
> > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > index 0e17bfe2e6..8be413200e 100644
> > --- a/northd/inc-proc-northd.c
> > +++ b/northd/inc-proc-northd.c
> > @@ -228,11 +228,12 @@ 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_ls_stateful, NULL);
>
> This is a duplicate.  Probably a typo.

Yes.  Fixed in v4.



>
> >      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 f56916b3da..235b9e100f 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1227,7 +1227,7 @@ ovn_port_create(struct hmap *ports, const char *key,
> >
> >      op->lflow_ref = lflow_ref_alloc(key);
> >      op->stateful_lflow_ref = lflow_ref_alloc(key);
> > -
> > +    op->routable_lflow_ref = lflow_ref_alloc(key);
> >      return op;
> >  }
> >
> > @@ -1268,6 +1268,7 @@ ovn_port_destroy_orphan(struct ovn_port *port)
> >      free(port->key);
> >      lflow_ref_destroy(port->lflow_ref);
> >      lflow_ref_destroy(port->stateful_lflow_ref);
> > +    lflow_ref_destroy(port->routable_lflow_ref);
> >
> >      free(port);
> >  }
> > @@ -12832,63 +12833,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_sful_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 (int 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_sful_rec =
> > -            lr_stateful_table_find_by_index(lr_sful_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_sful_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,
> > @@ -13130,42 +13074,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_sful_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_sful_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
> > @@ -13408,52 +13316,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_sful_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_sful_rec;
> > -            lr_sful_rec = lr_stateful_table_find_by_index(lr_sful_table,
> > -                                                    router_port->od->index);
> > -            routable_addresses_to_lflows(lflows, router_port, peer,
> > -                                         lr_sful_rec, match, actions,
> > -                                         lflow_ref);
> > -        }
> > -    }
> > -}
> > -
> >  static void
> >  build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu,
> >                              struct lflow_table *lflows,
> > @@ -15670,8 +15532,6 @@ static void
> >  build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
> >                              struct ovn_port *lrp_peer,
> >                              const struct lr_stateful_record *lr_sful_rec,
> > -                            const struct lr_stateful_table *lr_sful_table,
> > -                            const struct hmap *lr_ports,
> >                              struct lflow_table *lflows,
> >                              struct ds *match,
> >                              struct ds *actions,
> > @@ -15681,19 +15541,101 @@ build_lsp_lflows_for_lbnats(struct ovn_port *lsp,
> >      build_lswitch_rport_arp_req_flows_for_lbnats(
> >          lrp_peer, lr_sful_rec, lsp->od, lsp,
> >          lflows, &lsp->nbsp->header_, lflow_ref);
> > -    build_ip_routing_flows_for_router_type_lsp(lsp, lr_sful_table,
> > -                                               lr_ports, lflows,
> > -                                               lflow_ref);
> > -    build_arp_resolve_flows_for_lsp_routable_addresses(
> > -        lsp, lflows, lr_ports, lr_sful_table, match, actions, lflow_ref);
> >      build_lswitch_ip_unicast_lookup_for_nats(lsp, lr_sful_rec, lflows,
> >                                               match, actions, lflow_ref);
> >  }
> >
> > +/* Logical router ingress table IP_ROUTING : IP Routing.
> > + *
> > + * For the LRP 'lrp's peer's logical switch, if there are logical router
> > + * ports ('peer_lrp's) other than the 'lrp', then for routable addresses
> > + * (such as NAT IPs, LB VIPs, etc.) of the 'lrp' add routes to the
> > + * peer_lrp's datapath.
> > + */
> > +static void
> > +build_routable_flows_for_router_port(
> > +    struct ovn_port *lrp, const struct lr_stateful_record *lr_sful_rec,
> > +    struct lflow_table *lflows,
> > +    struct ds *match,
> > +    struct ds *actions,
> > +    struct lflow_ref *lflow_ref)
> > +{
> > +    ovs_assert(lrp->nbrp && lrp->od == lr_sful_rec->od);
> > +
> > +    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_sful_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,
> > +                            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),
> > +                              lflow_ref);
> > +            }
> > +        }
> > +    }
> > +
> > +    destroy_routable_addresses(&ra);
> > +}
> > +
> >  static void
> >  build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op,
> >                                    const struct lr_stateful_table *lr_sful_table,
> > -                                  const struct hmap *lr_ports,
> >                                    struct ds *match,
> >                                    struct ds *actions,
> >                                    struct lflow_table *lflows,
> > @@ -15711,8 +15653,8 @@ build_lbnat_lflows_iterate_by_lsp(struct ovn_port *op,
> >      ovs_assert(lr_sful_rec);
> >
> >      build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec,
> > -                                lr_sful_table, lr_ports, lflows,
> > -                                match, actions, lflow_ref);
> > +                                lflows,match, actions,
> > +                                lflow_ref);
> >  }
> >
> >  static void
> > @@ -15762,7 +15704,8 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port *op,
> >                                    struct ds *match,
> >                                    struct ds *actions,
> >                                    struct lflow_table *lflows,
> > -                                  struct lflow_ref *lflow_ref)
> > +                                  struct lflow_ref *lflow_ref,
> > +                                  struct lflow_ref *routable_lflow_ref)
> >  {
> >      ovs_assert(op->nbrp);
> >
> > @@ -15773,6 +15716,9 @@ build_lbnat_lflows_iterate_by_lrp(struct ovn_port *op,
> >
> >      build_lrp_lflows_for_lbnats(op, lr_sful_rec, meter_groups, match,
> >                                  actions, lflows, lflow_ref);
> > +
> > +    build_routable_flows_for_router_port(op, lr_sful_rec, lflows, match,
> > +                                         actions, routable_lflow_ref);
> >  }
> >
> >  static void
> > @@ -15787,15 +15733,15 @@ build_lr_stateful_flows(const struct lr_stateful_record *lr_sful_rec,
> >  {
> >      build_lrouter_nat_defrag_and_lb(lr_sful_rec, lflows, ls_ports, lr_ports,
> >                                      match, actions, meter_groups, features,
> > -                                    NULL);
> > +                                    lr_sful_rec->lflow_ref);
> >      build_lr_gateway_redirect_flows_for_nats(lr_sful_rec->od,
> >                                               lr_sful_rec->lrnat_rec, lflows,
> >                                               match, actions,
> > -                                             NULL);
> > +                                             lr_sful_rec->lflow_ref);
> >      build_lrouter_arp_nd_for_datapath(lr_sful_rec->od,
> >                                        lr_sful_rec->lrnat_rec, lflows,
> >                                        meter_groups,
> > -                                      NULL);
> > +                                      lr_sful_rec->lflow_ref);
> >  }
> >
> >  static void
> > @@ -16026,7 +15972,6 @@ build_lflows_thread(void *arg)
> >                                                               &lsi->actions,
> >                                                               lsi->lflows);
> >                      build_lbnat_lflows_iterate_by_lsp(op, lsi->lr_sful_table,
> > -                                                      lsi->lr_ports,
> >                                                        &lsi->match,
> >                                                        &lsi->actions,
> >                                                        lsi->lflows,
> > @@ -16048,7 +15993,8 @@ build_lflows_thread(void *arg)
> >                                                        &lsi->match,
> >                                                        &lsi->actions,
> >                                                        lsi->lflows,
> > -                                                      op->stateful_lflow_ref);
> > +                                                      op->stateful_lflow_ref,
> > +                                                      op->routable_lflow_ref);
> >                  }
> >              }
> >              for (bnum = control->id;
> > @@ -16280,9 +16226,9 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths,
> >                                                       &lsi.match,
> >                                                       &lsi.actions,
> >                                                       lsi.lflows);
> > -            build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table, lsi.lr_ports,
> > -                                              &lsi.match, &lsi.actions,
> > -                                              lsi.lflows,
> > +            build_lbnat_lflows_iterate_by_lsp(op, lsi.lr_sful_table,
> > +                                              &lsi.match,
> > +                                              &lsi.actions, lsi.lflows,
> >                                                op->stateful_lflow_ref);
> >          }
> >          HMAP_FOR_EACH (op, key_node, lr_ports) {
> > @@ -16292,7 +16238,8 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths,
> >                                                &lsi.match,
> >                                                &lsi.actions,
> >                                                lsi.lflows,
> > -                                              op->stateful_lflow_ref);
> > +                                              op->stateful_lflow_ref,
> > +                                              op->routable_lflow_ref);
> >          }
> >          stopwatch_stop(LFLOWS_PORTS_STOPWATCH_NAME, time_msec());
> >          stopwatch_start(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
> > @@ -16485,12 +16432,24 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
> >  void
> >  reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
> >  {
> > +    const struct lr_stateful_record *lr_sful_rec;
> >      struct ovn_lb_datapaths *lb_dps;
> >      struct ovn_port *op;
> >
> > +    LR_STATEFUL_TABLE_FOR_EACH (lr_sful_rec, lflow_input->lr_sful_table) {
> > +        lflow_ref_reset(lr_sful_rec->lflow_ref);
> > +    }
> > +
> >      HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
> >          lflow_ref_reset(op->lflow_ref);
> >          lflow_ref_reset(op->stateful_lflow_ref);
> > +        lflow_ref_reset(op->routable_lflow_ref);
> > +    }
> > +
> > +    HMAP_FOR_EACH (op, key_node, lflow_input->lr_ports) {
> > +        lflow_ref_reset(op->lflow_ref);
> > +        lflow_ref_reset(op->stateful_lflow_ref);
> > +        lflow_ref_reset(op->routable_lflow_ref);
> >      }
> >
> >      HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
> > @@ -16547,11 +16506,10 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
> >              ovs_assert(lr_sful_rec);
> >              lflow_ref_clear_lflows(op->stateful_lflow_ref);
> >              build_lsp_lflows_for_lbnats(op, op->peer, lr_sful_rec,
> > -                                        lflow_input->lr_sful_table,
> > -                                        lflow_input->lr_ports,
> >                                          lflows, &match, &actions,
> >                                          op->stateful_lflow_ref);
> > -            lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows, ovnsb_txn,
> > +            lflow_ref_sync_lflows_to_sb(op->stateful_lflow_ref, lflows,
> > +                             ovnsb_txn,
> >                               lflow_input->ls_datapaths,
> >                               lflow_input->lr_datapaths,
> >                               lflow_input->ovn_internal_version_changed,
> > @@ -16705,6 +16663,92 @@ 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_sful_rec;
> > +    struct hmapx_node *hmapx_node;
> > +
> > +    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
> > +        lr_sful_rec = hmapx_node->data;
> > +
> > +        lflow_ref_clear_lflows(lr_sful_rec->lflow_ref);
> > +
> > +        /* Generate new lflows. */
> > +        struct ds match = DS_EMPTY_INITIALIZER;
> > +        struct ds actions = DS_EMPTY_INITIALIZER;
> > +
> > +        build_lr_stateful_flows(lr_sful_rec, lflows, lflow_input->ls_ports,
> > +                                lflow_input->lr_ports, &match, &actions,
> > +                                lflow_input->meter_groups,
> > +                                lflow_input->features);
> > +
> > +        /* Sync the new flows to SB. */
> > +        lflow_ref_sync_lflows_to_sb(lr_sful_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);
> > +
> > +        struct ovn_port *op;
> > +        HMAP_FOR_EACH (op, dp_node, &lr_sful_rec->od->ports) {
> > +            lflow_ref_clear_lflows(op->stateful_lflow_ref);
> > +            lflow_ref_clear_lflows_for_all_dps(op->routable_lflow_ref,
> > +                                        ods_size(lflow_input->ls_datapaths),
> > +                                        ods_size(lflow_input->lr_datapaths));
> > +
> > +            build_lbnat_lflows_iterate_by_lrp(op, lflow_input->lr_sful_table,
> > +                                              lflow_input->meter_groups,
> > +                                              &match, &actions,
> > +                                              lflows,
> > +                                              op->stateful_lflow_ref,
> > +                                              op->routable_lflow_ref);
> > +
> > +            lflow_ref_sync_lflows_to_sb(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);
> > +
> > +            lflow_ref_sync_lflows_to_sb(op->routable_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 (op->peer && op->peer->nbsp) {
> > +                lflow_ref_clear_lflows(op->peer->stateful_lflow_ref);
> > +
> > +                build_lbnat_lflows_iterate_by_lsp(op->peer,
> > +                                                lflow_input->lr_sful_table,
> > +                                                &match, &actions,
> > +                                                lflows,
> > +                                                op->peer->stateful_lflow_ref);
> > +
> > +                lflow_ref_sync_lflows_to_sb(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);
> > +            }
> > +        }
> > +
> > +        ds_destroy(&match);
> > +        ds_destroy(&actions);
> > +    }
> > +
> > +    return true;
> > +}
> > +
> >  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 863bcce001..6adf06c26f 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -688,9 +688,13 @@ struct ovn_port {
> >       * 'stateful_lflow_ref' is used for logical switch ports of type
> >       * 'patch/router' to referenece logical flows generated fo this ovn_port
> >       *  from the 'lr_stateful' record of the peer port's datapath.
> > +     *
> > +     * 'routable_lflow_ref' is used to reference logical flows generated for
> > +     * the routable ips of a logical router port.
> >       */
> >      struct lflow_ref *lflow_ref;
> >      struct lflow_ref *stateful_lflow_ref;
> > +    struct lflow_ref *routable_lflow_ref;
> >  };
> >
> >  void ovnnb_db_run(struct northd_input *input_data,
> > @@ -715,6 +719,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 *);
> > @@ -728,6 +734,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 50d88fecd5..f517057534 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
> > @@ -10687,7 +10688,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
> >
> > @@ -10697,7 +10698,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
> >
> > @@ -10707,7 +10708,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
> >
> > @@ -10717,7 +10718,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
> >
> > @@ -10727,7 +10728,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
> >
> > @@ -10735,6 +10736,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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
> > @@ -10850,7 +10852,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
> >
> > @@ -10860,7 +10862,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
> >
> > @@ -10870,7 +10872,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
> >
> > @@ -10880,7 +10882,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
> >
> > @@ -10890,7 +10892,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
> >
> > @@ -10971,7 +10973,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
> >
> > @@ -10999,7 +11001,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
> >
> > @@ -11177,7 +11179,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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
> >
> > @@ -11186,7 +11188,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 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
> >
> > @@ -11196,7 +11198,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
> >
> > @@ -11206,7 +11208,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
> >
> > @@ -11216,7 +11218,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
> >
> > @@ -11226,7 +11228,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
> >
> > @@ -11237,7 +11239,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
> >
> > @@ -11258,6 +11260,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 NORTHD_TYPE 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
> > @@ -11267,6 +11271,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 NORTHD_TYPE 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
> > @@ -11276,6 +11282,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 NORTHD_TYPE 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
> > @@ -11291,7 +11299,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
> >
>
> Regards,
> Dumitru
>
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>