[ovs-dev] [service controllers 3/3] ofproto: Add support for remote "service controllers".

Ben Pfaff blp at nicira.com
Fri Aug 6 18:26:38 UTC 2010


CC: Dan Wendlandt <dan at nicira.com>
---
 ofproto/ofproto.c         |  217 ++++++++++++++++++++++++++++++++-----------
 ofproto/ofproto.h         |    3 +-
 ofproto/status.c          |    9 --
 utilities/ovs-openflowd.c |   38 ++++----
 vswitchd/bridge.c         |  226 ++++++++++++++++++++++++++-------------------
 vswitchd/vswitch.xml      |  101 ++++++++++++++++++--
 6 files changed, 404 insertions(+), 190 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5ed7bf2..a6bd094 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -177,12 +177,37 @@ static void send_flow_removed(struct ofproto *p, struct rule *rule,
  *   - "Service" connections, e.g. from ovs-ofctl.  When these connections
  *     drop, it is the other side's responsibility to reconnect them if
  *     necessary.  ofproto does not send them asynchronous messages by default.
+ *
+ * Currently, active (tcp, ssl, unix) connections are always "primary"
+ * connections and passive (ptcp, pssl, punix) connections are always "service"
+ * connections.  There is no inherent reason for this, but it reflects the
+ * common case.
  */
 enum ofconn_type {
     OFCONN_PRIMARY,             /* An ordinary OpenFlow controller. */
     OFCONN_SERVICE              /* A service connection, e.g. "ovs-ofctl". */
 };
 
+/* A listener for incoming OpenFlow "service" connections. */
+struct ofservice {
+    struct hmap_node node;      /* In struct ofproto's "services" hmap. */
+    struct pvconn *pvconn;      /* OpenFlow connection listener. */
+
+    /* These are not used by ofservice directly.  They are settings for
+     * accepted "struct ofconn"s from the pvconn. */
+    int probe_interval;         /* Max idle time before probing, in seconds. */
+    int rate_limit;             /* Max packet-in rate in packets per second. */
+    int burst_limit;            /* Limit on accumulating packet credits. */
+};
+
+static struct ofservice *ofservice_lookup(struct ofproto *,
+                                          const char *target);
+static int ofservice_create(struct ofproto *,
+                            const struct ofproto_controller *);
+static void ofservice_reconfigure(struct ofservice *,
+                                  const struct ofproto_controller *);
+static void ofservice_destroy(struct ofproto *, struct ofservice *);
+
 /* An OpenFlow connection. */
 struct ofconn {
     struct ofproto *ofproto;    /* The ofproto that owns this connection. */
@@ -227,6 +252,7 @@ static void ofconn_run(struct ofconn *, struct ofproto *);
 static void ofconn_wait(struct ofconn *);
 static bool ofconn_receives_async_msgs(const struct ofconn *);
 static char *ofconn_make_name(const struct ofproto *, const char *target);
+static void ofconn_set_rate_limit(struct ofconn *, int rate, int burst);
 
 static void queue_tx(struct ofpbuf *msg, const struct ofconn *ofconn,
                      struct rconn_packet_counter *counter);
@@ -275,8 +301,9 @@ struct ofproto {
     struct hmap controllers;   /* Controller "struct ofconn"s. */
     struct list all_conns;     /* Contains "struct ofconn"s. */
     enum ofproto_fail_mode fail_mode;
-    struct pvconn **listeners;
-    size_t n_listeners;
+
+    /* OpenFlow listeners. */
+    struct hmap services;       /* Contains "struct ofservice"s. */
     struct pvconn **snoops;
     size_t n_snoops;
 
@@ -382,8 +409,7 @@ ofproto_create(const char *datapath, const char *datapath_type,
     /* Initialize OpenFlow connections. */
     list_init(&p->all_conns);
     hmap_init(&p->controllers);
-    p->listeners = NULL;
-    p->n_listeners = 0;
+    hmap_init(&p->services);
     p->snoops = NULL;
     p->n_snoops = 0;
 
@@ -473,9 +499,7 @@ add_controller(struct ofproto *ofproto, const struct ofproto_controller *c)
 static void
 update_controller(struct ofconn *ofconn, const struct ofproto_controller *c)
 {
-    struct ofproto *ofproto = ofconn->ofproto;
     int probe_interval;
-    int i;
 
     ofconn->band = (is_in_band_controller(c)
                     ? OFPROTO_IN_BAND : OFPROTO_OUT_OF_BAND);
@@ -491,21 +515,7 @@ update_controller(struct ofconn *ofconn, const struct ofproto_controller *c)
         discovery_set_accept_controller_re(ofconn->discovery, c->accept_re);
     }
 
-    for (i = 0; i < N_SCHEDULERS; i++) {
-        struct pinsched **s = &ofconn->schedulers[i];
-
-        if (c->rate_limit > 0) {
-            if (!*s) {
-                *s = pinsched_create(c->rate_limit, c->burst_limit,
-                                     ofproto->switch_status);
-            } else {
-                pinsched_set_limits(*s, c->rate_limit, c->burst_limit);
-            }
-        } else {
-            pinsched_destroy(*s);
-            *s = NULL;
-        }
-    }
+    ofconn_set_rate_limit(ofconn, c->rate_limit, c->burst_limit);
 }
 
 static const char *
@@ -621,22 +631,38 @@ ofproto_set_controllers(struct ofproto *p,
                         size_t n_controllers)
 {
     struct shash new_controllers;
-    struct ofconn *ofconn, *next;
+    struct ofconn *ofconn, *next_ofconn;
+    struct ofservice *ofservice, *next_ofservice;
     bool ss_exists;
     size_t i;
 
+    /* Create newly configured controllers and services.
+     * Create a name to ofproto_controller mapping in 'new_controllers'. */
     shash_init(&new_controllers);
     for (i = 0; i < n_controllers; i++) {
         const struct ofproto_controller *c = &controllers[i];
 
-        shash_add_once(&new_controllers, c->target, &controllers[i]);
-        if (!find_controller_by_target(p, c->target)) {
-            add_controller(p, c);
+        if (!vconn_verify_name(c->target) || !strcmp(c->target, "discover")) {
+            if (!find_controller_by_target(p, c->target)) {
+                add_controller(p, c);
+            }
+        } else if (!pvconn_verify_name(c->target)) {
+            if (!ofservice_lookup(p, c->target) && ofservice_create(p, c)) {
+                continue;
+            }
+        } else {
+            VLOG_WARN_RL(&rl, "%s: unsupported controller \"%s\"",
+                         dpif_name(p->dpif), c->target);
+            continue;
         }
+
+        shash_add_once(&new_controllers, c->target, &controllers[i]);
     }
 
+    /* Delete controllers that are no longer configured.
+     * Update configuration of all now-existing controllers. */
     ss_exists = false;
-    HMAP_FOR_EACH_SAFE (ofconn, next, struct ofconn, hmap_node,
+    HMAP_FOR_EACH_SAFE (ofconn, next_ofconn, struct ofconn, hmap_node,
                         &p->controllers) {
         struct ofproto_controller *c;
 
@@ -650,10 +676,25 @@ ofproto_set_controllers(struct ofproto *p,
             }
         }
     }
+
+    /* Delete services that are no longer configured.
+     * Update configuration of all now-existing services. */
+    HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, struct ofservice, node,
+                        &p->services) {
+        struct ofproto_controller *c;
+
+        c = shash_find_data(&new_controllers,
+                            pvconn_get_name(ofservice->pvconn));
+        if (!c) {
+            ofservice_destroy(p, ofservice);
+        } else {
+            ofservice_reconfigure(ofservice, c);
+        }
+    }
+
     shash_destroy(&new_controllers);
 
     update_in_band_remotes(p);
-
     update_fail_open(p);
 
     if (!hmap_is_empty(&p->controllers) && !ss_exists) {
@@ -814,12 +855,6 @@ set_pvconns(struct pvconn ***pvconnsp, size_t *n_pvconnsp,
 }
 
 int
-ofproto_set_listeners(struct ofproto *ofproto, const struct svec *listeners)
-{
-    return set_pvconns(&ofproto->listeners, &ofproto->n_listeners, listeners);
-}
-
-int
 ofproto_set_snoops(struct ofproto *ofproto, const struct svec *snoops)
 {
     return set_pvconns(&ofproto->snoops, &ofproto->n_snoops, snoops);
@@ -884,7 +919,7 @@ ofproto_get_datapath_id(const struct ofproto *ofproto)
 }
 
 bool
-ofproto_has_controller(const struct ofproto *ofproto)
+ofproto_has_primary_controller(const struct ofproto *ofproto)
 {
     return !hmap_is_empty(&ofproto->controllers);
 }
@@ -896,16 +931,6 @@ ofproto_get_fail_mode(const struct ofproto *p)
 }
 
 void
-ofproto_get_listeners(const struct ofproto *ofproto, struct svec *listeners)
-{
-    size_t i;
-
-    for (i = 0; i < ofproto->n_listeners; i++) {
-        svec_add(listeners, pvconn_get_name(ofproto->listeners[i]));
-    }
-}
-
-void
 ofproto_get_snoops(const struct ofproto *ofproto, struct svec *snoops)
 {
     size_t i;
@@ -918,6 +943,7 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct svec *snoops)
 void
 ofproto_destroy(struct ofproto *p)
 {
+    struct ofservice *ofservice, *next_ofservice;
     struct ofconn *ofconn, *next_ofconn;
     struct ofport *ofport;
     unsigned int port_no;
@@ -955,10 +981,11 @@ ofproto_destroy(struct ofproto *p)
     netflow_destroy(p->netflow);
     ofproto_sflow_destroy(p->sflow);
 
-    for (i = 0; i < p->n_listeners; i++) {
-        pvconn_close(p->listeners[i]);
+    HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, struct ofservice, node,
+                        &p->services) {
+        ofservice_destroy(p, ofservice);
     }
-    free(p->listeners);
+    hmap_destroy(&p->services);
 
     for (i = 0; i < p->n_snoops; i++) {
         pvconn_close(p->snoops[i]);
@@ -1046,6 +1073,7 @@ int
 ofproto_run1(struct ofproto *p)
 {
     struct ofconn *ofconn, *next_ofconn;
+    struct ofservice *ofservice;
     char *devname;
     int error;
     int i;
@@ -1101,21 +1129,24 @@ ofproto_run1(struct ofproto *p)
         fail_open_run(p->fail_open);
     }
 
-    for (i = 0; i < p->n_listeners; i++) {
+    HMAP_FOR_EACH (ofservice, struct ofservice, node, &p->services) {
         struct vconn *vconn;
         int retval;
 
-        retval = pvconn_accept(p->listeners[i], OFP_VERSION, &vconn);
+        retval = pvconn_accept(ofservice->pvconn, OFP_VERSION, &vconn);
         if (!retval) {
+            struct ofconn *ofconn;
             struct rconn *rconn;
             char *name;
 
-            rconn = rconn_create(60, 0);
+            rconn = rconn_create(ofservice->probe_interval, 0);
             name = ofconn_make_name(p, vconn_get_name(vconn));
             rconn_connect_unreliably(rconn, vconn, name);
             free(name);
 
-            ofconn_create(p, rconn, OFCONN_SERVICE);
+            ofconn = ofconn_create(p, rconn, OFCONN_SERVICE);
+            ofconn_set_rate_limit(ofconn, ofservice->rate_limit,
+                                  ofservice->burst_limit);
         } else if (retval != EAGAIN) {
             VLOG_WARN_RL(&rl, "accept failed (%s)", strerror(retval));
         }
@@ -1188,6 +1219,7 @@ ofproto_run2(struct ofproto *p, bool revalidate_all)
 void
 ofproto_wait(struct ofproto *p)
 {
+    struct ofservice *ofservice;
     struct ofconn *ofconn;
     size_t i;
 
@@ -1217,8 +1249,8 @@ ofproto_wait(struct ofproto *p)
     } else if (p->next_expiration != LLONG_MAX) {
         poll_timer_wait_until(p->next_expiration);
     }
-    for (i = 0; i < p->n_listeners; i++) {
-        pvconn_wait(p->listeners[i]);
+    HMAP_FOR_EACH (ofservice, struct ofservice, node, &p->services) {
+        pvconn_wait(ofservice->pvconn);
     }
     for (i = 0; i < p->n_snoops; i++) {
         pvconn_wait(p->snoops[i]);
@@ -1746,6 +1778,85 @@ ofconn_make_name(const struct ofproto *ofproto, const char *target)
 {
     return xasprintf("%s<->%s", dpif_base_name(ofproto->dpif), target);
 }
+
+static void
+ofconn_set_rate_limit(struct ofconn *ofconn, int rate, int burst)
+{
+    int i;
+
+    for (i = 0; i < N_SCHEDULERS; i++) {
+        struct pinsched **s = &ofconn->schedulers[i];
+
+        if (rate > 0) {
+            if (!*s) {
+                *s = pinsched_create(rate, burst,
+                                     ofconn->ofproto->switch_status);
+            } else {
+                pinsched_set_limits(*s, rate, burst);
+            }
+        } else {
+            pinsched_destroy(*s);
+            *s = NULL;
+        }
+    }
+}
+
+static void
+ofservice_reconfigure(struct ofservice *ofservice,
+                      const struct ofproto_controller *c)
+{
+    ofservice->probe_interval = c->probe_interval;
+    ofservice->rate_limit = c->rate_limit;
+    ofservice->burst_limit = c->burst_limit;
+}
+
+/* Creates a new ofservice in 'ofproto'.  Returns 0 if successful, otherwise a
+ * positive errno value. */
+static int
+ofservice_create(struct ofproto *ofproto, const struct ofproto_controller *c)
+{
+    struct ofservice *ofservice;
+    struct pvconn *pvconn;
+    int error;
+
+    error = pvconn_open(c->target, &pvconn);
+    if (error) {
+        return error;
+    }
+
+    ofservice = xzalloc(sizeof *ofservice);
+    hmap_insert(&ofproto->services, &ofservice->node,
+                hash_string(c->target, 0));
+    ofservice->pvconn = pvconn;
+
+    ofservice_reconfigure(ofservice, c);
+
+    return 0;
+}
+
+static void
+ofservice_destroy(struct ofproto *ofproto, struct ofservice *ofservice)
+{
+    hmap_remove(&ofproto->services, &ofservice->node);
+    pvconn_close(ofservice->pvconn);
+    free(ofservice);
+}
+
+/* Finds and returns the ofservice within 'ofproto' that has the given
+ * 'target', or a null pointer if none exists. */
+static struct ofservice *
+ofservice_lookup(struct ofproto *ofproto, const char *target)
+{
+    struct ofservice *ofservice;
+
+    HMAP_FOR_EACH_WITH_HASH (ofservice, struct ofservice, node,
+                             hash_string(target, 0), &ofproto->services) {
+        if (!strcmp(pvconn_get_name(ofservice->pvconn), target)) {
+            return ofservice;
+        }
+    }
+    return NULL;
+}
 
 /* Caller is responsible for initializing the 'cr' member of the returned
  * rule. */
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 507c565..0750289 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -111,7 +111,6 @@ 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_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);
@@ -120,7 +119,7 @@ int ofproto_set_stp(struct ofproto *, bool enable_stp);
 
 /* Configuration querying. */
 uint64_t ofproto_get_datapath_id(const struct ofproto *);
-bool ofproto_has_controller(const struct ofproto *);
+bool ofproto_has_primary_controller(const struct ofproto *);
 enum ofproto_fail_mode ofproto_get_fail_mode(const struct ofproto *);
 void ofproto_get_listeners(const struct ofproto *, struct svec *);
 void ofproto_get_snoops(const struct ofproto *, struct svec *);
diff --git a/ofproto/status.c b/ofproto/status.c
index a8f522d..644e3a4 100644
--- a/ofproto/status.c
+++ b/ofproto/status.c
@@ -132,20 +132,11 @@ config_status_cb(struct status_reply *sr, void *ofproto_)
 {
     const struct ofproto *ofproto = ofproto_;
     uint64_t datapath_id;
-    struct svec listeners;
-    size_t i;
 
     datapath_id = ofproto_get_datapath_id(ofproto);
     if (datapath_id) {
         status_reply_put(sr, "datapath-id=%016"PRIx64, datapath_id);
     }
-
-    svec_init(&listeners);
-    ofproto_get_listeners(ofproto, &listeners);
-    for (i = 0; i < listeners.n; i++) {
-        status_reply_put(sr, "management%zu=%s", i, listeners.names[i]);
-    }
-    svec_destroy(&listeners);
 }
 
 static void
diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c
index 3ace4fd..d3b7ff5 100644
--- a/utilities/ovs-openflowd.c
+++ b/utilities/ovs-openflowd.c
@@ -69,7 +69,6 @@ struct ofsettings {
     const char *dp_desc;        /* Datapath description. */
 
     /* Related vconns and network devices. */
-    struct svec listeners;       /* Listen for management connections. */
     struct svec snoops;          /* Listen for controller snooping conns. */
 
     /* Failure behavior. */
@@ -140,16 +139,6 @@ main(int argc, char *argv[])
     }
     ofproto_set_desc(ofproto, s.mfr_desc, s.hw_desc, s.sw_desc,
                      s.serial_desc, s.dp_desc);
-    if (!s.listeners.n) {
-        svec_add_nocopy(&s.listeners, xasprintf("punix:%s/%s.mgmt",
-                                              ovs_rundir, s.dp_name));
-    } else if (s.listeners.n == 1 && !strcmp(s.listeners.names[0], "none")) {
-        svec_clear(&s.listeners);
-    }
-    error = ofproto_set_listeners(ofproto, &s.listeners);
-    if (error) {
-        ovs_fatal(error, "failed to configure management connections");
-    }
     error = ofproto_set_snoops(ofproto, &s.snoops);
     if (error) {
         ovs_fatal(error,
@@ -262,6 +251,8 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
     };
     char *short_options = long_options_to_short_options(long_options);
     struct ofproto_controller controller_opts;
+    struct svec controllers;
+    int i;
 
     /* Set defaults that we can figure out before parsing options. */
     controller_opts.target = NULL;
@@ -279,7 +270,7 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
     s->sw_desc = NULL;
     s->serial_desc = NULL;
     s->dp_desc = NULL;
-    svec_init(&s->listeners);
+    svec_init(&controllers);
     svec_init(&s->snoops);
     s->max_idle = 0;
     s->enable_stp = false;
@@ -408,7 +399,7 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
             break;
 
         case 'l':
-            svec_add(&s->listeners, optarg);
+            svec_add(&controllers, optarg);
             break;
 
         case OPT_SNOOP:
@@ -471,19 +462,28 @@ parse_options(int argc, char *argv[], struct ofsettings *s)
     /* Local vconns. */
     dp_parse_name(argv[0], &s->dp_name, &s->dp_type);
 
-    /* Controllers. */
-    s->n_controllers = argc > 1 ? argc - 1 : 1;
+    /* Figure out controller names. */
+    if (!controllers.n) {
+        svec_add_nocopy(&controllers,
+                        xasprintf("punix:%s/%s.mgmt", ovs_rundir, s->dp_name));
+    }
+    for (i = 1; i < argc; i++) {
+        svec_add(&controllers, argv[i]);
+    }
+    if (argc < 2) {
+        svec_add(&controllers, "discover");
+    }
+
+    /* Set up controllers. */
+    s->n_controllers = controllers.n;
     s->controllers = xmalloc(s->n_controllers * sizeof *s->controllers);
     if (argc > 1) {
         size_t i;
 
         for (i = 0; i < s->n_controllers; i++) {
             s->controllers[i] = controller_opts;
-            s->controllers[i].target = argv[i + 1];
+            s->controllers[i].target = controllers.names[i];
         }
-    } else {
-        s->controllers[0] = controller_opts;
-        s->controllers[0].target = "discover";
     }
 
     /* Sanity check. */
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 7174f2c..e32d869 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1172,7 +1172,7 @@ bridge_wait(void)
 
     LIST_FOR_EACH (br, struct bridge, node, &all_bridges) {
         ofproto_wait(br->ofproto);
-        if (ofproto_has_controller(br->ofproto)) {
+        if (ofproto_has_primary_controller(br->ofproto)) {
             continue;
         }
 
@@ -1418,7 +1418,6 @@ static void
 bridge_reconfigure_one(struct bridge *br)
 {
     struct shash old_ports, new_ports;
-    struct svec listeners, old_listeners;
     struct svec snoops, old_snoops;
     struct shash_node *node;
     enum ofproto_fail_mode fail_mode;
@@ -1495,8 +1494,8 @@ bridge_reconfigure_one(struct bridge *br)
                 || !strcmp(br->cfg->fail_mode, "standalone")
                     ? OFPROTO_FAIL_STANDALONE
                     : OFPROTO_FAIL_SECURE;
-    if ((ofproto_get_fail_mode(br->ofproto) != fail_mode)
-            && !ofproto_has_controller(br->ofproto)) {
+    if (ofproto_get_fail_mode(br->ofproto) != fail_mode
+        && !ofproto_has_primary_controller(br->ofproto)) {
         ofproto_flush_flows(br->ofproto);
     }
     ofproto_set_fail_mode(br->ofproto, fail_mode);
@@ -1505,18 +1504,6 @@ bridge_reconfigure_one(struct bridge *br)
      * versa.  (XXX Should we delete all flows if we are switching from one
      * controller to another?) */
 
-    /* Configure OpenFlow management listener. */
-    svec_init(&listeners);
-    svec_add_nocopy(&listeners, xasprintf("punix:%s/%s.mgmt",
-                                          ovs_rundir, br->name));
-    svec_init(&old_listeners);
-    ofproto_get_listeners(br->ofproto, &old_listeners);
-    if (!svec_equal(&listeners, &old_listeners)) {
-        ofproto_set_listeners(br->ofproto, &listeners);
-    }
-    svec_destroy(&listeners);
-    svec_destroy(&old_listeners);
-
     /* Configure OpenFlow controller connection snooping. */
     svec_init(&snoops);
     svec_add_nocopy(&snoops, xasprintf("punix:%s/%s.snoop",
@@ -1532,6 +1519,90 @@ bridge_reconfigure_one(struct bridge *br)
     mirror_reconfigure(br);
 }
 
+/* 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->accept_re = NULL;
+    oc->update_resolv_conf = false;
+    oc->rate_limit = 0;
+    oc->burst_limit = 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)
+{
+    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->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);
+}
+
+/* Configures the IP stack for 'br''s local interface properly according to the
+ * configuration in 'c'.  */
+static void
+bridge_configure_local_iface_netdev(struct bridge *br,
+                                    struct ovsrec_controller *c)
+{
+    struct netdev *netdev;
+    struct in_addr mask, gateway;
+
+    struct iface *local_iface;
+    struct in_addr ip;
+
+    /* Controller discovery does its own TCP/IP configuration later. */
+    if (strcmp(c->target, "discover")) {
+        return;
+    }
+
+    /* If there's no local interface or no IP address, give up. */
+    local_iface = bridge_get_local_iface(br);
+    if (!local_iface || !c->local_ip || !inet_aton(c->local_ip, &ip)) {
+        return;
+    }
+
+    /* Bring up the local interface. */
+    netdev = local_iface->netdev;
+    netdev_turn_flags_on(netdev, NETDEV_UP, true);
+
+    /* Configure the IP address and netmask. */
+    if (!c->local_netmask
+        || !inet_aton(c->local_netmask, &mask)
+        || !mask.s_addr) {
+        mask.s_addr = guess_netmask(ip.s_addr);
+    }
+    if (!netdev_set_in4(netdev, ip, mask)) {
+        VLOG_INFO("bridge %s: configured IP address "IP_FMT", netmask "IP_FMT,
+                  br->name, IP_ARGS(&ip.s_addr), IP_ARGS(&mask.s_addr));
+    }
+
+    /* Configure the default gateway. */
+    if (c->local_gateway
+        && inet_aton(c->local_gateway, &gateway)
+        && gateway.s_addr) {
+        if (!netdev_add_router(netdev, gateway)) {
+            VLOG_INFO("bridge %s: configured gateway "IP_FMT,
+                      br->name, IP_ARGS(&gateway.s_addr));
+        }
+    }
+}
+
 static void
 bridge_reconfigure_remotes(struct bridge *br,
                            const struct sockaddr_in *managers,
@@ -1539,99 +1610,62 @@ bridge_reconfigure_remotes(struct bridge *br,
 {
     struct ovsrec_controller **controllers;
     size_t n_controllers;
+    bool had_primary;
+
+    struct ofproto_controller *ocs;
+    size_t n_ocs;
+    size_t i;
 
     ofproto_set_extra_in_band_remotes(br->ofproto, managers, n_managers);
+    had_primary = ofproto_has_primary_controller(br->ofproto);
 
     n_controllers = bridge_get_controllers(br, &controllers);
-    if (ofproto_has_controller(br->ofproto) != (n_controllers != 0)) {
-        ofproto_flush_flows(br->ofproto);
-    }
 
-    if (!n_controllers) {
-        union ofp_action action;
-        flow_t flow;
+    ocs = xmalloc((n_controllers + 1) * sizeof *ocs);
+    n_ocs = 0;
 
-        /* Clear out controllers. */
-        ofproto_set_controllers(br->ofproto, NULL, 0);
-
-        /* If there are no controllers and the bridge is in standalone
-         * mode, set up a flow that matches every packet and directs
-         * them to OFPP_NORMAL (which goes to us).  Otherwise, the
-         * switch is in secure mode and we won't pass any traffic until
-         * a controller has been defined and it tells us to do so. */
-        if (ofproto_get_fail_mode(br->ofproto) == OFPROTO_FAIL_STANDALONE) {
-            memset(&action, 0, sizeof action);
-            action.type = htons(OFPAT_OUTPUT);
-            action.output.len = htons(sizeof action);
-            action.output.port = htons(OFPP_NORMAL);
-            memset(&flow, 0, sizeof flow);
-            ofproto_add_flow(br->ofproto, &flow, OVSFW_ALL, 0, &action, 1, 0);
-        }
-    } else {
-        struct ofproto_controller *ocs;
-        size_t i;
+    bridge_ofproto_controller_for_mgmt(br, &ocs[n_ocs++]);
+    for (i = 0; i < n_controllers; i++) {
+        struct ovsrec_controller *c = controllers[i];
 
-        ocs = xmalloc(n_controllers * sizeof *ocs);
-        for (i = 0; i < n_controllers; i++) {
-            struct ovsrec_controller *c = controllers[i];
-            struct ofproto_controller *oc = &ocs[i];
+        if (!strncmp(c->target, "punix:", 6)
+            || !strncmp(c->target, "unix:", 5)) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
-            if (strcmp(c->target, "discover")) {
-                struct iface *local_iface;
-                struct in_addr ip;
+            VLOG_ERR_RL(&rl, "%s: not adding Unix domain socket controller "
+                        "\"%s\" due to possibility for remote exploit",
+                        dpif_name(br->dpif), c->target);
+            continue;
+        }
 
-                local_iface = bridge_get_local_iface(br);
-                if (local_iface && c->local_ip
-                    && inet_aton(c->local_ip, &ip)) {
-                    struct netdev *netdev = local_iface->netdev;
-                    struct in_addr mask, gateway;
+        bridge_configure_local_iface_netdev(br, c);
+        bridge_ofproto_controller_from_ovsrec(c, &ocs[n_ocs++]);
+    }
 
-                    if (!c->local_netmask
-                        || !inet_aton(c->local_netmask, &mask)) {
-                        mask.s_addr = 0;
-                    }
-                    if (!c->local_gateway
-                        || !inet_aton(c->local_gateway, &gateway)) {
-                        gateway.s_addr = 0;
-                    }
+    ofproto_set_controllers(br->ofproto, ocs, n_ocs);
+    free(ocs[0].target);
+    free(ocs);
 
-                    netdev_turn_flags_on(netdev, NETDEV_UP, true);
-                    if (!mask.s_addr) {
-                        mask.s_addr = guess_netmask(ip.s_addr);
-                    }
-                    if (!netdev_set_in4(netdev, ip, mask)) {
-                        VLOG_INFO("bridge %s: configured IP address "IP_FMT", "
-                                  "netmask "IP_FMT,
-                                  br->name, IP_ARGS(&ip.s_addr),
-                                  IP_ARGS(&mask.s_addr));
-                    }
+    if (had_primary != ofproto_has_primary_controller(br->ofproto)) {
+        ofproto_flush_flows(br->ofproto);
+    }
 
-                    if (gateway.s_addr) {
-                        if (!netdev_add_router(netdev, gateway)) {
-                            VLOG_INFO("bridge %s: configured gateway "IP_FMT,
-                                      br->name, IP_ARGS(&gateway.s_addr));
-                        }
-                    }
-                }
-            }
+    /* If there are no controllers and the bridge is in standalone
+     * mode, set up a flow that matches every packet and directs
+     * them to OFPP_NORMAL (which goes to us).  Otherwise, the
+     * switch is in secure mode and we won't pass any traffic until
+     * a controller has been defined and it tells us to do so. */
+    if (!n_controllers
+        && ofproto_get_fail_mode(br->ofproto) == OFPROTO_FAIL_STANDALONE) {
+        union ofp_action action;
+        flow_t flow;
 
-            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->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_controllers(br->ofproto, ocs, n_controllers);
-        free(ocs);
+        memset(&action, 0, sizeof action);
+        action.type = htons(OFPAT_OUTPUT);
+        action.output.len = htons(sizeof action);
+        action.output.port = htons(OFPP_NORMAL);
+        memset(&flow, 0, sizeof flow);
+        ofproto_add_flow(br->ofproto, &flow, OVSFW_ALL, 0, &action, 1, 0);
     }
 }
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 6bcc0a2..05a3f91 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -853,18 +853,70 @@
   <table name="Controller" title="OpenFlow controller configuration.">
     <p>An OpenFlow controller.</p>
 
-    <p>Open vSwitch permits a bridge to have any number of OpenFlow
-       controllers.  When multiple controllers are configured, Open vSwitch
-       connects to all of them simultaneously.  OpenFlow 1.0 does not specify
-       how multiple controllers coordinate in interacting with a single switch,
-       so more than one controller should be specified only if the controllers
-       are themselves designed to coordinate with each other.</p>
+    <p>
+      Open vSwitch supports two kinds of OpenFlow controllers:
+    </p>
+    
+    <dl>
+      <dt>Primary controllers</dt>
+      <dd>
+        <p>
+          This is the kind of controller envisioned by the OpenFlow 1.0
+          specification.  Usually, a primary controller implements a network
+          policy by taking charge of the switch's flow table.
+        </p>
+
+        <p>
+          Open vSwitch initiates and maintains persistent connections to
+          primary controllers, retrying the connection each time it fails or
+          drops.  The <ref table="Bridge" column="fail_mode"/> column in the
+          <ref table="Bridge"/> table applies to primary controllers.
+        </p>
+
+        <p>
+          Open vSwitch permits a bridge to have any number of primary
+          controllers.  When multiple controllers are configured, Open vSwitch
+          connects to all of them simultaneously.  Because OpenFlow 1.0 does
+          not specify how multiple controllers coordinate in interacting with a
+          single switch, so more than one primary controller should be
+          specified only if the controllers are themselves designed to
+          coordinate with each other.
+        </p>
+      </dd>
+      <dt>Service controllers</dt>
+      <dd>
+        <p>
+          These kinds of OpenFlow controller connections are intended for
+          occasional support and maintenance use, e.g. with
+          <code>ovs-ofctl</code>.  Usually a service controller connects only
+          briefly to inspect or modify some of a switch's state.
+        </p>
+
+        <p>
+          Open vSwitch listens for incoming connections from service
+          controllers.  The service controllers initiate and, if necessary,
+          maintain the connections from their end.  The <ref table="Bridge"
+          column="fail_mode"/> column in the <ref table="Bridge"/> table does
+          not apply to service controllers.
+        </p>
+
+        <p>
+          Open vSwitch supports configuring any number of service controllers.
+        </p>
+      </dd>
+    </dl>
+
+    <p>
+      The <ref column="target"/> determines the type of controller.
+    </p>
 
     <group title="Core Features">
       <column name="target">
-        <p>Connection method for controller.
-          The following connection methods are currently
-          supported:</p>
+        <p>Connection method for controller.</p>
+        <p>
+          The following connection methods are currently supported for primary
+          controllers:
+        </p>
         <dl>
           <dt><code>ssl:<var>ip</var></code>[<code>:<var>port</var></code>]</dt>
           <dd>
@@ -897,8 +949,35 @@
               used only for bootstrapping the OpenFlow PKI at initial switch
               setup; <code>ovs-vswitchd</code> does not use it at all.</p>
           </dd>
-          <dt><code>none</code></dt>
-          <dd>Disables the controller.</dd>
+        </dl>
+        <p>
+          The following connection methods are currently supported for service
+          controllers:
+        </p>
+        <dl>
+          <dt><code>pssl:</code>[<var>port</var>][<code>:<var>ip</var></code>]</dt>
+          <dd>
+            <p>
+              Listens for SSL connections on the specified TCP <var>port</var>
+              (default: 6633).  If <var>ip</var>, which must be expressed as an
+              IP address (not a DNS name), is specified, then connections are
+              restricted to the specified local IP address.
+            </p>
+            <p>
+              The <ref table="Open_vSwitch" column="ssl"/> column in the <ref
+              table="Open_vSwitch"/> must point to a valid SSL configuration
+              when this form is used.
+            </p>
+            <p>SSL support is an optional feature that is not always built as
+              part of Open vSwitch.</p>
+          </dd>
+          <dt><code>ptcp:</code>[<var>port</var>][<code>:<var>ip</var></code>]</dt>
+          <dd>
+            Listens for connections on the specified TCP <var>port</var>
+            (default: 6633).  If <var>ip</var>, which must be expressed as an
+            IP address (not a DNS name), is specified, then connections are
+            restricted to the specified local IP address.
+          </dd>
         </dl>
 	<p>When multiple controllers are configured for a single bridge, the
 	  <ref column="target"/> values must be unique.  Duplicate
-- 
1.7.1





More information about the dev mailing list