secchan: Make fail-open work using a wildcard rule (and flush the flow table).

Fail-open was implemented by grabbing packets as they came in and then
shoving them into a learning switch, but this interacted badly with
vswitch.  First, it didn't respect the vswitch architecture; e.g. VLAN
and bonding settings were ignored.  Second, it didn't override any rules
that were still in the flow table from the last time we were connected to
a controller, which could cause other things to break if those rules were
bad (or if some host just wasn't authenticated etc.)

This commit fixes these problems by making fail-open use a wildcard rule
and OFPP_NORMAL, so that vswitch then implements VLANs etc., and by
flushing the flow table when we go into fail-open, getting rid of any
bad rules.

diff --git a/secchan/fail-open.c b/secchan/fail-open.c
index 619b6f46d102fc66a8a55330fc5f1f2160a46096..9a8a3d49c3978c1d5b676e081f1357f47c39321a 100644 (file)
--- a/secchan/fail-open.c
+++ b/secchan/fail-open.c
@@ -47,88 +47,70 @@
 #include "vlog.h"
 
 struct fail_open {
+    struct ofproto *ofproto;
     struct rconn *controller;
     int trigger_duration;
     int last_disconn_secs;
-    struct mac_learning *mac_learning;
     struct status_category *ss_cat;
 };
 
-/* Causes 'r' to enter or leave fail-open mode, if appropriate. */
+/* Causes the switch to enter or leave fail-open mode, if appropriate. */
 void
 fail_open_run(struct fail_open *fo)
 {
     int disconn_secs = rconn_failure_duration(fo->controller);
     bool open = disconn_secs >= fo->trigger_duration;
-    if (open != (fo->mac_learning != NULL)) {
+    if (open != (fo->last_disconn_secs != 0)) {
         if (!open) {
+            flow_t flow;
+
             VLOG_WARN("No longer in fail-open mode");
-            mac_learning_destroy(fo->mac_learning);
-            fo->mac_learning = NULL;
+            fo->last_disconn_secs = 0;
+
+            memset(&flow, 0, sizeof flow);
+            ofproto_delete_flow(fo->ofproto, &flow, OFPFW_ALL, 70000);
         } else {
             VLOG_WARN("Could not connect to controller for %d seconds, "
                       "failing open", disconn_secs);
-            fo->mac_learning = mac_learning_create();
             fo->last_disconn_secs = disconn_secs;
+
+            /* Flush all OpenFlow and datapath flows.  We will set up our
+             * fail-open rule from fail_open_flushed() when
+             * ofproto_flush_flows() calls back to us. */
+            ofproto_flush_flows(fo->ofproto);
         }
     } else if (open && disconn_secs > fo->last_disconn_secs + 60) {
         VLOG_INFO("Still in fail-open mode after %d seconds disconnected "
                   "from controller", disconn_secs);
         fo->last_disconn_secs = disconn_secs;
     }
-    if (fo->mac_learning) {
-        mac_learning_run(fo->mac_learning, NULL);
-    }
 }
 
 void
-fail_open_wait(struct fail_open *fo)
+fail_open_wait(struct fail_open *fo UNUSED)
 {
-    if (fo->mac_learning) {
-        mac_learning_wait(fo->mac_learning);
-    }
+    /* Nothing to do. */
 }
 
-bool
-fail_open_handle_flow_miss(struct fail_open *fo, struct ofproto *ofproto,
-                           const flow_t *flow, const struct ofpbuf *payload)
+void
+fail_open_flushed(struct fail_open *fo)
 {
-    /* -1 (FLOOD) is coincidentally the value returned by mac_learning_lookup()
-     * when it doesn't have a entry for that address. */
-    enum { FLOOD = -1, DROP = -2 };
-    union ofp_action action;
-    int out_port;
-
-    if (rconn_is_connected(fo->controller) || !fo->mac_learning) {
-        return false;
-    }
-
-    if (mac_learning_learn(fo->mac_learning, flow->dl_src, 0, flow->in_port)) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
-        VLOG_DBG_RL(&rl, "learned that "ETH_ADDR_FMT" is on port %"PRIu16,
-                    ETH_ADDR_ARGS(flow->dl_src), flow->in_port);
-    }
+    int disconn_secs = rconn_failure_duration(fo->controller);
+    bool open = disconn_secs >= fo->trigger_duration;
+    if (open) {
+        union ofp_action action;
+        flow_t flow;
 
-    out_port = (eth_addr_is_reserved(flow->dl_src) ? DROP
-                : mac_learning_lookup(fo->mac_learning, flow->dl_dst, 0));
-    memset(&action, 0, sizeof action);
-    action.output.type = htons(OFPAT_OUTPUT);
-    action.output.len = htons(sizeof action);
-    if (flow->in_port == out_port || out_port == DROP) {
-        /* Set up a flow to drop packets. */
-        ofproto_add_flow(ofproto, flow, 0, UINT16_MAX, NULL, 0, NULL, -1);
-    } else if (out_port != FLOOD) {
-        /* The output port is known, so add a new flow. */
-        action.output.port = htons(odp_port_to_ofp_port(out_port));
-        ofproto_add_flow(ofproto, flow, 0, UINT16_MAX,
-                         &action, 1, payload, -1);
-    } else {
-        /* We don't know that MAC.  Send along the packet without setting up a
-         * flow. */
-        action.output.port = htons(OFPP_FLOOD);
-        ofproto_send_packet(ofproto, flow, &action, 1, payload);
+        /* Set up a flow that matches every packet and directs them to
+         * OFPP_NORMAL. */
+        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(fo->ofproto, &flow, OFPFW_ALL, 70000,
+                         &action, 1, NULL, 0);
     }
-    return true;
 }
 
 static void
@@ -144,14 +126,15 @@ fail_open_status_cb(struct status_reply *sr, void *fo_)
 }
 
 struct fail_open *
-fail_open_create(int trigger_duration, struct switch_status *switch_status,
+fail_open_create(struct ofproto *ofproto,
+                 int trigger_duration, struct switch_status *switch_status,
                  struct rconn *controller)
 {
     struct fail_open *fo = xmalloc(sizeof *fo);
+    fo->ofproto = ofproto;
     fo->controller = controller;
     fo->trigger_duration = trigger_duration;
     fo->last_disconn_secs = 0;
-    fo->mac_learning = NULL;
     fo->ss_cat = switch_status_register(switch_status, "fail-open",
                                         fail_open_status_cb, fo);
     return fo;
@@ -168,7 +151,6 @@ fail_open_destroy(struct fail_open *fo)
 {
     if (fo) {
         /* We don't own fo->controller. */
-        mac_learning_destroy(fo->mac_learning);
         switch_status_unregister(fo->ss_cat);
         free(fo);
     }

diff --git a/secchan/fail-open.h b/secchan/fail-open.h
index 96f923501810878145da1b014c1d1b2f808bad14..76cbb439258f063118ea3c1bc12388e2d38e192c 100644 (file)
--- a/secchan/fail-open.h
+++ b/secchan/fail-open.h
@@ -43,14 +43,13 @@ struct ofproto;
 struct rconn;
 struct switch_status;
 
-struct fail_open *fail_open_create(int trigger_duration,
+struct fail_open *fail_open_create(struct ofproto *, int trigger_duration,
                                    struct switch_status *,
                                    struct rconn *controller);
 void fail_open_set_trigger_duration(struct fail_open *, int trigger_duration);
 void fail_open_destroy(struct fail_open *);
 void fail_open_wait(struct fail_open *);
 void fail_open_run(struct fail_open *);
-bool fail_open_handle_flow_miss(struct fail_open *, struct ofproto *,
-                                const flow_t *, const struct ofpbuf *payload);
+void fail_open_flushed(struct fail_open *);
 
 #endif /* fail-open.h */

diff --git a/secchan/ofproto.c b/secchan/ofproto.c
index e2c69d5d0587da4cabb2d56b0f0806253be1bd51..3ff45acd2b8d6706fc6cab5c303029fb6e4d6fa8 100644 (file)
--- a/secchan/ofproto.c
+++ b/secchan/ofproto.c
@@ -553,7 +553,7 @@ ofproto_set_failure(struct ofproto *ofproto, bool 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(trigger_duration,
+            ofproto->fail_open = fail_open_create(ofproto, trigger_duration,
                                                   ofproto->switch_status,
                                                   rconn);
         } else {
@@ -1027,6 +1027,9 @@ ofproto_flush_flows(struct ofproto *ofproto)
     if (ofproto->in_band) {
         in_band_flushed(ofproto->in_band);
     }
+    if (ofproto->fail_open) {
+        fail_open_flushed(ofproto->fail_open);
+    }
 }
 \f
 static void
@@ -2765,12 +2768,6 @@ handle_odp_msg(struct ofproto *p, struct ofpbuf *packet)
     if (!rule) {
         struct ofport *port;
 
-        if (p->fail_open && fail_open_handle_flow_miss(p->fail_open, p,
-                                                       &flow, &payload)) {
-            ofpbuf_delete(packet);
-            return;
-        }
-
         if (p->ofhooks->packet_in_cb
             && p->ofhooks->packet_in_cb(&flow, &payload, p->aux)) {
             ofpbuf_delete(packet);