Skip to content

Commit

Permalink
ovn-controller: Store conntrack zone mappings to OVS database.
Browse files Browse the repository at this point in the history
If ovn-controller is restarted, it may choose different conntrack zones
than had been previously used, which could cause the wrong conntrack
entries to be associated with a logical port.  This commit stores in the
integration bridge's OVS "Bridge" table the mapping to the conntrack zone.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
justinpettit committed Sep 23, 2016
1 parent 8ba0e38 commit 8cb9eba
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 4 deletions.
14 changes: 14 additions & 0 deletions ovn/controller/ovn-controller.8.xml
Expand Up @@ -199,6 +199,20 @@
chassis.
</dd>

<dt>
<code>external_ids:ct-zone-*</code> in the <code>Bridge</code> table
</dt>
<dd>
Logical ports and gateway routers are assigned a connection
tracking zone by <code>ovn-controller</code> for stateful
services. To keep state across restarts of
<code>ovn-controller</code>, these keys are stored in the
integration bridge's Bridge table. The name contains a prefix
of <code>ct-zone-</code> followed by the name of the logical
port or gateway router's zone key. The value for this key
identifies the zone used for this port.
</dd>

<dt>
<code>external_ids:ovn-localnet-port</code> in the <code>Port</code>
table
Expand Down
136 changes: 132 additions & 4 deletions ovn/controller/ovn-controller.c
Expand Up @@ -229,9 +229,21 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
}
}

enum ct_zone_pending_state {
CT_ZONE_DB_QUEUED, /* Waiting for DB transaction to open. */
CT_ZONE_DB_SENT, /* Sent and waiting for confirmation from DB. */
};

struct ct_zone_pending_entry {
enum ct_zone_pending_state state;
int zone;
bool add; /* Is the entry being added? */
};

static void
update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
struct simap *ct_zones, unsigned long *ct_zone_bitmap)
struct simap *ct_zones, unsigned long *ct_zone_bitmap,
struct shash *pending_ct_zones)
{
struct simap_node *ct_zone, *ct_zone_next;
int scan_start = 1;
Expand Down Expand Up @@ -260,6 +272,15 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
/* Delete zones that do not exist in above sset. */
SIMAP_FOR_EACH_SAFE(ct_zone, ct_zone_next, ct_zones) {
if (!sset_contains(&all_users, ct_zone->name)) {
VLOG_DBG("removing ct zone %"PRId32" for '%s'",
ct_zone->data, ct_zone->name);

struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending);
pending->state = CT_ZONE_DB_QUEUED;
pending->zone = ct_zone->data;
pending->add = false;
shash_add(pending_ct_zones, ct_zone->name, pending);

bitmap_set0(ct_zone_bitmap, ct_zone->data);
simap_delete(ct_zones, ct_zone);
}
Expand All @@ -271,7 +292,7 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
/* Assign a unique zone id for each logical port and two zones
* to a gateway router. */
SSET_FOR_EACH(user, &all_users) {
size_t zone;
int zone;

if (simap_contains(ct_zones, user)) {
continue;
Expand All @@ -286,6 +307,14 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
}
scan_start = zone + 1;

VLOG_DBG("assigning ct zone %"PRId32" to '%s'", zone, user);

struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending);
pending->state = CT_ZONE_DB_QUEUED;
pending->zone = zone;
pending->add = true;
shash_add(pending_ct_zones, user, pending);

bitmap_set1(ct_zone_bitmap, zone);
simap_put(ct_zones, user, zone);

Expand All @@ -297,6 +326,90 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
sset_destroy(&all_users);
}

static void
commit_ct_zones(struct controller_ctx *ctx,
const struct ovsrec_bridge *br_int,
struct shash *pending_ct_zones)
{
if (!ctx->ovs_idl_txn) {
return;
}

struct smap new_ids;
smap_clone(&new_ids, &br_int->external_ids);

bool updated = false;
struct shash_node *iter;
SHASH_FOR_EACH(iter, pending_ct_zones) {
struct ct_zone_pending_entry *ctzpe = iter->data;

/* The transaction is open, so any pending entries in the
* CT_ZONE_DB_QUEUED must be sent and any in CT_ZONE_DB_QUEUED
* need to be retried. */
if (ctzpe->state != CT_ZONE_DB_QUEUED
&& ctzpe->state != CT_ZONE_DB_SENT) {
continue;
}

char *user_str = xasprintf("ct-zone-%s", iter->name);
if (ctzpe->add) {
char *zone_str = xasprintf("%"PRId32, ctzpe->zone);
smap_replace(&new_ids, user_str, zone_str);
free(zone_str);
} else {
smap_remove(&new_ids, user_str);
}
free(user_str);

ctzpe->state = CT_ZONE_DB_SENT;
updated = true;
}

if (updated) {
ovsrec_bridge_verify_external_ids(br_int);
ovsrec_bridge_set_external_ids(br_int, &new_ids);
}
smap_destroy(&new_ids);
}

static void
restore_ct_zones(struct ovsdb_idl *ovs_idl,
struct simap *ct_zones, unsigned long *ct_zone_bitmap)
{
const struct ovsrec_open_vswitch *cfg;
cfg = ovsrec_open_vswitch_first(ovs_idl);
if (!cfg) {
return;
}

const char *br_int_name = smap_get_def(&cfg->external_ids, "ovn-bridge",
DEFAULT_BRIDGE_NAME);

const struct ovsrec_bridge *br_int;
br_int = get_bridge(ovs_idl, br_int_name);
if (!br_int) {
/* If the integration bridge hasn't been defined, assume that
* any existing ct-zone definitions aren't valid. */
return;
}

struct smap_node *node;
SMAP_FOR_EACH(node, &br_int->external_ids) {
if (strncmp(node->key, "ct-zone-", 8)) {
continue;
}

const char *user = node->key + 8;
int zone = atoi(node->value);

if (user[0] && zone) {
VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user);
bitmap_set1(ct_zone_bitmap, zone);
simap_put(ct_zones, user, zone);
}
}
}

static int64_t
get_nb_cfg(struct ovsdb_idl *idl)
{
Expand Down Expand Up @@ -362,6 +475,7 @@ main(int argc, char *argv[])
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_bridge_col_name);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_bridge_col_fail_mode);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_bridge_col_other_config);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_bridge_col_external_ids);
chassis_register_ovs_idl(ovs_idl_loop.idl);
encaps_register_ovs_idl(ovs_idl_loop.idl);
binding_register_ovs_idl(ovs_idl_loop.idl);
Expand All @@ -381,9 +495,11 @@ main(int argc, char *argv[])

/* Initialize connection tracking zones. */
struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones);
struct shash pending_ct_zones = SHASH_INITIALIZER(&pending_ct_zones);
unsigned long ct_zone_bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)];
memset(ct_zone_bitmap, 0, sizeof ct_zone_bitmap);
bitmap_set1(ct_zone_bitmap, 0); /* Zone 0 is reserved. */
restore_ct_zones(ovs_idl_loop.idl, &ct_zones, ct_zone_bitmap);
unixctl_command_register("ct-zone-list", "", 0, 0,
ct_zone_list, &ct_zones);

Expand Down Expand Up @@ -440,7 +556,8 @@ main(int argc, char *argv[])

pinctrl_run(&ctx, &lports, br_int, chassis_id, &local_datapaths);
update_ct_zones(&all_lports, &patched_datapaths, &ct_zones,
ct_zone_bitmap);
ct_zone_bitmap, &pending_ct_zones);
commit_ct_zones(&ctx, br_int, &pending_ct_zones);

struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
Expand Down Expand Up @@ -493,9 +610,20 @@ main(int argc, char *argv[])
pinctrl_wait(&ctx);
}
ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop);
ovsdb_idl_track_clear(ovnsb_idl_loop.idl);

if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) {
struct shash_node *iter, *iter_next;
SHASH_FOR_EACH_SAFE(iter, iter_next, &pending_ct_zones) {
struct ct_zone_pending_entry *ctzpe = iter->data;
if (ctzpe->state == CT_ZONE_DB_SENT) {
shash_delete(&pending_ct_zones, iter);
free(ctzpe);
}
}
}
ovsdb_idl_track_clear(ovs_idl_loop.idl);

poll_block();
if (should_service_stop()) {
exiting = true;
Expand Down

0 comments on commit 8cb9eba

Please sign in to comment.