diff mbox series

[ovs-dev,v1,6/8] northd: Handle load balancer group changes for a logical switch.

Message ID 20230624064843.12161-1-numans@ovn.org
State Superseded
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-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test fail github build: failed

Commit Message

Numan Siddique June 24, 2023, 6:48 a.m. UTC
From: Numan Siddique <numans@ovn.org>

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

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 lib/lb.c            | 45 +++++++++++++++++++++----
 lib/lb.h            | 33 +++++++++----------
 northd/northd.c     | 80 ++++++++++++++++++++++++++++++++++++---------
 tests/ovn-northd.at |  6 ++--
 4 files changed, 121 insertions(+), 43 deletions(-)
diff mbox series

Patch

diff --git a/lib/lb.c b/lib/lb.c
index b2b6473c1d..a0426cc42e 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -1100,7 +1100,7 @@  ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
 
 void
 ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
-                        struct ovn_datapath **ods)
+                           struct ovn_datapath **ods)
 {
     for (size_t i = 0; i < n; i++) {
         if (bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
@@ -1112,14 +1112,14 @@  ovn_lb_datapaths_remove_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
 
 struct ovn_lb_group_datapaths *
 ovn_lb_group_datapaths_create(const struct ovn_lb_group *lb_group,
-                              size_t max_ls_datapaths,
-                              size_t max_lr_datapaths)
+                              size_t n_ls_datapaths,
+                              size_t n_lr_datapaths)
 {
     struct ovn_lb_group_datapaths *lb_group_dps =
         xzalloc(sizeof *lb_group_dps);
     lb_group_dps->lb_group = lb_group;
-    lb_group_dps->ls = xmalloc(max_ls_datapaths * sizeof *lb_group_dps->ls);
-    lb_group_dps->lr = xmalloc(max_lr_datapaths * sizeof *lb_group_dps->lr);
+    lb_group_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
+    lb_group_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
 
     return lb_group_dps;
 }
@@ -1127,8 +1127,8 @@  ovn_lb_group_datapaths_create(const struct ovn_lb_group *lb_group,
 void
 ovn_lb_group_datapaths_destroy(struct ovn_lb_group_datapaths *lb_group_dps)
 {
-    free(lb_group_dps->ls);
-    free(lb_group_dps->lr);
+    bitmap_free(lb_group_dps->nb_lr_map);
+    bitmap_free(lb_group_dps->nb_ls_map);
     free(lb_group_dps);
 }
 
@@ -1146,3 +1146,34 @@  ovn_lb_group_datapaths_find(const struct hmap *lb_group_dps_map,
     }
     return NULL;
 }
+
+void
+ovn_lb_group_datapaths_add_ls(struct ovn_lb_group_datapaths *lbg_dps, size_t n,
+                              struct ovn_datapath **ods)
+{
+    for (size_t i = 0; i < n; i++) {
+        if (!bitmap_is_set(lbg_dps->nb_ls_map, ods[i]->index)) {
+            bitmap_set1(lbg_dps->nb_ls_map, ods[i]->index);
+            lbg_dps->n_nb_ls++;
+        }
+    }
+}
+
+void
+ovn_lb_group_datapaths_remove_ls(struct ovn_lb_group_datapaths *lbg_dps,
+                                 size_t n, struct ovn_datapath **ods)
+{
+    for (size_t i = 0; i < n; i++) {
+        if (bitmap_is_set(lbg_dps->nb_ls_map, ods[i]->index)) {
+            bitmap_set0(lbg_dps->nb_ls_map, ods[i]->index);
+            lbg_dps->n_nb_ls--;
+        }
+    }
+}
+
+void
+ovn_lb_group_datapaths_add_lr(struct ovn_lb_group_datapaths *lbg_dps,
+                              struct ovn_datapath *lr)
+{
+    bitmap_set1(lbg_dps->nb_lr_map, lr->index);
+}
diff --git a/lib/lb.h b/lib/lb.h
index ac33333a7f..08723e8ef3 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -19,6 +19,7 @@ 
 
 #include <sys/types.h>
 #include <netinet/in.h>
+#include "lib/bitmap.h"
 #include "lib/smap.h"
 #include "openvswitch/hmap.h"
 #include "ovn-util.h"
@@ -179,6 +180,9 @@  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_set_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 **);
 
@@ -188,10 +192,11 @@  struct ovn_lb_group_datapaths {
     const struct ovn_lb_group *lb_group;
 
     /* Datapaths to which 'lb_group' is applied. */
-    size_t n_ls;
-    struct ovn_datapath **ls;
-    size_t n_lr;
-    struct ovn_datapath **lr;
+    size_t n_nb_ls;
+    unsigned long *nb_ls_map;
+
+    size_t n_nb_lr;
+    unsigned long *nb_lr_map;
 };
 
 struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_create(
@@ -202,21 +207,13 @@  void ovn_lb_group_datapaths_destroy(struct ovn_lb_group_datapaths *);
 struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_find(
     const struct hmap *lb_group_dps, const struct uuid *);
 
-static inline void
-ovn_lb_group_datapaths_add_ls(struct ovn_lb_group_datapaths *lbg_dps, size_t n,
-                               struct ovn_datapath **ods)
-{
-    memcpy(&lbg_dps->ls[lbg_dps->n_ls], ods, n * sizeof *ods);
-    lbg_dps->n_ls += n;
-}
-
-static inline void
-ovn_lb_group_datapaths_add_lr(struct ovn_lb_group_datapaths *lbg_dps,
-                               struct ovn_datapath *lr)
-{
-    lbg_dps->lr[lbg_dps->n_lr++] = lr;
-}
+void ovn_lb_group_datapaths_add_ls(struct ovn_lb_group_datapaths *, size_t n,
+                                   struct ovn_datapath **);
+void ovn_lb_group_datapaths_remove_ls(struct ovn_lb_group_datapaths *,
+                                      size_t n, struct ovn_datapath **);
 
+void ovn_lb_group_datapaths_add_lr(struct ovn_lb_group_datapaths *,
+                                   struct ovn_datapath *lr);
 struct ovn_controller_lb {
     struct hmap_node hmap_node;
 
diff --git a/northd/northd.c b/northd/northd.c
index 8309ca52e4..3183057bd9 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4058,7 +4058,8 @@  associate_ls_lbs(struct ovn_datapath *ls_od, struct hmap *lb_datapaths_map)
 
 static void
 associate_ls_lb_groups(struct ovn_datapath *ls_od,
-                       struct hmap *lb_group_datapaths_map)
+                       struct hmap *lb_group_datapaths_map,
+                       struct hmap *lb_datapaths_map)
 {
     const struct nbrec_load_balancer_group *nbrec_lb_group;
     struct ovn_lb_group_datapaths *lb_group_dps;
@@ -4076,6 +4077,15 @@  associate_ls_lb_groups(struct ovn_datapath *ls_od,
         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;
+
+        struct ovn_lb_datapaths *lb_dps;
+        for (size_t j = 0; j < lb_group_dps->lb_group->n_lbs; j++) {
+            const struct uuid *lb_uuid =
+                &lb_group_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_ls(lb_dps, 1, &ls_od);
+        }
     }
 }
 
@@ -4115,7 +4125,7 @@  build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups,
             continue;
         }
         associate_ls_lbs(od, lb_datapaths_map);
-        associate_ls_lb_groups(od, lb_group_datapaths_map);
+        associate_ls_lb_groups(od, lb_group_datapaths_map, lb_datapaths_map);
     }
 
     HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
@@ -4175,10 +4185,12 @@  build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups,
                 &lb_group_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_ls(lb_dps, lb_group_dps->n_ls,
-                                    lb_group_dps->ls);
-            ovn_lb_datapaths_add_lr(lb_dps, lb_group_dps->n_lr,
-                                    lb_group_dps->lr);
+            size_t index;
+            BITMAP_FOR_EACH_1 (index, ods_size(lr_datapaths),
+                               lb_group_dps->nb_lr_map) {
+                od = lr_datapaths->array[index];
+                ovn_lb_datapaths_add_lr(lb_dps, 1, &od);
+            }
         }
     }
 }
@@ -4357,8 +4369,9 @@  build_lswitch_lbs_from_lrouter(struct ovn_datapaths *lr_datapaths,
 
     struct ovn_lb_group_datapaths *lb_group_dps;
     HMAP_FOR_EACH (lb_group_dps, hmap_node, lb_group_dps_map) {
-        for (size_t i = 0; i < lb_group_dps->n_lr; i++) {
-            struct ovn_datapath *od = lb_group_dps->lr[i];
+        BITMAP_FOR_EACH_1 (index, ods_size(lr_datapaths),
+                           lb_group_dps->nb_lr_map) {
+            struct ovn_datapath *od = lr_datapaths->array[index];
             ovn_lb_group_datapaths_add_ls(lb_group_dps, od->n_ls_peers,
                                           od->ls_peers);
             for (size_t j = 0; j < lb_group_dps->lb_group->n_lbs; j++) {
@@ -5024,7 +5037,8 @@  check_unsupported_inc_proc_for_ls_changes(
     for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) {
         if (nbrec_logical_switch_is_updated(ls, col)) {
             if (col == NBREC_LOGICAL_SWITCH_COL_PORTS ||
-                col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) {
+                col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER ||
+                col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP) {
                 continue;
             }
             return true;
@@ -5055,12 +5069,6 @@  check_unsupported_inc_proc_for_ls_changes(
             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) {
-            return true;
-        }
-    }
     for (size_t i = 0; i < ls->n_qos_rules; i++) {
         if (nbrec_qos_row_get_seqno(ls->qos_rules[i],
                                 OVSDB_IDL_CHANGE_MODIFY) > 0) {
@@ -5282,6 +5290,40 @@  ls_check_and_handle_lb_changes(const struct nbrec_logical_switch *changed_ls,
     return true;
 }
 
+static bool
+ls_check_and_handle_lb_group_changes(
+    struct hmap *lb_group_datapaths_map,
+    struct hmap *lb_datapaths_map,
+    struct ovn_datapath *od,
+    struct ls_change *ls_change,
+    bool *updated)
+{
+    struct ovn_lb_group_datapaths *lbg_dps;
+    for (size_t i = 0; i < od->n_lb_group_uuids; i++) {
+        lbg_dps = ovn_lb_group_datapaths_find(lb_group_datapaths_map,
+                                              &od->lb_group_uuids[i]);
+        if (lbg_dps) {
+            ovn_lb_group_datapaths_remove_ls(lbg_dps, 1, &od);
+
+            struct ovn_lb_datapaths *lb_dps;
+            for (size_t j = 0; j < lbg_dps->lb_group->n_lbs; j++) {
+                const struct uuid *lb_uuid =
+                    &lbg_dps->lb_group->lbs[j]->nlb->header_.uuid;
+                lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
+                if (lb_dps) {
+                    ovn_lb_datapaths_remove_ls(lb_dps, 1, &od);
+                }
+            }
+        }
+    }
+
+    associate_ls_lb_groups(od, lb_group_datapaths_map, 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
@@ -5341,6 +5383,14 @@  northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
             goto fail;
         }
 
+        if (!ls_check_and_handle_lb_group_changes(&nd->lb_group_datapaths_map,
+                                                  &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);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index f133a89263..da9bc16fd9 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9710,16 +9710,16 @@  check_engine_stats norecompute 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 recompute recompute
+check_engine_stats norecompute recompute
 
 # Update lb and this should result in recompute
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb set load_balancer . options:bar=foo
-check_engine_stats recompute recompute
+check_engine_stats norecompute 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 recompute recompute
+check_engine_stats norecompute recompute
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl add logical_router lr0 load_balancer_group $lbg1_uuid