Drop controller-bound traffic that arrives on the controller's port.

Before, if a hub connected a number of OpenFlow switches and the controller,
then in-band control traffic from one of the OpenFlow switches would be
seen by each of the other switches and forwarded up to the controller as
an ofp_packet_in message.  That message would then be seen by all of the
other OpenFlow switches and also forwarded, and so on in an infinite loop.

This change prevents this situation by keeping secchan from forwarding
ofp_packet_in messages for a packet destined to the controller that
arrives on the port where the controller is located.

This code has at least two weaknesses.  First, if the controller's port
changes, then the flows set up to drop packets will not be deleted.  This
should not be a major problem: if this inadvertently kills a switch's
connection to the controller, then the switch will realize it after it
stops receiving data and re-connect.  Its new connection will have new
flow data and therefore its packets will not be dropped.

Second, the notion of the "controller's port" does not take into account
the possibility of loops in the network topology.  We need spanning tree
protocol for that.

diff --git a/include/vconn.h b/include/vconn.h
index 4b0eaa7ed13d110fda77d0a83465dfbcf75b53b8..22a1da835ec5e1956570537003bc104474312a7e 100644 (file)
--- a/include/vconn.h
+++ b/include/vconn.h
@@ -83,6 +83,8 @@ void *make_openflow(size_t openflow_len, uint8_t type, struct buffer **);
 void *make_openflow_xid(size_t openflow_len, uint8_t type,
                         uint32_t xid, struct buffer **);
 void update_openflow_length(struct buffer *);
+struct buffer *make_add_flow(const struct flow *, uint32_t buffer_id,
+                             uint16_t max_idle, size_t n_actions);
 struct buffer *make_add_simple_flow(const struct flow *,
                                     uint32_t buffer_id, uint16_t out_port,
                                     uint16_t max_idle);

diff --git a/lib/vconn.c b/lib/vconn.c
index 7af59f38cd341f6806c4f47ce4ec78ba3cb0b165..332f5de766c79432e7a1579b2bbfb83e6cabfd55 100644 (file)
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -488,11 +488,11 @@ update_openflow_length(struct buffer *buffer)
 }
 
 struct buffer *
-make_add_simple_flow(const struct flow *flow,
-                     uint32_t buffer_id, uint16_t out_port, uint16_t max_idle)
+make_add_flow(const struct flow *flow, uint32_t buffer_id, uint16_t max_idle,
+              size_t n_actions)
 {
     struct ofp_flow_mod *ofm;
-    size_t size = sizeof *ofm + sizeof ofm->actions[0];
+    size_t size = sizeof *ofm + n_actions * sizeof ofm->actions[0];
     struct buffer *out = buffer_new(size);
     ofm = buffer_put_uninit(out, size);
     memset(ofm, 0, size);
@@ -513,10 +513,19 @@ make_add_simple_flow(const struct flow *flow,
     ofm->command = htons(OFPFC_ADD);
     ofm->max_idle = htons(max_idle);
     ofm->buffer_id = htonl(buffer_id);
+    return out;
+}
+
+struct buffer *
+make_add_simple_flow(const struct flow *flow,
+                     uint32_t buffer_id, uint16_t out_port, uint16_t max_idle)
+{
+    struct buffer *buffer = make_add_flow(flow, buffer_id, max_idle, 1);
+    struct ofp_flow_mod *ofm = buffer->data;
     ofm->actions[0].type = htons(OFPAT_OUTPUT);
     ofm->actions[0].arg.output.max_len = htons(0);
     ofm->actions[0].arg.output.port = htons(out_port);
-    return out;
+    return buffer;
 }
 
 struct buffer *

diff --git a/secchan/secchan.c b/secchan/secchan.c
index 50ebebb558d4e8fe89c8f8cd1ee16a95aad6885f..30531988a8108c971eb27a12e43031f096ebfc42 100644 (file)
--- a/secchan/secchan.c
+++ b/secchan/secchan.c
@@ -451,9 +451,8 @@ queue_tx(struct rconn *rc, struct buffer *b)
     }
 }
 
-static bool
-is_controller_mac(const uint8_t dl_addr[ETH_ADDR_LEN], struct netdev *netdev,
-                  struct rconn *controller)
+static const uint8_t *
+get_controller_mac(struct netdev *netdev, struct rconn *controller)
 {
     static uint32_t ip, last_nonzero_ip;
     static uint8_t mac[ETH_ADDR_LEN], last_nonzero_mac[ETH_ADDR_LEN];
@@ -498,7 +497,14 @@ is_controller_mac(const uint8_t dl_addr[ETH_ADDR_LEN], struct netdev *netdev,
          * Otherwise, we can afford to wait a little while. */
         next_refresh = now + (!ip || have_mac ? 10 : 1);
     }
-    return !eth_addr_is_zero(mac) && eth_addr_equals(mac, dl_addr);
+    return !eth_addr_is_zero(mac) ? mac : NULL;
+}
+
+static bool
+is_controller_mac(const uint8_t mac[ETH_ADDR_LEN],
+                  const uint8_t *controller_mac)
+{
+    return controller_mac && eth_addr_equals(mac, controller_mac);
 }
 
 static bool
@@ -513,6 +519,7 @@ in_band_packet_cb(struct relay *r, int half, void *in_band_)
     struct buffer pkt;
     struct flow flow;
     uint16_t in_port, out_port;
+    const uint8_t *controller_mac;
 
     if (half != HALF_LOCAL || r->is_mgmt_conn) {
         return false;
@@ -537,9 +544,13 @@ in_band_packet_cb(struct relay *r, int half, void *in_band_)
     flow_extract(&pkt, in_port, &flow);
 
     /* Deal with local stuff. */
+    controller_mac = get_controller_mac(in_band->of_device,
+                                        r->halves[HALF_REMOTE].rconn);
     if (in_port == OFPP_LOCAL) {
+        /* Sent by secure channel. */
         out_port = mac_learning_lookup(in_band->ml, flow.dl_dst);
     } else if (eth_addr_equals(flow.dl_dst, in_band->mac)) {
+        /* Sent to secure channel. */
         out_port = OFPP_LOCAL;
         if (mac_learning_learn(in_band->ml, flow.dl_src, in_port)) {
             VLOG_DBG("learned that "ETH_ADDR_FMT" is on port %"PRIu16,
@@ -547,9 +558,16 @@ in_band_packet_cb(struct relay *r, int half, void *in_band_)
         }
     } else if (flow.dl_type == htons(ETH_TYPE_ARP)
                && eth_addr_is_broadcast(flow.dl_dst)
-               && is_controller_mac(flow.dl_src, in_band->of_device,
-                                    r->halves[HALF_REMOTE].rconn)) {
+               && is_controller_mac(flow.dl_src, controller_mac)) {
+        /* ARP sent by controller. */
         out_port = OFPP_FLOOD;
+    } else if (is_controller_mac(flow.dl_dst, controller_mac)
+               && in_port == mac_learning_lookup(in_band->ml,
+                                                 controller_mac)) {
+        /* Drop controller traffic that arrives on the controller port. */
+        queue_tx(rc, make_add_flow(&flow, ntohl(opi->buffer_id),
+                                   in_band->s->max_idle, 0));
+        return true;
     } else {
         return false;
     }