From: Ben Pfaff <blp@nicira.com>
Date: Tue, 20 Apr 2010 17:05:57 +0000 (-0700)
Subject: ofproto: Bundle all controller-related settings into a struct.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=79c9f2ee7883b52860c76c3730725f5731402874;p=openvswitch

ofproto: Bundle all controller-related settings into a struct.

Many ofproto settings are controller-related.  Upcoming commits will add
to ofproto the ability to support multiple controllers, so it is important
to be able to refer to controller settings as a group.  Hence, this commit
bundles them into a new "struct ofproto_controller".
---

diff --git a/ofproto/discovery.c b/ofproto/discovery.c
index ae36b4dc..2044ea5e 100644
--- a/ofproto/discovery.c
+++ b/ofproto/discovery.c
@@ -155,6 +155,12 @@ discovery_destroy(struct discovery *d)
     }
 }
 
+bool
+discovery_get_update_resolv_conf(const struct discovery *d)
+{
+    return d->update_resolv_conf;
+}
+
 void
 discovery_set_update_resolv_conf(struct discovery *d,
                                  bool update_resolv_conf)
@@ -162,6 +168,12 @@ discovery_set_update_resolv_conf(struct discovery *d,
     d->update_resolv_conf = update_resolv_conf;
 }
 
+const char *
+discovery_get_accept_controller_re(const struct discovery *d)
+{
+    return d->re;
+}
+
 int
 discovery_set_accept_controller_re(struct discovery *d, const char *re_)
 {
diff --git a/ofproto/discovery.h b/ofproto/discovery.h
index 98ff5b8b..2288ff60 100644
--- a/ofproto/discovery.h
+++ b/ofproto/discovery.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,8 +28,10 @@ int discovery_create(const char *accept_controller_re, bool update_resolv_conf,
                      struct dpif *, struct switch_status *,
                      struct discovery **);
 void discovery_destroy(struct discovery *);
+bool discovery_get_update_resolv_conf(const struct discovery *);
 void discovery_set_update_resolv_conf(struct discovery *,
                                       bool update_resolv_conf);
+const char *discovery_get_accept_controller_re(const struct discovery *);
 int discovery_set_accept_controller_re(struct discovery *, const char *re);
 void discovery_question_connectivity(struct discovery *);
 bool discovery_run(struct discovery *, char **controller_name);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c5bac6bd..ddd079bc 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -376,20 +376,98 @@ ofproto_set_datapath_id(struct ofproto *p, uint64_t datapath_id)
 }
 
 void
-ofproto_set_probe_interval(struct ofproto *p, int probe_interval)
+ofproto_set_controller(struct ofproto *p, const struct ofproto_controller *c)
 {
-    probe_interval = probe_interval ? MAX(probe_interval, 5) : 0;
-    rconn_set_probe_interval(p->controller->rconn, probe_interval);
-    if (p->fail_open) {
-        int trigger_duration = probe_interval ? probe_interval * 3 : 15;
-        fail_open_set_trigger_duration(p->fail_open, trigger_duration);
+    int rate_limit, burst_limit;
+    bool in_band;
+
+    if (c) {
+        int probe_interval;
+        bool discovery;
+
+        discovery = !strcmp(c->target, "discover");
+        in_band = discovery || c->band == OFPROTO_IN_BAND;
+
+        rconn_set_max_backoff(p->controller->rconn, c->max_backoff);
+
+        probe_interval = c->probe_interval ? MAX(c->probe_interval, 5) : 0;
+        rconn_set_probe_interval(p->controller->rconn, probe_interval);
+
+        if (discovery != (p->discovery != NULL)) {
+            rconn_disconnect(p->controller->rconn);
+            if (discovery) {
+                if (discovery_create(c->accept_re, c->update_resolv_conf,
+                                     p->dpif, p->switch_status,
+                                     &p->discovery)) {
+                    return;
+                }
+            } else {
+                discovery_destroy(p->discovery);
+                p->discovery = NULL;
+            }
+        }
+
+        if (discovery) {
+            discovery_set_update_resolv_conf(p->discovery,
+                                             c->update_resolv_conf);
+            discovery_set_accept_controller_re(p->discovery, c->accept_re);
+        } else {
+            if (strcmp(rconn_get_name(p->controller->rconn), c->target)) {
+                rconn_connect(p->controller->rconn, c->target);
+            }
+        }
+    } else {
+        rconn_disconnect(p->controller->rconn);
+        in_band = false;
     }
-}
 
-void
-ofproto_set_max_backoff(struct ofproto *p, int max_backoff)
-{
-    rconn_set_max_backoff(p->controller->rconn, max_backoff);
+    if (in_band != (p->in_band != NULL)) {
+        if (in_band) {
+            int error;
+
+            error = in_band_create(p, p->dpif, p->switch_status, &p->in_band);
+            if (!error) {
+                in_band_set_remotes(p->in_band, &p->controller->rconn, 1);
+            }
+        } else {
+            in_band_destroy(p->in_band);
+            p->in_band = NULL;
+        }
+        rconn_reconnect(p->controller->rconn);
+    }
+
+    if (c && c->fail == OFPROTO_FAIL_STANDALONE) {
+        struct rconn *rconn = p->controller->rconn;
+        int trigger_duration = rconn_get_probe_interval(rconn) * 3;
+        if (!p->fail_open) {
+            p->fail_open = fail_open_create(p, trigger_duration,
+                                            p->switch_status, rconn);
+        } else {
+            fail_open_set_trigger_duration(p->fail_open, trigger_duration);
+        }
+    } else {
+        fail_open_destroy(p->fail_open);
+        p->fail_open = NULL;
+    }
+
+    rate_limit = c ? c->rate_limit : 0;
+    burst_limit = c ? c->burst_limit : 0;
+    if (rate_limit > 0) {
+        if (!p->miss_sched) {
+            p->miss_sched = pinsched_create(rate_limit, burst_limit,
+                                                  p->switch_status);
+            p->action_sched = pinsched_create(rate_limit, burst_limit,
+                                                    NULL);
+        } else {
+            pinsched_set_limits(p->miss_sched, rate_limit, burst_limit);
+            pinsched_set_limits(p->action_sched, rate_limit, burst_limit);
+        }
+    } else {
+        pinsched_destroy(p->miss_sched);
+        p->miss_sched = NULL;
+        pinsched_destroy(p->action_sched);
+        p->action_sched = NULL;
+    }
 }
 
 void
@@ -443,73 +521,6 @@ ofproto_set_desc(struct ofproto *p,
     }
 }
 
-int
-ofproto_set_in_band(struct ofproto *p, bool in_band)
-{
-    if (in_band != (p->in_band != NULL)) {
-        if (in_band) {
-            int error;
-
-            error = in_band_create(p, p->dpif, p->switch_status, &p->in_band);
-            if (error) {
-                return error;
-            }
-            in_band_set_remotes(p->in_band, &p->controller->rconn, 1);
-        } else {
-            ofproto_set_discovery(p, false, NULL, true);
-            in_band_destroy(p->in_band);
-            p->in_band = NULL;
-        }
-        rconn_reconnect(p->controller->rconn);
-    }
-    return 0;
-}
-
-int
-ofproto_set_discovery(struct ofproto *p, bool discovery,
-                      const char *re, bool update_resolv_conf)
-{
-    if (discovery != (p->discovery != NULL)) {
-        if (discovery) {
-            int error = ofproto_set_in_band(p, true);
-            if (error) {
-                return error;
-            }
-            error = discovery_create(re, update_resolv_conf,
-                                     p->dpif, p->switch_status,
-                                     &p->discovery);
-            if (error) {
-                return error;
-            }
-        } else {
-            discovery_destroy(p->discovery);
-            p->discovery = NULL;
-        }
-        rconn_disconnect(p->controller->rconn);
-    } else if (discovery) {
-        discovery_set_update_resolv_conf(p->discovery, update_resolv_conf);
-        return discovery_set_accept_controller_re(p->discovery, re);
-    }
-    return 0;
-}
-
-int
-ofproto_set_controller(struct ofproto *ofproto, const char *controller)
-{
-    if (ofproto->discovery) {
-        return EINVAL;
-    } else if (controller) {
-        if (strcmp(rconn_get_name(ofproto->controller->rconn), controller)) {
-            return rconn_connect(ofproto->controller->rconn, controller);
-        } else {
-            return 0;
-        }
-    } else {
-        rconn_disconnect(ofproto->controller->rconn);
-        return 0;
-    }
-}
-
 static int
 set_pvconns(struct pvconn ***pvconnsp, size_t *n_pvconnsp,
             const struct svec *svec)
@@ -600,49 +611,6 @@ ofproto_set_sflow(struct ofproto *ofproto,
     }
 }
 
-void
-ofproto_set_failure(struct ofproto *ofproto, bool fail_open)
-{
-    if (fail_open) {
-        struct rconn *rconn = ofproto->controller->rconn;
-        int trigger_duration = rconn_get_probe_interval(rconn) * 3;
-        if (!ofproto->fail_open) {
-            ofproto->fail_open = fail_open_create(ofproto, trigger_duration,
-                                                  ofproto->switch_status,
-                                                  rconn);
-        } else {
-            fail_open_set_trigger_duration(ofproto->fail_open,
-                                           trigger_duration);
-        }
-    } else {
-        fail_open_destroy(ofproto->fail_open);
-        ofproto->fail_open = NULL;
-    }
-}
-
-void
-ofproto_set_rate_limit(struct ofproto *ofproto,
-                       int rate_limit, int burst_limit)
-{
-    if (rate_limit > 0) {
-        if (!ofproto->miss_sched) {
-            ofproto->miss_sched = pinsched_create(rate_limit, burst_limit,
-                                                  ofproto->switch_status);
-            ofproto->action_sched = pinsched_create(rate_limit, burst_limit,
-                                                    NULL);
-        } else {
-            pinsched_set_limits(ofproto->miss_sched, rate_limit, burst_limit);
-            pinsched_set_limits(ofproto->action_sched,
-                                rate_limit, burst_limit);
-        }
-    } else {
-        pinsched_destroy(ofproto->miss_sched);
-        ofproto->miss_sched = NULL;
-        pinsched_destroy(ofproto->action_sched);
-        ofproto->action_sched = NULL;
-    }
-}
-
 int
 ofproto_set_stp(struct ofproto *ofproto OVS_UNUSED, bool enable_stp)
 {
@@ -661,34 +629,27 @@ ofproto_get_datapath_id(const struct ofproto *ofproto)
     return ofproto->datapath_id;
 }
 
-int
-ofproto_get_probe_interval(const struct ofproto *ofproto)
-{
-    return rconn_get_probe_interval(ofproto->controller->rconn);
-}
-
-int
-ofproto_get_max_backoff(const struct ofproto *ofproto)
-{
-    return rconn_get_max_backoff(ofproto->controller->rconn);
-}
-
-bool
-ofproto_get_in_band(const struct ofproto *ofproto)
+void
+ofproto_get_controller(const struct ofproto *p, struct ofproto_controller *c)
 {
-    return ofproto->in_band != NULL;
-}
+    memset(c, 0, sizeof *c);
+    if (p->discovery) {
+        struct discovery *d = p->discovery;
 
-bool
-ofproto_get_discovery(const struct ofproto *ofproto)
-{
-    return ofproto->discovery != NULL;
-}
+        c->target = "discover";
+        c->accept_re = (char *) discovery_get_accept_controller_re(d);
+        c->update_resolv_conf = discovery_get_update_resolv_conf(d);
+    } else if (p->controller) {
+        c->target = (char *) rconn_get_name(p->controller->rconn);
+    } else {
+        return;
+    }
 
-const char *
-ofproto_get_controller(const struct ofproto *ofproto)
-{
-    return rconn_get_name(ofproto->controller->rconn);
+    c->max_backoff = rconn_get_max_backoff(p->controller->rconn);
+    c->probe_interval = rconn_get_probe_interval(p->controller->rconn);
+    c->fail = p->fail_open ? OFPROTO_FAIL_STANDALONE : OFPROTO_FAIL_SECURE;
+    c->band = p->in_band ? OFPROTO_IN_BAND : OFPROTO_OUT_OF_BAND;
+    pinsched_get_limits(p->miss_sched, &c->rate_limit, &c->burst_limit);
 }
 
 void
@@ -724,8 +685,11 @@ ofproto_destroy(struct ofproto *p)
     }
 
     /* Destroy fail-open and in-band early, since they touch the classifier. */
-    ofproto_set_failure(p, false);
-    ofproto_set_in_band(p, false);
+    fail_open_destroy(p->fail_open);
+    p->fail_open = NULL;
+
+    in_band_destroy(p->in_band);
+    p->in_band = NULL;
 
     ofproto_flush_flows(p);
     classifier_destroy(&p->cls);
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index d9e71d76..29f9ee92 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -55,6 +55,33 @@ struct ofproto_sflow_options {
     char *control_ip;
 };
 
+/* How the switch should act if the controller cannot be contacted. */
+enum ofproto_fail_mode {
+    OFPROTO_FAIL_SECURE,        /* Preserve flow table. */
+    OFPROTO_FAIL_STANDALONE     /* Act as a standalone switch. */
+};
+
+enum ofproto_band {
+    OFPROTO_IN_BAND,            /* In-band connection to controller. */
+    OFPROTO_OUT_OF_BAND         /* Out-of-band connection to controller. */
+};
+
+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_fail_mode fail; /* Controller failure handling mode. */
+    enum ofproto_band band;      /* In-band or out-of-band? */
+
+    /* Discovery options. */
+    char *accept_re;            /* Regexp for acceptable controllers.  */
+    bool update_resolv_conf;    /* Update /etc/resolv.conf? */
+
+    /* OpenFlow packet-in rate-limiting. */
+    int rate_limit;             /* Max packet-in rate in packets per second. */
+    int burst_limit;            /* Limit on accumulating packet credits. */
+};
+
 #define DEFAULT_MFR_DESC "Nicira Networks, Inc."
 #define DEFAULT_HW_DESC "Open vSwitch"
 #define DEFAULT_SW_DESC VERSION BUILDNR
@@ -73,33 +100,23 @@ bool ofproto_is_alive(const struct ofproto *);
 
 /* Configuration. */
 void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
-void ofproto_set_probe_interval(struct ofproto *, int probe_interval);
-void ofproto_set_max_backoff(struct ofproto *, int max_backoff);
+void ofproto_set_controller(struct ofproto *,
+                            const struct ofproto_controller *);
 void ofproto_set_desc(struct ofproto *,
                       const char *mfr_desc, const char *hw_desc,
                       const char *sw_desc, const char *serial_desc,
                       const char *dp_desc);
-int ofproto_set_in_band(struct ofproto *, bool in_band);
-int ofproto_set_discovery(struct ofproto *, bool discovery,
-                          const char *accept_controller_re,
-                          bool update_resolv_conf);
-int ofproto_set_controller(struct ofproto *, const char *controller);
 int ofproto_set_listeners(struct ofproto *, const struct svec *listeners);
 int ofproto_set_snoops(struct ofproto *, const struct svec *snoops);
 int ofproto_set_netflow(struct ofproto *,
                         const struct netflow_options *nf_options);
 void ofproto_set_sflow(struct ofproto *, const struct ofproto_sflow_options *);
-void ofproto_set_failure(struct ofproto *, bool fail_open);
-void ofproto_set_rate_limit(struct ofproto *, int rate_limit, int burst_limit);
 int ofproto_set_stp(struct ofproto *, bool enable_stp);
 
 /* Configuration querying. */
 uint64_t ofproto_get_datapath_id(const struct ofproto *);
-int ofproto_get_probe_interval(const struct ofproto *);
-int ofproto_get_max_backoff(const struct ofproto *);
-bool ofproto_get_in_band(const struct ofproto *);
-bool ofproto_get_discovery(const struct ofproto *);
-const char *ofproto_get_controller(const struct ofproto *);
+void ofproto_get_controller(const struct ofproto *,
+                            struct ofproto_controller *);
 void ofproto_get_listeners(const struct ofproto *, struct svec *);
 void ofproto_get_snoops(const struct ofproto *, struct svec *);
 void ofproto_get_all_flows(struct ofproto *p, struct ds *);
diff --git a/ofproto/pinsched.c b/ofproto/pinsched.c
index b9c6371a..6af2bb67 100644
--- a/ofproto/pinsched.c
+++ b/ofproto/pinsched.c
@@ -267,6 +267,14 @@ pinsched_destroy(struct pinsched *ps)
     }
 }
 
+void
+pinsched_get_limits(const struct pinsched *ps,
+                    int *rate_limit, int *burst_limit)
+{
+    *rate_limit = ps->rate_limit;
+    *burst_limit = ps->burst_limit;
+}
+
 void
 pinsched_set_limits(struct pinsched *ps, int rate_limit, int burst_limit)
 {
diff --git a/ofproto/pinsched.h b/ofproto/pinsched.h
index aead7a45..17e3db1d 100644
--- a/ofproto/pinsched.h
+++ b/ofproto/pinsched.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,6 +25,8 @@ struct switch_status;
 typedef void pinsched_tx_cb(struct ofpbuf *, void *aux);
 struct pinsched *pinsched_create(int rate_limit, int burst_limit,
                                  struct switch_status *);
+void pinsched_get_limits(const struct pinsched *,
+                         int *rate_limit, int *burst_limit);
 void pinsched_set_limits(struct pinsched *, int rate_limit, int burst_limit);
 void pinsched_destroy(struct pinsched *);
 void pinsched_send(struct pinsched *, uint16_t port_no, struct ofpbuf *,
diff --git a/ofproto/status.c b/ofproto/status.c
index 01cda01a..dbedf10e 100644
--- a/ofproto/status.c
+++ b/ofproto/status.c
@@ -130,9 +130,9 @@ static void
 config_status_cb(struct status_reply *sr, void *ofproto_)
 {
     const struct ofproto *ofproto = ofproto_;
+    struct ofproto_controller controller;
     uint64_t datapath_id;
     struct svec listeners;
-    int probe_interval, max_backoff;
     size_t i;
 
     datapath_id = ofproto_get_datapath_id(ofproto);
@@ -147,14 +147,12 @@ config_status_cb(struct status_reply *sr, void *ofproto_)
     }
     svec_destroy(&listeners);
 
-    probe_interval = ofproto_get_probe_interval(ofproto);
-    if (probe_interval) {
-        status_reply_put(sr, "probe-interval=%d", probe_interval);
+    ofproto_get_controller(ofproto, &controller);
+    if (controller.probe_interval) {
+        status_reply_put(sr, "probe-interval=%d", controller.probe_interval);
     }
-
-    max_backoff = ofproto_get_max_backoff(ofproto);
-    if (max_backoff) {
-        status_reply_put(sr, "max-backoff=%d", max_backoff);
+    if (controller.max_backoff) {
+        status_reply_put(sr, "max-backoff=%d", controller.max_backoff);
     }
 }
 
diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c
index 4e3f3809..70430c03 100644
--- a/utilities/ovs-openflowd.c
+++ b/utilities/ovs-openflowd.c
@@ -48,17 +48,10 @@
 #include "vlog.h"
 #define THIS_MODULE VLM_openflowd
 
-/* Behavior when the connection to the controller fails. */
-enum fail_mode {
-    FAIL_OPEN,                  /* Act as learning switch. */
-    FAIL_CLOSED                 /* Drop all packets. */
-};
-
 /* Settings that may be configured by the user. */
 struct ofsettings {
-    /* Overall mode of operation. */
-    bool discovery;           /* Discover the controller automatically? */
-    bool in_band;             /* Connect to controller in-band? */
+    /* Controller configuration. */
+    struct ofproto_controller controller;
 
     /* Datapath. */
     uint64_t datapath_id;       /* Datapath ID. */
@@ -74,23 +67,11 @@ struct ofsettings {
     const char *dp_desc;        /* Datapath description. */
 
     /* Related vconns and network devices. */
-    const char *controller_name; /* Controller (if not discovery mode). */
     struct svec listeners;       /* Listen for management connections. */
     struct svec snoops;          /* Listen for controller snooping conns. */
 
     /* Failure behavior. */
-    enum fail_mode fail_mode; /* Act as learning switch if no controller? */
     int max_idle;             /* Idle time for flows in fail-open mode. */
-    int probe_interval;       /* # seconds idle before sending echo request. */
-    int max_backoff;          /* Max # seconds between connection attempts. */
-
-    /* Packet-in rate-limiting. */
-    int rate_limit;           /* Tokens added to bucket per second. */
-    int burst_limit;          /* Maximum number token bucket size. */
-
-    /* Discovery behavior. */
-    const char *accept_controller_re; /* Controller vconns to accept. */
-    bool update_resolv_conf;          /* Update /etc/resolv.conf? */
 
     /* Spanning tree protocol. */
     bool enable_stp;
@@ -160,15 +141,6 @@ main(int argc, char *argv[])
     if (error) {
         ovs_fatal(error, "could not initialize openflow switch");
     }
-    error = ofproto_set_in_band(ofproto, s.in_band);
-    if (error) {
-        ovs_fatal(error, "failed to configure in-band control");
-    }
-    error = ofproto_set_discovery(ofproto, s.discovery, s.accept_controller_re,
-                                  s.update_resolv_conf);
-    if (error) {
-        ovs_fatal(error, "failed to configure controller discovery");
-    }
     if (s.datapath_id) {
         ofproto_set_datapath_id(ofproto, s.datapath_id);
     }
@@ -195,20 +167,11 @@ main(int argc, char *argv[])
     if (error) {
         ovs_fatal(error, "failed to configure NetFlow collectors");
     }
-    ofproto_set_failure(ofproto, s.fail_mode == FAIL_OPEN);
-    ofproto_set_probe_interval(ofproto, s.probe_interval);
-    ofproto_set_max_backoff(ofproto, s.max_backoff);
-    ofproto_set_rate_limit(ofproto, s.rate_limit, s.burst_limit);
     error = ofproto_set_stp(ofproto, s.enable_stp);
     if (error) {
         ovs_fatal(error, "failed to configure STP");
     }
-    if (!s.discovery) {
-        error = ofproto_set_controller(ofproto, s.controller_name);
-        if (error) {
-            ovs_fatal(error, "failed to configure controller");
-        }
-    }
+    ofproto_set_controller(ofproto, &s.controller);
 
     daemonize_complete();
 
@@ -305,6 +268,14 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
     char *short_options = long_options_to_short_options(long_options);
 
     /* Set defaults that we can figure out before parsing options. */
+    s->controller.max_backoff = 8;
+    s->controller.probe_interval = 5;
+    s->controller.fail = OFPROTO_FAIL_STANDALONE;
+    s->controller.band = OFPROTO_IN_BAND;
+    s->controller.accept_re = NULL;
+    s->controller.update_resolv_conf = true;
+    s->controller.rate_limit = 0;
+    s->controller.burst_limit = 0;
     s->datapath_id = 0;
     s->mfr_desc = NULL;
     s->hw_desc = NULL;
@@ -313,16 +284,8 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
     s->dp_desc = NULL;
     svec_init(&s->listeners);
     svec_init(&s->snoops);
-    s->fail_mode = FAIL_OPEN;
     s->max_idle = 0;
-    s->probe_interval = 0;
-    s->max_backoff = 8;
-    s->update_resolv_conf = true;
-    s->rate_limit = 0;
-    s->burst_limit = 0;
-    s->accept_controller_re = NULL;
     s->enable_stp = false;
-    s->in_band = true;
     svec_init(&s->netflow);
     svec_init(&s->ports);
     for (;;) {
@@ -362,26 +325,26 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
             break;
 
         case OPT_ACCEPT_VCONN:
-            s->accept_controller_re = optarg;
+            s->controller.accept_re = optarg;
             break;
 
         case OPT_NO_RESOLV_CONF:
-            s->update_resolv_conf = false;
+            s->controller.update_resolv_conf = false;
             break;
 
         case OPT_FAIL_MODE:
             if (!strcmp(optarg, "open")) {
-                s->fail_mode = FAIL_OPEN;
+                s->controller.fail = OFPROTO_FAIL_STANDALONE;
             } else if (!strcmp(optarg, "closed")) {
-                s->fail_mode = FAIL_CLOSED;
+                s->controller.fail = OFPROTO_FAIL_SECURE;
             } else {
                 ovs_fatal(0, "--fail argument must be \"open\" or \"closed\"");
             }
             break;
 
         case OPT_INACTIVITY_PROBE:
-            s->probe_interval = atoi(optarg);
-            if (s->probe_interval < 5) {
+            s->controller.probe_interval = atoi(optarg);
+            if (s->controller.probe_interval < 5) {
                 ovs_fatal(0, "--inactivity-probe argument must be at least 5");
             }
             break;
@@ -399,28 +362,28 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
             break;
 
         case OPT_MAX_BACKOFF:
-            s->max_backoff = atoi(optarg);
-            if (s->max_backoff < 1) {
+            s->controller.max_backoff = atoi(optarg);
+            if (s->controller.max_backoff < 1) {
                 ovs_fatal(0, "--max-backoff argument must be at least 1");
-            } else if (s->max_backoff > 3600) {
-                s->max_backoff = 3600;
+            } else if (s->controller.max_backoff > 3600) {
+                s->controller.max_backoff = 3600;
             }
             break;
 
         case OPT_RATE_LIMIT:
             if (optarg) {
-                s->rate_limit = atoi(optarg);
-                if (s->rate_limit < 1) {
+                s->controller.rate_limit = atoi(optarg);
+                if (s->controller.rate_limit < 1) {
                     ovs_fatal(0, "--rate-limit argument must be at least 1");
                 }
             } else {
-                s->rate_limit = 1000;
+                s->controller.rate_limit = 1000;
             }
             break;
 
         case OPT_BURST_LIMIT:
-            s->burst_limit = atoi(optarg);
-            if (s->burst_limit < 1) {
+            s->controller.burst_limit = atoi(optarg);
+            if (s->controller.burst_limit < 1) {
                 ovs_fatal(0, "--burst-limit argument must be at least 1");
             }
             break;
@@ -434,11 +397,11 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
             break;
 
         case OPT_OUT_OF_BAND:
-            s->in_band = false;
+            s->controller.band = OFPROTO_OUT_OF_BAND;
             break;
 
         case OPT_IN_BAND:
-            s->in_band = true;
+            s->controller.band = OFPROTO_IN_BAND;
             break;
 
         case OPT_NETFLOW:
@@ -497,23 +460,22 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
     /* Local and remote vconns. */
     dp_parse_name(argv[0], &s->dp_name, &s->dp_type);
 
-    s->controller_name = argc > 1 ? xstrdup(argv[1]) : NULL;
+    s->controller.target = argc > 1 ? argv[1] : "discover";
+    if (!strcmp(s->controller.target, "discover")
+        && s->controller.band == OFPROTO_OUT_OF_BAND) {
+        ovs_fatal(0, "Cannot perform discovery with out-of-band control");
+    }
 
     /* Set accept_controller_regex. */
-    if (!s->accept_controller_re) {
-        s->accept_controller_re
+    if (!s->controller.accept_re) {
+        s->controller.accept_re
             = stream_ssl_is_configured() ? "^ssl:.*" : "^tcp:.*";
     }
 
-    /* Mode of operation. */
-    s->discovery = s->controller_name == NULL;
-    if (s->discovery && !s->in_band) {
-        ovs_fatal(0, "Cannot perform discovery with out-of-band control");
-    }
-
     /* Rate limiting. */
-    if (s->rate_limit && s->rate_limit < 100) {
-        VLOG_WARN("Rate limit set to unusually low value %d", s->rate_limit);
+    if (s->controller.rate_limit && s->controller.rate_limit < 100) {
+        VLOG_WARN("Rate limit set to unusually low value %d",
+                  s->controller.rate_limit);
     }
 }
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 0cdf72f7..b9e39c9c 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1516,22 +1516,11 @@ bridge_reconfigure_controller(const struct ovsrec_open_vswitch *ovs_cfg,
     br->controller = c ? xstrdup(c->target) : NULL;
 
     if (c) {
-        int max_backoff, probe;
-        int rate_limit, burst_limit;
+        struct ofproto_controller oc;
 
-        if (!strcmp(c->target, "discover")) {
-            ofproto_set_discovery(br->ofproto, true,
-                                  c->discover_accept_regex,
-                                  c->discover_update_resolv_conf);
-        } else {
+        if (strcmp(c->target, "discover")) {
             struct iface *local_iface;
             struct in_addr ip;
-            bool in_band;
-
-            in_band = (!c->connection_mode
-                       || !strcmp(c->connection_mode, "out-of-band"));
-            ofproto_set_discovery(br->ofproto, false, NULL, NULL);
-            ofproto_set_in_band(br->ofproto, in_band);
 
             local_iface = bridge_get_local_iface(br);
             if (local_iface && c->local_ip && inet_aton(c->local_ip, &ip)) {
@@ -1566,20 +1555,26 @@ bridge_reconfigure_controller(const struct ovsrec_open_vswitch *ovs_cfg,
             }
         }
 
-        ofproto_set_failure(br->ofproto,
-                            (!c->fail_mode
-                             || !strcmp(c->fail_mode, "standalone")
-                             || !strcmp(c->fail_mode, "open")));
-
-        probe = c->inactivity_probe ? *c->inactivity_probe / 1000 : 5;
-        ofproto_set_probe_interval(br->ofproto, probe);
-
-        max_backoff = c->max_backoff ? *c->max_backoff / 1000 : 8;
-        ofproto_set_max_backoff(br->ofproto, max_backoff);
-
-        rate_limit = c->controller_rate_limit ? *c->controller_rate_limit : 0;
-        burst_limit = c->controller_burst_limit ? *c->controller_burst_limit : 0;
-        ofproto_set_rate_limit(br->ofproto, rate_limit, burst_limit);
+        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.fail = (!c->fail_mode
+                   || !strcmp(c->fail_mode, "standalone")
+                   || !strcmp(c->fail_mode, "open")
+                   ? OFPROTO_FAIL_STANDALONE
+                   : OFPROTO_FAIL_SECURE);
+        oc.band = (!c->connection_mode
+                   || !strcmp(c->connection_mode, "in-band")
+                   ? OFPROTO_IN_BAND
+                   : OFPROTO_OUT_OF_BAND);
+        oc.accept_re = c->discover_accept_regex;
+        oc.update_resolv_conf = c->discover_update_resolv_conf;
+        oc.rate_limit = (c->controller_rate_limit
+                         ? *c->controller_rate_limit : 0);
+        oc.burst_limit = (c->controller_burst_limit
+                          ? *c->controller_burst_limit : 0);
+        ofproto_set_controller(br->ofproto, &oc);
     } else {
         union ofp_action action;
         flow_t flow;
@@ -1593,13 +1588,8 @@ bridge_reconfigure_controller(const struct ovsrec_open_vswitch *ovs_cfg,
         memset(&flow, 0, sizeof flow);
         ofproto_add_flow(br->ofproto, &flow, OVSFW_ALL, 0, &action, 1, 0);
 
-        ofproto_set_in_band(br->ofproto, false);
-        ofproto_set_max_backoff(br->ofproto, 1);
-        ofproto_set_probe_interval(br->ofproto, 5);
-        ofproto_set_failure(br->ofproto, false);
+        ofproto_set_controller(br->ofproto, NULL);
     }
-
-    ofproto_set_controller(br->ofproto, br->controller);
 }
 
 static void