Skip to content

Commit

Permalink
controller: Prepare structure around CT zone limiting.
Browse files Browse the repository at this point in the history 
In order to be able to store CT limits for specified zone, store the
zone inside separate struct instead of simap. This allows to add
the addition of limit without changing the whole infrastructure again.

This is a preparation step for the CT zone limits.

Acked-by: Mark Michelson <[email protected]>
Signed-off-by: Ales Musil <[email protected]>
Signed-off-by: Numan Siddique <[email protected]>
  • Loading branch information
@almusil @numansiddique
almusil authored and numansiddique committed Jul 29, 2024
1 parent 601b3c6 commit 493ef70
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 99 deletions.
187 changes: 110 additions & 77 deletions controller/ct-zone.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,29 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table,
struct ct_zone_ctx *ctx, const char *name, int zone);
static void ct_zone_add_pending(struct shash *pending_ct_zones,
enum ct_zone_pending_state state,
int zone, bool add, const char *name);
struct ct_zone *zone, bool add,
const char *name);
static int ct_zone_get_snat(const struct sbrec_datapath_binding *dp);
static bool ct_zone_assign_unused(struct ct_zone_ctx *ctx,
const char *zone_name,
int *scan_start, int scan_stop);
static bool ct_zone_remove(struct ct_zone_ctx *ctx,
struct simap_node *ct_zone);
static bool ct_zone_remove(struct ct_zone_ctx *ctx, const char *name);
static void ct_zone_add(struct ct_zone_ctx *ctx, const char *name,
uint16_t zone, bool set_pending);

void
ct_zone_ctx_init(struct ct_zone_ctx *ctx)
{
shash_init(&ctx->pending);
shash_init(&ctx->current);
}

void
ct_zone_ctx_destroy(struct ct_zone_ctx *ctx)
{
shash_destroy_free_data(&ctx->current);
shash_destroy_free_data(&ctx->pending);
}

void
ct_zones_restore(struct ct_zone_ctx *ctx,
Expand All @@ -50,7 +66,8 @@ ct_zones_restore(struct ct_zone_ctx *ctx,
struct ct_zone_pending_entry *ctpe = pending_node->data;

if (ctpe->add) {
ct_zone_restore(dp_table, ctx, pending_node->name, ctpe->zone);
ct_zone_restore(dp_table, ctx, pending_node->name,
ctpe->ct_zone.zone);
}
}

Expand Down Expand Up @@ -144,7 +161,6 @@ ct_zones_update(const struct sset *local_lports,
const struct hmap *local_datapaths, struct ct_zone_ctx *ctx)
{
int min_ct_zone, max_ct_zone;
struct simap_node *ct_zone;
const char *user;
struct sset all_users = SSET_INITIALIZER(&all_users);
struct simap req_snat_zones = SIMAP_INITIALIZER(&req_snat_zones);
Expand Down Expand Up @@ -186,13 +202,15 @@ ct_zones_update(const struct sset *local_lports,
ct_zones_parse_range(ovs_table, &min_ct_zone, &max_ct_zone);

/* Delete zones that do not exist in above sset. */
SIMAP_FOR_EACH_SAFE (ct_zone, &ctx->current) {
if (!sset_contains(&all_users, ct_zone->name) ||
ct_zone->data < min_ct_zone || ct_zone->data > max_ct_zone) {
ct_zone_remove(ctx, ct_zone);
} else if (!simap_find(&req_snat_zones, ct_zone->name)) {
bitmap_set1(unreq_snat_zones_map, ct_zone->data);
simap_put(&unreq_snat_zones, ct_zone->name, ct_zone->data);
struct shash_node *node;
SHASH_FOR_EACH_SAFE (node, &ctx->current) {
struct ct_zone *ct_zone = node->data;
if (!sset_contains(&all_users, node->name) ||
ct_zone->zone < min_ct_zone || ct_zone->zone > max_ct_zone) {
ct_zone_remove(ctx, node->name);
} else if (!simap_find(&req_snat_zones, node->name)) {
bitmap_set1(unreq_snat_zones_map, ct_zone->zone);
simap_put(&unreq_snat_zones, node->name, ct_zone->zone);
}
}

Expand All @@ -207,7 +225,7 @@ ct_zones_update(const struct sset *local_lports,
struct simap_node *unreq_node;
SIMAP_FOR_EACH_SAFE (unreq_node, &unreq_snat_zones) {
if (unreq_node->data == snat_req_node->data) {
simap_find_and_delete(&ctx->current, unreq_node->name);
ct_zone_remove(ctx, unreq_node->name);
simap_delete(&unreq_snat_zones, unreq_node);
}
}
Expand All @@ -218,26 +236,12 @@ ct_zones_update(const struct sset *local_lports,
bitmap_set0(unreq_snat_zones_map, snat_req_node->data);
}

struct simap_node *node = simap_find(&ctx->current,
snat_req_node->name);
if (node) {
if (node->data != snat_req_node->data) {
/* Zone request has changed for this node. delete old entry and
* create new one*/
ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED,
snat_req_node->data, true,
snat_req_node->name);
bitmap_set0(ctx->bitmap, node->data);
}
bitmap_set1(ctx->bitmap, snat_req_node->data);
node->data = snat_req_node->data;
} else {
ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED,
snat_req_node->data, true,
snat_req_node->name);
bitmap_set1(ctx->bitmap, snat_req_node->data);
simap_put(&ctx->current, snat_req_node->name, snat_req_node->data);
struct ct_zone *ct_zone = shash_find_data(&ctx->current,
snat_req_node->name);
if (ct_zone && ct_zone->zone != snat_req_node->data) {
ct_zone_remove(ctx, snat_req_node->name);
}
ct_zone_add(ctx, snat_req_node->name, snat_req_node->data, true);
}

/* xxx This is wasteful to assign a zone to each port--even if no
Expand All @@ -246,7 +250,7 @@ ct_zones_update(const struct sset *local_lports,
/* Assign a unique zone id for each logical port and two zones
* to a gateway router. */
SSET_FOR_EACH (user, &all_users) {
if (simap_contains(&ctx->current, user)) {
if (shash_find(&ctx->current, user)) {
continue;
}

Expand Down Expand Up @@ -277,7 +281,7 @@ ct_zones_commit(const struct ovsrec_bridge *br_int,

char *user_str = xasprintf("ct-zone-%s", iter->name);
if (ctzpe->add) {
char *zone_str = xasprintf("%"PRId32, ctzpe->zone);
char *zone_str = xasprintf("%"PRIu16, ctzpe->ct_zone.zone);
struct smap_node *node =
smap_get_node(&br_int->external_ids, user_str);
if (!node || strcmp(node->value, zone_str)) {
Expand Down Expand Up @@ -336,12 +340,9 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
* or not. If so, then fall back to full recompute of
* ct_zone engine. */
char *snat_dp_zone_key = alloc_nat_zone_key(name, "snat");
struct simap_node *simap_node =
simap_find(&ctx->current, snat_dp_zone_key);
struct ct_zone *ct_zone = shash_find_data(&ctx->current, snat_dp_zone_key);
free(snat_dp_zone_key);
if (!simap_node || simap_node->data != req_snat_zone) {
/* There is no entry yet or the requested snat zone has changed.
* Trigger full recompute of ct_zones engine. */
if (!ct_zone || ct_zone->zone != req_snat_zone) {
return false;
}

Expand All @@ -354,24 +355,33 @@ ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name,
bool updated, int *scan_start,
int min_ct_zone, int max_ct_zone)
{
struct simap_node *ct_zone = simap_find(&ctx->current, name);
struct shash_node *node = shash_find(&ctx->current, name);

if (ct_zone &&
(ct_zone->data < min_ct_zone || ct_zone->data > max_ct_zone)) {
ct_zone_remove(ctx, ct_zone);
ct_zone = NULL;
if (node) {
struct ct_zone *ct_zone = node->data;
if (ct_zone->zone < min_ct_zone || ct_zone->zone > max_ct_zone) {
ct_zone_remove(ctx, node->name);
node = NULL;
}
}

if (updated && !ct_zone) {
if (updated && !node) {
ct_zone_assign_unused(ctx, name, scan_start, max_ct_zone);
return true;
} else if (!updated && ct_zone_remove(ctx, ct_zone)) {
} else if (!updated && node && ct_zone_remove(ctx, node->name)) {
return true;
}

return false;
}

uint16_t
ct_zone_find_zone(const struct shash *ct_zones, const char *name)
{
struct ct_zone *ct_zone = shash_find_data(ct_zones, name);
return ct_zone ? ct_zone->zone : 0;
}


static bool
ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name,
Expand All @@ -386,33 +396,53 @@ ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name,
}

*scan_start = zone + 1;
ct_zone_add(ctx, zone_name, zone, true);

ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED,
zone, true, zone_name);

bitmap_set1(ctx->bitmap, zone);
simap_put(&ctx->current, zone_name, zone);
return true;
}

static bool
ct_zone_remove(struct ct_zone_ctx *ctx, struct simap_node *ct_zone)
ct_zone_remove(struct ct_zone_ctx *ctx, const char *name)
{
if (!ct_zone) {
struct shash_node *node = shash_find(&ctx->current, name);
if (!node) {
return false;
}

VLOG_DBG("removing ct zone %"PRId32" for '%s'", ct_zone->data,
ct_zone->name);
struct ct_zone *ct_zone = node->data;

VLOG_DBG("removing ct zone %"PRIu16" for '%s'", ct_zone->zone, name);

ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED,
ct_zone->data, false, ct_zone->name);
bitmap_set0(ctx->bitmap, ct_zone->data);
simap_delete(&ctx->current, ct_zone);
ct_zone, false, name);
bitmap_set0(ctx->bitmap, ct_zone->zone);
shash_delete(&ctx->current, node);
free(ct_zone);

return true;
}

static void
ct_zone_add(struct ct_zone_ctx *ctx, const char *name, uint16_t zone,
bool set_pending)
{
VLOG_DBG("assigning ct zone %"PRIu16" for '%s'", zone, name);

struct ct_zone *ct_zone = shash_find_data(&ctx->current, name);
if (!ct_zone) {
ct_zone = xmalloc(sizeof *ct_zone);
shash_add(&ctx->current, name, ct_zone);
}

ct_zone->zone = zone;

if (set_pending) {
ct_zone_add_pending(&ctx->pending, CT_ZONE_OF_QUEUED,
ct_zone, true, name);
}
bitmap_set1(ctx->bitmap, zone);
}

static int
ct_zone_get_snat(const struct sbrec_datapath_binding *dp)
{
Expand All @@ -422,27 +452,29 @@ ct_zone_get_snat(const struct sbrec_datapath_binding *dp)
static void
ct_zone_add_pending(struct shash *pending_ct_zones,
enum ct_zone_pending_state state,
int zone, bool add, const char *name)
struct ct_zone *zone, bool add, const char *name)
{
VLOG_DBG("%s ct zone %"PRId32" for '%s'",
add ? "assigning" : "removing", zone, name);

struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending);
*pending = (struct ct_zone_pending_entry) {
.state = state,
.zone = zone,
.add = add,
};

/* Its important that we add only one entry for the key 'name'.
* Replace 'pending' with 'existing' and free up 'existing'.
* Otherwise, we may end up in a continuous loop of adding
* and deleting the zone entry in the 'external_ids' of
* integration bridge.
*/
struct ct_zone_pending_entry *existing =
shash_replace(pending_ct_zones, name, pending);
free(existing);
struct ct_zone_pending_entry *entry =
shash_find_data(pending_ct_zones, name);

if (!entry) {
entry = xmalloc(sizeof *entry);
entry->state = state;

shash_add(pending_ct_zones, name, entry);
}

*entry = (struct ct_zone_pending_entry) {
.ct_zone = *zone,
.state = MIN(entry->state, state),
.add = add,
};
}

/* Replaces a UUID prefix from 'uuid_zone' (if any) with the
Expand Down Expand Up @@ -483,19 +515,20 @@ ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table,
VLOG_DBG("ct zone %"PRId32" replace uuid name '%s' with '%s'",
zone, name, new_name);

struct ct_zone ct_zone = {
.zone = zone,
};
/* Make sure we remove the uuid one in the next OvS DB commit without
* flush. */
ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED,
zone, false, name);
&ct_zone, false, name);
/* Store the zone in OvS DB with name instead of uuid without flush.
* */
ct_zone_add_pending(&ctx->pending, CT_ZONE_DB_QUEUED,
zone, true, new_name);
&ct_zone, true, new_name);
current_name = new_name;
}

simap_put(&ctx->current, current_name, zone);
bitmap_set1(ctx->bitmap, zone);

ct_zone_add(ctx, current_name, zone, false);
free(new_name);
}
13 changes: 10 additions & 3 deletions controller/ct-zone.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ struct ct_zone_ctx {
struct shash pending; /* Pending entries,
* 'struct ct_zone_pending_entry'
* by name. */
struct simap current; /* Current CT zones mapping
* (zone id by name). */
struct shash current; /* Current CT zones mapping
* (struct ct_zone by name). */
};

struct ct_zone {
uint16_t zone;
};

/* States to move through when a new conntrack zone has been allocated. */
Expand All @@ -50,12 +54,14 @@ enum ct_zone_pending_state {
};

struct ct_zone_pending_entry {
int zone;
struct ct_zone ct_zone;
bool add; /* Is the entry being added? */
ovs_be32 of_xid; /* Transaction id for barrier. */
enum ct_zone_pending_state state;
};

void ct_zone_ctx_init(struct ct_zone_ctx *ctx);
void ct_zone_ctx_destroy(struct ct_zone_ctx *ctx);
void ct_zones_parse_range(const struct ovsrec_open_vswitch_table *ovs_table,
int *min_ct_zone, int *max_ct_zone);
void ct_zones_restore(struct ct_zone_ctx *ctx,
Expand All @@ -74,5 +80,6 @@ bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx,
bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const char *name,
bool updated, int *scan_start,
int min_ct_zone, int max_ct_zone);
uint16_t ct_zone_find_zone(const struct shash *ct_zones, const char *name);

#endif /* controller/ct-zone.h */
2 changes: 1 addition & 1 deletion controller/ofctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2709,7 +2709,7 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
SHASH_FOR_EACH(iter, pending_ct_zones) {
struct ct_zone_pending_entry *ctzpe = iter->data;
if (ctzpe->state == CT_ZONE_OF_QUEUED) {
add_ct_flush_zone(ctzpe->zone, &msgs);
add_ct_flush_zone(ctzpe->ct_zone.zone, &msgs);
ctzpe->state = CT_ZONE_OF_SENT;
ctzpe->of_xid = 0;
}
Expand Down
Loading

0 comments on commit 493ef70

Please sign in to comment.