diff mbox series

[ovs-dev,v2,5/8] northd: Handle load balancer changes for a logical switch.

Message ID 20230707055416.961752-1-numans@ovn.org
State Changes Requested
Headers show
Series northd: I-P for load balancer and lb groups | expand

Checks

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

Commit Message

Numan Siddique July 7, 2023, 5:54 a.m. UTC 
From: Numan Siddique <numans@ovn.org>

Logical switch change handler of the 'northd' engine node
now also handles changes to load balancers.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 lib/lb.c            |  17 +++-
 lib/lb.h            |   2 +
 northd/northd.c     | 212 +++++++++++++++++++++++++++++++++-----------
 northd/northd.h     |   9 ++
 tests/ovn-northd.at |   8 +-
 5 files changed, 191 insertions(+), 57 deletions(-)

Comments

Han Zhou July 17, 2023, 2:57 a.m. UTC | #1 
On Fri, Jul 7, 2023 at 1:54 PM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> Logical switch change handler of the 'northd' engine node
> now also handles changes to load balancers.
>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  lib/lb.c            |  17 +++-
>  lib/lb.h            |   2 +
>  northd/northd.c     | 212 +++++++++++++++++++++++++++++++++-----------
>  northd/northd.h     |   9 ++
>  tests/ovn-northd.at |   8 +-
>  5 files changed, 191 insertions(+), 57 deletions(-)
>
> diff --git a/lib/lb.c b/lib/lb.c
> index a51fe225fa..b2b6473c1d 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -1091,7 +1091,22 @@ ovn_lb_datapaths_add_ls(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_ls_map, ods[i]->index);
> +        if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
> +            bitmap_set1(lb_dps->nb_ls_map, ods[i]->index);
> +            lb_dps->n_nb_ls++;
> +        }
> +    }
> +}
> +
> +void
> +ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
> +                        struct ovn_datapath **ods)
> +{
> +    for (size_t i = 0; i < n; i++) {
> +        if (bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
> +            bitmap_set0(lb_dps->nb_ls_map, ods[i]->index);
> +            lb_dps->n_nb_ls--;
> +        }
>      }
>  }
>
> diff --git a/lib/lb.h b/lib/lb.h
> index 74905c73b7..ac33333a7f 100644
> --- a/lib/lb.h
> +++ b/lib/lb.h
> @@ -179,6 +179,8 @@ void ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths
*, size_t n,
>                               struct ovn_datapath **);
>  void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *, size_t n,
>                               struct ovn_datapath **);
> +void ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *, size_t n,
> +                                struct ovn_datapath **);
>
>  struct ovn_lb_group_datapaths {
>      struct hmap_node hmap_node;
> diff --git a/northd/northd.c b/northd/northd.c
> index fc98ab7bfd..bf02190f7c 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -853,7 +853,8 @@ ovn_datapath_destroy(struct hmap *datapaths, struct
ovn_datapath *od)
>          ovn_ls_port_group_destroy(&od->nb_pgs);
>          destroy_mcast_info_for_datapath(od);
>          destroy_ports_for_datapath(od);
> -
> +        free(od->lb_uuids);
> +        free(od->lb_group_uuids);
>          free(od);
>      }
>  }
> @@ -4080,6 +4081,48 @@ build_lb_vip_actions(const struct ovn_northd_lb
*lb,
>      return reject;
>  }
>
> +static void
> +associate_ls_lbs(struct ovn_datapath *ls_od, struct hmap
*lb_datapaths_map)
> +{
> +    struct ovn_lb_datapaths *lb_dps;
> +
> +    free(ls_od->lb_uuids);
> +    ls_od->lb_uuids = xcalloc(ls_od->nbs->n_load_balancer,
> +                              sizeof *ls_od->lb_uuids);
> +    ls_od->n_lb_uuids = ls_od->nbs->n_load_balancer;
> +    for (size_t i = 0; i < ls_od->nbs->n_load_balancer; i++) {
> +        const struct uuid *lb_uuid =
> +            &ls_od->nbs->load_balancer[i]->header_.uuid;
> +        lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
> +        ovs_assert(lb_dps);
> +        ovn_lb_datapaths_add_ls(lb_dps, 1, &ls_od);
> +        ls_od->lb_uuids[i] = *lb_uuid;
> +    }
> +}
> +
> +static void
> +associate_ls_lb_groups(struct ovn_datapath *ls_od,
> +                       struct hmap *lb_group_datapaths_map)
> +{
> +    const struct nbrec_load_balancer_group *nbrec_lb_group;
> +    struct ovn_lb_group_datapaths *lb_group_dps;
> +
> +    free(ls_od->lb_group_uuids);
> +    ls_od->lb_group_uuids = xcalloc(ls_od->nbs->n_load_balancer_group,
> +                                    sizeof *ls_od->lb_group_uuids);
> +    ls_od->n_lb_group_uuids = ls_od->nbs->n_load_balancer_group;
> +    for (size_t i = 0; i < ls_od->nbs->n_load_balancer_group; i++) {
> +        nbrec_lb_group = ls_od->nbs->load_balancer_group[i];
> +        const struct uuid *lb_group_uuid = &nbrec_lb_group->header_.uuid;
> +        lb_group_dps =
> +            ovn_lb_group_datapaths_find(lb_group_datapaths_map,
> +                                        lb_group_uuid);
> +        ovs_assert(lb_group_dps);
> +        ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &ls_od);
> +        ls_od->lb_group_uuids[i] = *lb_group_uuid;
> +    }
> +}
> +
>  static void
>  build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups,
>                     struct ovn_datapaths *ls_datapaths,
> @@ -4115,24 +4158,8 @@ build_lb_datapaths(const struct hmap *lbs, const
struct hmap *lb_groups,
>          if (!od->nbs) {
>              continue;
>          }
> -
> -        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> -            const struct uuid *lb_uuid =
> -                &od->nbs->load_balancer[i]->header_.uuid;
> -            lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
> -            ovs_assert(lb_dps);
> -            ovn_lb_datapaths_add_ls(lb_dps, 1, &od);
> -        }
> -
> -        for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) {
> -            nbrec_lb_group = od->nbs->load_balancer_group[i];
> -            const struct uuid *lb_group_uuid =
&nbrec_lb_group->header_.uuid;
> -            lb_group_dps =
> -                ovn_lb_group_datapaths_find(lb_group_datapaths_map,
> -                                            lb_group_uuid);
> -            ovs_assert(lb_group_dps);
> -            ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &od);
> -        }
> +        associate_ls_lbs(od, lb_datapaths_map);
> +        associate_ls_lb_groups(od, lb_group_datapaths_map);
>      }
>
>      HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
> @@ -4891,23 +4918,29 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
>      sset_destroy(&active_ha_chassis_grps);
>  }
>
> +static void
> +destroy_tracked_ls_change(struct ls_change *ls_change)
> +{
> +    struct ovn_port *op;
> +    LIST_FOR_EACH (op, list, &ls_change->added_ports) {
> +        ovs_list_remove(&op->list);
> +    }
> +    LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
> +        ovs_list_remove(&op->list);
> +    }
> +    LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) {
> +        ovs_list_remove(&op->list);
> +        ovn_port_destroy_orphan(op);
> +    }
> +}
> +
>  void
>  destroy_northd_data_tracked_changes(struct northd_data *nd)
>  {
>      struct ls_change *ls_change;
>      LIST_FOR_EACH_SAFE (ls_change, list_node,
>                          &nd->tracked_ls_changes.updated) {
> -        struct ovn_port *op;
> -        LIST_FOR_EACH (op, list, &ls_change->added_ports) {
> -            ovs_list_remove(&op->list);
> -        }
> -        LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
> -            ovs_list_remove(&op->list);
> -        }
> -        LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) {
> -            ovs_list_remove(&op->list);
> -            ovn_port_destroy_orphan(op);
> -        }
> +        destroy_tracked_ls_change(ls_change);

Does the above change belong to the previous refactor patch?

>          ovs_list_remove(&ls_change->list_node);
>          free(ls_change);
>      }
> @@ -5028,6 +5061,7 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn,
struct hmap *ls_ports,
>   * incrementally handled.
>   * Presently supports i-p for the below changes:
>   *    - logical switch ports.
> + *    - load balancers.
>   */
>  static bool
>  check_unsupported_inc_proc_for_ls_changes(
> @@ -5036,8 +5070,11 @@ check_unsupported_inc_proc_for_ls_changes(
>      /* Check if the columns are changed in this row. */
>      enum nbrec_logical_switch_column_id col;
>      for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) {
> -        if (nbrec_logical_switch_is_updated(ls, col) &&
> -            col != NBREC_LOGICAL_SWITCH_COL_PORTS) {
> +        if (nbrec_logical_switch_is_updated(ls, col)) {
> +            if (col == NBREC_LOGICAL_SWITCH_COL_PORTS ||
> +                col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) {
> +                continue;
> +            }
>              return true;
>          }
>      }
> @@ -5066,12 +5103,6 @@ check_unsupported_inc_proc_for_ls_changes(
>              return true;
>          }
>      }
> -    for (size_t i = 0; i < ls->n_load_balancer; i++) {
> -        if (nbrec_load_balancer_row_get_seqno(ls->load_balancer[i],
> -                                OVSDB_IDL_CHANGE_MODIFY) > 0) {
> -            return true;
> -        }
> -    }
>      for (size_t i = 0; i < ls->n_load_balancer_group; i++) {
>          if
(nbrec_load_balancer_group_row_get_seqno(ls->load_balancer_group[i],
>                                  OVSDB_IDL_CHANGE_MODIFY) > 0) {
> @@ -5130,7 +5161,9 @@ ls_check_and_handle_lsp_changes(struct
ovsdb_idl_txn *ovnsb_idl_txn,
>                                  const struct nbrec_logical_switch
*changed_ls,
>                                  const struct northd_input *ni,
>                                  struct northd_data *nd,
> -                                struct ovn_datapath *od)
> +                                struct ovn_datapath *od,
> +                                struct ls_change *ls_change,
> +                                bool *updated)
>  {
>      bool ls_ports_changed = false;
>      if (!nbrec_logical_switch_is_updated(changed_ls,
> @@ -5151,12 +5184,6 @@ ls_check_and_handle_lsp_changes(struct
ovsdb_idl_txn *ovnsb_idl_txn,
>          return true;
>      }
>
> -    struct ls_change *ls_change = xzalloc(sizeof *ls_change);
> -    ls_change->od = od;
> -    ovs_list_init(&ls_change->added_ports);
> -    ovs_list_init(&ls_change->deleted_ports);
> -    ovs_list_init(&ls_change->updated_ports);
> -
>      struct ovn_port *op;
>      HMAP_FOR_EACH (op, dp_node, &od->ports) {
>          op->visited = false;
> @@ -5246,10 +5273,7 @@ ls_check_and_handle_lsp_changes(struct
ovsdb_idl_txn *ovnsb_idl_txn,
>      if (!ovs_list_is_empty(&ls_change->added_ports) ||
>          !ovs_list_is_empty(&ls_change->updated_ports) ||
>          !ovs_list_is_empty(&ls_change->deleted_ports)) {
> -        ovs_list_push_back(&nd->tracked_ls_changes.updated,
> -                            &ls_change->list_node);
> -    } else {
> -        free(ls_change);
> +        *updated = true;
>      }
>
>      return true;
> @@ -5260,10 +5284,60 @@ fail_clean_deleted:
>      }
>
>  fail:
> -    free(ls_change);
> +
> +    LIST_FOR_EACH (op, list, &ls_change->added_ports) {
> +        ovs_list_remove(&op->list);
> +    }
> +    LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
> +        ovs_list_remove(&op->list);
> +    }
> +    LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) {
> +        ovs_list_remove(&op->list);
> +        ovn_port_destroy_orphan(op);
> +    }

Should it just call destroy_tracked_ls_change()? Should this also belong to
the previous patch?

>      return false;
>  }
>
> +static bool
> +ls_check_and_handle_lb_changes(const struct nbrec_logical_switch
*changed_ls,

nit: Maybe shorten the name as ls_handle_lb_changes? It is common to check
before handling something so I think it is ok to omit the "check" in the
name. (same for ls_check_and_handle_lsp_changes - sorry that I forgot to
mention in the previous patch)

> +                               struct hmap *lb_datapaths_map,
> +                               struct ovn_datapath *od,
> +                               struct ls_change *ls_change,
> +                               bool *updated)
> +{
> +
> +    if (!nbrec_logical_switch_is_updated(changed_ls,
> +                        NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER)) {
> +        for (size_t i = 0; i < changed_ls->n_load_balancer; i++) {
> +            if
(nbrec_load_balancer_row_get_seqno(changed_ls->load_balancer[i],
> +                                    OVSDB_IDL_CHANGE_MODIFY) > 0) {
> +                ls_change->lbs_changed = true;
> +                *updated = true;
> +                /* Re-evaluate 'od->has_lb_vip' */
> +                init_lb_for_datapath(od);

The above logic is not needed, because now we break the loop and then the
same logic is repeated later in this function.

> +                break;
> +            }
> +        }
> +
> +        return true;
> +    }
> +
> +    struct ovn_lb_datapaths *lb_dps;
> +    for (size_t i = 0; i < od->n_lb_uuids; i++) {
> +        lb_dps = ovn_lb_datapaths_find(lb_datapaths_map,
&od->lb_uuids[i]);
> +        if (lb_dps) {
> +            ovn_lb_datapaths_remove_ls(lb_dps, 1, &od);
> +        }
> +    }
> +
> +    associate_ls_lbs(od, lb_datapaths_map);

I think the incremental-processing needs to be more fine-grained. This
function removes all LB associations for this datapath and re-attach all,
so it can't tell the real delta, and then the lflow node can't handle the
real change but needs to reprocess lflows related to all LBs of this
datapath. As we know there can be big number of LBs associated to every
datapath, this may not help much for the performance. So I think it is
better to do a diff for the real changes and only process those (similar to
the lsp handling).

> +    ls_change->lbs_changed = true;
> +    *updated = true;
> +    /* Re-evaluate 'od->has_lb_vip' */
> +    init_lb_for_datapath(od);
> +    return true;
> +}
> +
>
>  /* Return true if changes are handled incrementally, false otherwise.
>   * When there are any changes, try to track what's exactly changed and
set
> @@ -5293,20 +5367,47 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
>              goto fail;
>          }
>
> -        /* Now only able to handle lsp changes. */
> +        /* Now only able to handle lsp, load balancerload and
> +         * load balancer group changes. */

nit: Not yet handling lb group changes in this patch :)

>          if (check_unsupported_inc_proc_for_ls_changes(changed_ls)) {
>              goto fail;
>          }
>
> +        struct ls_change *ls_change = xzalloc(sizeof *ls_change);
> +        ls_change->od = od;
> +        ovs_list_init(&ls_change->added_ports);
> +        ovs_list_init(&ls_change->deleted_ports);
> +        ovs_list_init(&ls_change->updated_ports);
> +
> +        bool updated = false;
>          if (!ls_check_and_handle_lsp_changes(ovnsb_idl_txn, changed_ls,
> -                                             ni, nd, od)) {
> +                                             ni, nd, od, ls_change,
> +                                             &updated)) {
> +            destroy_tracked_ls_change(ls_change);
> +            free(ls_change);
> +            goto fail;
> +        }
> +
> +        if (!ls_check_and_handle_lb_changes(changed_ls,
> +                                            &nd->lb_datapaths_map, od,
> +                                            ls_change, &updated)) {
> +            destroy_tracked_ls_change(ls_change);
> +            free(ls_change);
>              goto fail;
>          }
> +
> +        if (updated) {
> +            ovs_list_push_back(&nd->tracked_ls_changes.updated,
> +                               &ls_change->list_node);
> +        } else {
> +            free(ls_change);
> +        }
>      }
>
>      if (!ovs_list_is_empty(&nd->tracked_ls_changes.updated)) {
>          nd->change_tracked = true;
>      }
> +
>      return true;
>
>  fail:
> @@ -16628,6 +16729,13 @@ bool lflow_handle_northd_ls_changes(struct
ovsdb_idl_txn *ovnsb_txn,
>                                      struct hmap *lflows)
>  {
>      struct ls_change *ls_change;
> +
> +    LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) {
> +     if (ls_change->lbs_changed) {
> +            return false;
> +        }
> +    }
> +
>      LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) {
>          const struct sbrec_multicast_group *sbmc_flood =
>              mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp,
> diff --git a/northd/northd.h b/northd/northd.h
> index 7d17921fa2..3afa4eaff2 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -94,6 +94,7 @@ struct ls_change {
>      struct ovs_list added_ports;
>      struct ovs_list deleted_ports;
>      struct ovs_list updated_ports;
> +    bool lbs_changed;
>  };
>
>  /* Track what's changed for logical switches.
> @@ -318,6 +319,14 @@ struct ovn_datapath {
>      /* Map of ovn_port objects belonging to this datapath.
>       * This map doesn't include derived ports. */
>      struct hmap ports;
> +
> +    /* LB uuids associated with this datapath. */
> +    struct uuid *lb_uuids;
> +    size_t n_lb_uuids;
> +
> +    /* LB group uuids associated with this datapath. */
> +    struct uuid *lb_group_uuids;
> +    size_t n_lb_group_uuids;
>  };
>
>  void ovnnb_db_run(struct northd_input *input_data,
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index dd0bd8b36a..d9f3917a3e 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -9750,15 +9750,15 @@ check ovn-nbctl ls-add sw0
>  check ovn-nbctl --wait=sb lr-add lr0
>  check_engine_stats recompute recompute
>
> -# Associate lb1 to sw0. There should be a full recompute of northd
engine node
> +# Associate lb1 to sw0. There should be no recompute of northd engine
node
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
> -check_engine_stats recompute recompute
> +check_engine_stats norecompute recompute
>
> -# Disassociate lb1 from sw0. There should be a full recompute of northd
engine node.
> +# Disassociate lb1 from sw0. There should be a no recompute of northd
engine node.
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
> -check_engine_stats recompute recompute
> +check_engine_stats norecompute recompute

Suggest using the CHECK_NO_CHANGE_AFTER_RECOMPUTE to make sure the I-P is
not missing anything.

Thanks,
Han

>
>  # Test load balancer group now
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> --
> 2.40.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/lib/lb.c b/lib/lb.c
index a51fe225fa..b2b6473c1d 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -1091,7 +1091,22 @@  ovn_lb_datapaths_add_ls(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_ls_map, ods[i]->index);
+        if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
+            bitmap_set1(lb_dps->nb_ls_map, ods[i]->index);
+            lb_dps->n_nb_ls++;
+        }
+    }
+}
+
+void
+ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
+                        struct ovn_datapath **ods)
+{
+    for (size_t i = 0; i < n; i++) {
+        if (bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
+            bitmap_set0(lb_dps->nb_ls_map, ods[i]->index);
+            lb_dps->n_nb_ls--;
+        }
     }
 }
 
diff --git a/lib/lb.h b/lib/lb.h
index 74905c73b7..ac33333a7f 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -179,6 +179,8 @@  void ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *, size_t n,
                              struct ovn_datapath **);
 void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *, size_t n,
                              struct ovn_datapath **);
+void ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *, size_t n,
+                                struct ovn_datapath **);
 
 struct ovn_lb_group_datapaths {
     struct hmap_node hmap_node;
diff --git a/northd/northd.c b/northd/northd.c
index fc98ab7bfd..bf02190f7c 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -853,7 +853,8 @@  ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
         ovn_ls_port_group_destroy(&od->nb_pgs);
         destroy_mcast_info_for_datapath(od);
         destroy_ports_for_datapath(od);
-
+        free(od->lb_uuids);
+        free(od->lb_group_uuids);
         free(od);
     }
 }
@@ -4080,6 +4081,48 @@  build_lb_vip_actions(const struct ovn_northd_lb *lb,
     return reject;
 }
 
+static void
+associate_ls_lbs(struct ovn_datapath *ls_od, struct hmap *lb_datapaths_map)
+{
+    struct ovn_lb_datapaths *lb_dps;
+
+    free(ls_od->lb_uuids);
+    ls_od->lb_uuids = xcalloc(ls_od->nbs->n_load_balancer,
+                              sizeof *ls_od->lb_uuids);
+    ls_od->n_lb_uuids = ls_od->nbs->n_load_balancer;
+    for (size_t i = 0; i < ls_od->nbs->n_load_balancer; i++) {
+        const struct uuid *lb_uuid =
+            &ls_od->nbs->load_balancer[i]->header_.uuid;
+        lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
+        ovs_assert(lb_dps);
+        ovn_lb_datapaths_add_ls(lb_dps, 1, &ls_od);
+        ls_od->lb_uuids[i] = *lb_uuid;
+    }
+}
+
+static void
+associate_ls_lb_groups(struct ovn_datapath *ls_od,
+                       struct hmap *lb_group_datapaths_map)
+{
+    const struct nbrec_load_balancer_group *nbrec_lb_group;
+    struct ovn_lb_group_datapaths *lb_group_dps;
+
+    free(ls_od->lb_group_uuids);
+    ls_od->lb_group_uuids = xcalloc(ls_od->nbs->n_load_balancer_group,
+                                    sizeof *ls_od->lb_group_uuids);
+    ls_od->n_lb_group_uuids = ls_od->nbs->n_load_balancer_group;
+    for (size_t i = 0; i < ls_od->nbs->n_load_balancer_group; i++) {
+        nbrec_lb_group = ls_od->nbs->load_balancer_group[i];
+        const struct uuid *lb_group_uuid = &nbrec_lb_group->header_.uuid;
+        lb_group_dps =
+            ovn_lb_group_datapaths_find(lb_group_datapaths_map,
+                                        lb_group_uuid);
+        ovs_assert(lb_group_dps);
+        ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &ls_od);
+        ls_od->lb_group_uuids[i] = *lb_group_uuid;
+    }
+}
+
 static void
 build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups,
                    struct ovn_datapaths *ls_datapaths,
@@ -4115,24 +4158,8 @@  build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups,
         if (!od->nbs) {
             continue;
         }
-
-        for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
-            const struct uuid *lb_uuid =
-                &od->nbs->load_balancer[i]->header_.uuid;
-            lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
-            ovs_assert(lb_dps);
-            ovn_lb_datapaths_add_ls(lb_dps, 1, &od);
-        }
-
-        for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) {
-            nbrec_lb_group = od->nbs->load_balancer_group[i];
-            const struct uuid *lb_group_uuid = &nbrec_lb_group->header_.uuid;
-            lb_group_dps =
-                ovn_lb_group_datapaths_find(lb_group_datapaths_map,
-                                            lb_group_uuid);
-            ovs_assert(lb_group_dps);
-            ovn_lb_group_datapaths_add_ls(lb_group_dps, 1, &od);
-        }
+        associate_ls_lbs(od, lb_datapaths_map);
+        associate_ls_lb_groups(od, lb_group_datapaths_map);
     }
 
     HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
@@ -4891,23 +4918,29 @@  build_ports(struct ovsdb_idl_txn *ovnsb_txn,
     sset_destroy(&active_ha_chassis_grps);
 }
 
+static void
+destroy_tracked_ls_change(struct ls_change *ls_change)
+{
+    struct ovn_port *op;
+    LIST_FOR_EACH (op, list, &ls_change->added_ports) {
+        ovs_list_remove(&op->list);
+    }
+    LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
+        ovs_list_remove(&op->list);
+    }
+    LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) {
+        ovs_list_remove(&op->list);
+        ovn_port_destroy_orphan(op);
+    }
+}
+
 void
 destroy_northd_data_tracked_changes(struct northd_data *nd)
 {
     struct ls_change *ls_change;
     LIST_FOR_EACH_SAFE (ls_change, list_node,
                         &nd->tracked_ls_changes.updated) {
-        struct ovn_port *op;
-        LIST_FOR_EACH (op, list, &ls_change->added_ports) {
-            ovs_list_remove(&op->list);
-        }
-        LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
-            ovs_list_remove(&op->list);
-        }
-        LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) {
-            ovs_list_remove(&op->list);
-            ovn_port_destroy_orphan(op);
-        }
+        destroy_tracked_ls_change(ls_change);
         ovs_list_remove(&ls_change->list_node);
         free(ls_change);
     }
@@ -5028,6 +5061,7 @@  ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports,
  * incrementally handled.
  * Presently supports i-p for the below changes:
  *    - logical switch ports.
+ *    - load balancers.
  */
 static bool
 check_unsupported_inc_proc_for_ls_changes(
@@ -5036,8 +5070,11 @@  check_unsupported_inc_proc_for_ls_changes(
     /* Check if the columns are changed in this row. */
     enum nbrec_logical_switch_column_id col;
     for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) {
-        if (nbrec_logical_switch_is_updated(ls, col) &&
-            col != NBREC_LOGICAL_SWITCH_COL_PORTS) {
+        if (nbrec_logical_switch_is_updated(ls, col)) {
+            if (col == NBREC_LOGICAL_SWITCH_COL_PORTS ||
+                col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) {
+                continue;
+            }
             return true;
         }
     }
@@ -5066,12 +5103,6 @@  check_unsupported_inc_proc_for_ls_changes(
             return true;
         }
     }
-    for (size_t i = 0; i < ls->n_load_balancer; i++) {
-        if (nbrec_load_balancer_row_get_seqno(ls->load_balancer[i],
-                                OVSDB_IDL_CHANGE_MODIFY) > 0) {
-            return true;
-        }
-    }
     for (size_t i = 0; i < ls->n_load_balancer_group; i++) {
         if (nbrec_load_balancer_group_row_get_seqno(ls->load_balancer_group[i],
                                 OVSDB_IDL_CHANGE_MODIFY) > 0) {
@@ -5130,7 +5161,9 @@  ls_check_and_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
                                 const struct nbrec_logical_switch *changed_ls,
                                 const struct northd_input *ni,
                                 struct northd_data *nd,
-                                struct ovn_datapath *od)
+                                struct ovn_datapath *od,
+                                struct ls_change *ls_change,
+                                bool *updated)
 {
     bool ls_ports_changed = false;
     if (!nbrec_logical_switch_is_updated(changed_ls,
@@ -5151,12 +5184,6 @@  ls_check_and_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
         return true;
     }
 
-    struct ls_change *ls_change = xzalloc(sizeof *ls_change);
-    ls_change->od = od;
-    ovs_list_init(&ls_change->added_ports);
-    ovs_list_init(&ls_change->deleted_ports);
-    ovs_list_init(&ls_change->updated_ports);
-
     struct ovn_port *op;
     HMAP_FOR_EACH (op, dp_node, &od->ports) {
         op->visited = false;
@@ -5246,10 +5273,7 @@  ls_check_and_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
     if (!ovs_list_is_empty(&ls_change->added_ports) ||
         !ovs_list_is_empty(&ls_change->updated_ports) ||
         !ovs_list_is_empty(&ls_change->deleted_ports)) {
-        ovs_list_push_back(&nd->tracked_ls_changes.updated,
-                            &ls_change->list_node);
-    } else {
-        free(ls_change);
+        *updated = true;
     }
 
     return true;
@@ -5260,10 +5284,60 @@  fail_clean_deleted:
     }
 
 fail:
-    free(ls_change);
+
+    LIST_FOR_EACH (op, list, &ls_change->added_ports) {
+        ovs_list_remove(&op->list);
+    }
+    LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
+        ovs_list_remove(&op->list);
+    }
+    LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) {
+        ovs_list_remove(&op->list);
+        ovn_port_destroy_orphan(op);
+    }
     return false;
 }
 
+static bool
+ls_check_and_handle_lb_changes(const struct nbrec_logical_switch *changed_ls,
+                               struct hmap *lb_datapaths_map,
+                               struct ovn_datapath *od,
+                               struct ls_change *ls_change,
+                               bool *updated)
+{
+
+    if (!nbrec_logical_switch_is_updated(changed_ls,
+                        NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER)) {
+        for (size_t i = 0; i < changed_ls->n_load_balancer; i++) {
+            if (nbrec_load_balancer_row_get_seqno(changed_ls->load_balancer[i],
+                                    OVSDB_IDL_CHANGE_MODIFY) > 0) {
+                ls_change->lbs_changed = true;
+                *updated = true;
+                /* Re-evaluate 'od->has_lb_vip' */
+                init_lb_for_datapath(od);
+                break;
+            }
+        }
+
+        return true;
+    }
+
+    struct ovn_lb_datapaths *lb_dps;
+    for (size_t i = 0; i < od->n_lb_uuids; i++) {
+        lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, &od->lb_uuids[i]);
+        if (lb_dps) {
+            ovn_lb_datapaths_remove_ls(lb_dps, 1, &od);
+        }
+    }
+
+    associate_ls_lbs(od, lb_datapaths_map);
+    ls_change->lbs_changed = true;
+    *updated = true;
+    /* Re-evaluate 'od->has_lb_vip' */
+    init_lb_for_datapath(od);
+    return true;
+}
+
 
 /* Return true if changes are handled incrementally, false otherwise.
  * When there are any changes, try to track what's exactly changed and set
@@ -5293,20 +5367,47 @@  northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
             goto fail;
         }
 
-        /* Now only able to handle lsp changes. */
+        /* Now only able to handle lsp, load balancerload and
+         * load balancer group changes. */
         if (check_unsupported_inc_proc_for_ls_changes(changed_ls)) {
             goto fail;
         }
 
+        struct ls_change *ls_change = xzalloc(sizeof *ls_change);
+        ls_change->od = od;
+        ovs_list_init(&ls_change->added_ports);
+        ovs_list_init(&ls_change->deleted_ports);
+        ovs_list_init(&ls_change->updated_ports);
+
+        bool updated = false;
         if (!ls_check_and_handle_lsp_changes(ovnsb_idl_txn, changed_ls,
-                                             ni, nd, od)) {
+                                             ni, nd, od, ls_change,
+                                             &updated)) {
+            destroy_tracked_ls_change(ls_change);
+            free(ls_change);
+            goto fail;
+        }
+
+        if (!ls_check_and_handle_lb_changes(changed_ls,
+                                            &nd->lb_datapaths_map, od,
+                                            ls_change, &updated)) {
+            destroy_tracked_ls_change(ls_change);
+            free(ls_change);
             goto fail;
         }
+
+        if (updated) {
+            ovs_list_push_back(&nd->tracked_ls_changes.updated,
+                               &ls_change->list_node);
+        } else {
+            free(ls_change);
+        }
     }
 
     if (!ovs_list_is_empty(&nd->tracked_ls_changes.updated)) {
         nd->change_tracked = true;
     }
+
     return true;
 
 fail:
@@ -16628,6 +16729,13 @@  bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn,
                                     struct hmap *lflows)
 {
     struct ls_change *ls_change;
+
+    LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) {
+     if (ls_change->lbs_changed) {
+            return false;
+        }
+    }
+
     LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) {
         const struct sbrec_multicast_group *sbmc_flood =
             mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp,
diff --git a/northd/northd.h b/northd/northd.h
index 7d17921fa2..3afa4eaff2 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -94,6 +94,7 @@  struct ls_change {
     struct ovs_list added_ports;
     struct ovs_list deleted_ports;
     struct ovs_list updated_ports;
+    bool lbs_changed;
 };
 
 /* Track what's changed for logical switches.
@@ -318,6 +319,14 @@  struct ovn_datapath {
     /* Map of ovn_port objects belonging to this datapath.
      * This map doesn't include derived ports. */
     struct hmap ports;
+
+    /* LB uuids associated with this datapath. */
+    struct uuid *lb_uuids;
+    size_t n_lb_uuids;
+
+    /* LB group uuids associated with this datapath. */
+    struct uuid *lb_group_uuids;
+    size_t n_lb_group_uuids;
 };
 
 void ovnnb_db_run(struct northd_input *input_data,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index dd0bd8b36a..d9f3917a3e 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9750,15 +9750,15 @@  check ovn-nbctl ls-add sw0
 check ovn-nbctl --wait=sb lr-add lr0
 check_engine_stats recompute recompute
 
-# Associate lb1 to sw0. There should be a full recompute of northd engine node
+# Associate lb1 to sw0. There should be no recompute of northd engine node
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
-check_engine_stats recompute recompute
+check_engine_stats norecompute recompute
 
-# Disassociate lb1 from sw0. There should be a full recompute of northd engine node.
+# Disassociate lb1 from sw0. There should be a no recompute of northd engine node.
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
-check_engine_stats recompute recompute
+check_engine_stats norecompute recompute
 
 # Test load balancer group now
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats