On Fri, Aug 18, 2023 at 10:59 AM <num...@ovn.org> wrote:

> From: Numan Siddique <num...@ovn.org>
>
> When a logical router gets updated due to load balancer or load balancer
> groups changes, it is now incrementally handled first in 'lb_data'
> engine node similar to how logical switch changes are handled.  The
> tracking data of 'lb_data' is updated similarly so that northd engine
> handler - northd_handle_lb_data_changes_post_od() handles it.
>
> A new handler northd_handle_lr_changes() is added in the 'northd' engine
> node for logical router changes.  This handler returns true if only
> load balancer or load balancer group columns are changed.  It returns
> false for any other changes.
>
> northd_handle_lb_data_changes_post_od() also sets the logical router
> od's lb_ips accordingly.
>
> Below are the scale testing results done with these patches applied
> using ovn-heater.  The test ran the scenario  -
> ocp-500-density-heavy.yml [1].
>
> With these patches applied (with load balancer I-P handling in northd
> engine node) the resuts are:
>
>
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>                         Min (s)         Median (s)      90%ile (s)
> 99%ile (s)      Max (s)         Mean (s)        Total (s)       Count
>  Failed
>
> -------------------------------------------------------------------------------------------------------------------------------------------------------
> Iteration Total         0.132929        2.157103        3.314847
> 3.331561        4.378626        1.581889        197.736147      125     0
> Namespace.add_ports     0.005217        0.005760        0.006565
> 0.013348        0.021014        0.006106        0.763214        125     0
> WorkerNode.bind_port    0.035205        0.045458        0.052278
> 0.059804        0.063941        0.045652        11.413122       250     0
> WorkerNode.ping_port    0.005075        0.006814        3.088548
> 3.192577        4.242026        0.726453        181.613284      250     0
>
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> The results with the present main are:
>
>
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>                         Min (s)         Median (s)      90%ile (s)
> 99%ile (s)      Max (s)         Mean (s)        Total (s)       Count
>  Failed
>
> -------------------------------------------------------------------------------------------------------------------------------------------------------
> Iteration Total         4.377260        6.486962        7.502040
> 8.322587        8.334701        6.559002        819.875306      125     0
> Namespace.add_ports     0.005112        0.005484        0.005953
> 0.009153        0.011452        0.005662        0.707752        125     0
> WorkerNode.bind_port    0.035360        0.042732        0.049152
> 0.053698        0.056635        0.043215        10.803700       250     0
> WorkerNode.ping_port    0.005338        1.599904        7.229649
> 7.798039        8.206537        3.209860        802.464911      250     0
>
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Few observations:
>
>  - The total time taken has come down significantly from 819 seconds to
> 197.
>  - 99%ile with these patches is 3.3 seconds compared to 8.3 seconds for the
>    main.
>  - 90%file with these patches is 3.3 seconds compared to 7.5 seconds for
>    the main.
>  - CPU utilization of northd during the test with these patches
>    is between 100% to 300% which is almost the same as main.
>    Main difference being that, with these patches the test duration is
>    less and hence overall less CPU utilization.
>
> [1] -
> https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
>
> Signed-off-by: Numan Siddique <num...@ovn.org>
> ---
>  lib/lb.c                 |  46 +++++-
>  lib/lb.h                 |   9 ++
>  northd/en-lb-data.c      | 328 +++++++++++++++++++++++++++++++--------
>  northd/en-lb-data.h      |  15 ++
>  northd/en-northd.c       |  26 ++++
>  northd/en-northd.h       |   1 +
>  northd/inc-proc-northd.c |   5 +-
>  northd/northd.c          | 242 ++++++++++++++++++++++++++---
>  northd/northd.h          |   3 +
>  tests/ovn-northd.at      |  42 ++---
>  10 files changed, 604 insertions(+), 113 deletions(-)
>
> diff --git a/lib/lb.c b/lib/lb.c
> index e6c9dc2be2..d0d562b6fb 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -794,6 +794,7 @@ ovn_lb_group_init(struct ovn_lb_group *lb_group,
>          const struct uuid *lb_uuid =
>              &nbrec_lb_group->load_balancer[i]->header_.uuid;
>          lb_group->lbs[i] = ovn_northd_lb_find(lbs, lb_uuid);
> +        lb_group->has_routable_lb |= lb_group->lbs[i]->routable;
>      }
>  }
>
> @@ -815,6 +816,7 @@ ovn_lb_group_cleanup(struct ovn_lb_group *lb_group)
>  {
>      ovn_lb_ip_set_destroy(lb_group->lb_ips);
>      lb_group->lb_ips = NULL;
> +    lb_group->has_routable_lb = false;
>      free(lb_group->lbs);
>  }
>
> @@ -1022,23 +1024,54 @@ ovn_lb_5tuples_destroy(struct hmap *tuples)
>  void
>  build_lrouter_lb_ips(struct ovn_lb_ip_set *lb_ips,
>                       const struct ovn_northd_lb *lb)
> +{
> +    add_ips_to_lb_ip_set(lb_ips, lb->routable, &lb->ips_v4, &lb->ips_v6);
> +}
> +
> +void
> +add_ips_to_lb_ip_set(struct ovn_lb_ip_set *lb_ips,
> +                     bool is_routable,
> +                     const struct sset *lb_ips_v4,
> +                     const struct sset *lb_ips_v6)
>  {
>      const char *ip_address;
>
> -    SSET_FOR_EACH (ip_address, &lb->ips_v4) {
> +    SSET_FOR_EACH (ip_address, lb_ips_v4) {
>          sset_add(&lb_ips->ips_v4, ip_address);
> -        if (lb->routable) {
> +        if (is_routable) {
>              sset_add(&lb_ips->ips_v4_routable, ip_address);
>          }
>      }
> -    SSET_FOR_EACH (ip_address, &lb->ips_v6) {
> +    SSET_FOR_EACH (ip_address, lb_ips_v6) {
>          sset_add(&lb_ips->ips_v6, ip_address);
> -        if (lb->routable) {
> +        if (is_routable) {
>              sset_add(&lb_ips->ips_v6_routable, ip_address);
>          }
>      }
>  }
>
> +void
> +remove_ips_from_lb_ip_set(struct ovn_lb_ip_set *lb_ips,
> +                          bool is_routable,
> +                          const struct sset *lb_ips_v4,
> +                          const struct sset *lb_ips_v6)
> +{
> +    const char *ip_address;
> +
> +    SSET_FOR_EACH (ip_address, lb_ips_v4) {
> +        sset_find_and_delete(&lb_ips->ips_v4, ip_address);
> +        if (is_routable) {
> +            sset_find_and_delete(&lb_ips->ips_v4_routable, ip_address);
> +        }
> +    }
> +    SSET_FOR_EACH (ip_address, lb_ips_v6) {
> +        sset_find_and_delete(&lb_ips->ips_v6, ip_address);
> +        if (is_routable) {
> +            sset_find_and_delete(&lb_ips->ips_v6_routable, ip_address);
> +        }
> +    }
> +}
> +
>  /* lb datapaths functions */
>  struct  ovn_lb_datapaths *
>  ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t
> n_ls_datapaths,
> @@ -1079,7 +1112,10 @@ ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths
> *lb_dps, size_t n,
>                          struct ovn_datapath **ods)
>  {
>      for (size_t i = 0; i < n; i++) {
> -        bitmap_set1(lb_dps->nb_lr_map, ods[i]->index);
> +        if (!bitmap_is_set(lb_dps->nb_lr_map, ods[i]->index)) {
> +            bitmap_set1(lb_dps->nb_lr_map, ods[i]->index);
> +            lb_dps->n_nb_lr++;
> +        }
>      }
>  }
>
> diff --git a/lib/lb.h b/lib/lb.h
> index 74905c73b7..b8e3c1e8fb 100644
> --- a/lib/lb.h
> +++ b/lib/lb.h
> @@ -138,6 +138,14 @@ void ovn_northd_lb_reinit(struct ovn_northd_lb *,
>
>  void build_lrouter_lb_ips(struct ovn_lb_ip_set *,
>                            const struct ovn_northd_lb *);
> +void add_ips_to_lb_ip_set(struct ovn_lb_ip_set *lb_ips,
> +                          bool is_routable,
> +                          const struct sset *lb_ips_v4,
> +                          const struct sset *lb_ips_v6);
> +void remove_ips_from_lb_ip_set(struct ovn_lb_ip_set *lb_ips,
> +                               bool is_routable,
> +                               const struct sset *lb_ips_v4,
> +                               const struct sset *lb_ips_v6);
>
>  struct ovn_lb_group {
>      struct hmap_node hmap_node;
> @@ -145,6 +153,7 @@ struct ovn_lb_group {
>      size_t n_lbs;
>      struct ovn_northd_lb **lbs;
>      struct ovn_lb_ip_set *lb_ips;
> +    bool has_routable_lb;
>  };
>
>  struct ovn_lb_group *ovn_lb_group_create(
> diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
> index 8619b4cc14..80cb959a47 100644
> --- a/northd/en-lb-data.c
> +++ b/northd/en-lb-data.c
> @@ -40,20 +40,30 @@ static void build_lbs(const struct
> nbrec_load_balancer_table *,
>                        const struct nbrec_load_balancer_group_table *,
>                        struct hmap *lbs, struct hmap *lb_groups);
>  static void build_od_lb_map(const struct nbrec_logical_switch_table *,
> -                             struct hmap *od_lb_map);
> +                            const struct nbrec_logical_router_table *,
> +                            struct hmap *ls_lb_map, struct hmap
> *lr_lb_map);
>  static struct od_lb_data *find_od_lb_data(struct hmap *od_lb_map,
>                                            const struct uuid *od_uuid);
>  static void destroy_od_lb_data(struct od_lb_data *od_lb_data);
>  static struct od_lb_data *create_od_lb_data(struct hmap *od_lb_map,
>                                              const struct uuid *od_uuid);
> +static void handle_od_lb_changes(struct nbrec_load_balancer **,
> +                                 size_t n_nbrec_lbs,
> +                                 struct od_lb_data *od_lb_data,
> +                                 struct ed_type_lb_data *lb_data,
> +                                 struct crupdated_od_lb_data *);
> +static void handle_od_lbgrp_changes(struct nbrec_load_balancer_group **,
> +                                    size_t n_nbrec_lbs,
> +                                    struct od_lb_data *,
> +                                    struct ed_type_lb_data *lb_data,
> +                                    struct crupdated_od_lb_data *);
>
>  static struct ovn_lb_group *create_lb_group(
>      const struct nbrec_load_balancer_group *, struct hmap *lbs,
>      struct hmap *lb_groups);
>  static void destroy_tracked_data(struct ed_type_lb_data *);
> -static void add_crupdated_lb_to_tracked_data(struct ovn_northd_lb *,
> -                                                    struct
> tracked_lb_data *,
> -                                                    bool health_checks);
> +static struct crupdated_lb *add_crupdated_lb_to_tracked_data(
> +    struct ovn_northd_lb *, struct tracked_lb_data *, bool health_checks);
>  static void add_deleted_lb_to_tracked_data(struct ovn_northd_lb *,
>                                                    struct tracked_lb_data
> *,
>                                                    bool health_checks);
> @@ -64,6 +74,8 @@ static void add_deleted_lb_group_to_tracked_data(
>      struct ovn_lb_group *, struct tracked_lb_data *);
>  static bool is_ls_lbs_changed(const struct nbrec_logical_switch *nbs);
>  static bool is_ls_lbgrps_changed(const struct nbrec_logical_switch *nbs);
> +static bool is_lr_lbs_changed(const struct nbrec_logical_router *);
> +static bool is_lr_lbgrps_changed(const struct nbrec_logical_router *);
>
>  /* 'lb_data' engine node manages the NB load balancers and load balancer
>   * groups.  For each NB LB, it creates 'struct ovn_northd_lb' and
> @@ -92,10 +104,13 @@ en_lb_data_run(struct engine_node *node, void *data)
>          EN_OVSDB_GET(engine_get_input("NB_load_balancer_group", node));
>      const struct nbrec_logical_switch_table *nb_ls_table =
>          EN_OVSDB_GET(engine_get_input("NB_logical_switch", node));
> +    const struct nbrec_logical_router_table *nb_lr_table =
> +        EN_OVSDB_GET(engine_get_input("NB_logical_router", node));
>
>      lb_data->tracked = false;
>      build_lbs(nb_lb_table, nb_lbg_table, &lb_data->lbs,
> &lb_data->lb_groups);
> -    build_od_lb_map(nb_ls_table, &lb_data->ls_lb_map);
> +    build_od_lb_map(nb_ls_table, nb_lr_table, &lb_data->ls_lb_map,
> +                    &lb_data->lr_lb_map);
>
>      engine_set_node_state(node, EN_UPDATED);
>  }
> @@ -137,6 +152,7 @@ lb_data_load_balancer_handler(struct engine_node
> *node, void *data)
>                          uuid_hash(&tracked_lb->header_.uuid));
>              add_crupdated_lb_to_tracked_data(lb, trk_lb_data,
>                                               lb->health_checks);
> +            trk_lb_data->has_routable_lb |= lb->routable;
>          } else if (nbrec_load_balancer_is_deleted(tracked_lb)) {
>              lb = ovn_northd_lb_find(&lb_data->lbs,
>                                      &tracked_lb->header_.uuid);
> @@ -144,15 +160,44 @@ lb_data_load_balancer_handler(struct engine_node
> *node, void *data)
>              hmap_remove(&lb_data->lbs, &lb->hmap_node);
>              add_deleted_lb_to_tracked_data(lb, trk_lb_data,
>                                             lb->health_checks);
> +            trk_lb_data->has_routable_lb |= lb->routable;
>          } else {
>              /* Load balancer updated. */
>              lb = ovn_northd_lb_find(&lb_data->lbs,
>                                      &tracked_lb->header_.uuid);
>              ovs_assert(lb);
>              bool health_checks = lb->health_checks;
> +            struct sset old_ips_v4 = SSET_INITIALIZER(&old_ips_v4);
> +            struct sset old_ips_v6 = SSET_INITIALIZER(&old_ips_v6);
> +            sset_swap(&lb->ips_v4, &old_ips_v4);
> +            sset_swap(&lb->ips_v6, &old_ips_v6);
>              ovn_northd_lb_reinit(lb, tracked_lb);
>              health_checks |= lb->health_checks;
> -            add_crupdated_lb_to_tracked_data(lb, trk_lb_data,
> health_checks);
> +            struct crupdated_lb *clb = add_crupdated_lb_to_tracked_data(
> +                lb, trk_lb_data, health_checks);
> +            trk_lb_data->has_routable_lb |= lb->routable;
> +
> +            /* Determine the inserted and deleted vips and store them in
> +             * the tracked data. */
> +            const char *vip;
> +            SSET_FOR_EACH (vip, &lb->ips_v4) {
> +                if (!sset_find_and_delete(&old_ips_v4, vip)) {
> +                    sset_add(&clb->inserted_vips_v4, vip);
> +                }
> +            }
> +
> +            sset_swap(&old_ips_v4, &clb->deleted_vips_v4);
> +
> +            SSET_FOR_EACH (vip, &lb->ips_v6) {
> +                if (!sset_find_and_delete(&old_ips_v6, vip)) {
> +                    sset_add(&clb->inserted_vips_v6, vip);
> +                }
> +            }
> +
> +            sset_swap(&old_ips_v6, &clb->deleted_vips_v6);
> +
> +            sset_destroy(&old_ips_v4);
> +            sset_destroy(&old_ips_v6);
>          }
>      }
>
> @@ -181,6 +226,8 @@ lb_data_load_balancer_group_handler(struct engine_node
> *node, void *data)
>              for (size_t i = 0; i < lb_group->n_lbs; i++) {
>                  hmapx_add(&clbg->assoc_lbs, lb_group->lbs[i]);
>              }
> +
> +            trk_lb_data->has_routable_lb |= lb_group->has_routable_lb;
>          } else if
> (nbrec_load_balancer_group_is_deleted(tracked_lb_group)) {
>              struct ovn_lb_group *lb_group;
>              lb_group = ovn_lb_group_find(&lb_data->lb_groups,
> @@ -188,6 +235,7 @@ lb_data_load_balancer_group_handler(struct engine_node
> *node, void *data)
>              ovs_assert(lb_group);
>              hmap_remove(&lb_data->lb_groups, &lb_group->hmap_node);
>              add_deleted_lb_group_to_tracked_data(lb_group, trk_lb_data);
> +            trk_lb_data->has_routable_lb |= lb_group->has_routable_lb;
>          } else {
>
>              struct ovn_lb_group *lb_group;
> @@ -209,6 +257,8 @@ lb_data_load_balancer_group_handler(struct engine_node
> *node, void *data)
>                  build_lrouter_lb_ips(lb_group->lb_ips, lb_group->lbs[i]);
>              }
>
> +            trk_lb_data->has_routable_lb |= lb_group->has_routable_lb;
> +
>              struct crupdated_lb_group *clbg =
>                  add_crupdated_lb_group_to_tracked_data(lb_group,
> trk_lb_data);
>
> @@ -277,6 +327,7 @@ lb_data_logical_switch_handler(struct engine_node
> *node, void *data)
>              if (!ls_lbs_changed && !ls_lbgrps_changed) {
>                  continue;
>              }
> +            changed = true;
>              struct crupdated_od_lb_data *codlb = xzalloc(sizeof *codlb);
>              codlb->od_uuid = nbs->header_.uuid;
>              uuidset_init(&codlb->assoc_lbs);
> @@ -290,66 +341,77 @@ lb_data_logical_switch_handler(struct engine_node
> *node, void *data)
>              }
>
>              if (ls_lbs_changed) {
> -                struct uuidset *pre_lb_uuids = od_lb_data->lbs;
> -                od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs);
> -                uuidset_init(od_lb_data->lbs);
> -
> -                for (size_t i = 0; i < nbs->n_load_balancer; i++) {
> -                    const struct uuid *lb_uuid =
> -                        &nbs->load_balancer[i]->header_.uuid;
> -                    uuidset_insert(od_lb_data->lbs, lb_uuid);
> -
> -                    struct uuidset_node *unode =
> uuidset_find(pre_lb_uuids,
> -                                                            lb_uuid);
> -
> -                    if (!unode || (nbrec_load_balancer_row_get_seqno(
> -                            nbs->load_balancer[i],
> -                            OVSDB_IDL_CHANGE_MODIFY) > 0)) {
> -                        /* Add this lb to the tracked data. */
> -                        uuidset_insert(&codlb->assoc_lbs, lb_uuid);
> -                        changed = true;
> -                    }
> -
> -                    if (unode) {
> -                        uuidset_delete(pre_lb_uuids, unode);
> -                    }
> -                }
> -                if (!uuidset_is_empty(pre_lb_uuids)) {
> -                    trk_lb_data->has_dissassoc_lbs_from_od = true;
> -                    changed = true;
> -                }
> -
> -                uuidset_destroy(pre_lb_uuids);
> -                free(pre_lb_uuids);
> +                handle_od_lb_changes(nbs->load_balancer,
> nbs->n_load_balancer,
> +                                     od_lb_data, lb_data, codlb);
>              }
>
>              if (ls_lbgrps_changed) {
> -                struct uuidset *pre_lbgrp_uuids = od_lb_data->lbgrps;
> -                od_lb_data->lbgrps = xzalloc(sizeof *od_lb_data->lbgrps);
> -                uuidset_init(od_lb_data->lbgrps);
> -                for (size_t i = 0; i < nbs->n_load_balancer_group; i++) {
> -                    const struct uuid *lbg_uuid =
> -                        &nbs->load_balancer_group[i]->header_.uuid;
> -                    uuidset_insert(od_lb_data->lbgrps, lbg_uuid);
> -
> -                    if (!uuidset_find_and_delete(pre_lbgrp_uuids,
> -                                                 lbg_uuid)) {
> -                        /* Add this lb group to the tracked data. */
> -                        uuidset_insert(&codlb->assoc_lbgrps, lbg_uuid);
> -                        changed = true;
> -                    }
> -                }
> +                handle_od_lbgrp_changes(nbs->load_balancer_group,
> +                                        nbs->n_load_balancer_group,
> +                                        od_lb_data, lb_data, codlb);
> +            }
>
> -                if (!uuidset_is_empty(pre_lbgrp_uuids)) {
> -                    trk_lb_data->has_dissassoc_lbgrps_from_od = true;
> -                    changed = true;
> -                }
> +            ovs_list_insert(&trk_lb_data->crupdated_ls_lbs,
> &codlb->list_node);
> +        }
> +    }
> +
> +    if (changed) {
> +        engine_set_node_state(node, EN_UPDATED);
> +    }
> +    return true;
> +}
> +
> +bool
> +lb_data_logical_router_handler(struct engine_node *node, void *data)
> +{
> +    struct ed_type_lb_data *lb_data = (struct ed_type_lb_data *) data;
> +    const struct nbrec_logical_router_table *nbrec_lr_table =
> +        EN_OVSDB_GET(engine_get_input("NB_logical_router", node));
>
> -                uuidset_destroy(pre_lbgrp_uuids);
> -                free(pre_lbgrp_uuids);
> +    struct tracked_lb_data *trk_lb_data = &lb_data->tracked_lb_data;
> +    lb_data->tracked = true;
> +
> +    bool changed = false;
> +    const struct nbrec_logical_router *nbr;
> +    NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH_TRACKED (nbr, nbrec_lr_table) {
> +        if (nbrec_logical_router_is_deleted(nbr)) {
> +            struct od_lb_data *od_lb_data =
> +                find_od_lb_data(&lb_data->lr_lb_map, &nbr->header_.uuid);
> +            if (od_lb_data) {
> +                hmap_remove(&lb_data->lr_lb_map, &od_lb_data->hmap_node);
> +                destroy_od_lb_data(od_lb_data);
>              }
> +        } else {
> +            bool lr_lbs_changed = is_lr_lbs_changed(nbr);
> +            bool lr_lbgrps_changed = is_lr_lbgrps_changed(nbr);
> +            if (!lr_lbs_changed && !lr_lbgrps_changed) {
> +                continue;
> +            }
> +            changed = true;
> +            struct crupdated_od_lb_data *codlb = xzalloc(sizeof *codlb);
> +            codlb->od_uuid = nbr->header_.uuid;
> +            uuidset_init(&codlb->assoc_lbs);
> +            uuidset_init(&codlb->assoc_lbgrps);
>
> -            ovs_list_insert(&trk_lb_data->crupdated_ls_lbs,
> &codlb->list_node);
> +            struct od_lb_data *od_lb_data =
> +                find_od_lb_data(&lb_data->lr_lb_map, &nbr->header_.uuid);
> +            if (!od_lb_data) {
> +                od_lb_data = create_od_lb_data(&lb_data->lr_lb_map,
> +                                                &nbr->header_.uuid);
> +            }
> +
> +            if (lr_lbs_changed) {
> +                handle_od_lb_changes(nbr->load_balancer,
> nbr->n_load_balancer,
> +                                     od_lb_data, lb_data, codlb);
> +            }
> +
> +            if (lr_lbgrps_changed) {
> +                handle_od_lbgrp_changes(nbr->load_balancer_group,
> +                                        nbr->n_load_balancer_group,
> +                                        od_lb_data, lb_data, codlb);
> +            }
> +
> +            ovs_list_insert(&trk_lb_data->crupdated_lr_lbs,
> &codlb->list_node);
>          }
>      }
>
> @@ -366,6 +428,7 @@ lb_data_init(struct ed_type_lb_data *lb_data)
>      hmap_init(&lb_data->lbs);
>      hmap_init(&lb_data->lb_groups);
>      hmap_init(&lb_data->ls_lb_map);
> +    hmap_init(&lb_data->lr_lb_map);
>
>      struct tracked_lb_data *trk_lb_data = &lb_data->tracked_lb_data;
>      hmap_init(&trk_lb_data->crupdated_lbs);
> @@ -373,6 +436,7 @@ lb_data_init(struct ed_type_lb_data *lb_data)
>      hmap_init(&trk_lb_data->crupdated_lb_groups);
>      hmapx_init(&trk_lb_data->deleted_lb_groups);
>      ovs_list_init(&trk_lb_data->crupdated_ls_lbs);
> +    ovs_list_init(&trk_lb_data->crupdated_lr_lbs);
>  }
>
>  static void
> @@ -396,6 +460,11 @@ lb_data_destroy(struct ed_type_lb_data *lb_data)
>      }
>      hmap_destroy(&lb_data->ls_lb_map);
>
> +    HMAP_FOR_EACH_POP (od_lb_data, hmap_node, &lb_data->lr_lb_map) {
> +        destroy_od_lb_data(od_lb_data);
> +    }
> +    hmap_destroy(&lb_data->lr_lb_map);
> +
>      destroy_tracked_data(lb_data);
>      hmap_destroy(&lb_data->tracked_lb_data.crupdated_lbs);
>      hmapx_destroy(&lb_data->tracked_lb_data.deleted_lbs);
> @@ -440,7 +509,8 @@ create_lb_group(const struct nbrec_load_balancer_group
> *nbrec_lb_group,
>
>  static void
>  build_od_lb_map(const struct nbrec_logical_switch_table *nbrec_ls_table,
> -                 struct hmap *od_lb_map)
> +                const struct nbrec_logical_router_table *nbrec_lr_table,
> +                struct hmap *ls_lb_map, struct hmap *lr_lb_map)
>  {
>      const struct nbrec_logical_switch *nbrec_ls;
>      NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbrec_ls, nbrec_ls_table) {
> @@ -448,17 +518,35 @@ build_od_lb_map(const struct
> nbrec_logical_switch_table *nbrec_ls_table,
>              continue;
>          }
>
> -        struct od_lb_data *od_lb_data =
> -            create_od_lb_data(od_lb_map, &nbrec_ls->header_.uuid);
> +        struct od_lb_data *ls_lb_data =
> +            create_od_lb_data(ls_lb_map, &nbrec_ls->header_.uuid);
>          for (size_t i = 0; i < nbrec_ls->n_load_balancer; i++) {
> -            uuidset_insert(od_lb_data->lbs,
> +            uuidset_insert(ls_lb_data->lbs,
>                             &nbrec_ls->load_balancer[i]->header_.uuid);
>          }
>          for (size_t i = 0; i < nbrec_ls->n_load_balancer_group; i++) {
> -            uuidset_insert(od_lb_data->lbgrps,
> +            uuidset_insert(ls_lb_data->lbgrps,
>
> &nbrec_ls->load_balancer_group[i]->header_.uuid);
>          }
>      }
> +
> +    const struct nbrec_logical_router *nbrec_lr;
> +    NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH (nbrec_lr, nbrec_lr_table) {
> +        if (!nbrec_lr->n_load_balancer &&
> !nbrec_lr->n_load_balancer_group) {
> +            continue;
> +        }
> +
> +        struct od_lb_data *lr_lb_data =
> +            create_od_lb_data(lr_lb_map, &nbrec_lr->header_.uuid);
> +        for (size_t i = 0; i < nbrec_lr->n_load_balancer; i++) {
> +            uuidset_insert(lr_lb_data->lbs,
> +                           &nbrec_lr->load_balancer[i]->header_.uuid);
> +        }
> +        for (size_t i = 0; i < nbrec_lr->n_load_balancer_group; i++) {
> +            uuidset_insert(lr_lb_data->lbgrps,
> +
>  &nbrec_lr->load_balancer_group[i]->header_.uuid);
> +        }
> +    }
>  }
>
>  static struct od_lb_data *
> @@ -500,6 +588,84 @@ destroy_od_lb_data(struct od_lb_data *od_lb_data)
>      free(od_lb_data);
>  }
>
> +static void
> +handle_od_lb_changes(struct nbrec_load_balancer **nbrec_lbs,
> +                     size_t n_nbrec_lbs, struct od_lb_data *od_lb_data,
> +                     struct ed_type_lb_data *lb_data,
> +                     struct crupdated_od_lb_data *codlb)
> +{
> +    struct tracked_lb_data *trk_lb_data = &lb_data->tracked_lb_data;
> +    struct uuidset *pre_lb_uuids = od_lb_data->lbs;
> +    od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs);
> +    uuidset_init(od_lb_data->lbs);
> +
> +    for (size_t i = 0; i < n_nbrec_lbs; i++) {
> +        const struct uuid *lb_uuid = &nbrec_lbs[i]->header_.uuid;
> +        uuidset_insert(od_lb_data->lbs, lb_uuid);
> +
> +        struct uuidset_node *unode = uuidset_find(pre_lb_uuids, lb_uuid);
> +
> +        if (!unode || (nbrec_load_balancer_row_get_seqno(
> +                nbrec_lbs[i], OVSDB_IDL_CHANGE_MODIFY) > 0)) {
> +            /* Add this lb to the tracked data. */
> +            uuidset_insert(&codlb->assoc_lbs, lb_uuid);
> +
> +            if (!trk_lb_data->has_routable_lb) {
> +                struct ovn_northd_lb *lb =
> ovn_northd_lb_find(&lb_data->lbs,
> +                                                              lb_uuid);
> +                ovs_assert(lb);
> +                trk_lb_data->has_routable_lb |= lb->routable;
> +            }
> +        }
> +
> +        if (unode) {
> +            uuidset_delete(pre_lb_uuids, unode);
> +        }
> +    }
> +
> +    if (!uuidset_is_empty(pre_lb_uuids)) {
> +        trk_lb_data->has_dissassoc_lbs_from_od = true;
> +    }
> +
> +    uuidset_destroy(pre_lb_uuids);
> +    free(pre_lb_uuids);
> +}
> +
> +static void
> +handle_od_lbgrp_changes(struct nbrec_load_balancer_group **nbrec_lbgrps,
> +                        size_t n_nbrec_lbgrps, struct od_lb_data
> *od_lb_data,
> +                        struct ed_type_lb_data *lb_data,
> +                        struct crupdated_od_lb_data *codlb)
> +{
> +    struct tracked_lb_data *trk_lb_data = &lb_data->tracked_lb_data;
> +    struct uuidset *pre_lbgrp_uuids = od_lb_data->lbgrps;
> +    od_lb_data->lbgrps = xzalloc(sizeof *od_lb_data->lbgrps);
> +    uuidset_init(od_lb_data->lbgrps);
> +    for (size_t i = 0; i < n_nbrec_lbgrps; i++) {
> +        const struct uuid *lbgrp_uuid = &nbrec_lbgrps[i]->header_.uuid;
> +        uuidset_insert(od_lb_data->lbgrps, lbgrp_uuid);
> +
> +        if (!uuidset_find_and_delete(pre_lbgrp_uuids, lbgrp_uuid)) {
> +            /* Add this lb group to the tracked data. */
> +            uuidset_insert(&codlb->assoc_lbgrps, lbgrp_uuid);
> +
> +            if (!trk_lb_data->has_routable_lb) {
> +                struct ovn_lb_group *lbgrp =
> +                    ovn_lb_group_find(&lb_data->lb_groups, lbgrp_uuid);
> +                ovs_assert(lbgrp);
> +                trk_lb_data->has_routable_lb |= lbgrp->has_routable_lb;
> +            }
> +        }
> +    }
> +
> +    if (!uuidset_is_empty(pre_lbgrp_uuids)) {
> +        trk_lb_data->has_dissassoc_lbgrps_from_od = true;
> +    }
> +
> +    uuidset_destroy(pre_lbgrp_uuids);
> +    free(pre_lbgrp_uuids);
> +}
> +
>  static void
>  destroy_tracked_data(struct ed_type_lb_data *lb_data)
>  {
> @@ -508,6 +674,7 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
>      lb_data->tracked_lb_data.has_dissassoc_lbs_from_lb_grops = false;
>      lb_data->tracked_lb_data.has_dissassoc_lbs_from_od = false;
>      lb_data->tracked_lb_data.has_dissassoc_lbgrps_from_od = false;
> +    lb_data->tracked_lb_data.has_routable_lb = false;
>
>      struct hmapx_node *node;
>      HMAPX_FOR_EACH_SAFE (node, &lb_data->tracked_lb_data.deleted_lbs) {
> @@ -523,6 +690,10 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
>      struct crupdated_lb *clb;
>      HMAP_FOR_EACH_POP (clb, hmap_node,
>                         &lb_data->tracked_lb_data.crupdated_lbs) {
> +        sset_destroy(&clb->inserted_vips_v4);
> +        sset_destroy(&clb->inserted_vips_v6);
> +        sset_destroy(&clb->deleted_vips_v4);
> +        sset_destroy(&clb->deleted_vips_v6);
>          free(clb);
>      }
>
> @@ -541,9 +712,16 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
>          uuidset_destroy(&codlb->assoc_lbgrps);
>          free(codlb);
>      }
> +    LIST_FOR_EACH_SAFE (codlb, list_node,
> +                        &lb_data->tracked_lb_data.crupdated_lr_lbs) {
> +        ovs_list_remove(&codlb->list_node);
> +        uuidset_destroy(&codlb->assoc_lbs);
> +        uuidset_destroy(&codlb->assoc_lbgrps);
> +        free(codlb);
> +    }
>  }
>
> -static void
> +static struct crupdated_lb *
>  add_crupdated_lb_to_tracked_data(struct ovn_northd_lb *lb,
>                                   struct tracked_lb_data *tracked_lb_data,
>                                   bool health_checks)
> @@ -552,9 +730,15 @@ add_crupdated_lb_to_tracked_data(struct ovn_northd_lb
> *lb,
>      clb->lb = lb;
>      hmap_insert(&tracked_lb_data->crupdated_lbs, &clb->hmap_node,
>                  uuid_hash(&lb->nlb->header_.uuid));
> +    sset_init(&clb->inserted_vips_v4);
> +    sset_init(&clb->inserted_vips_v6);
> +    sset_init(&clb->deleted_vips_v4);
> +    sset_init(&clb->deleted_vips_v6);
>      if (health_checks) {
>          tracked_lb_data->has_health_checks = true;
>      }
> +
> +    return clb;
>  }
>
>  static void
> @@ -600,3 +784,17 @@ is_ls_lbgrps_changed(const struct
> nbrec_logical_switch *nbs) {
>              ||  nbrec_logical_switch_is_updated(nbs,
>                          NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP));
>  }
> +
> +static bool
> +is_lr_lbs_changed(const struct nbrec_logical_router *nbr) {
> +    return ((nbrec_logical_router_is_new(nbr) && nbr->n_load_balancer)
> +            ||  nbrec_logical_router_is_updated(nbr,
> +                        NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER));
> +}
> +
> +static bool
> +is_lr_lbgrps_changed(const struct nbrec_logical_router *nbr) {
> +    return ((nbrec_logical_router_is_new(nbr) &&
> nbr->n_load_balancer_group)
> +            ||  nbrec_logical_router_is_updated(nbr,
> +                        NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP));
> +}
> diff --git a/northd/en-lb-data.h b/northd/en-lb-data.h
> index bc09ddb7eb..28b7367b51 100644
> --- a/northd/en-lb-data.h
> +++ b/northd/en-lb-data.h
> @@ -6,6 +6,7 @@
>  #include "openvswitch/hmap.h"
>  #include "include/openvswitch/list.h"
>  #include "lib/hmapx.h"
> +#include "lib/sset.h"
>  #include "lib/uuidset.h"
>
>  #include "lib/inc-proc-eng.h"
> @@ -17,8 +18,13 @@ struct crupdated_lb {
>      struct hmap_node hmap_node;
>
>      struct ovn_northd_lb *lb;
> +    struct sset inserted_vips_v4;
> +    struct sset inserted_vips_v6;
> +    struct sset deleted_vips_v4;
> +    struct sset deleted_vips_v6;
>  };
>
> +
>  struct crupdated_lb_group {
>      struct hmap_node hmap_node;
>
> @@ -54,6 +60,10 @@ struct tracked_lb_data {
>       * 'struct crupdated_od_lb_data' */
>      struct ovs_list crupdated_ls_lbs;
>
> +    /* List of logical router <-> lb changes. List node is
> +     * 'struct crupdated_od_lb_data' */
> +    struct ovs_list crupdated_lr_lbs;
> +
>      /* Indicates if any of the tracked lb has health checks enabled. */
>      bool has_health_checks;
>
> @@ -66,6 +76,9 @@ struct tracked_lb_data {
>
>      /* Indicates if a lb group was disassociated from a logical switch. */
>      bool has_dissassoc_lbgrps_from_od;
> +
> +    /* Indicates if any lb (in the tracked data) has 'routable' flag set.
> */
> +    bool has_routable_lb;
>  };
>
>  /* struct which maintains the data of the engine node lb_data. */
> @@ -76,6 +89,7 @@ struct ed_type_lb_data {
>      /* hmap of load balancer groups.  hmap node is 'struct ovn_lb_group
> *' */
>      struct hmap lb_groups;
>      struct hmap ls_lb_map;
> +    struct hmap lr_lb_map;
>
>      /* tracked data*/
>      bool tracked;
> @@ -90,5 +104,6 @@ void en_lb_data_clear_tracked_data(void *data);
>  bool lb_data_load_balancer_handler(struct engine_node *, void *data);
>  bool lb_data_load_balancer_group_handler(struct engine_node *, void
> *data);
>  bool lb_data_logical_switch_handler(struct engine_node *, void *data);
> +bool lb_data_logical_router_handler(struct engine_node *, void *data);
>
>  #endif /* end of EN_NORTHD_LB_DATA_H */
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 545971f76f..3be0f79e19 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -206,6 +206,26 @@ northd_sb_port_binding_handler(struct engine_node
> *node,
>      return true;
>  }
>
> +bool
> +northd_nb_logical_router_handler(struct engine_node *node,
> +                                 void *data)
> +{
> +    struct northd_data *nd = data;
> +    struct northd_input input_data;
> +
> +    northd_get_input_data(node, &input_data);
> +
> +    if (!northd_handle_lr_changes(&input_data, nd)) {
> +        return false;
> +    }
> +
> +    if (nd->change_tracked) {
> +        engine_set_node_state(node, EN_UPDATED);
> +    }
> +
> +    return true;
> +}
> +
>  bool
>  northd_lb_data_handler_pre_od(struct engine_node *node, void *data)
>  {
> @@ -240,6 +260,10 @@ northd_lb_data_handler_pre_od(struct engine_node
> *node, void *data)
>          return false;
>      }
>
> +    if (lb_data->tracked_lb_data.has_routable_lb) {
> +        return false;
> +    }
> +
>      struct northd_data *nd = data;
>
>      if (!northd_handle_lb_data_changes_pre_od(&lb_data->tracked_lb_data,
> @@ -264,11 +288,13 @@ northd_lb_data_handler_post_od(struct engine_node
> *node, void *data)
>      ovs_assert(!lb_data->tracked_lb_data.has_dissassoc_lbs_from_od);
>      ovs_assert(!lb_data->tracked_lb_data.has_dissassoc_lbgrps_from_od);
>      ovs_assert(!lb_data->tracked_lb_data.has_dissassoc_lbs_from_lb_grops);
> +    ovs_assert(!lb_data->tracked_lb_data.has_routable_lb);
>
>      struct northd_data *nd = data;
>
>      if (!northd_handle_lb_data_changes_post_od(&lb_data->tracked_lb_data,
>                                                 &nd->ls_datapaths,
> +                                               &nd->lr_datapaths,
>                                                 &nd->lb_datapaths_map,
>
> &nd->lb_group_datapaths_map)) {
>          return false;
> diff --git a/northd/en-northd.h b/northd/en-northd.h
> index 5926e7a9d3..84d8673e1b 100644
> --- a/northd/en-northd.h
> +++ b/northd/en-northd.h
> @@ -16,6 +16,7 @@ void en_northd_cleanup(void *data);
>  void en_northd_clear_tracked_data(void *data);
>  bool northd_nb_nb_global_handler(struct engine_node *, void *data
> OVS_UNUSED);
>  bool northd_nb_logical_switch_handler(struct engine_node *, void *data);
> +bool northd_nb_logical_router_handler(struct engine_node *, void *data);
>  bool northd_sb_port_binding_handler(struct engine_node *, void *data);
>  bool northd_lb_data_handler_pre_od(struct engine_node *, void *data);
>  bool northd_lb_data_handler_post_od(struct engine_node *, void *data);
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index dc8b880fd8..9dbc2ec81a 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -155,10 +155,11 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>                       lb_data_load_balancer_group_handler);
>      engine_add_input(&en_lb_data, &en_nb_logical_switch,
>                       lb_data_logical_switch_handler);
> +    engine_add_input(&en_lb_data, &en_nb_logical_router,
> +                     lb_data_logical_router_handler);
>
>      engine_add_input(&en_northd, &en_nb_port_group, NULL);
>      engine_add_input(&en_northd, &en_nb_acl, NULL);
> -    engine_add_input(&en_northd, &en_nb_logical_router, NULL);
>      engine_add_input(&en_northd, &en_nb_mirror, NULL);
>      engine_add_input(&en_northd, &en_nb_meter, NULL);
>      engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL);
> @@ -186,6 +187,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>      engine_add_input(&en_northd, &en_lb_data,
> northd_lb_data_handler_pre_od);
>      engine_add_input(&en_northd, &en_nb_logical_switch,
>                       northd_nb_logical_switch_handler);
> +    engine_add_input(&en_northd, &en_nb_logical_router,
> +                     northd_nb_logical_router_handler);
>      engine_add_input(&en_northd, &en_lb_data,
> northd_lb_data_handler_post_od);
>
>      engine_add_input(&en_mac_binding_aging, &en_nb_nb_global, NULL);
> diff --git a/northd/northd.c b/northd/northd.c
> index a3bd21e0b4..41bca6a432 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -4127,54 +4127,63 @@ static bool lrouter_port_ipv4_reachable(const
> struct ovn_port *op,
>                                          ovs_be32 addr);
>  static bool lrouter_port_ipv6_reachable(const struct ovn_port *op,
>                                          const struct in6_addr *addr);
> +
>  static void
> -build_lrouter_lb_reachable_ips(struct ovn_datapath *od,
> -                               const struct ovn_northd_lb *lb)
> +add_neigh_ips_to_lrouter(struct ovn_datapath *od,
> +                         enum lb_neighbor_responder_mode neigh_mode,
> +                         const struct sset *lb_ips_v4,
> +                         const struct sset *lb_ips_v6)
>  {
>      /* If configured to not reply to any neighbor requests for all VIPs
>       * return early.
>       */
> -    if (lb->neigh_mode == LB_NEIGH_RESPOND_NONE) {
> +    if (neigh_mode == LB_NEIGH_RESPOND_NONE) {
>          return;
>      }
>
> +    const char *ip_address;
> +
>      /* If configured to reply to neighbor requests for all VIPs force them
>       * all to be considered "reachable".
>       */
> -    if (lb->neigh_mode == LB_NEIGH_RESPOND_ALL) {
> -        for (size_t i = 0; i < lb->n_vips; i++) {
> -            if (lb->vips[i].address_family == AF_INET) {
> -                sset_add(&od->lb_ips->ips_v4_reachable,
> lb->vips[i].vip_str);
> -            } else {
> -                sset_add(&od->lb_ips->ips_v6_reachable,
> lb->vips[i].vip_str);
> -            }
> +    if (neigh_mode == LB_NEIGH_RESPOND_ALL) {
> +        SSET_FOR_EACH (ip_address, lb_ips_v4) {
> +            sset_add(&od->lb_ips->ips_v4_reachable, ip_address);
> +        }
> +        SSET_FOR_EACH (ip_address, lb_ips_v6) {
> +            sset_add(&od->lb_ips->ips_v6_reachable, ip_address);
>          }
> +
>          return;
>      }
>
>      /* Otherwise, a VIP is reachable if there's at least one router
>       * subnet that includes it.
>       */
> -    ovs_assert(lb->neigh_mode == LB_NEIGH_RESPOND_REACHABLE);
> -    for (size_t i = 0; i < lb->n_vips; i++) {
> -        if (lb->vips[i].address_family == AF_INET) {
> -            ovs_be32 vip_ip4 = in6_addr_get_mapped_ipv4(&lb->vips[i].vip);
> -            struct ovn_port *op;
> +    ovs_assert(neigh_mode == LB_NEIGH_RESPOND_REACHABLE);
>
> +    SSET_FOR_EACH (ip_address, lb_ips_v4) {
> +        struct ovn_port *op;
> +        ovs_be32 vip_ip4;
> +        if (ip_parse(ip_address, &vip_ip4)) {
>              HMAP_FOR_EACH (op, dp_node, &od->ports) {
>                  if (lrouter_port_ipv4_reachable(op, vip_ip4)) {
>                      sset_add(&od->lb_ips->ips_v4_reachable,
> -                             lb->vips[i].vip_str);
> +                             ip_address);
>                      break;
>                  }
>              }
> -        } else {
> -            struct ovn_port *op;
> +        }
> +    }
>
> +    SSET_FOR_EACH (ip_address, lb_ips_v6) {
> +        struct ovn_port *op;
> +        struct in6_addr vip;
> +        if (ipv6_parse(ip_address, &vip)) {
>              HMAP_FOR_EACH (op, dp_node, &od->ports) {
> -                if (lrouter_port_ipv6_reachable(op, &lb->vips[i].vip)) {
> +                if (lrouter_port_ipv6_reachable(op, &vip)) {
>                      sset_add(&od->lb_ips->ips_v6_reachable,
> -                             lb->vips[i].vip_str);
> +                             ip_address);
>                      break;
>                  }
>              }
> @@ -4182,6 +4191,34 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath
> *od,
>      }
>  }
>
> +static void
> +remove_lrouter_lb_reachable_ips(struct ovn_datapath *od,
> +                                enum lb_neighbor_responder_mode
> neigh_mode,
> +                                const struct sset *lb_ips_v4,
> +                                const struct sset *lb_ips_v6)
> +{
> +    if (neigh_mode == LB_NEIGH_RESPOND_NONE) {
> +        return;
> +    }
> +
> +    const char *ip_address;
> +    SSET_FOR_EACH (ip_address, lb_ips_v4) {
> +        sset_find_and_delete(&od->lb_ips->ips_v4_reachable, ip_address);
> +    }
> +    SSET_FOR_EACH (ip_address, lb_ips_v6) {
> +        sset_find_and_delete(&od->lb_ips->ips_v6_reachable, ip_address);
> +    }
> +}
> +
> +static void
> +build_lrouter_lb_reachable_ips(struct ovn_datapath *od,
> +                               const struct ovn_northd_lb *lb)
> +{
> +    add_neigh_ips_to_lrouter(od, lb->neigh_mode, &lb->ips_v4,
> +                             &lb->ips_v6);
> +}
> +
> +
>  static void
>  build_lrouter_lbs_check(const struct ovn_datapaths *lr_datapaths)
>  {
> @@ -5382,6 +5419,95 @@ fail:
>      return false;
>  }
>
> +/* Returns true if the logical router has changes which are not
> + * incrementally handled.
> + * Presently supports i-p for the below changes:
> + *    - load balancers and load balancer groups.
> + */
> +static bool
> +check_unsupported_inc_proc_for_lr_changes(
> +    const struct nbrec_logical_router *lr)
> +{
> +    /* Check if the columns are changed in this row. */
> +    enum nbrec_logical_router_column_id col;
> +    for (col = 0; col < NBREC_LOGICAL_ROUTER_N_COLUMNS; col++) {
> +        if (nbrec_logical_router_is_updated(lr, col)) {
> +            if (col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER ||
> +                col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP) {
> +                continue;
> +            }
> +            return true;
> +        }
> +    }
> +
> +    /* Check if the referenced rows are changed.
> +       XXX: Need a better OVSDB IDL interface for this check. */
> +    for (size_t i = 0; i < lr->n_ports; i++) {
> +        if (nbrec_logical_router_port_row_get_seqno(lr->ports[i],
> +                                OVSDB_IDL_CHANGE_MODIFY) > 0) {
> +            return true;
> +        }
> +    }
> +    if (lr->copp && nbrec_copp_row_get_seqno(lr->copp,
> +                                OVSDB_IDL_CHANGE_MODIFY) > 0) {
> +        return true;
> +    }
> +    for (size_t i = 0; i < lr->n_nat; i++) {
> +        if (nbrec_nat_row_get_seqno(lr->nat[i],
> +                                OVSDB_IDL_CHANGE_MODIFY) > 0) {
> +            return true;
> +        }
> +    }
> +    for (size_t i = 0; i < lr->n_policies; i++) {
> +        if (nbrec_logical_router_policy_row_get_seqno(lr->policies[i],
> +                                OVSDB_IDL_CHANGE_MODIFY) > 0) {
> +            return true;
> +        }
> +    }
> +    for (size_t i = 0; i < lr->n_static_routes; i++) {
> +        if (nbrec_logical_router_static_route_row_get_seqno(
> +            lr->static_routes[i], OVSDB_IDL_CHANGE_MODIFY) > 0) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +/* Return true if changes are handled incrementally, false otherwise.
> + * When there are any changes, try to track what's exactly changed and set
> + * northd_data->change_tracked accordingly: change tracked - true,
> otherwise,
> + * false.
> + * Note: Changes to load balancer and load balancer groups associated with
> + * the logical routers are handled separately in the lb_data change
> + * handlers (northd_handle_lb_data_changes_pre_od and
> + * northd_handle_lb_data_changes_post_od).
> + * */
> +bool
> +northd_handle_lr_changes(const struct northd_input *ni,
> +                         struct northd_data *nd)
> +{
> +    const struct nbrec_logical_router *changed_lr;
> +
> +    NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH_TRACKED (changed_lr,
> +
>  ni->nbrec_logical_router_table) {
> +        if (nbrec_logical_router_is_new(changed_lr) ||
> +            nbrec_logical_router_is_deleted(changed_lr)) {
> +            goto fail;
> +        }
> +
> +        /* Presently only able to handle load balancer and
> +         * load balancer group changes. */
> +        if (check_unsupported_inc_proc_for_lr_changes(changed_lr)) {
> +            goto fail;
> +        }
> +    }
> +
> +    return true;
> +fail:
> +    destroy_northd_data_tracked_changes(nd);
> +    return false;
> +}
> +
>  bool
>  northd_handle_sb_port_binding_changes(
>      const struct sbrec_port_binding_table *sbrec_port_binding_table,
> @@ -5530,6 +5656,7 @@ northd_handle_lb_data_changes_pre_od(struct
> tracked_lb_data *trk_lb_data,
>  bool
>  northd_handle_lb_data_changes_post_od(struct tracked_lb_data *trk_lb_data,
>                                        struct ovn_datapaths *ls_datapaths,
> +                                      struct ovn_datapaths *lr_datapaths,
>                                        struct hmap *lb_datapaths_map,
>                                        struct hmap *lbgrp_datapaths_map)
>  {
> @@ -5573,6 +5700,45 @@ northd_handle_lb_data_changes_post_od(struct
> tracked_lb_data *trk_lb_data,
>          init_lb_for_datapath(od);
>      }
>
> +    LIST_FOR_EACH (codlb, list_node, &trk_lb_data->crupdated_lr_lbs) {
> +        od = ovn_datapath_find(&lr_datapaths->datapaths, &codlb->od_uuid);
> +        ovs_assert(od);
> +
> +        struct uuidset_node *uuidnode;
> +        UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbs) {
> +            lb_dps = ovn_lb_datapaths_find(lb_datapaths_map,
> &uuidnode->uuid);
> +            ovs_assert(lb_dps);
> +            ovn_lb_datapaths_add_lr(lb_dps, 1, &od);
> +
> +            /* Add the lb_ips of lb_dps to the od. */
> +            build_lrouter_lb_ips(od->lb_ips, lb_dps->lb);
> +            build_lrouter_lb_reachable_ips(od, lb_dps->lb);
> +        }
> +
> +        UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbgrps) {
> +            lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map,
> +                                                    &uuidnode->uuid);
> +            ovs_assert(lbgrp_dps);
> +            ovn_lb_group_datapaths_add_lr(lbgrp_dps, od);
> +
> +            /* Associate all the lbs of the lbgrp to the datapath 'od' */
> +            for (size_t j = 0; j < lbgrp_dps->lb_group->n_lbs; j++) {
> +                const struct uuid *lb_uuid
> +                    = &lbgrp_dps->lb_group->lbs[j]->nlb->header_.uuid;
> +                lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
> +                ovs_assert(lb_dps);
> +                ovn_lb_datapaths_add_lr(lb_dps, 1, &od);
> +
> +                /* Add the lb_ips of lb_dps to the od. */
> +                build_lrouter_lb_ips(od->lb_ips, lb_dps->lb);
> +                build_lrouter_lb_reachable_ips(od, lb_dps->lb);
> +            }
> +        }
> +
> +        /* Re-evaluate 'od->has_lb_vip' */
> +        init_lb_for_datapath(od);
> +    }
> +
>      struct crupdated_lb *clb;
>      HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) {
>          lb = clb->lb;
> @@ -5587,6 +5753,29 @@ northd_handle_lb_data_changes_post_od(struct
> tracked_lb_data *trk_lb_data,
>              /* Re-evaluate 'od->has_lb_vip' */
>              init_lb_for_datapath(od);
>          }
> +
> +        BITMAP_FOR_EACH_1 (index, ods_size(lr_datapaths),
> +                           lb_dps->nb_lr_map) {
> +            od = lr_datapaths->array[index];
> +            /* Re-evaluate 'od->has_lb_vip' */
> +            init_lb_for_datapath(od);
> +
> +            /* Update the od->lb_ips with the deleted and inserted
> +             * vips (if any). */
> +            remove_ips_from_lb_ip_set(od->lb_ips, lb->routable,
> +                                      &clb->deleted_vips_v4,
> +                                      &clb->deleted_vips_v6);
> +            add_ips_to_lb_ip_set(od->lb_ips, lb->routable,
> +                                 &clb->inserted_vips_v4,
> +                                 &clb->inserted_vips_v6);
> +
> +            remove_lrouter_lb_reachable_ips(od, lb->neigh_mode,
> +                                            &clb->deleted_vips_v4,
> +                                            &clb->deleted_vips_v6);
> +            add_neigh_ips_to_lrouter(od, lb->neigh_mode,
> +                                     &clb->inserted_vips_v4,
> +                                     &clb->inserted_vips_v6);
> +        }
>      }
>
>      struct ovn_lb_group *lbgrp;
> @@ -5606,8 +5795,19 @@ northd_handle_lb_data_changes_post_od(struct
> tracked_lb_data *trk_lb_data,
>              lb_uuid = &lb->nlb->header_.uuid;
>              lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
>              ovs_assert(lb_dps);
> +            for (size_t i = 0; i < lbgrp_dps->n_lr; i++) {
> +                od = lbgrp_dps->lr[i];
> +                ovn_lb_datapaths_add_lr(lb_dps, 1, &od);
> +
> +                /* Re-evaluate 'od->has_lb_vip' */
> +                init_lb_for_datapath(od);
> +
> +                /* Add the lb_ips of lb_dps to the od. */
> +                build_lrouter_lb_ips(od->lb_ips, lb_dps->lb);
> +            }
> +
>              for (size_t i = 0; i < lbgrp_dps->n_ls; i++) {
> -                od = lbgrp_dps->ls[i];
> +               od = lbgrp_dps->ls[i];
>                  ovn_lb_datapaths_add_ls(lb_dps, 1, &od);
>
>                  /* Re-evaluate 'od->has_lb_vip' */
> diff --git a/northd/northd.h b/northd/northd.h
> index cd2e5394c2..1d344d57d6 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -335,6 +335,8 @@ void ovnsb_db_run(struct ovsdb_idl_txn *ovnnb_txn,
>  bool northd_handle_ls_changes(struct ovsdb_idl_txn *,
>                                const struct northd_input *,
>                                struct northd_data *);
> +bool northd_handle_lr_changes(const struct northd_input *,
> +                              struct northd_data *);
>  void destroy_northd_data_tracked_changes(struct northd_data *);
>  void northd_destroy(struct northd_data *data);
>  void northd_init(struct northd_data *data);
> @@ -357,6 +359,7 @@ bool northd_handle_lb_data_changes_pre_od(struct
> tracked_lb_data *,
>                                            struct hmap
> *lbgrp_datapaths_map);
>  bool northd_handle_lb_data_changes_post_od(struct tracked_lb_data *,
>                                             struct ovn_datapaths
> *ls_datapaths,
> +                                           struct ovn_datapaths
> *lr_datapaths,
>                                             struct hmap *lb_datapaths_map,
>                                             struct hmap
> *lbgrp_datapaths_map);
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index b8b2ee390e..234d5a8bbd 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -9947,8 +9947,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  # Add lb1 to lr0 and then disassociate
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lr-lb-add lr0 lb1
> -check_engine_stats lb_data norecompute nocompute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -9956,7 +9956,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"
> 10.0.0.100:80"'
>  check_engine_stats lb_data norecompute compute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -9964,7 +9964,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
>  check_engine_stats lb_data norecompute compute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -9972,7 +9972,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  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 recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -9980,13 +9980,13 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  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 recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  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 nocompute
> +check_engine_stats lb_data norecompute compute
>  check_engine_stats northd recompute nocompute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> @@ -10078,8 +10078,8 @@ check_engine_stats lflow recompute nocompute
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl add logical_router lr0 load_balancer_group $lbg1_uuid
> -check_engine_stats lb_data norecompute nocompute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10087,7 +10087,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"
> 10.0.0.100:80"'
>  check_engine_stats lb_data norecompute compute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10095,7 +10095,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
>  check_engine_stats lb_data norecompute compute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10103,7 +10103,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  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 recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10111,13 +10111,13 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  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 recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl clear logical_router lr0 load_balancer_group
> -check_engine_stats lb_data norecompute nocompute
> +check_engine_stats lb_data norecompute compute
>  check_engine_stats northd recompute nocompute
>  check_engine_stats lflow recompute nocompute
>
> @@ -10172,8 +10172,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl set logical_router lr1 load_balancer_group=$lbg1_uuid
> -check_engine_stats lb_data norecompute nocompute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10194,8 +10194,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lr-lb-add lr1 lb1
>  check ovn-nbctl --wait=sb lr-lb-add lr1 lb2
> -check_engine_stats lb_data norecompute nocompute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10208,7 +10208,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lr-lb-del lr1 lb2
> -check_engine_stats lb_data norecompute nocompute
> +check_engine_stats lb_data norecompute compute
>  check_engine_stats northd recompute nocompute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> @@ -10218,7 +10218,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lb-del lb4
>  check_engine_stats lb_data norecompute compute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10227,7 +10227,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lb-del lb2
>  check_engine_stats lb_data norecompute compute
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
>  check_engine_stats lflow recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> --
> 2.40.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>

Looks good to me, thanks.

Reviewed-by: Ales Musil <amu...@redhat.com>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

amu...@redhat.com    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to