Skip to content

Commit

Permalink
ovn-controller: Avoid blocking to commit OVSDB transactions.
Browse files Browse the repository at this point in the history 
Until now, ovn-controller has been full of loops that commit a transaction
to the OVS or OVN Southbound database.  These blocking loops delay other
work within ovn-controller.  They also make it unsafe to keep pointers to
database records within a single ovn-controller main loop, since calls
to ovsdb_idl_run() can cause IDL records to be destroyed.  This commit
drops all of the blocking calls, instead doing a single commit to the
databases at the end of each main loop.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Russell Bryant <rbryant@redhat.com>
  • Loading branch information
@blp
blp committed Jul 28, 2015
1 parent acd55f5 commit f1fd765
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 125 deletions.
61 changes: 23 additions & 38 deletions ovn/controller/binding.c
View file
Expand Up @@ -76,10 +76,12 @@ binding_run(struct controller_ctx *ctx)
{
const struct sbrec_chassis *chassis_rec;
const struct sbrec_binding *binding_rec;
struct ovsdb_idl_txn *txn;
struct sset lports, all_lports;
const char *name;
int retval;

if (!ctx->ovnsb_idl_txn) {
return;
}

chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (!chassis_rec) {
Expand All @@ -91,8 +93,7 @@ binding_run(struct controller_ctx *ctx)
get_local_iface_ids(ctx, &lports);
sset_clone(&all_lports, &lports);

txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
ovsdb_idl_txn_add_comment(txn,
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: updating bindings for '%s'",
ctx->chassis_id);

Expand All @@ -115,55 +116,39 @@ binding_run(struct controller_ctx *ctx)
}
}

retval = ovsdb_idl_txn_commit_block(txn);
if (retval == TXN_ERROR) {
VLOG_INFO("Problem committing binding information: %s",
ovsdb_idl_txn_status_to_string(retval));
}

ovsdb_idl_txn_destroy(txn);

SSET_FOR_EACH (name, &lports) {
VLOG_DBG("No binding record for lport %s", name);
}
sset_destroy(&lports);
sset_destroy(&all_lports);
}

void
binding_destroy(struct controller_ctx *ctx)
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
binding_cleanup(struct controller_ctx *ctx)
{
const struct sbrec_chassis *chassis_rec;
int retval = TXN_TRY_AGAIN;

ovs_assert(ctx->ovnsb_idl);
if (!ctx->ovnsb_idl_txn) {
return false;
}

chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (!chassis_rec) {
return;
return true;
}

while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
const struct sbrec_binding *binding_rec;
struct ovsdb_idl_txn *txn;

txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
ovsdb_idl_txn_add_comment(txn,
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: removing all bindings for '%s'",
ctx->chassis_id);

SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
if (binding_rec->chassis == chassis_rec) {
sbrec_binding_set_chassis(binding_rec, NULL);
}
}

retval = ovsdb_idl_txn_commit_block(txn);
if (retval == TXN_ERROR) {
VLOG_INFO("Problem removing bindings: %s",
ovsdb_idl_txn_status_to_string(retval));
const struct sbrec_binding *binding_rec;
bool any_changes = false;
SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
if (binding_rec->chassis == chassis_rec) {
sbrec_binding_set_chassis(binding_rec, NULL);
any_changes = true;
}

ovsdb_idl_txn_destroy(txn);
}
return !any_changes;
}
4 changes: 3 additions & 1 deletion ovn/controller/binding.h
View file
Expand Up @@ -17,10 +17,12 @@
#ifndef OVN_BINDING_H
#define OVN_BINDING_H 1

#include <stdbool.h>

struct controller_ctx;

void binding_init(struct controller_ctx *);
void binding_run(struct controller_ctx *);
void binding_destroy(struct controller_ctx *);
bool binding_cleanup(struct controller_ctx *);

#endif /* ovn/binding.h */
117 changes: 41 additions & 76 deletions ovn/controller/chassis.c
View file
Expand Up @@ -52,8 +52,6 @@ register_chassis(struct controller_ctx *ctx)
const char *encap_type, *encap_ip;
struct sbrec_encap *encap_rec;
static bool inited = false;
int retval = TXN_TRY_AGAIN;
struct ovsdb_idl_txn *txn;

chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);

Expand Down Expand Up @@ -92,31 +90,22 @@ register_chassis(struct controller_ctx *ctx)
}
}

txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
ovsdb_idl_txn_add_comment(txn,
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: registering chassis '%s'",
ctx->chassis_id);

if (!chassis_rec) {
chassis_rec = sbrec_chassis_insert(txn);
chassis_rec = sbrec_chassis_insert(ctx->ovnsb_idl_txn);
sbrec_chassis_set_name(chassis_rec, ctx->chassis_id);
}

encap_rec = sbrec_encap_insert(txn);
encap_rec = sbrec_encap_insert(ctx->ovnsb_idl_txn);

sbrec_encap_set_type(encap_rec, encap_type);
sbrec_encap_set_ip(encap_rec, encap_ip);

sbrec_chassis_set_encaps(chassis_rec, &encap_rec, 1);

retval = ovsdb_idl_txn_commit_block(txn);
if (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
VLOG_INFO("Problem registering chassis: %s",
ovsdb_idl_txn_status_to_string(retval));
poll_immediate_wake();
}
ovsdb_idl_txn_destroy(txn);

inited = true;
}

Expand Down Expand Up @@ -311,15 +300,14 @@ update_encaps(struct controller_ctx *ctx)
{
const struct sbrec_chassis *chassis_rec;
const struct ovsrec_bridge *br;
int retval;

struct tunnel_ctx tc = {
.tunnel_hmap = HMAP_INITIALIZER(&tc.tunnel_hmap),
.port_names = SSET_INITIALIZER(&tc.port_names),
.br_int = ctx->br_int
};

tc.ovs_txn = ovsdb_idl_txn_create(ctx->ovs_idl);
tc.ovs_txn = ctx->ovs_idl_txn;
ovsdb_idl_txn_add_comment(tc.ovs_txn,
"ovn-controller: modifying OVS tunnels '%s'",
ctx->chassis_id);
Expand Down Expand Up @@ -366,81 +354,58 @@ update_encaps(struct controller_ctx *ctx)
}
hmap_destroy(&tc.tunnel_hmap);
sset_destroy(&tc.port_names);

retval = ovsdb_idl_txn_commit_block(tc.ovs_txn);
if (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
VLOG_INFO("Problem modifying OVS tunnels: %s",
ovsdb_idl_txn_status_to_string(retval));
poll_immediate_wake();
}
ovsdb_idl_txn_destroy(tc.ovs_txn);
}

void
chassis_run(struct controller_ctx *ctx)
{
register_chassis(ctx);
update_encaps(ctx);
if (ctx->ovnsb_idl_txn) {
register_chassis(ctx);
}

if (ctx->ovs_idl_txn) {
update_encaps(ctx);
}
}

void
chassis_destroy(struct controller_ctx *ctx)
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
chassis_cleanup(struct controller_ctx *ctx)
{
int retval = TXN_TRY_AGAIN;

ovs_assert(ctx->ovnsb_idl);

while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
const struct sbrec_chassis *chassis_rec;
struct ovsdb_idl_txn *txn;
if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
return false;
}

chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (!chassis_rec) {
break;
}
bool any_changes = false;

txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
ovsdb_idl_txn_add_comment(txn,
/* Delete Chassis row. */
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (chassis_rec) {
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: unregistering chassis '%s'",
ctx->chassis_id);
sbrec_chassis_delete(chassis_rec);

retval = ovsdb_idl_txn_commit_block(txn);
if (retval == TXN_ERROR) {
VLOG_INFO("Problem unregistering chassis: %s",
ovsdb_idl_txn_status_to_string(retval));
}
ovsdb_idl_txn_destroy(txn);
any_changes = true;
}

retval = TXN_TRY_AGAIN;
while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
struct ovsrec_port **ports;
struct ovsdb_idl_txn *txn;
size_t i, n;

txn = ovsdb_idl_txn_create(ctx->ovs_idl);
ovsdb_idl_txn_add_comment(txn,
"ovn-controller: destroying tunnels");

/* Delete all the OVS-created tunnels from the integration
* bridge. */
ports = xmalloc(sizeof *ctx->br_int->ports * ctx->br_int->n_ports);
for (i = n = 0; i < ctx->br_int->n_ports; i++) {
if (!smap_get(&ctx->br_int->ports[i]->external_ids,
"ovn-chassis-id")) {
ports[n++] = ctx->br_int->ports[i];
}
}
ovsrec_bridge_verify_ports(ctx->br_int);
ovsrec_bridge_set_ports(ctx->br_int, ports, n);
free(ports);

retval = ovsdb_idl_txn_commit_block(txn);
if (retval == TXN_ERROR) {
VLOG_INFO("Problem destroying tunnels: %s",
ovsdb_idl_txn_status_to_string(retval));
/* Delete all the OVS-created tunnels from the integration bridge. */
ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
"ovn-controller: destroying tunnels");
struct ovsrec_port **ports
= xmalloc(sizeof *ctx->br_int->ports * ctx->br_int->n_ports);
size_t n = 0;
for (size_t i = 0; i < ctx->br_int->n_ports; i++) {
if (!smap_get(&ctx->br_int->ports[i]->external_ids,
"ovn-chassis-id")) {
ports[n++] = ctx->br_int->ports[i];
any_changes = true;
}
ovsdb_idl_txn_destroy(txn);
}
ovsrec_bridge_verify_ports(ctx->br_int);
ovsrec_bridge_set_ports(ctx->br_int, ports, n);
free(ports);

return !any_changes;
}
4 changes: 3 additions & 1 deletion ovn/controller/chassis.h
View file
Expand Up @@ -16,10 +16,12 @@
#ifndef OVN_CHASSIS_H
#define OVN_CHASSIS_H 1

#include <stdbool.h>

struct controller_ctx;

void chassis_init(struct controller_ctx *);
void chassis_run(struct controller_ctx *);
void chassis_destroy(struct controller_ctx *);
bool chassis_cleanup(struct controller_ctx *);

#endif /* ovn/chassis.h */

0 comments on commit f1fd765

Please sign in to comment.