ofp-util: Work on decoding OF1.1 flow_mods.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Tested-by: Simon Horman <horms@verge.net.au>
Reviewed-by: Simon Horman <horms@verge.net.au>
---
v3->v4: Builds and passes unit tests.  Somewhat incomplete, I think.

diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 7c78c6394671db412771493ae42003c93967514f..696c3ec45167c3df65961f9c38fd4866efb19d31 100644 (file)
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -467,8 +467,8 @@ struct ofp11_flow_mod {
                                     indicates no restriction. */
     ovs_be16 flags;              /* One of OFPFF_*. */
     uint8_t pad[2];
-    /* Open Flow version specific match */
-    /* struct ofp_instruction instructions[0];  Instruction set */
+    /* Followed by an ofp11_match structure. */
+    /* Followed by an instruction set. */
 };
 OFP_ASSERT(sizeof(struct ofp11_flow_mod) == 40);
 
@@ -528,7 +528,7 @@ struct ofp11_stats_msg {
     ovs_be16 type;              /* One of the OFPST_* constants. */
     ovs_be16 flags;             /* OFPSF_REQ_* flags (none yet defined). */
     uint8_t pad[4];
-    /* uint8_t body[0];             Body of the request. */
+    /* Followed by the body of the request. */
 };
 OFP_ASSERT(sizeof(struct ofp11_stats_msg) == 16);
 
@@ -560,9 +560,9 @@ struct ofp11_flow_stats_request {
     ovs_be64 cookie_mask;     /* Mask used to restrict the cookie bits that
                                  must match. A value of 0 indicates
                                  no restriction. */
-    struct ofp11_match match; /* Fields to match. */
+    /* Followed by an ofp11_match structure. */
 };
-OFP_ASSERT(sizeof(struct ofp11_flow_stats_request) == 120);
+OFP_ASSERT(sizeof(struct ofp11_flow_stats_request) == 32);
 
 /* Body of reply to OFPST_FLOW request. */
 struct ofp11_flow_stats {
@@ -743,8 +743,8 @@ struct ofp11_flow_removed {
     uint8_t pad2[2];          /* Align to 64-bits. */
     ovs_be64 packet_count;
     ovs_be64 byte_count;
-    struct ofp11_match match; /* Description of fields. */
+    /* Followed by an ofp11_match structure. */
 };
-OFP_ASSERT(sizeof(struct ofp11_flow_removed) == 128);
+OFP_ASSERT(sizeof(struct ofp11_flow_removed) == 40);
 
 #endif /* openflow/openflow-1.1.h */

diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index 4ab378046b00feaf78e71302945bd9686dee9cc1..202ba1ed69baecd1230b7549867535e7c00eea1f 100644 (file)
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -322,4 +322,14 @@ enum ofp_match_type {
     OFPMT_OXM = 1,              /* OpenFlow Extensible Match */
 };
 
+/* Group numbering. Groups can use any number up to OFPG_MAX. */
+enum ofp_group {
+    /* Last usable group number. */
+    OFPG_MAX        = 0xffffff00,
+
+    /* Fake groups. */
+    OFPG_ALL        = 0xfffffffc,  /* All groups, for group delete commands. */
+    OFPG_ANY        = 0xffffffff   /* Wildcard, for flow stats requests. */
+};
+
 #endif /* openflow/openflow-common.h */

diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index 1af70da90a06bcead19f82853e31a10df6a69a55..76b65746fa83e0e0cda5383fb6f4acadd4fb47f9 100644 (file)
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -339,6 +339,9 @@ enum ofperr {
      * extension is enabled. */
     OFPERR_NXFMFC_BAD_TABLE_ID,
 
+    /* NX1.0+(3,258).  'out_group' specified but groups not yet supported. */
+    OFPERR_NXFMFC_GROUPS_NOT_SUPPORTED,
+
 /* ## ---------------------- ## */
 /* ## OFPET_GROUP_MOD_FAILED ## */
 /* ## ---------------------- ## */

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 92d3fe465d2b914e3dc8e4adf5fa3d644a6e4933..8d0f99384c58d421eaae315b428f7ed60beb1141 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -265,6 +265,31 @@ ofputil_cls_rule_to_ofp10_match(const struct cls_rule *rule,
     memset(match->pad2, '\0', sizeof match->pad2);
 }
 
+enum ofperr
+ofputil_pull_ofp11_match(struct ofpbuf *buf, unsigned int priority,
+                         struct cls_rule *rule)
+{
+    struct ofp11_match_header *omh;
+    struct ofp11_match *om;
+
+    if (buf->size < sizeof(struct ofp11_match_header)) {
+        return OFPERR_OFPBMC_BAD_LEN;
+    }
+
+    omh = buf->data;
+    switch (ntohs(omh->type)) {
+    case OFPMT_STANDARD:
+        if (omh->length != htons(sizeof *om) || buf->size < sizeof *om) {
+            return OFPERR_OFPBMC_BAD_LEN;
+        }
+        om = ofpbuf_pull(buf, sizeof *om);
+        return ofputil_cls_rule_from_ofp11_match(om, priority, rule);
+
+    default:
+        return OFPERR_OFPBMC_BAD_TYPE;
+    }
+}
+
 /* Converts the ofp11_match in 'match' into a cls_rule in 'rule', with the
  * given 'priority'.  Returns 0 if successful, otherwise an OFPERR_* value. */
 enum ofperr
@@ -1102,87 +1127,131 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
 
     ofpbuf_use_const(&b, oh, ntohs(oh->length));
     raw = ofpraw_pull_assert(&b);
-    if (raw == OFPRAW_OFPT10_FLOW_MOD) {
+    if (raw == OFPRAW_OFPT11_FLOW_MOD) {
         /* Standard OpenFlow 1.1 flow_mod. */
-        const struct ofp10_flow_mod *ofm;
-        uint16_t priority;
+        const struct ofp11_flow_mod *ofm;
         enum ofperr error;
 
-        /* Get the ofp_flow_mod. */
         ofm = ofpbuf_pull(&b, sizeof *ofm);
 
-        /* Set priority based on original wildcards.  Normally we'd allow
-         * ofputil_cls_rule_from_match() to do this for us, but
-         * ofputil_normalize_rule() can put wildcards where the original flow
-         * didn't have them. */
-        priority = ntohs(ofm->priority);
-        if (!(ofm->match.wildcards & htonl(OFPFW10_ALL))) {
-            priority = UINT16_MAX;
+        error = ofputil_pull_ofp11_match(&b, ntohs(ofm->priority), &fm->cr);
+        if (error) {
+            return error;
         }
 
-        /* Translate the rule. */
-        ofputil_cls_rule_from_ofp10_match(&ofm->match, priority, &fm->cr);
-        ofputil_normalize_rule(&fm->cr);
-
-        /* Now get the actions. */
-        error = ofpacts_pull_openflow10(&b, b.size, ofpacts);
+        error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
         if (error) {
             return error;
         }
 
         /* Translate the message. */
-        command = ntohs(ofm->command);
-        fm->cookie = htonll(0);
-        fm->cookie_mask = htonll(0);
-        fm->new_cookie = ofm->cookie;
+        if (ofm->command == OFPFC_ADD) {
+            fm->cookie = htonll(0);
+            fm->cookie_mask = htonll(0);
+            fm->new_cookie = ofm->cookie;
+        } else {
+            /* XXX */
+            fm->cookie = ofm->cookie;
+            fm->cookie_mask = ofm->cookie_mask;
+            fm->new_cookie = htonll(UINT64_MAX);
+        }
+        fm->command = ofm->command;
+        fm->table_id = ofm->table_id;
         fm->idle_timeout = ntohs(ofm->idle_timeout);
         fm->hard_timeout = ntohs(ofm->hard_timeout);
         fm->buffer_id = ntohl(ofm->buffer_id);
-        fm->out_port = ntohs(ofm->out_port);
-        fm->flags = ntohs(ofm->flags);
-    } else if (raw == OFPRAW_NXT_FLOW_MOD) {
-        /* Nicira extended flow_mod. */
-        const struct nx_flow_mod *nfm;
-        enum ofperr error;
-
-        /* Dissect the message. */
-        nfm = ofpbuf_pull(&b, sizeof *nfm);
-        error = nx_pull_match(&b, ntohs(nfm->match_len), ntohs(nfm->priority),
-                              &fm->cr, &fm->cookie, &fm->cookie_mask);
+        error = ofputil_port_from_ofp11(ofm->out_port, &fm->out_port);
         if (error) {
             return error;
         }
-        error = ofpacts_pull_openflow10(&b, b.size, ofpacts);
-        if (error) {
-            return error;
+        if (ofm->out_group != htonl(OFPG_ANY)) {
+            return OFPERR_NXFMFC_GROUPS_NOT_SUPPORTED;
         }
-
-        /* Translate the message. */
-        command = ntohs(nfm->command);
-        if ((command & 0xff) == OFPFC_ADD && fm->cookie_mask) {
-            /* Flow additions may only set a new cookie, not match an
-             * existing cookie. */
-            return OFPERR_NXBRC_NXM_INVALID;
-        }
-        fm->new_cookie = nfm->cookie;
-        fm->idle_timeout = ntohs(nfm->idle_timeout);
-        fm->hard_timeout = ntohs(nfm->hard_timeout);
-        fm->buffer_id = ntohl(nfm->buffer_id);
-        fm->out_port = ntohs(nfm->out_port);
-        fm->flags = ntohs(nfm->flags);
+        fm->flags = ntohs(ofm->flags);
     } else {
-        NOT_REACHED();
+        if (raw == OFPRAW_OFPT10_FLOW_MOD) {
+            /* Standard OpenFlow 1.0 flow_mod. */
+            const struct ofp10_flow_mod *ofm;
+            uint16_t priority;
+            enum ofperr error;
+
+            /* Get the ofp10_flow_mod. */
+            ofm = ofpbuf_pull(&b, sizeof *ofm);
+
+            /* Set priority based on original wildcards.  Normally we'd allow
+             * ofputil_cls_rule_from_match() to do this for us, but
+             * ofputil_normalize_rule() can put wildcards where the original
+             * flow didn't have them. */
+            priority = ntohs(ofm->priority);
+            if (!(ofm->match.wildcards & htonl(OFPFW10_ALL))) {
+                priority = UINT16_MAX;
+            }
+
+            /* Translate the rule. */
+            ofputil_cls_rule_from_ofp10_match(&ofm->match, priority, &fm->cr);
+            ofputil_normalize_rule(&fm->cr);
+
+            /* Now get the actions. */
+            error = ofpacts_pull_openflow10(&b, b.size, ofpacts);
+            if (error) {
+                return error;
+            }
+
+            /* Translate the message. */
+            command = ntohs(ofm->command);
+            fm->cookie = htonll(0);
+            fm->cookie_mask = htonll(0);
+            fm->new_cookie = ofm->cookie;
+            fm->idle_timeout = ntohs(ofm->idle_timeout);
+            fm->hard_timeout = ntohs(ofm->hard_timeout);
+            fm->buffer_id = ntohl(ofm->buffer_id);
+            fm->out_port = ntohs(ofm->out_port);
+            fm->flags = ntohs(ofm->flags);
+        } else if (raw == OFPRAW_NXT_FLOW_MOD) {
+            /* Nicira extended flow_mod. */
+            const struct nx_flow_mod *nfm;
+            enum ofperr error;
+
+            /* Dissect the message. */
+            nfm = ofpbuf_pull(&b, sizeof *nfm);
+            error = nx_pull_match(&b, ntohs(nfm->match_len), ntohs(nfm->priority),
+                                  &fm->cr, &fm->cookie, &fm->cookie_mask);
+            if (error) {
+                return error;
+            }
+            error = ofpacts_pull_openflow10(&b, b.size, ofpacts);
+            if (error) {
+                return error;
+            }
+
+            /* Translate the message. */
+            command = ntohs(nfm->command);
+            if ((command & 0xff) == OFPFC_ADD && fm->cookie_mask) {
+                /* Flow additions may only set a new cookie, not match an
+                 * existing cookie. */
+                return OFPERR_NXBRC_NXM_INVALID;
+            }
+            fm->new_cookie = nfm->cookie;
+            fm->idle_timeout = ntohs(nfm->idle_timeout);
+            fm->hard_timeout = ntohs(nfm->hard_timeout);
+            fm->buffer_id = ntohl(nfm->buffer_id);
+            fm->out_port = ntohs(nfm->out_port);
+            fm->flags = ntohs(nfm->flags);
+        } else {
+            NOT_REACHED();
+        }
+
+        if (protocol & OFPUTIL_P_TID) {
+            fm->command = command & 0xff;
+            fm->table_id = command >> 8;
+        } else {
+            fm->command = command;
+            fm->table_id = 0xff;
+        }
     }
 
     fm->ofpacts = ofpacts->data;
     fm->ofpacts_len = ofpacts->size;
-    if (protocol & OFPUTIL_P_TID) {
-        fm->command = command & 0xff;
-        fm->table_id = command >> 8;
-    } else {
-        fm->command = command;
-        fm->table_id = 0xff;
-    }
 
     return 0;
 }

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 54bec75f6efe48f7f204a0eea14a9b2e5be556c2..c87d6488bb89ccf1390eabc8d39192c0d3388c74 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -117,6 +117,8 @@ void ofputil_cls_rule_to_ofp10_match(const struct cls_rule *,
                                      struct ofp10_match *);
 
 /* Work with ofp11_match. */
+enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *, unsigned int priority,
+                                     struct cls_rule *);
 enum ofperr ofputil_cls_rule_from_ofp11_match(const struct ofp11_match *,
                                               unsigned int priority,
                                               struct cls_rule *);