Skip to content

Commit

Permalink
northd: Use the same UUID for SB representation of Load_Balancer.
Browse files Browse the repository at this point in the history 
The NB Load_Balancer has exactly one corresponding row in SB database.
Use the NB UUID for the SB representation which makes the processing easier
and the link between the rows is obvious at first glance.

Signed-off-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Acked-by: Numan Siddique <numans@ovn.org>
---
v2: Rebase on top of current main.
    Add ack from Numan and Mark.
  • Loading branch information
@almusil
almusil committed Nov 14, 2024
1 parent daf1680 commit 704fd10
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 73 deletions.
80 changes: 11 additions & 69 deletions northd/en-sync-sb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ struct sb_lb_record {
const struct sbrec_load_balancer *sbrec_lb;
struct ovn_dp_group *ls_dpg;
struct ovn_dp_group *lr_dpg;
struct uuid sb_uuid;
};

struct sb_lb_table {
Expand Down Expand Up @@ -268,7 +267,6 @@ static bool sync_changed_lbs(struct sb_lb_table *,
struct ovn_datapaths *ls_datapaths,
struct ovn_datapaths *lr_datapaths,
struct chassis_features *);
static bool check_sb_lb_duplicates(const struct sbrec_load_balancer_table *);

void *
en_sync_to_sb_lb_init(struct engine_node *node OVS_UNUSED,
Expand Down Expand Up @@ -346,23 +344,6 @@ sync_to_sb_lb_northd_handler(struct engine_node *node, void *data_)
return true;
}

bool
sync_to_sb_lb_sb_load_balancer(struct engine_node *node, void *data OVS_UNUSED)
{
const struct sbrec_load_balancer_table *sb_load_balancer_table =
EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));

/* The only reason to handle SB.Load_Balancer updates is to detect
* spurious records being created in clustered databases due to
* lack of indexing on the SB.Load_Balancer table. All other changes
* are valid and performed by northd, the only write-client for
* this table. */
if (check_sb_lb_duplicates(sb_load_balancer_table)) {
return false;
}
return true;
}

/* sync_to_sb_pb engine node functions.
* This engine node syncs the SB Port Bindings (partly).
* en_northd engine create the SB Port binding rows and
Expand Down Expand Up @@ -690,14 +671,7 @@ sb_lb_table_build_and_sync(
const struct sbrec_load_balancer *sbrec_lb;
SBREC_LOAD_BALANCER_TABLE_FOR_EACH_SAFE (sbrec_lb,
sb_lb_table) {
const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
struct uuid lb_uuid;
if (!nb_lb_uuid || !uuid_from_string(&lb_uuid, nb_lb_uuid)) {
sbrec_load_balancer_delete(sbrec_lb);
continue;
}

sb_lb = sb_lb_table_find(&tmp_sb_lbs, &lb_uuid);
sb_lb = sb_lb_table_find(&tmp_sb_lbs, &sbrec_lb->header_.uuid);
if (sb_lb) {
sb_lb->sbrec_lb = sbrec_lb;
bool success = sync_sb_lb_record(sb_lb, sbrec_lb, sb_dpgrp_table,
Expand Down Expand Up @@ -751,17 +725,9 @@ sync_sb_lb_record(struct sb_lb_record *sb_lb,
pre_sync_lr_dpg = sb_lb->lr_dpg;

if (!sbrec_lb) {
sb_lb->sb_uuid = uuid_random();
sbrec_lb = sbrec_load_balancer_insert_persist_uuid(ovnsb_txn,
&sb_lb->sb_uuid);
char *lb_id = xasprintf(
UUID_FMT, UUID_ARGS(&lb_dps->lb->nlb->header_.uuid));
const struct smap external_ids =
SMAP_CONST1(&external_ids, "lb_id", lb_id);
sbrec_load_balancer_set_external_ids(sbrec_lb, &external_ids);
free(lb_id);
const struct uuid *nb_uuid = &lb_dps->lb->nlb->header_.uuid;
sbrec_lb = sbrec_load_balancer_insert_persist_uuid(ovnsb_txn, nb_uuid);
} else {
sb_lb->sb_uuid = sbrec_lb->header_.uuid;
sbrec_ls_dp_group =
chassis_features->ls_dpg_column
? sbrec_lb->ls_datapath_group
Expand Down Expand Up @@ -923,13 +889,11 @@ sync_changed_lbs(struct sb_lb_table *sb_lbs,

HMAPX_FOR_EACH (hmapx_node, &trk_lbs->deleted) {
lb_dps = hmapx_node->data;

sb_lb = sb_lb_table_find(&sb_lbs->entries,
&lb_dps->lb->nlb->header_.uuid);
const struct uuid *nb_uuid = &lb_dps->lb->nlb->header_.uuid;
sb_lb = sb_lb_table_find(&sb_lbs->entries, nb_uuid);
if (sb_lb) {
const struct sbrec_load_balancer *sbrec_lb =
sbrec_load_balancer_table_get_for_uuid(sb_lb_table,
&sb_lb->sb_uuid);
sbrec_load_balancer_table_get_for_uuid(sb_lb_table, nb_uuid);
if (sbrec_lb) {
sbrec_load_balancer_delete(sbrec_lb);
}
Expand All @@ -941,9 +905,9 @@ sync_changed_lbs(struct sb_lb_table *sb_lbs,

HMAPX_FOR_EACH (hmapx_node, &trk_lbs->crupdated) {
lb_dps = hmapx_node->data;
const struct uuid *nb_uuid = &lb_dps->lb->nlb->header_.uuid;

sb_lb = sb_lb_table_find(&sb_lbs->entries,
&lb_dps->lb->nlb->header_.uuid);
sb_lb = sb_lb_table_find(&sb_lbs->entries, nb_uuid);

if (!sb_lb && !lb_dps->n_nb_ls && !lb_dps->n_nb_lr) {
continue;
Expand All @@ -953,17 +917,15 @@ sync_changed_lbs(struct sb_lb_table *sb_lbs,
sb_lb = xzalloc(sizeof *sb_lb);
sb_lb->lb_dps = lb_dps;
hmap_insert(&sb_lbs->entries, &sb_lb->key_node,
uuid_hash(&lb_dps->lb->nlb->header_.uuid));
uuid_hash(nb_uuid));
} else {
sb_lb->sbrec_lb =
sbrec_load_balancer_table_get_for_uuid(sb_lb_table,
&sb_lb->sb_uuid);
sbrec_load_balancer_table_get_for_uuid(sb_lb_table, nb_uuid);
}

if (sb_lb && !lb_dps->n_nb_ls && !lb_dps->n_nb_lr) {
const struct sbrec_load_balancer *sbrec_lb =
sbrec_load_balancer_table_get_for_uuid(sb_lb_table,
&sb_lb->sb_uuid);
sbrec_load_balancer_table_get_for_uuid(sb_lb_table, nb_uuid);
if (sbrec_lb) {
sbrec_load_balancer_delete(sbrec_lb);
}
Expand All @@ -981,23 +943,3 @@ sync_changed_lbs(struct sb_lb_table *sb_lbs,

return true;
}

static bool
check_sb_lb_duplicates(const struct sbrec_load_balancer_table *table)
{
struct sset existing_nb_lb_uuids =
SSET_INITIALIZER(&existing_nb_lb_uuids);
const struct sbrec_load_balancer *sbrec_lb;
bool duplicates = false;

SBREC_LOAD_BALANCER_TABLE_FOR_EACH (sbrec_lb, table) {
const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
if (nb_lb_uuid && !sset_add(&existing_nb_lb_uuids, nb_lb_uuid)) {
duplicates = true;
break;
}
}

sset_destroy(&existing_nb_lb_uuids);
return duplicates;
}
2 changes: 0 additions & 2 deletions northd/en-sync-sb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ void *en_sync_to_sb_lb_init(struct engine_node *, struct engine_arg *);
void en_sync_to_sb_lb_run(struct engine_node *, void *data);
void en_sync_to_sb_lb_cleanup(void *data);
bool sync_to_sb_lb_northd_handler(struct engine_node *, void *data OVS_UNUSED);
bool sync_to_sb_lb_sb_load_balancer(struct engine_node *,
void *data OVS_UNUSED);

void *en_sync_to_sb_pb_init(struct engine_node *, struct engine_arg *);
void en_sync_to_sb_pb_run(struct engine_node *, void *data);
Expand Down
4 changes: 3 additions & 1 deletion northd/inc-proc-northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
node_global_config_handler);
engine_add_input(&en_sync_to_sb_lb, &en_northd,
sync_to_sb_lb_northd_handler);
/* There is nothing to handle as duplicates are prevented by using
* UUID mapping between NB and SB. */
engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer,
sync_to_sb_lb_sb_load_balancer);
engine_noop_handler);
engine_add_input(&en_sync_to_sb_lb, &en_sb_logical_dp_group, NULL);

engine_add_input(&en_sync_to_sb_pb, &en_northd,
Expand Down
3 changes: 2 additions & 1 deletion tests/ovn-northd.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6469,7 +6469,8 @@ check ovn-nbctl --wait=sb sync

dps=$(fetch_column Load_Balancer ls_datapath_group)
nlb=$(fetch_column nb:Load_Balancer _uuid)
AT_CHECK([ovn-sbctl create Load_Balancer name=lb1 ls_datapath_group="$dps" external_ids="lb_id=$nlb"], [0], [ignore])
AT_CHECK([ovn-sbctl --id="$nlb" create Load_Balancer name=lb1 ls_datapath_group="$dps"], [1], [ignore], [stderr])
check grep -q "duplicate uuid" stderr

check ovn-nbctl --wait=sb sync
check_row_count Load_Balancer 1
Expand Down

0 comments on commit 704fd10

Please sign in to comment.