ofproto: Fix treatment of out-of-band controllers.
[openvswitch] / ofproto / ofproto.c
index 2f105ecec7c6ea50f02f8d93a56327ab143ba25e..d8215a85d5bb83375a066559c85faa57fe280cb7 100644 (file)
@@ -19,6 +19,7 @@
 #include "ofproto.h"
 #include <errno.h>
 #include <inttypes.h>
+#include <sys/socket.h>
 #include <net/if.h>
 #include <netinet/in.h>
 #include <stdbool.h>
@@ -223,6 +224,8 @@ static struct ofconn *ofconn_create(struct ofproto *, struct rconn *,
 static void ofconn_destroy(struct ofconn *);
 static void ofconn_run(struct ofconn *, struct ofproto *);
 static void ofconn_wait(struct ofconn *);
+static bool ofconn_receives_async_msgs(const struct ofconn *);
+
 static void queue_tx(struct ofpbuf *msg, const struct ofconn *ofconn,
                      struct rconn_packet_counter *counter);
 
@@ -543,6 +546,10 @@ update_in_band_remotes(struct ofproto *ofproto)
     HMAP_FOR_EACH (ofconn, struct ofconn, hmap_node, &ofproto->controllers) {
         struct sockaddr_in *sin = &addrs[n_addrs];
 
+        if (ofconn->band == OFPROTO_OUT_OF_BAND) {
+            continue;
+        }
+
         sin->sin_addr.s_addr = rconn_get_remote_ip(ofconn->rconn);
         if (sin->sin_addr.s_addr) {
             sin->sin_port = rconn_get_remote_port(ofconn->rconn);
@@ -961,25 +968,47 @@ process_port_change(struct ofproto *ofproto, int error, char *devname)
     }
 }
 
+/* Returns a "preference level" for snooping 'ofconn'.  A higher return value
+ * means that 'ofconn' is more interesting for monitoring than a lower return
+ * value. */
+static int
+snoop_preference(const struct ofconn *ofconn)
+{
+    switch (ofconn->role) {
+    case NX_ROLE_MASTER:
+        return 3;
+    case NX_ROLE_OTHER:
+        return 2;
+    case NX_ROLE_SLAVE:
+        return 1;
+    default:
+        /* Shouldn't happen. */
+        return 0;
+    }
+}
+
 /* One of ofproto's "snoop" pvconns has accepted a new connection on 'vconn'.
  * Connects this vconn to a controller. */
 static void
 add_snooper(struct ofproto *ofproto, struct vconn *vconn)
 {
-    struct ofconn *ofconn;
+    struct ofconn *ofconn, *best;
 
-    /* Arbitrarily pick the first controller in the list for monitoring.  We
-     * could do something smarter or more flexible later, if it ever proves
-     * useful. */
+    /* Pick a controller for monitoring. */
+    best = NULL;
     LIST_FOR_EACH (ofconn, struct ofconn, node, &ofproto->all_conns) {
-        if (ofconn->type == OFCONN_CONTROLLER) {
-            rconn_add_monitor(ofconn->rconn, vconn);
-            return;
+        if (ofconn->type == OFCONN_CONTROLLER
+            && (!best || snoop_preference(ofconn) > snoop_preference(best))) {
+            best = ofconn;
         }
+    }
 
+    if (best) {
+        rconn_add_monitor(best->rconn, vconn);
+    } else {
+        VLOG_INFO_RL(&rl, "no controller connection to snoop");
+        vconn_close(vconn);
     }
-    VLOG_INFO_RL(&rl, "no controller connection to monitor");
-    vconn_close(vconn);
 }
 
 int
@@ -1131,7 +1160,7 @@ ofproto_wait(struct ofproto *p)
         ofconn_wait(ofconn);
     }
     if (p->in_band) {
-        poll_timer_wait(p->next_in_band_update - time_msec());
+        poll_timer_wait_until(p->next_in_band_update);
         in_band_wait(p->in_band);
     }
     if (p->fail_open) {
@@ -1148,7 +1177,7 @@ ofproto_wait(struct ofproto *p)
         VLOG_DBG_RL(&rl, "need revalidate in ofproto_wait_cb()");
         poll_immediate_wake();
     } else if (p->next_expiration != LLONG_MAX) {
-        poll_timer_wait(p->next_expiration - time_msec());
+        poll_timer_wait_until(p->next_expiration);
     }
     for (i = 0; i < p->n_listeners; i++) {
         pvconn_wait(p->listeners[i]);
@@ -1402,7 +1431,7 @@ send_port_status(struct ofproto *p, const struct ofport *ofport,
         struct ofp_port_status *ops;
         struct ofpbuf *b;
 
-        if (ofconn->role == NX_ROLE_SLAVE) {
+        if (!ofconn_receives_async_msgs(ofconn)) {
             continue;
         }
 
@@ -1651,6 +1680,22 @@ ofconn_wait(struct ofconn *ofconn)
         COVERAGE_INC(ofproto_ofconn_stuck);
     }
 }
+
+/* Returns true if 'ofconn' should receive asynchronous messages. */
+static bool
+ofconn_receives_async_msgs(const struct ofconn *ofconn)
+{
+    if (ofconn->type == OFCONN_CONTROLLER) {
+        /* Ordinary controllers always get asynchronous messages unless they
+         * have configured themselves as "slaves".  */
+        return ofconn->role != NX_ROLE_SLAVE;
+    } else {
+        /* Transient connections don't get asynchronous messages unless they
+         * have explicitly asked for them by setting a nonzero miss send
+         * length. */
+        return ofconn->miss_send_len > 0;
+    }
+}
 \f
 /* Caller is responsible for initializing the 'cr' member of the returned
  * rule. */
@@ -3500,7 +3545,7 @@ handle_role_request(struct ofproto *ofproto,
     uint32_t role;
 
     if (ntohs(msg->header.length) != sizeof *nrr) {
-        VLOG_WARN_RL(&rl, "received role request of length %zu (expected %zu)",
+        VLOG_WARN_RL(&rl, "received role request of length %u (expected %zu)",
                      ntohs(msg->header.length), sizeof *nrr);
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
@@ -3550,7 +3595,7 @@ handle_vendor(struct ofproto *p, struct ofconn *ofconn, void *msg)
     struct nicira_header *nh;
 
     if (ntohs(ovh->header.length) < sizeof(struct ofp_vendor_header)) {
-        VLOG_WARN_RL(&rl, "received vendor message of length %zu "
+        VLOG_WARN_RL(&rl, "received vendor message of length %u "
                           "(expected at least %zu)",
                    ntohs(ovh->header.length), sizeof(struct ofp_vendor_header));
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
@@ -3559,7 +3604,7 @@ handle_vendor(struct ofproto *p, struct ofconn *ofconn, void *msg)
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR);
     }
     if (ntohs(ovh->header.length) < sizeof(struct nicira_header)) {
-        VLOG_WARN_RL(&rl, "received Nicira vendor message of length %zu "
+        VLOG_WARN_RL(&rl, "received Nicira vendor message of length %u "
                           "(expected at least %zu)",
                      ntohs(ovh->header.length), sizeof(struct nicira_header));
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
@@ -3864,7 +3909,7 @@ send_flow_removed(struct ofproto *p, struct rule *rule,
     prev = NULL;
     LIST_FOR_EACH (ofconn, struct ofconn, node, &p->all_conns) {
         if (rule->send_flow_removed && rconn_is_connected(ofconn->rconn)
-            && ofconn->role != NX_ROLE_SLAVE) {
+            && ofconn_receives_async_msgs(ofconn)) {
             if (prev) {
                 queue_tx(ofpbuf_clone(buf), prev, prev->reply_counter);
             } else {
@@ -4028,6 +4073,8 @@ schedule_packet_in(struct ofconn *ofconn, struct ofpbuf *packet, int max_len,
         buffer_id = UINT32_MAX;
     } else if (ofproto->fail_open && fail_open_is_active(ofproto->fail_open)) {
         buffer_id = pktbuf_get_null();
+    } else if (!ofconn->pktbuf) {
+        buffer_id = UINT32_MAX;
     } else {
         struct ofpbuf payload;
         payload.data = opi->data;
@@ -4124,7 +4171,7 @@ send_packet_in(struct ofproto *ofproto, struct ofpbuf *packet)
 
     prev = NULL;
     LIST_FOR_EACH (ofconn, struct ofconn, node, &ofproto->all_conns) {
-        if (ofconn->role != NX_ROLE_SLAVE) {
+        if (ofconn_receives_async_msgs(ofconn)) {
             if (prev) {
                 schedule_packet_in(prev, packet, max_len, true);
             }