learning-switch: Make lswitch own its rconn.

Until now, ovs-controller and the learning-switch code split responsibility
for the OpenFlow connection.  This commit moves all the responsibility into
the learning-switch code.

The rationale here is twofold.  First, the split itself seems odd; I think
there must have been a reason for it at one time, but I don't remember it
and don't see one anymore.  Second, I intend to make the lswitch code more
stateful in upcoming commits, and it seems odd to have the lswitch manage
quite a bit of state but not the entity that that state applies to.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index b7a435c0ecb49e2c41a312b182fd6485c6ec4552..d53f147a3656952caa6568e2ed7314021f344a1e 100644 (file)
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -53,6 +53,8 @@ struct lswitch_port {
 };
 
 struct lswitch {
+    struct rconn *rconn;
+
     /* If nonnegative, the switch sets up flows that expire after the given
      * number of seconds (or never expire, if the value is OFP_FLOW_PERMANENT).
      * Otherwise, the switch processes every packet. */
@@ -72,21 +74,24 @@ struct lswitch {
 
     /* Number of outgoing queued packets on the rconn. */
     struct rconn_packet_counter *queued;
+
+    /* If true, do not reply to any messages from the switch (for debugging
+     * fail-open mode). */
+    bool mute;
 };
 
 /* The log messages here could actually be useful in debugging, so keep the
  * rate limit relatively high. */
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
 
-static void queue_tx(struct lswitch *, struct rconn *, struct ofpbuf *);
-static void send_features_request(struct lswitch *, struct rconn *);
+static void queue_tx(struct lswitch *, struct ofpbuf *);
+static void send_features_request(struct lswitch *);
 
+static void lswitch_process_packet(struct lswitch *, const struct ofpbuf *);
 static enum ofperr process_switch_features(struct lswitch *,
                                            struct ofp_header *);
-static void process_packet_in(struct lswitch *, struct rconn *,
-                              const struct ofp_header *);
-static void process_echo_request(struct lswitch *, struct rconn *,
-                                 const struct ofp_header *);
+static void process_packet_in(struct lswitch *, const struct ofp_header *);
+static void process_echo_request(struct lswitch *, const struct ofp_header *);
 
 /* Creates and returns a new learning switch whose configuration is given by
  * 'cfg'.
@@ -99,6 +104,7 @@ lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
     struct lswitch *sw;
 
     sw = xzalloc(sizeof *sw);
+    sw->rconn = rconn;
     sw->max_idle = cfg->max_idle;
     sw->datapath_id = 0;
     sw->last_features_request = time_now() - 1;
@@ -141,7 +147,7 @@ lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
     }
 
     sw->queued = rconn_packet_counter_create();
-    send_features_request(sw, rconn);
+    send_features_request(sw);
 
     protocol = ofputil_protocol_from_ofp_version(rconn_get_version(rconn));
     if (cfg->default_flows) {
@@ -185,6 +191,12 @@ lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
     return sw;
 }
 
+bool
+lswitch_is_alive(const struct lswitch *sw)
+{
+    return rconn_is_alive(sw->rconn);
+}
+
 /* Destroys 'sw'. */
 void
 lswitch_destroy(struct lswitch *sw)
@@ -192,6 +204,7 @@ lswitch_destroy(struct lswitch *sw)
     if (sw) {
         struct lswitch_port *node, *next;
 
+        rconn_destroy(sw->rconn);
         HMAP_FOR_EACH_SAFE (node, next, hmap_node, &sw->queue_numbers) {
             hmap_remove(&sw->queue_numbers, &node->hmap_node);
             free(node);
@@ -208,9 +221,27 @@ lswitch_destroy(struct lswitch *sw)
 void
 lswitch_run(struct lswitch *sw)
 {
+    int i;
+
     if (sw->ml) {
         mac_learning_run(sw->ml, NULL);
     }
+
+    rconn_run(sw->rconn);
+
+    for (i = 0; i < 50; i++) {
+        struct ofpbuf *msg;
+
+        msg = rconn_recv(sw->rconn);
+        if (!msg) {
+            break;
+        }
+
+        if (!sw->mute) {
+            lswitch_process_packet(sw, msg);
+        }
+        ofpbuf_delete(msg);
+    }
 }
 
 void
@@ -219,15 +250,16 @@ lswitch_wait(struct lswitch *sw)
     if (sw->ml) {
         mac_learning_wait(sw->ml);
     }
+    rconn_run(sw->rconn);
+    rconn_recv_wait(sw->rconn);
 }
 
 /* Processes 'msg', which should be an OpenFlow received on 'rconn', according
  * to the learning switch state in 'sw'.  The most likely result of processing
  * is that flow-setup and packet-out OpenFlow messages will be sent out on
  * 'rconn'.  */
-void
-lswitch_process_packet(struct lswitch *sw, struct rconn *rconn,
-                       const struct ofpbuf *msg)
+static void
+lswitch_process_packet(struct lswitch *sw, const struct ofpbuf *msg)
 {
     enum ofptype type;
     struct ofpbuf b;
@@ -240,13 +272,13 @@ lswitch_process_packet(struct lswitch *sw, struct rconn *rconn,
     if (sw->datapath_id == 0
         && type != OFPTYPE_ECHO_REQUEST
         && type != OFPTYPE_FEATURES_REPLY) {
-        send_features_request(sw, rconn);
+        send_features_request(sw);
         return;
     }
 
     switch (type) {
     case OFPTYPE_ECHO_REQUEST:
-        process_echo_request(sw, rconn, msg->data);
+        process_echo_request(sw, msg->data);
         break;
 
     case OFPTYPE_FEATURES_REPLY:
@@ -254,7 +286,7 @@ lswitch_process_packet(struct lswitch *sw, struct rconn *rconn,
         break;
 
     case OFPTYPE_PACKET_IN:
-        process_packet_in(sw, rconn, msg->data);
+        process_packet_in(sw, msg->data);
         break;
 
     case OFPTYPE_FLOW_REMOVED:
@@ -312,41 +344,41 @@ lswitch_process_packet(struct lswitch *sw, struct rconn *rconn,
 }
 \f
 static void
-send_features_request(struct lswitch *sw, struct rconn *rconn)
+send_features_request(struct lswitch *sw)
 {
     time_t now = time_now();
     if (now >= sw->last_features_request + 1) {
         struct ofpbuf *b;
         struct ofp_switch_config *osc;
-        int ofp_version = rconn_get_version(rconn);
+        int ofp_version = rconn_get_version(sw->rconn);
 
         assert(ofp_version > 0 && ofp_version < 0xff);
 
         /* Send OFPT_FEATURES_REQUEST. */
         b = ofpraw_alloc(OFPRAW_OFPT_FEATURES_REQUEST, ofp_version, 0);
-        queue_tx(sw, rconn, b);
+        queue_tx(sw, b);
 
         /* Send OFPT_SET_CONFIG. */
         b = ofpraw_alloc(OFPRAW_OFPT_SET_CONFIG, ofp_version, sizeof *osc);
         osc = ofpbuf_put_zeros(b, sizeof *osc);
         osc->miss_send_len = htons(OFP_DEFAULT_MISS_SEND_LEN);
-        queue_tx(sw, rconn, b);
+        queue_tx(sw, b);
 
         sw->last_features_request = now;
     }
 }
 
 static void
-queue_tx(struct lswitch *sw, struct rconn *rconn, struct ofpbuf *b)
+queue_tx(struct lswitch *sw, struct ofpbuf *b)
 {
-    int retval = rconn_send_with_limit(rconn, b, sw->queued, 10);
+    int retval = rconn_send_with_limit(sw->rconn, b, sw->queued, 10);
     if (retval && retval != ENOTCONN) {
         if (retval == EAGAIN) {
             VLOG_INFO_RL(&rl, "%016llx: %s: tx queue overflow",
-                         sw->datapath_id, rconn_get_name(rconn));
+                         sw->datapath_id, rconn_get_name(sw->rconn));
         } else {
             VLOG_WARN_RL(&rl, "%016llx: %s: send: %s",
-                         sw->datapath_id, rconn_get_name(rconn),
+                         sw->datapath_id, rconn_get_name(sw->rconn),
                          strerror(retval));
         }
     }
@@ -441,8 +473,7 @@ get_queue_id(const struct lswitch *sw, uint16_t in_port)
 }
 
 static void
-process_packet_in(struct lswitch *sw, struct rconn *rconn,
-                  const struct ofp_header *oh)
+process_packet_in(struct lswitch *sw, const struct ofp_header *oh)
 {
     struct ofputil_packet_in pi;
     uint32_t queue_id;
@@ -523,24 +554,23 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
         fm.ofpacts_len = ofpacts.size;
         buffer = ofputil_encode_flow_mod(&fm, sw->protocol);
 
-        queue_tx(sw, rconn, buffer);
+        queue_tx(sw, buffer);
 
         /* If the switch didn't buffer the packet, we need to send a copy. */
         if (pi.buffer_id == UINT32_MAX && out_port != OFPP_NONE) {
-            queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
+            queue_tx(sw, ofputil_encode_packet_out(&po));
         }
     } else {
         /* We don't know that MAC, or we don't set up flows.  Send along the
          * packet without setting up a flow. */
         if (pi.buffer_id != UINT32_MAX || out_port != OFPP_NONE) {
-            queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
+            queue_tx(sw, ofputil_encode_packet_out(&po));
         }
     }
 }
 
 static void
-process_echo_request(struct lswitch *sw, struct rconn *rconn,
-                     const struct ofp_header *rq)
+process_echo_request(struct lswitch *sw, const struct ofp_header *rq)
 {
-    queue_tx(sw, rconn, make_echo_reply(rq));
+    queue_tx(sw, make_echo_reply(rq));
 }

diff --git a/lib/learning-switch.h b/lib/learning-switch.h
index 1b760d8e5ed74156d36e1d8677beee6ff68c59bd..dcfb5a270b69ea3108df6a143cf6d3b8bad49229 100644 (file)
--- a/lib/learning-switch.h
+++ b/lib/learning-switch.h
@@ -57,15 +57,19 @@ struct lswitch_config {
 
     /* Maps from a port name to a queue_id. */
     const struct simap *port_queues;
+
+    /* If true, do not reply to any messages from the switch (for debugging
+     * fail-open mode). */
+    bool mute;
 };
 
 struct lswitch *lswitch_create(struct rconn *, const struct lswitch_config *);
+bool lswitch_is_alive(const struct lswitch *);
 void lswitch_set_queue(struct lswitch *sw, uint32_t queue);
 void lswitch_run(struct lswitch *);
 void lswitch_wait(struct lswitch *);
 void lswitch_destroy(struct lswitch *);
-void lswitch_process_packet(struct lswitch *, struct rconn *,
-                            const struct ofpbuf *);
 
+void lswitch_mute(struct lswitch *);
 
 #endif /* learning-switch.h */

diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c
index 04b16e8709b529479392207e49334e27c47f6f56..07e300bc94aba036e4be8dd10afff80c4ccf6e09 100644 (file)
--- a/utilities/ovs-controller.c
+++ b/utilities/ovs-controller.c
@@ -49,7 +49,6 @@ VLOG_DEFINE_THIS_MODULE(controller);
 
 struct switch_ {
     struct lswitch *lswitch;
-    struct rconn *rconn;
 };
 
 /* -H, --hub: Learn the ports on which MAC addresses appear? */
@@ -86,7 +85,6 @@ static size_t n_default_flows;
 /* --unixctl: Name of unixctl socket, or null to use the default. */
 static char *unixctl_path = NULL;
 
-static int do_switching(struct switch_ *);
 static void new_switch(struct switch_ *, struct vconn *);
 static void parse_options(int argc, char *argv[]);
 static void usage(void) NO_RETURN;
@@ -151,8 +149,6 @@ main(int argc, char *argv[])
     daemonize_complete();
 
     while (n_switches > 0 || n_listeners > 0) {
-        int iteration;
-
         /* Accept connections on listening vconns. */
         for (i = 0; i < n_listeners && n_switches < MAX_SWITCHES; ) {
             struct vconn *new_vconn;
@@ -169,32 +165,16 @@ main(int argc, char *argv[])
             }
         }
 
-        /* Do some switching work.  Limit the number of iterations so that
-         * callbacks registered with the poll loop don't starve. */
-        for (iteration = 0; iteration < 50; iteration++) {
-            bool progress = false;
-            for (i = 0; i < n_switches; ) {
-                struct switch_ *this = &switches[i];
-
-                retval = do_switching(this);
-                if (!retval || retval == EAGAIN) {
-                    if (!retval) {
-                        progress = true;
-                    }
-                    i++;
-                } else {
-                    rconn_destroy(this->rconn);
-                    lswitch_destroy(this->lswitch);
-                    switches[i] = switches[--n_switches];
-                }
-            }
-            if (!progress) {
-                break;
-            }
-        }
-        for (i = 0; i < n_switches; i++) {
+        /* Do some switching work.  . */
+        for (i = 0; i < n_switches; ) {
             struct switch_ *this = &switches[i];
             lswitch_run(this->lswitch);
+            if (lswitch_is_alive(this->lswitch)) {
+                i++;
+            } else {
+                lswitch_destroy(this->lswitch);
+                switches[i] = switches[--n_switches];
+            }
         }
 
         unixctl_server_run(unixctl);
@@ -207,8 +187,6 @@ main(int argc, char *argv[])
         }
         for (i = 0; i < n_switches; i++) {
             struct switch_ *sw = &switches[i];
-            rconn_run_wait(sw->rconn);
-            rconn_recv_wait(sw->rconn);
             lswitch_wait(sw->lswitch);
         }
         unixctl_server_wait(unixctl);
@@ -222,9 +200,10 @@ static void
 new_switch(struct switch_ *sw, struct vconn *vconn)
 {
     struct lswitch_config cfg;
+    struct rconn *rconn;
 
-    sw->rconn = rconn_create(60, 0, DSCP_DEFAULT);
-    rconn_connect_unreliably(sw->rconn, vconn, NULL);
+    rconn = rconn_create(60, 0, DSCP_DEFAULT);
+    rconn_connect_unreliably(rconn, vconn, NULL);
 
     cfg.mode = (action_normal ? LSW_NORMAL
                 : learn_macs ? LSW_LEARN
@@ -235,29 +214,8 @@ new_switch(struct switch_ *sw, struct vconn *vconn)
     cfg.n_default_flows = n_default_flows;
     cfg.default_queue = default_queue;
     cfg.port_queues = &port_queues;
-    sw->lswitch = lswitch_create(sw->rconn, &cfg);
-}
-
-static int
-do_switching(struct switch_ *sw)
-{
-    unsigned int packets_sent;
-    struct ofpbuf *msg;
-
-    packets_sent = rconn_packets_sent(sw->rconn);
-
-    msg = rconn_recv(sw->rconn);
-    if (msg) {
-        if (!mute) {
-            lswitch_process_packet(sw->lswitch, sw->rconn, msg);
-        }
-        ofpbuf_delete(msg);
-    }
-    rconn_run(sw->rconn);
-
-    return (!rconn_is_alive(sw->rconn) ? EOF
-            : rconn_packets_sent(sw->rconn) != packets_sent ? 0
-            : EAGAIN);
+    cfg.mute = mute;
+    sw->lswitch = lswitch_create(rconn, &cfg);
 }
 
 static void