diff mbox series

[ovs-dev,v8,1/4] northd: Handle load balancer changes for a logical switch.

Message ID 20230912174651.258656-1-numans@ovn.org
State Accepted
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 success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Numan Siddique Sept. 12, 2023, 5:46 p.m. UTC
From: Numan Siddique <numans@ovn.org>

'lb_data' engine node now also handles logical switch changes.
Its data maintains ls to lb related information. i.e if a
logical switch sw0 has lb1, lb2 and lb3 associated then
it stores this info in its data.  And when a new load balancer
lb4 is associated to it, it stores this information in its
tracked data so that 'northd' engine node can handle it
accordingly.  Tracked data will have information like:
  changed ls -> {sw0 : {associated_lbs: [lb4]}

The first handler 'northd_lb_data_handler_pre_od' is called before the
'northd_nb_logical_switch_handler' handler and it just creates or
deletes the lb_datapaths hmap for the tracked lbs.

The northd handler 'northd_lb_data_handler' updates the
ovn_lb_datapaths's 'nb_ls_map' bitmap accordingly.

Eg.  If the lb_data has the below tracked data:

tracked_data = {'crupdated_lbs': [lb1, lb2],
                'deleted_lbs': [lb3],
                'crupdated_lb_groups': [lbg1, lbg2],
                'crupdated_ls_lbs': [{ls: sw0, assoc_lbs: [lb1],
                                     {ls: sw1, assoc_lbs: [lb1, lb2]}

The handler northd_lb_data_handler(), creates the
ovn_lb_datapaths object for lb1 and lb2 and deletes lb3 from
the ovn_lb_datapaths hmap.  It does the same for the created or updated lb
groups lbg1 and lbg2 in the ovn_lbgrp_datapaths map.  It also updates the
nb_ls_bitmap of lb1 for sw0 and sw1 and nb_ls_bitmap of lb2 for sw1.

Reviewed-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 lib/lb.c                 |   5 +-
 northd/en-lb-data.c      | 176 +++++++++++++++++++++++++++++++++++++++
 northd/en-lb-data.h      |  31 +++++++
 northd/en-lflow.c        |   6 ++
 northd/en-northd.c       |   6 +-
 northd/en-sync-from-sb.c |  10 ++-
 northd/en-sync-sb.c      |  18 ++--
 northd/inc-proc-northd.c |   2 +
 northd/northd.c          |  89 +++++++++++++++++---
 northd/northd.h          |   4 +-
 tests/ovn-northd.at      |  56 +++++++++----
 11 files changed, 359 insertions(+), 44 deletions(-)
diff mbox series

Patch

diff --git a/lib/lb.c b/lib/lb.c
index 6fd67e2218..e6c9dc2be2 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -1088,7 +1088,10 @@  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++;
+        }
     }
 }
 
diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
index 8acd9c8cb2..d854042feb 100644
--- a/northd/en-lb-data.c
+++ b/northd/en-lb-data.c
@@ -39,6 +39,14 @@  static void lb_data_destroy(struct ed_type_lb_data *);
 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);
+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 struct ovn_lb_group *create_lb_group(
     const struct nbrec_load_balancer_group *, struct hmap *lbs,
     struct hmap *lb_groups);
@@ -54,6 +62,7 @@  static struct crupdated_lbgrp *
                                            struct tracked_lb_data *);
 static void add_deleted_lbgrp_to_tracked_data(
     struct ovn_lb_group *, struct tracked_lb_data *);
+static bool is_ls_lbs_changed(const struct nbrec_logical_switch *nbs);
 
 /* 'lb_data' engine node manages the NB load balancers and load balancer
  * groups.  For each NB LB, it creates 'struct ovn_northd_lb' and
@@ -80,9 +89,13 @@  en_lb_data_run(struct engine_node *node, void *data)
         EN_OVSDB_GET(engine_get_input("NB_load_balancer", node));
     const struct nbrec_load_balancer_group_table *nb_lbg_table =
         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));
 
     lb_data->tracked = false;
     build_lbs(nb_lb_table, nb_lbg_table, &lb_data->lbs, &lb_data->lbgrps);
+    build_od_lb_map(nb_ls_table, &lb_data->ls_lb_map);
+
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -230,18 +243,98 @@  lb_data_load_balancer_group_handler(struct engine_node *node, void *data)
     return true;
 }
 
+bool
+lb_data_logical_switch_handler(struct engine_node *node, void *data)
+{
+    struct ed_type_lb_data *lb_data = (struct ed_type_lb_data *) data;
+    const struct nbrec_logical_switch_table *nb_ls_table =
+        EN_OVSDB_GET(engine_get_input("NB_logical_switch", node));
+
+    struct tracked_lb_data *trk_lb_data = &lb_data->tracked_lb_data;
+    lb_data->tracked = true;
+
+    bool changed = false;
+    const struct nbrec_logical_switch *nbs;
+    NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (nbs, nb_ls_table) {
+        if (nbrec_logical_switch_is_deleted(nbs)) {
+            struct od_lb_data *od_lb_data =
+                find_od_lb_data(&lb_data->ls_lb_map, &nbs->header_.uuid);
+            if (od_lb_data) {
+                hmap_remove(&lb_data->ls_lb_map, &od_lb_data->hmap_node);
+                hmapx_add(&trk_lb_data->deleted_od_lb_data, od_lb_data);
+            }
+        } else {
+            if (!is_ls_lbs_changed(nbs)) {
+                continue;
+            }
+            struct crupdated_od_lb_data *codlb = xzalloc(sizeof *codlb);
+            codlb->od_uuid = nbs->header_.uuid;
+            uuidset_init(&codlb->assoc_lbs);
+
+            struct od_lb_data *od_lb_data =
+                find_od_lb_data(&lb_data->ls_lb_map, &nbs->header_.uuid);
+            if (!od_lb_data) {
+                od_lb_data = create_od_lb_data(&lb_data->ls_lb_map,
+                                                &nbs->header_.uuid);
+            }
+
+            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);
+
+            ovs_list_insert(&trk_lb_data->crupdated_ls_lbs, &codlb->list_node);
+        }
+    }
+
+    if (changed) {
+        engine_set_node_state(node, EN_UPDATED);
+    }
+    return true;
+}
+
 /* static functions. */
 static void
 lb_data_init(struct ed_type_lb_data *lb_data)
 {
     hmap_init(&lb_data->lbs);
     hmap_init(&lb_data->lbgrps);
+    hmap_init(&lb_data->ls_lb_map);
 
     struct tracked_lb_data *trk_lb_data = &lb_data->tracked_lb_data;
     hmap_init(&trk_lb_data->crupdated_lbs);
     hmapx_init(&trk_lb_data->deleted_lbs);
     hmap_init(&trk_lb_data->crupdated_lbgrps);
     hmapx_init(&trk_lb_data->deleted_lbgrps);
+    ovs_list_init(&trk_lb_data->crupdated_ls_lbs);
+    hmapx_init(&trk_lb_data->deleted_od_lb_data);
 }
 
 static void
@@ -259,11 +352,18 @@  lb_data_destroy(struct ed_type_lb_data *lb_data)
     }
     hmap_destroy(&lb_data->lbgrps);
 
+    struct od_lb_data *od_lb_data;
+    HMAP_FOR_EACH_POP (od_lb_data, hmap_node, &lb_data->ls_lb_map) {
+        destroy_od_lb_data(od_lb_data);
+    }
+    hmap_destroy(&lb_data->ls_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);
     hmapx_destroy(&lb_data->tracked_lb_data.deleted_lbgrps);
     hmap_destroy(&lb_data->tracked_lb_data.crupdated_lbgrps);
+    hmapx_destroy(&lb_data->tracked_lb_data.deleted_od_lb_data);
 }
 
 static void
@@ -301,12 +401,67 @@  create_lb_group(const struct nbrec_load_balancer_group *nbrec_lb_group,
     return 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_switch *nbrec_ls;
+    NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbrec_ls, nbrec_ls_table) {
+        if (!nbrec_ls->n_load_balancer) {
+            continue;
+        }
+
+        struct od_lb_data *od_lb_data =
+            create_od_lb_data(od_lb_map, &nbrec_ls->header_.uuid);
+        for (size_t i = 0; i < nbrec_ls->n_load_balancer; i++) {
+            uuidset_insert(od_lb_data->lbs,
+                           &nbrec_ls->load_balancer[i]->header_.uuid);
+        }
+    }
+}
+
+static struct od_lb_data *
+create_od_lb_data(struct hmap *od_lb_map, const struct uuid *od_uuid)
+{
+    struct od_lb_data *od_lb_data = xzalloc(sizeof *od_lb_data);
+    od_lb_data->od_uuid = *od_uuid;
+    od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs);
+    uuidset_init(od_lb_data->lbs);
+
+    hmap_insert(od_lb_map, &od_lb_data->hmap_node,
+                uuid_hash(&od_lb_data->od_uuid));
+    return od_lb_data;
+}
+
+static struct od_lb_data *
+find_od_lb_data(struct hmap *od_lb_map, const struct uuid *od_uuid)
+{
+    struct od_lb_data *od_lb_data;
+    HMAP_FOR_EACH_WITH_HASH (od_lb_data, hmap_node, uuid_hash(od_uuid),
+                             od_lb_map) {
+        if (uuid_equals(&od_lb_data->od_uuid, od_uuid)) {
+            return od_lb_data;
+        }
+    }
+
+    return NULL;
+}
+
+static void
+destroy_od_lb_data(struct od_lb_data *od_lb_data)
+{
+    uuidset_destroy(od_lb_data->lbs);
+    free(od_lb_data->lbs);
+    free(od_lb_data);
+}
+
 static void
 destroy_tracked_data(struct ed_type_lb_data *lb_data)
 {
     lb_data->tracked = false;
     lb_data->tracked_lb_data.has_health_checks = false;
     lb_data->tracked_lb_data.has_dissassoc_lbs_from_lbgrps = false;
+    lb_data->tracked_lb_data.has_dissassoc_lbs_from_od = false;
 
     struct hmapx_node *node;
     HMAPX_FOR_EACH_SAFE (node, &lb_data->tracked_lb_data.deleted_lbs) {
@@ -331,6 +486,20 @@  destroy_tracked_data(struct ed_type_lb_data *lb_data)
         hmapx_destroy(&crupdated_lbg->assoc_lbs);
         free(crupdated_lbg);
     }
+
+    struct crupdated_od_lb_data *codlb;
+    LIST_FOR_EACH_SAFE (codlb, list_node,
+                        &lb_data->tracked_lb_data.crupdated_ls_lbs) {
+        ovs_list_remove(&codlb->list_node);
+        uuidset_destroy(&codlb->assoc_lbs);
+
+        free(codlb);
+    }
+
+    HMAPX_FOR_EACH_SAFE (node, &lb_data->tracked_lb_data.deleted_od_lb_data) {
+        destroy_od_lb_data(node->data);
+        hmapx_delete(&lb_data->tracked_lb_data.deleted_od_lb_data, node);
+    }
 }
 
 static void
@@ -376,3 +545,10 @@  add_deleted_lbgrp_to_tracked_data(struct ovn_lb_group *lbg,
 {
     hmapx_add(&tracked_lb_data->deleted_lbgrps, lbg);
 }
+
+static bool
+is_ls_lbs_changed(const struct nbrec_logical_switch *nbs) {
+    return ((nbrec_logical_switch_is_new(nbs) && nbs->n_load_balancer)
+            ||  nbrec_logical_switch_is_updated(nbs,
+                        NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER));
+}
diff --git a/northd/en-lb-data.h b/northd/en-lb-data.h
index afb163dd9f..6b2ba11cb8 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/uuidset.h"
 
 #include "lib/inc-proc-eng.h"
 
@@ -27,6 +28,13 @@  struct crupdated_lbgrp {
     struct hmapx assoc_lbs;
 };
 
+struct crupdated_od_lb_data {
+    struct ovs_list list_node;
+
+    struct uuid od_uuid;
+    struct uuidset assoc_lbs;
+};
+
 struct tracked_lb_data {
     /* Both created and updated lbs. hmapx node is 'struct crupdated_lb *'. */
     struct hmap crupdated_lbs;
@@ -41,12 +49,31 @@  struct tracked_lb_data {
     /* Deleted lb_groups. hmapx node is  'struct ovn_lb_group *'. */
     struct hmapx deleted_lbgrps;
 
+    /* List of logical switch <-> lb changes. List node is
+     * 'struct crupdated_od_lb_data' */
+    struct ovs_list crupdated_ls_lbs;
+
+    /* hmapx of deleted logical switches which have load balancer or lb groups
+     * associated with it.  hmapx_node is 'struct od_lb_data'. */
+    struct hmapx deleted_od_lb_data;
+
     /* Indicates if any of the tracked lb has health checks enabled. */
     bool has_health_checks;
 
     /* Indicates if any lb got disassociated from a lb group
      * but not deleted. */
     bool has_dissassoc_lbs_from_lbgrps;
+
+    /* Indicates if a lb was disassociated from a logical switch. */
+    bool has_dissassoc_lbs_from_od;
+};
+
+/* Datapath (logical switch) to lb/lbgrp association data. */
+struct od_lb_data {
+    struct hmap_node hmap_node;
+    struct uuid od_uuid;
+    struct uuidset *lbs;
+    struct uuidset *lbgrps;
 };
 
 /* struct which maintains the data of the engine node lb_data. */
@@ -57,6 +84,9 @@  struct ed_type_lb_data {
     /* hmap of load balancer groups.  hmap node is 'struct ovn_lb_group *' */
     struct hmap lbgrps;
 
+    /* hmap of ls to lb map.  hmap node is 'struct od_lb_data'. */
+    struct hmap ls_lb_map;
+
     /* tracked data*/
     bool tracked;
     struct tracked_lb_data tracked_lb_data;
@@ -69,5 +99,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);
 
 #endif /* end of EN_NORTHD_LB_DATA_H */
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index ad8b62c735..2b84fef0ef 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -107,6 +107,12 @@  lflow_northd_handler(struct engine_node *node,
     if (!northd_data->change_tracked) {
         return false;
     }
+
+    /* Fall back to recompute if lb related data has changed. */
+    if (northd_data->lb_changed) {
+        return false;
+    }
+
     const struct engine_context *eng_ctx = engine_get_context();
     struct lflow_data *lflow_data = data;
 
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 1704a18834..8487b003f7 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -1,4 +1,4 @@ 
-/*
+    /*
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at:
@@ -214,6 +214,10 @@  northd_lb_data_handler(struct engine_node *node, void *data)
         return false;
     }
 
+    /* Indicate the depedendant engine nodes that load balancer/group
+     * related data has changed (including association to logical
+     * switch/router). */
+    nd->lb_changed = true;
     engine_set_node_state(node, EN_UPDATED);
     return true;
 }
diff --git a/northd/en-sync-from-sb.c b/northd/en-sync-from-sb.c
index 4109aebe45..ba293d1a2a 100644
--- a/northd/en-sync-from-sb.c
+++ b/northd/en-sync-from-sb.c
@@ -70,10 +70,12 @@  sync_from_sb_northd_handler(struct engine_node *node,
 {
     struct northd_data *nd = engine_get_input_data("northd", node);
     if (nd->change_tracked) {
-        /* There are only NB LSP related changes and the only field this node
-         * cares about is the "up" column, which is considered write-only to
-         * this node, so it is safe to ignore the change. (The real change
-         * matters to this node is always from the SB DB.) */
+        /* So far the changes tracked in northd don't impact this node.
+         *
+         * In particular, for the LS related changes, the only field this node
+         * cares about is the "up" column of LSP, which is considered
+         * write-only to this node, so it is safe to ignore the change. (The
+         * real change matters to this node is always from the SB DB.) */
         return true;
     }
     return false;
diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index f6dbd7b2b6..37ec631b6e 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -234,14 +234,18 @@  bool
 sync_to_sb_lb_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
 {
     struct northd_data *nd = engine_get_input_data("northd", node);
-    if (nd->change_tracked) {
-        /* There are only NB LSP related changes and these can be safely
-         * ignore and returned true.  However in case the northd engine
-         * tracking data includes other changes, we need to do additional
-         * checks before safely ignoring. */
-        return true;
+
+    if (!nd->change_tracked || nd->lb_changed) {
+        /* Return false if no tracking data or if lbs changed. */
+        return false;
     }
-    return false;
+
+
+    /* There are only NB LSP related changes and these can be safely
+     * ignore and returned true.  However in case the northd engine
+     * tracking data includes other changes, we need to do additional
+     * checks before safely ignoring. */
+    return true;
 }
 
 /* static functions. */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index ccc6687207..303b58d43f 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -155,6 +155,8 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                      lb_data_load_balancer_handler);
     engine_add_input(&en_lb_data, &en_nb_load_balancer_group,
                      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_northd, &en_nb_logical_router, NULL);
     engine_add_input(&en_northd, &en_nb_mirror, NULL);
diff --git a/northd/northd.c b/northd/northd.c
index e9cb906e2a..492d9b4f88 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -73,7 +73,7 @@  static bool install_ls_lb_from_router;
 /* Use common zone for SNAT and DNAT if this option is set to "true". */
 static bool use_common_zone = false;
 
-/* MAC allocated for service monitor usage. Just one mac is allocated
+/* MAC allocated for service monitor usage. Just one mac is allocatedg5534
  * for this purpose and ovn-controller's on each chassis will make use
  * of this mac when sending out the packets to monitor the services
  * defined in Service_Monitor Southbound table. Since these packets
@@ -852,7 +852,6 @@  ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
         free(od->l3dgw_ports);
         destroy_mcast_info_for_datapath(od);
         destroy_ports_for_datapath(od);
-
         free(od);
     }
 }
@@ -4959,6 +4958,7 @@  destroy_northd_data_tracked_changes(struct northd_data *nd)
     }
 
     nd->change_tracked = false;
+    nd->lb_changed = false;
 }
 
 /* Check if a changed LSP can be handled incrementally within the I-P engine
@@ -5074,6 +5074,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
 ls_changes_can_be_handled(
@@ -5084,7 +5085,8 @@  ls_changes_can_be_handled(
     for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) {
         if (nbrec_logical_switch_is_updated(ls, col)) {
             if (col == NBREC_LOGICAL_SWITCH_COL_ACLS ||
-                col == NBREC_LOGICAL_SWITCH_COL_PORTS) {
+                col == NBREC_LOGICAL_SWITCH_COL_PORTS ||
+                col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) {
                 continue;
             }
             return false;
@@ -5109,12 +5111,6 @@  ls_changes_can_be_handled(
             return false;
         }
     }
-    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 false;
-        }
-    }
     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) {
@@ -5365,6 +5361,7 @@  northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
     if (!ovs_list_is_empty(&nd->tracked_ls_changes.updated)) {
         nd->change_tracked = true;
     }
+
     return true;
 
 fail:
@@ -5431,9 +5428,20 @@  northd_handle_sb_port_binding_changes(
     return true;
 }
 
-/* Handler for lb_data engine changes.  For every tracked lb_data
- * it creates or deletes the ovn_lb_datapaths/ovn_lb_group_datapaths
- * from the lb_datapaths hmap and lb_group_datapaths hmap. */
+/* Handler for lb_data engine changes.  It does the following
+ * For every tracked 'lb' and 'lb_group'
+ *  - it creates or deletes the ovn_lb_datapaths/ovn_lb_group_datapaths
+ *    from the lb_datapaths hmap and lb_group_datapaths hmap.
+ *
+ *  - For any changes to a logical switch (in 'trk_lb_data->crupdated_ls_lbs')
+ *    due to association of a load balancer (eg. ovn-nbctl ls-lb-add sw0 lb1),
+ *    the logical switch datapath is added to the load balancer (represented
+ *    by 'struct ovn_lb_datapaths') by calling ovn_lb_datapaths_add_ls().
+ *
+ *  - For every 'lb' in the tracked data (trk_lb_data->crupdated_lbs) ,
+ *    it gets the associated logical switches and for each switch it
+ *    re-evaluates 'od->has_lb_vip' to reflect any changes to the lb vips.
+ * */
 bool
 northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
                               struct ovn_datapaths *ls_datapaths,
@@ -5459,8 +5467,21 @@  northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
         return false;
     }
 
+    /* Fall back to recompute if any load balancer has been disassociated from
+     * a logical switch or router. */
+    if (trk_lb_data->has_dissassoc_lbs_from_od) {
+        return false;
+    }
+
+    /* Fall back to recompute if any logical switch or router was deleted which
+     * had load balancer or lb group association. */
+    if (!hmapx_is_empty(&trk_lb_data->deleted_od_lb_data)) {
+        return false;
+    }
+
     struct ovn_lb_datapaths *lb_dps;
     struct ovn_northd_lb *lb;
+    struct ovn_datapath *od;
     struct hmapx_node *hmapx_node;
     HMAPX_FOR_EACH (hmapx_node, &trk_lb_data->deleted_lbs) {
         lb = hmapx_node->data;
@@ -5468,10 +5489,22 @@  northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
 
         lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
         ovs_assert(lb_dps);
+
+        /* Re-evaluate 'od->has_lb_vip for od's associated with the
+         * deleted lb. */
+        size_t index;
+        BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths),
+                           lb_dps->nb_ls_map) {
+            od = ls_datapaths->array[index];
+            init_lb_for_datapath(od);
+        }
+
         hmap_remove(lb_datapaths_map, &lb_dps->hmap_node);
         ovn_lb_datapaths_destroy(lb_dps);
     }
 
+    /* Create the 'lb_dps' if not already created for each
+     * 'lb' in the trk_lb_data->crupdated_lbs. */
     struct crupdated_lb *clb;
     HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) {
         lb = clb->lb;
@@ -5504,6 +5537,37 @@  northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
         }
     }
 
+    struct crupdated_od_lb_data *codlb;
+    LIST_FOR_EACH (codlb, list_node, &trk_lb_data->crupdated_ls_lbs) {
+        od = ovn_datapath_find(&ls_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_ls(lb_dps, 1, &od);
+        }
+
+        /* Re-evaluate 'od->has_lb_vip' */
+        init_lb_for_datapath(od);
+    }
+
+    HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) {
+        lb = clb->lb;
+        const struct uuid *lb_uuid = &lb->nlb->header_.uuid;
+
+        lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
+        ovs_assert(lb_dps);
+        size_t index;
+        BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths),
+                           lb_dps->nb_ls_map) {
+            od = ls_datapaths->array[index];
+            /* Re-evaluate 'od->has_lb_vip' */
+            init_lb_for_datapath(od);
+        }
+    }
+
     return true;
 }
 
@@ -16529,6 +16593,7 @@  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) {
         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 2bb6910945..0d206a4e52 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -115,6 +115,8 @@  struct northd_data {
     struct hmap svc_monitor_map;
     bool change_tracked;
     struct tracked_ls_changes tracked_ls_changes;
+    bool lb_changed; /* Indicates if load balancers changed or association of
+                      * load balancer to logical switch/router changed. */
 };
 
 struct lflow_data {
@@ -345,7 +347,7 @@  bool northd_handle_lb_data_changes(struct tracked_lb_data *,
                                    struct ovn_datapaths *ls_datapaths,
                                    struct ovn_datapaths *lr_datapaths,
                                    struct hmap *lb_datapaths_map,
-                                   struct hmap *lb_group_datapaths_map);
+                                   struct hmap *lbgrp_datapaths_map);
 
 void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
                      const struct nbrec_bfd_table *,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 5c9da811fa..f0f8d1f503 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10398,22 +10398,23 @@  ovn-nbctl lsp-set-type sw0-lr0 router
 ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0
-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
 
-# 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 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
 
 # Modify the backend of the lb1 vip
 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
 
@@ -10421,7 +10422,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
 
@@ -10429,7 +10430,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
 
@@ -10437,14 +10438,33 @@  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
 
 # Disassociate lb1 from sw0. There should be a full 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 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
+
+# Associate lb1 to sw0 and also create a port sw0p1.  This should not result in
+# full recompute of northd, but should rsult in full recompute of lflow node.
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 -- lsp-add sw0 sw0p1
+check_engine_stats lb_data norecompute compute
+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
+
+# Disassociate lb1 from sw0. There should be a 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 lb_data norecompute compute
 check_engine_stats northd recompute nocompute
 check_engine_stats lflow recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -10532,7 +10552,7 @@  CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl add logical_switch sw0 load_balancer_group $lbg1_uuid
-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
 
@@ -10577,7 +10597,7 @@  CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl clear logical_switch sw0 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
 
@@ -10629,7 +10649,7 @@  check_engine_stats lflow recompute nocompute
 # Add back lb group to logical switch and then delete it.
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl add logical_switch sw0 load_balancer_group $lbg1_uuid
-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
 
@@ -10670,7 +10690,7 @@  check_engine_stats lflow recompute nocompute
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl set logical_switch sw0 load_balancer_group=$lbg1_uuid
-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
@@ -10684,15 +10704,15 @@  CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw0 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
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw0 lb3
-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
 
@@ -10706,7 +10726,7 @@  CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-del sw0 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