Skip to content

Commit

Permalink
connmgr: Improve interface for setting controllers.
Browse files Browse the repository at this point in the history 
Using an shash instead of an array simplifies the code for both the caller
and the callee.  Putting the set of allowed OpenFlow versions into the
ofproto_controller data structure also simplifies the overall function
interface slightly.

Tested-by: Yifeng Sun <pkusunyifeng@gmail.com>
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
@blp
blp committed Oct 31, 2018
1 parent 8645f9c commit af26093
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 103 deletions.
51 changes: 21 additions & 30 deletions ofproto/connmgr.c
View file
Expand Up @@ -580,9 +580,7 @@ connmgr_free_controller_info(struct shash *info)
/* Changes 'mgr''s set of controllers to the 'n_controllers' controllers in
* 'controllers'. */
void
connmgr_set_controllers(struct connmgr *mgr,
const struct ofproto_controller *controllers,
size_t n_controllers, uint32_t allowed_versions)
connmgr_set_controllers(struct connmgr *mgr, struct shash *controllers)
OVS_EXCLUDED(ofproto_mutex)
{
bool had_controllers = connmgr_has_controllers(mgr);
Expand All @@ -591,61 +589,57 @@ connmgr_set_controllers(struct connmgr *mgr,
* cover a smaller amount of code, if that yielded some benefit. */
ovs_mutex_lock(&ofproto_mutex);

/* Create newly configured controllers and services.
* Create a name to ofproto_controller mapping in 'new_controllers'. */
struct shash new_controllers = SHASH_INITIALIZER(&new_controllers);
for (size_t i = 0; i < n_controllers; i++) {
const struct ofproto_controller *c = &controllers[i];
/* Create newly configured controllers and services. */
struct shash_node *node;
SHASH_FOR_EACH (node, controllers) {
const char *target = node->name;
const struct ofproto_controller *c = node->data;

if (!vconn_verify_name(c->target)) {
if (!vconn_verify_name(target)) {
bool add = false;
struct ofconn *ofconn = find_controller_by_target(mgr, c->target);
struct ofconn *ofconn = find_controller_by_target(mgr, target);
if (!ofconn) {
VLOG_INFO("%s: added primary controller \"%s\"",
mgr->name, c->target);
mgr->name, target);
add = true;
} else if (rconn_get_allowed_versions(ofconn->rconn) !=
allowed_versions) {
c->allowed_versions) {
VLOG_INFO("%s: re-added primary controller \"%s\"",
mgr->name, c->target);
mgr->name, target);
add = true;
ofconn_destroy(ofconn);
}
if (add) {
add_controller(mgr, c->target, c->dscp, allowed_versions);
add_controller(mgr, target, c->dscp, c->allowed_versions);
}
} else if (!pvconn_verify_name(c->target)) {
} else if (!pvconn_verify_name(target)) {
bool add = false;
struct ofservice *ofservice = ofservice_lookup(mgr, c->target);
struct ofservice *ofservice = ofservice_lookup(mgr, target);
if (!ofservice) {
VLOG_INFO("%s: added service controller \"%s\"",
mgr->name, c->target);
mgr->name, target);
add = true;
} else if (ofservice->allowed_versions != allowed_versions) {
} else if (ofservice->allowed_versions != c->allowed_versions) {
VLOG_INFO("%s: re-added service controller \"%s\"",
mgr->name, c->target);
mgr->name, target);
ofservice_destroy(mgr, ofservice);
add = true;
}
if (add) {
ofservice_create(mgr, c->target, allowed_versions, c->dscp);
ofservice_create(mgr, target, c->allowed_versions, c->dscp);
}
} else {
VLOG_WARN_RL(&rl, "%s: unsupported controller \"%s\"",
mgr->name, c->target);
continue;
mgr->name, target);
}

shash_add_once(&new_controllers, c->target, &controllers[i]);
}

/* Delete controllers that are no longer configured.
* Update configuration of all now-existing controllers. */
struct ofconn *ofconn, *next_ofconn;
HMAP_FOR_EACH_SAFE (ofconn, next_ofconn, hmap_node, &mgr->controllers) {
const char *target = ofconn_get_target(ofconn);
struct ofproto_controller *c = shash_find_data(&new_controllers,
target);
struct ofproto_controller *c = shash_find_data(controllers, target);
if (!c) {
VLOG_INFO("%s: removed primary controller \"%s\"",
mgr->name, target);
Expand All @@ -660,8 +654,7 @@ connmgr_set_controllers(struct connmgr *mgr,
struct ofservice *ofservice, *next_ofservice;
HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, node, &mgr->services) {
const char *target = pvconn_get_name(ofservice->pvconn);
struct ofproto_controller *c = shash_find_data(&new_controllers,
target);
struct ofproto_controller *c = shash_find_data(controllers, target);
if (!c) {
VLOG_INFO("%s: removed service controller \"%s\"",
mgr->name, target);
Expand All @@ -671,8 +664,6 @@ connmgr_set_controllers(struct connmgr *mgr,
}
}

shash_destroy(&new_controllers);

ovs_mutex_unlock(&ofproto_mutex);

update_in_band_remotes(mgr);
Expand Down
5 changes: 2 additions & 3 deletions ofproto/connmgr.h
View file
Expand Up @@ -56,6 +56,7 @@ enum ofconn_type {
OFCONN_PRIMARY, /* An ordinary OpenFlow controller. */
OFCONN_SERVICE /* A service connection, e.g. "ovs-ofctl". */
};
const char *ofconn_type_to_string(enum ofconn_type);

/* An asynchronous message that might need to be queued between threads. */
struct ofproto_async_msg {
Expand Down Expand Up @@ -94,9 +95,7 @@ void connmgr_retry(struct connmgr *);
bool connmgr_has_controllers(const struct connmgr *);
void connmgr_get_controller_info(struct connmgr *, struct shash *);
void connmgr_free_controller_info(struct shash *);
void connmgr_set_controllers(struct connmgr *,
const struct ofproto_controller[], size_t n,
uint32_t allowed_versions);
void connmgr_set_controllers(struct connmgr *, struct shash *);
void connmgr_reconnect(const struct connmgr *);

int connmgr_set_snoops(struct connmgr *, const struct sset *snoops);
Expand Down
7 changes: 2 additions & 5 deletions ofproto/ofproto.c
View file
Expand Up @@ -639,12 +639,9 @@ ofproto_set_datapath_id(struct ofproto *p, uint64_t datapath_id)
}

void
ofproto_set_controllers(struct ofproto *p,
const struct ofproto_controller *controllers,
size_t n_controllers, uint32_t allowed_versions)
ofproto_set_controllers(struct ofproto *p, struct shash *controllers)
{
connmgr_set_controllers(p->connmgr, controllers, n_controllers,
allowed_versions);
connmgr_set_controllers(p->connmgr, controllers);
}

void
Expand Down
7 changes: 3 additions & 4 deletions ofproto/ofproto.h
View file
Expand Up @@ -208,11 +208,12 @@ enum ofproto_band {
};

struct ofproto_controller {
char *target; /* e.g. "tcp:127.0.0.1" */
int max_backoff; /* Maximum reconnection backoff, in seconds. */
int probe_interval; /* Max idle time before probing, in seconds. */
enum ofproto_band band; /* In-band or out-of-band? */
bool enable_async_msgs; /* Initially enable asynchronous messages? */
uint32_t allowed_versions; /* OpenFlow protocol versions that may
* be negotiated for a session. */

/* OpenFlow packet-in rate-limiting. */
int rate_limit; /* Max packet-in rate in packets per second. */
Expand Down Expand Up @@ -304,9 +305,7 @@ int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
/* Top-level configuration. */
uint64_t ofproto_get_datapath_id(const struct ofproto *);
void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
void ofproto_set_controllers(struct ofproto *,
const struct ofproto_controller *, size_t n,
uint32_t allowed_versions);
void ofproto_set_controllers(struct ofproto *, struct shash *controllers);
void ofproto_set_fail_mode(struct ofproto *, enum ofproto_fail_mode fail_mode);
void ofproto_reconnect_controllers(struct ofproto *);
void ofproto_set_extra_in_band_remotes(struct ofproto *,
Expand Down
102 changes: 41 additions & 61 deletions vswitchd/bridge.c
View file
Expand Up @@ -3462,48 +3462,6 @@ bridge_del_ports(struct bridge *br, const struct shash *wanted_ports)
}
}

/* Initializes 'oc' appropriately as a management service controller for
* 'br'.
*
* The caller must free oc->target when it is no longer needed. */
static void
bridge_ofproto_controller_for_mgmt(const struct bridge *br,
struct ofproto_controller *oc)
{
oc->target = xasprintf("punix:%s/%s.mgmt", ovs_rundir(), br->name);
oc->max_backoff = 0;
oc->probe_interval = 60;
oc->band = OFPROTO_OUT_OF_BAND;
oc->rate_limit = 0;
oc->burst_limit = 0;
oc->enable_async_msgs = true;
oc->dscp = 0;
}

/* Converts ovsrec_controller 'c' into an ofproto_controller in 'oc'. */
static void
bridge_ofproto_controller_from_ovsrec(const struct ovsrec_controller *c,
struct ofproto_controller *oc)
{
int dscp;

oc->target = c->target;
oc->max_backoff = c->max_backoff ? *c->max_backoff / 1000 : 8;
oc->probe_interval = c->inactivity_probe ? *c->inactivity_probe / 1000 : 5;
oc->band = (!c->connection_mode || !strcmp(c->connection_mode, "in-band")
? OFPROTO_IN_BAND : OFPROTO_OUT_OF_BAND);
oc->rate_limit = c->controller_rate_limit ? *c->controller_rate_limit : 0;
oc->burst_limit = (c->controller_burst_limit
? *c->controller_burst_limit : 0);
oc->enable_async_msgs = (!c->enable_async_messages
|| *c->enable_async_messages);
dscp = smap_get_int(&c->other_config, "dscp", DSCP_DEFAULT);
if (dscp < 0 || dscp > 63) {
dscp = DSCP_DEFAULT;
}
oc->dscp = dscp;
}

/* Configures the IP stack for 'br''s local interface properly according to the
* configuration in 'c'. */
static void
Expand Down Expand Up @@ -3589,10 +3547,6 @@ bridge_configure_remotes(struct bridge *br,

enum ofproto_fail_mode fail_mode;

struct ofproto_controller *ocs;
size_t n_ocs;
size_t i;

/* Check if we should disable in-band control on this bridge. */
disable_in_band = smap_get_bool(&br->cfg->other_config, "disable-in-band",
false);
Expand All @@ -3611,13 +3565,22 @@ bridge_configure_remotes(struct bridge *br,
n_controllers = (ofproto_get_flow_restore_wait() ? 0
: bridge_get_controllers(br, &controllers));

ocs = xmalloc((n_controllers + 1) * sizeof *ocs);
n_ocs = 0;

bridge_ofproto_controller_for_mgmt(br, &ocs[n_ocs++]);
for (i = 0; i < n_controllers; i++) {
/* The set of controllers to pass down to ofproto. */
struct shash ocs = SHASH_INITIALIZER(&ocs);

/* Add managment controller. */
struct ofproto_controller *oc = xmalloc(sizeof *oc);
*oc = (struct ofproto_controller) {
.probe_interval = 60,
.band = OFPROTO_OUT_OF_BAND,
.enable_async_msgs = true,
.allowed_versions = bridge_get_allowed_versions(br),
};
shash_add_nocopy(
&ocs, xasprintf("punix:%s/%s.mgmt", ovs_rundir(), br->name), oc);

for (size_t i = 0; i < n_controllers; i++) {
struct ovsrec_controller *c = controllers[i];

if (daemon_should_self_confine()
&& (!strncmp(c->target, "punix:", 6)
|| !strncmp(c->target, "unix:", 5))) {
Expand Down Expand Up @@ -3668,17 +3631,34 @@ bridge_configure_remotes(struct bridge *br,
}

bridge_configure_local_iface_netdev(br, c);
bridge_ofproto_controller_from_ovsrec(c, &ocs[n_ocs]);
if (disable_in_band) {
ocs[n_ocs].band = OFPROTO_OUT_OF_BAND;

int dscp = smap_get_int(&c->other_config, "dscp", DSCP_DEFAULT);
if (dscp < 0 || dscp > 63) {
dscp = DSCP_DEFAULT;
}
n_ocs++;
}

ofproto_set_controllers(br->ofproto, ocs, n_ocs,
bridge_get_allowed_versions(br));
free(ocs[0].target); /* From bridge_ofproto_controller_for_mgmt(). */
free(ocs);
oc = xmalloc(sizeof *oc);
*oc = (struct ofproto_controller) {
.max_backoff = c->max_backoff ? *c->max_backoff / 1000 : 8,
.probe_interval = (c->inactivity_probe
? *c->inactivity_probe / 1000 : 5),
.band = ((!c->connection_mode
|| !strcmp(c->connection_mode, "in-band"))
&& !disable_in_band
? OFPROTO_IN_BAND : OFPROTO_OUT_OF_BAND),
.enable_async_msgs = (!c->enable_async_messages
|| *c->enable_async_messages),
.allowed_versions = bridge_get_allowed_versions(br),
.rate_limit = (c->controller_rate_limit
? *c->controller_rate_limit : 0),
.burst_limit = (c->controller_burst_limit
? *c->controller_burst_limit : 0),
.dscp = dscp,
};
shash_add(&ocs, c->target, oc);
}
ofproto_set_controllers(br->ofproto, &ocs);
shash_destroy_free_data(&ocs);

/* Set the fail-mode. */
fail_mode = !br->cfg->fail_mode
Expand Down

0 comments on commit af26093

Please sign in to comment.