From: Numan Siddique <num...@ovn.org>
For every a given load balancer group 'A', northd engine data maintains
a bitmap of datapaths associated to this lb group. So when lb group 'A'
gets associated to a logical switch 's1', the bitmap index of 's1' is set
in its bitmap.
In order to handle the load balancer group changes incrementally for a
logical switch, we need to set and clear the bitmap bits accordingly.
And this patch does it.
Signed-off-by: Numan Siddique <num...@ovn.org>
---
lib/lb.c | 45 +++++++++++++++---
lib/lb.h | 33 ++++++--------
northd/northd.c | 109 ++++++++++++++++++++++++++++++++++++++------
tests/ovn-northd.at | 6 +--
4 files changed, 150 insertions(+), 43 deletions(-)
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 bf02190f7c..20a58afa6b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4102,7 +4102,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;
@@ -4120,6 +4121,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);
+ }
}
}
@@ -4159,7 +4169,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) {
@@ -4219,10 +4229,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);
+ }
}
}
}
@@ -4401,8 +4413,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++) {
@@ -5072,7 +5085,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;
@@ -5103,12 +5117,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) {
@@ -5338,6 +5346,68 @@ ls_check_and_handle_lb_changes(const struct
nbrec_logical_switch *changed_ls,
return true;
}
+static bool
+ls_check_and_handle_lb_group_changes(
+ const struct nbrec_logical_switch *changed_ls,
+ struct hmap *lb_group_datapaths_map,
+ struct hmap *lb_datapaths_map,
+ struct ovn_datapath *od,
+ struct ls_change *ls_change,
+ bool *updated)
+{
+ bool lbg_modified = false;
+ /* Check if lb_groups changed or not */
+ if (!nbrec_logical_switch_is_updated(changed_ls,
+ NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP)) {
+ /* load_balancer_group column of logical switch hasn't changed, but
+ * its possible that a load balancer group may have changed (like
+ * a load balancer added or removed from the group). */
+ for (size_t i = 0; i < changed_ls->n_load_balancer_group; i++) {
+ if (nbrec_load_balancer_group_row_get_seqno(
+ changed_ls->load_balancer_group[i],
+ OVSDB_IDL_CHANGE_MODIFY) > 0) {
+ lbg_modified = true;
+ break;
+ }
+ }
+ } else {
+ /* load_balancer_group column of logical switch has changed. */
+ lbg_modified = true;
+ }
+
+ if (!lbg_modified) {
+ /* Nothing changed */
+ return true;
+ }
+
+ /* Disassociate load balancer group (and its load balancers) from
+ * the datapath and rebuild the association again. */
+ 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
@@ -5396,6 +5466,15 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
goto fail;
}
+ if (!ls_check_and_handle_lb_group_changes(changed_ls,
+ &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 d9f3917a3e..d3a5851e35 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9783,16 +9783,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
--
2.40.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
