ofproto: Check overlap, emerg flow cache, and error code sync (OpenFlow 0.9)

This commit adds (some) support for a couple new OpenFlow 0.9 features:

    - The OFPFF_CHECK_OVERLAP flag in Flow Mod messages allows the
      controller to prevent flows that would conflict at the same
      priority.

    - An emergency flow cache that contains a small flow table that is
      used if the switch loses connectivity with the controller.  I
      believe the design has fundamental flaws and looks likely to be
      retired.  If a controller attempts to add a flow to the emergency
      flow cache, OVS always responds that the tables are full.

The OpenFlow 0.9 error codes are also sync'd in the commit.

NOTE: OVS at this point is not wire-compatible with OpenFlow 0.9 until the
final commit in this OpenFlow 0.9 set.

diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h
index c45dbeb..a9199c7 100644 (file)
--- a/include/openflow/openflow.h
+++ b/include/openflow/openflow.h
@@ -529,6 +529,13 @@ OFP_ASSERT(sizeof(struct ofp_match) == 40);
 /* By default, choose a priority in the middle. */
 #define OFP_DEFAULT_PRIORITY 0x8000
 
+enum ofp_flow_mod_flags {
+    OFPFF_SEND_FLOW_REM = 1 << 0,  /* Send flow removed message when flow
+                                    * expires or is deleted. */
+    OFPFF_CHECK_OVERLAP = 1 << 1,  /* Check for overlapping entries first. */
+    OFPFF_EMERG         = 1 << 2   /* Ramark this is for emergency. */
+};
+
 /* Flow setup and teardown (controller -> datapath). */
 struct ofp_flow_mod {
     struct ofp_header header;
@@ -545,7 +552,7 @@ struct ofp_flow_mod {
                                      matching entries to include this as an 
                                      output port.  A value of OFPP_NONE 
                                      indicates no restriction. */
-    uint8_t pad[2];               /* Align to 32-bits. */
+    uint16_t flags;               /* One of OFPFF_*. */
     uint32_t reserved;            /* Reserved for future use. */
     struct ofp_action_header actions[0]; /* The action length is inferred 
                                             from the length field in the 
@@ -589,7 +596,8 @@ enum ofp_error_type {
 /* ofp_error_msg 'code' values for OFPET_HELLO_FAILED.  'data' contains an
  * ASCII text string that may give failure details. */
 enum ofp_hello_failed_code {
-    OFPHFC_INCOMPATIBLE         /* No compatible version. */
+    OFPHFC_INCOMPATIBLE,        /* No compatible version. */
+    OFPHFC_EPERM                /* Permissions error. */
 };
 
 /* ofp_error_msg 'code' values for OFPET_BAD_REQUEST.  'data' contains at least
@@ -601,9 +609,10 @@ enum ofp_bad_request_code {
     OFPBRC_BAD_VENDOR,          /* Vendor not supported (in ofp_vendor_header 
                                  * or ofp_stats_request or ofp_stats_reply). */
     OFPBRC_BAD_SUBTYPE,         /* Vendor subtype not supported. */
-    OFPBRC_BAD_LENGTH,          /* Wrong request length for type. */
+    OFPBRC_EPERM,               /* Permissions error. */
+    OFPBRC_BAD_LEN,             /* Wrong request length for type. */
     OFPBRC_BUFFER_EMPTY,        /* Specified buffer has already been used. */
-    OFPBRC_BAD_COOKIE           /* Specified buffer does not exist. */
+    OFPBRC_BUFFER_UNKNOWN       /* Specified buffer does not exist. */
 };
 
 /* ofp_error_msg 'code' values for OFPET_BAD_ACTION.  'data' contains at least 
@@ -615,6 +624,7 @@ enum ofp_bad_action_code {
     OFPBAC_BAD_VENDOR_TYPE,    /* Unknown action type for vendor id. */
     OFPBAC_BAD_OUT_PORT,       /* Problem validating output action. */
     OFPBAC_BAD_ARGUMENT,       /* Bad action argument. */
+    OFPBAC_EPERM,              /* Permissions error. */
     OFPBAC_TOO_MANY            /* Can't handle this many actions. */
 };
 
@@ -622,6 +632,11 @@ enum ofp_bad_action_code {
  * at least the first 64 bytes of the failed request. */
 enum ofp_flow_mod_failed_code {
     OFPFMFC_ALL_TABLES_FULL,    /* Flow not added because of full tables. */
+    OFPFMFC_OVERLAP,            /* Attempted to add overlapping flow with
+                                 * CHECK_OVERLAP flag set. */
+    OFPFMFC_EPERM,              /* Permissions error. */
+    OFPFMFC_BAD_EMERG_TIMEOUT,  /* Flow not added because of non-zero idle/hard
+                                 * timeout. */
     OFPFMFC_BAD_COMMAND         /* Unknown command. */
 };
 

diff --git a/lib/classifier.c b/lib/classifier.c
index b0f60e7..c0d421d 100644 (file)
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -48,6 +48,8 @@ static struct cls_rule *search_exact_table(const struct classifier *,
                                            size_t hash, const flow_t *);
 static bool rules_match_1wild(const struct cls_rule *fixed,
                               const struct cls_rule *wild, int field_idx);
+static bool rules_match_2wild(const struct cls_rule *wild1,
+                              const struct cls_rule *wild2, int field_idx);
 
 /* Converts the flow in 'flow' into a cls_rule in 'rule', with the given
  * 'wildcards' and 'priority'.*/
@@ -334,6 +336,43 @@ classifier_find_rule_exactly(const struct classifier *cls,
     return NULL;
 }
 
+/* Checks if the flow defined by 'target' with 'wildcards' at 'priority' 
+ * overlaps with any other rule at the same priority in the classifier.  
+ * Two rules are considered overlapping if a packet could match both. */
+bool
+classifier_rule_overlaps(const struct classifier *cls,
+                         const flow_t *target, uint32_t wildcards,
+                         unsigned int priority)
+{
+    struct cls_rule target_rule;
+    const struct hmap *tbl;
+
+    if (!wildcards) {
+        return search_exact_table(cls, flow_hash(target, 0), target) ?
+            true : false;
+    }
+
+    cls_rule_from_flow(&target_rule, target, wildcards, priority);
+
+    for (tbl = &cls->tables[0]; tbl < &cls->tables[CLS_N_FIELDS]; tbl++) {
+        struct cls_bucket *bucket;
+
+        HMAP_FOR_EACH (bucket, struct cls_bucket, hmap_node, tbl) {
+            struct cls_rule *rule;
+
+            LIST_FOR_EACH (rule, struct cls_rule, node.list,
+                           &bucket->rules) {
+                if (rule->priority == priority 
+                        && rules_match_2wild(rule, &target_rule, 0)) {
+                    return true;
+                }
+            }
+        }
+    }
+
+    return false;
+}
+
 /* Ignores target->priority.
  *
  * 'callback' is allowed to delete the rule that is passed as its argument, but
@@ -762,6 +801,23 @@ rules_match_1wild(const struct cls_rule *fixed, const struct cls_rule *wild,
                        wild->wc.nw_dst_mask, field_idx);
 }
 
+/* Returns true if 'wild1' and 'wild2' match, that is, if their fields
+ * are equal modulo wildcards in 'wild1' or 'wild2'.
+ *
+ * 'field_idx' is the index of the first field to be compared; fields before
+ * 'field_idx' are assumed to match.  Always returns true if 'field_idx' is
+ * CLS_N_FIELDS. */
+static bool
+rules_match_2wild(const struct cls_rule *wild1, const struct cls_rule *wild2,
+                  int field_idx)
+{
+    return rules_match(wild1, wild2, 
+                       wild1->wc.wildcards | wild2->wc.wildcards, 
+                       wild1->wc.nw_src_mask & wild2->wc.nw_src_mask,
+                       wild1->wc.nw_dst_mask & wild2->wc.nw_dst_mask, 
+                       field_idx);
+}
+
 /* Searches 'bucket' for a rule that matches 'target'.  Returns the
  * highest-priority match, if one is found, or a null pointer if there is no
  * match.

diff --git a/lib/classifier.h b/lib/classifier.h
index 84a3461..45cb957 100644 (file)
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -143,6 +143,8 @@ struct cls_rule *classifier_lookup_wild(const struct classifier *,
                                         const flow_t *);
 struct cls_rule *classifier_lookup_exact(const struct classifier *,
                                          const flow_t *);
+bool classifier_rule_overlaps(const struct classifier *, const flow_t *, 
+                              uint32_t wildcards, unsigned int priority);
 
 typedef void cls_cb_func(struct cls_rule *, void *aux);
 

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 5385dd1..1dd269d 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -804,6 +804,7 @@ static const struct error_type error_types[] = {
 #define ERROR_CODE(TYPE, CODE) {TYPE, CODE, #CODE}
     ERROR_TYPE(OFPET_HELLO_FAILED),
     ERROR_CODE(OFPET_HELLO_FAILED, OFPHFC_INCOMPATIBLE),
+    ERROR_CODE(OFPET_HELLO_FAILED, OFPHFC_EPERM),
 
     ERROR_TYPE(OFPET_BAD_REQUEST),
     ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_VERSION),
@@ -811,9 +812,10 @@ static const struct error_type error_types[] = {
     ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT),
     ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR),
     ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE),
-    ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH),
+    ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_EPERM),
+    ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN),
     ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BUFFER_EMPTY),
-    ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE),
+    ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BUFFER_UNKNOWN),
 
     ERROR_TYPE(OFPET_BAD_ACTION),
     ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE),
@@ -822,10 +824,14 @@ static const struct error_type error_types[] = {
     ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE),
     ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_OUT_PORT),
     ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT),
+    ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_EPERM),
     ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_TOO_MANY),
 
     ERROR_TYPE(OFPET_FLOW_MOD_FAILED),
     ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL),
+    ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_OVERLAP),
+    ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_EPERM),
+    ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_EMERG_TIMEOUT),
     ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_COMMAND),
 
     ERROR_TYPE(OFPET_PORT_MOD_FAILED),

diff --git a/lib/vconn.c b/lib/vconn.c
index 09f6976..533278b 100644 (file)
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -1046,7 +1046,7 @@ check_ofp_message(const struct ofp_header *msg, uint8_t type, size_t size)
                      "received %s message of length %zu (expected %zu)",
                      type_name, got_size, size);
         free(type_name);
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
 
     return 0;
@@ -1082,7 +1082,7 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type,
                      "(expected at least %zu)",
                      type_name, got_size, min_size);
         free(type_name);
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
     if ((got_size - min_size) % array_elt_size) {
         char *type_name = ofp_message_type_to_string(type);
@@ -1093,7 +1093,7 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type,
                      type_name, got_size, min_size, got_size - min_size,
                      array_elt_size, (got_size - min_size) % array_elt_size);
         free(type_name);
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
     if (n_array_elts) {
         *n_array_elts = (got_size - min_size) / array_elt_size;
@@ -1123,13 +1123,13 @@ check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data,
         VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out claims %u bytes of actions "
                      "but message has room for only %zu bytes",
                      actions_len, extra);
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
     if (actions_len % sizeof(union ofp_action)) {
         VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out claims %u bytes of actions, "
                      "which is not a multiple of %zu",
                      actions_len, sizeof(union ofp_action));
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
 
     n_actions = actions_len / sizeof(union ofp_action);

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index baa5a8b..dbb1a21 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2577,7 +2577,7 @@ handle_flow_stats_request(struct ofproto *p, struct ofconn *ofconn,
     struct cls_rule target;
 
     if (arg_size != sizeof *fsr) {
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
     fsr = (struct ofp_flow_stats_request *) osr->body;
 
@@ -2685,7 +2685,7 @@ handle_aggregate_stats_request(struct ofproto *p, struct ofconn *ofconn,
     struct ofpbuf *msg;
 
     if (arg_size != sizeof *asr) {
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
     asr = (struct ofp_aggregate_stats_request *) osr->body;
 
@@ -2790,6 +2790,17 @@ add_flow(struct ofproto *p, struct ofconn *ofconn,
     uint16_t in_port;
     int error;
 
+    if (ofm->flags & htons(OFPFF_CHECK_OVERLAP)) {
+        flow_t flow;
+        uint32_t wildcards;
+
+        flow_from_match(&flow, &wildcards, &ofm->match);
+        if (classifier_rule_overlaps(&p->cls, &flow, wildcards,
+                                     ntohs(ofm->priority))) {
+            return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_OVERLAP);
+        }
+    }
+
     rule = rule_create(p, NULL, (const union ofp_action *) ofm->actions,
                        n_actions, ntohs(ofm->idle_timeout),
                        ntohs(ofm->hard_timeout));
@@ -2923,6 +2934,14 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn,
         return error;
     }
 
+    /* We do not support the emergency flow cache.  It will hopefully
+     * get dropped from OpenFlow in the near future. */
+    if (ofm->flags & htons(OFPFF_EMERG)) {
+        /* There isn't a good fit for an error code, so just state that the
+         * flow table is full. */
+        return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL);
+    }
+
     normalize_match(&ofm->match);
     if (!ofm->match.wildcards) {
         ofm->priority = htons(UINT16_MAX);
@@ -2982,7 +3001,7 @@ handle_ofmp(struct ofproto *p, struct ofconn *ofconn,
     size_t msg_len = ntohs(ofmph->header.header.length);
     if (msg_len < sizeof(*ofmph)) {
         VLOG_WARN_RL(&rl, "dropping short managment message: %zu\n", msg_len);
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
 
     if (ofmph->type == htons(OFMPT_CAPABILITY_REQUEST)) {
@@ -2991,7 +3010,7 @@ handle_ofmp(struct ofproto *p, struct ofconn *ofconn,
         if (msg_len < sizeof(struct ofmp_capability_request)) {
             VLOG_WARN_RL(&rl, "dropping short capability request: %zu\n",
                     msg_len);
-            return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+            return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
         }
 
         ofmpcr = (struct ofmp_capability_request *)ofmph;
@@ -3014,13 +3033,13 @@ handle_vendor(struct ofproto *p, struct ofconn *ofconn, void *msg)
     struct nicira_header *nh;
 
     if (ntohs(ovh->header.length) < sizeof(struct ofp_vendor_header)) {
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
     if (ovh->vendor != htonl(NX_VENDOR_ID)) {
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR);
     }
     if (ntohs(ovh->header.length) < sizeof(struct nicira_header)) {
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
 
     nh = msg;

diff --git a/ofproto/pktbuf.c b/ofproto/pktbuf.c
index 450cc3b..ac60e3d 100644 (file)
--- a/ofproto/pktbuf.c
+++ b/ofproto/pktbuf.c
@@ -165,7 +165,7 @@ pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct ofpbuf **bufferp,
     if (!pb) {
         VLOG_WARN_RL(&rl, "attempt to send buffered packet via connection "
                      "without buffers");
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE);
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BUFFER_UNKNOWN);
     }
 
     p = &pb->packets[id & PKTBUF_MASK];
@@ -183,10 +183,10 @@ pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct ofpbuf **bufferp,
             error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BUFFER_EMPTY);
         }
     } else if (id >> PKTBUF_BITS != COOKIE_MAX) {
-        COVERAGE_INC(pktbuf_bad_cookie);
+        COVERAGE_INC(pktbuf_buffer_unknown);
         VLOG_WARN_RL(&rl, "cookie mismatch: %08"PRIx32" != %08"PRIx32,
                      id, (id & PKTBUF_MASK) | (p->cookie << PKTBUF_BITS));
-        error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE);
+        error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BUFFER_UNKNOWN);
     } else {
         COVERAGE_INC(pktbuf_null_cookie);
         VLOG_INFO_RL(&rl, "Received null cookie %08"PRIx32" (this is normal "