Skip to content

Commit

Permalink
northd: Handle load balancer group changes for a logical switch.
Browse files Browse the repository at this point in the history 
For 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.

Reviewed-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
@numansiddique
numansiddique committed Sep 14, 2023
1 parent 505ceec commit cc27dcc
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 44 deletions.
102 changes: 75 additions & 27 deletions northd/en-lb-data.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ static struct crupdated_lbgrp *
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);
static bool is_ls_lbgrps_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
Expand Down Expand Up @@ -264,12 +265,15 @@ lb_data_logical_switch_handler(struct engine_node *node, void *data)
hmapx_add(&trk_lb_data->deleted_od_lb_data, od_lb_data);
}
} else {
if (!is_ls_lbs_changed(nbs)) {
bool ls_lbs_changed = is_ls_lbs_changed(nbs);
bool ls_lbgrps_changed = is_ls_lbgrps_changed(nbs);
if (!ls_lbs_changed && !ls_lbgrps_changed) {
continue;
}
struct crupdated_od_lb_data *codlb = xzalloc(sizeof *codlb);
codlb->od_uuid = nbs->header_.uuid;
uuidset_init(&codlb->assoc_lbs);
uuidset_init(&codlb->assoc_lbgrps);

struct od_lb_data *od_lb_data =
find_od_lb_data(&lb_data->ls_lb_map, &nbs->header_.uuid);
Expand All @@ -278,38 +282,66 @@ lb_data_logical_switch_handler(struct engine_node *node, void *data)
&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);
if (ls_lbs_changed) {
struct uuidset *pre_lb_uuids = od_lb_data->lbs;
od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs);
uuidset_init(od_lb_data->lbs);

for (size_t i = 0; i < nbs->n_load_balancer; i++) {
const struct uuid *lb_uuid =
&nbs->load_balancer[i]->header_.uuid;
uuidset_insert(od_lb_data->lbs, lb_uuid);

struct uuidset_node *unode = uuidset_find(pre_lb_uuids,
lb_uuid);

if (!unode || (nbrec_load_balancer_row_get_seqno(
nbs->load_balancer[i],
OVSDB_IDL_CHANGE_MODIFY) > 0)) {
/* Add this lb to the tracked data. */
uuidset_insert(&codlb->assoc_lbs, lb_uuid);
changed = true;
}

if (unode) {
uuidset_delete(pre_lb_uuids, unode);
}
}
if (!uuidset_is_empty(pre_lb_uuids)) {
trk_lb_data->has_dissassoc_lbs_from_od = true;
changed = true;
}

struct uuidset_node *unode = uuidset_find(pre_lb_uuids,
lb_uuid);
uuidset_destroy(pre_lb_uuids);
free(pre_lb_uuids);
}

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 (ls_lbgrps_changed) {
struct uuidset *pre_lbgrp_uuids = od_lb_data->lbgrps;
od_lb_data->lbgrps = xzalloc(sizeof *od_lb_data->lbgrps);
uuidset_init(od_lb_data->lbgrps);
for (size_t i = 0; i < nbs->n_load_balancer_group; i++) {
const struct uuid *lbg_uuid =
&nbs->load_balancer_group[i]->header_.uuid;
uuidset_insert(od_lb_data->lbgrps, lbg_uuid);

if (!uuidset_find_and_delete(pre_lbgrp_uuids,
lbg_uuid)) {
/* Add this lb group to the tracked data. */
uuidset_insert(&codlb->assoc_lbgrps, lbg_uuid);
changed = true;
}
}

if (unode) {
uuidset_delete(pre_lb_uuids, unode);
if (!uuidset_is_empty(pre_lbgrp_uuids)) {
trk_lb_data->has_dissassoc_lbgrps_from_od = true;
changed = true;
}
}

if (!uuidset_is_empty(pre_lb_uuids)) {
trk_lb_data->has_dissassoc_lbs_from_od = true;
changed = true;
uuidset_destroy(pre_lbgrp_uuids);
free(pre_lbgrp_uuids);
}

uuidset_destroy(pre_lb_uuids);
free(pre_lb_uuids);

ovs_list_insert(&trk_lb_data->crupdated_ls_lbs, &codlb->list_node);
}
}
Expand Down Expand Up @@ -407,7 +439,7 @@ build_od_lb_map(const struct nbrec_logical_switch_table *nbrec_ls_table,
{
const struct nbrec_logical_switch *nbrec_ls;
NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbrec_ls, nbrec_ls_table) {
if (!nbrec_ls->n_load_balancer) {
if (!nbrec_ls->n_load_balancer && !nbrec_ls->n_load_balancer_group) {
continue;
}

Expand All @@ -417,6 +449,10 @@ build_od_lb_map(const struct nbrec_logical_switch_table *nbrec_ls_table,
uuidset_insert(od_lb_data->lbs,
&nbrec_ls->load_balancer[i]->header_.uuid);
}
for (size_t i = 0; i < nbrec_ls->n_load_balancer_group; i++) {
uuidset_insert(od_lb_data->lbgrps,
&nbrec_ls->load_balancer_group[i]->header_.uuid);
}
}
}

Expand All @@ -426,7 +462,9 @@ 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);
od_lb_data->lbgrps = xzalloc(sizeof *od_lb_data->lbgrps);
uuidset_init(od_lb_data->lbs);
uuidset_init(od_lb_data->lbgrps);

hmap_insert(od_lb_map, &od_lb_data->hmap_node,
uuid_hash(&od_lb_data->od_uuid));
Expand All @@ -451,7 +489,9 @@ static void
destroy_od_lb_data(struct od_lb_data *od_lb_data)
{
uuidset_destroy(od_lb_data->lbs);
uuidset_destroy(od_lb_data->lbgrps);
free(od_lb_data->lbs);
free(od_lb_data->lbgrps);
free(od_lb_data);
}

Expand All @@ -462,6 +502,7 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
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;
lb_data->tracked_lb_data.has_dissassoc_lbgrps_from_od = false;

struct hmapx_node *node;
HMAPX_FOR_EACH_SAFE (node, &lb_data->tracked_lb_data.deleted_lbs) {
Expand Down Expand Up @@ -492,7 +533,7 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
&lb_data->tracked_lb_data.crupdated_ls_lbs) {
ovs_list_remove(&codlb->list_node);
uuidset_destroy(&codlb->assoc_lbs);

uuidset_destroy(&codlb->assoc_lbgrps);
free(codlb);
}

Expand Down Expand Up @@ -552,3 +593,10 @@ is_ls_lbs_changed(const struct nbrec_logical_switch *nbs) {
|| nbrec_logical_switch_is_updated(nbs,
NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER));
}

static bool
is_ls_lbgrps_changed(const struct nbrec_logical_switch *nbs) {
return ((nbrec_logical_switch_is_new(nbs) && nbs->n_load_balancer_group)
|| nbrec_logical_switch_is_updated(nbs,
NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP));
}
4 changes: 4 additions & 0 deletions northd/en-lb-data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct crupdated_od_lb_data {

struct uuid od_uuid;
struct uuidset assoc_lbs;
struct uuidset assoc_lbgrps;
};

struct tracked_lb_data {
Expand Down Expand Up @@ -66,6 +67,9 @@ struct tracked_lb_data {

/* Indicates if a lb was disassociated from a logical switch. */
bool has_dissassoc_lbs_from_od;

/* Indicates if a lb group was disassociated from a logical switch. */
bool has_dissassoc_lbgrps_from_od;
};

/* Datapath (logical switch) to lb/lbgrp association data. */
Expand Down
63 changes: 55 additions & 8 deletions northd/northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5075,6 +5075,7 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports,
* Presently supports i-p for the below changes:
* - logical switch ports.
* - load balancers.
* - load balancer groups.
*/
static bool
ls_changes_can_be_handled(
Expand All @@ -5086,7 +5087,8 @@ ls_changes_can_be_handled(
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_LOAD_BALANCER) {
col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER ||
col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER_GROUP) {
continue;
}
return false;
Expand All @@ -5111,12 +5113,6 @@ ls_changes_can_be_handled(
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) {
return false;
}
}
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) {
Expand Down Expand Up @@ -5304,7 +5300,11 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
/* Return true if changes are handled incrementally, false otherwise.
* When there are any changes, try to track what's exactly changed and set
* northd_data->change_tracked accordingly: change tracked - true, otherwise,
* false. */
* false.
*
* Note: Changes to load balancer and load balancer groups associated with
* the logical switches are handled separately in the lb_data change handlers.
* */
bool
northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct northd_input *ni,
Expand Down Expand Up @@ -5479,6 +5479,12 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
return false;
}

/* Fall back to recompute if any load balancer group has been disassociated
* from a logical switch or router. */
if (trk_lb_data->has_dissassoc_lbgrps_from_od) {
return false;
}

struct ovn_lb_datapaths *lb_dps;
struct ovn_northd_lb *lb;
struct ovn_datapath *od;
Expand Down Expand Up @@ -5549,6 +5555,22 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
ovn_lb_datapaths_add_ls(lb_dps, 1, &od);
}

UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbgrps) {
lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map,
&uuidnode->uuid);
ovs_assert(lbgrp_dps);
ovn_lb_group_datapaths_add_ls(lbgrp_dps, 1, &od);

/* Associate all the lbs of the lbgrp to the datapath 'od' */
for (size_t j = 0; j < lbgrp_dps->lb_group->n_lbs; j++) {
const struct uuid *lb_uuid
= &lbgrp_dps->lb_group->lbs[j]->nlb->header_.uuid;
lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
ovs_assert(lb_dps);
ovn_lb_datapaths_add_ls(lb_dps, 1, &od);
}
}

/* Re-evaluate 'od->has_lb_vip' */
init_lb_for_datapath(od);
}
Expand All @@ -5568,6 +5590,31 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data,
}
}

HMAP_FOR_EACH (crupdated_lbgrp, hmap_node,
&trk_lb_data->crupdated_lbgrps) {
lbgrp = crupdated_lbgrp->lbgrp;
const struct uuid *lb_uuid = &lbgrp->uuid;

lbgrp_dps = ovn_lb_group_datapaths_find(lbgrp_datapaths_map,
lb_uuid);
ovs_assert(lbgrp_dps);

struct hmapx_node *hnode;
HMAPX_FOR_EACH (hnode, &crupdated_lbgrp->assoc_lbs) {
lb = hnode->data;
lb_uuid = &lb->nlb->header_.uuid;
lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid);
ovs_assert(lb_dps);
for (size_t i = 0; i < lbgrp_dps->n_ls; i++) {
od = lbgrp_dps->ls[i];
ovn_lb_datapaths_add_ls(lb_dps, 1, &od);

/* Re-evaluate 'od->has_lb_vip' */
init_lb_for_datapath(od);
}
}
}

return true;
}

Expand Down

0 comments on commit cc27dcc

Please sign in to comment.