nx-match: Separate raw match and header/pad pull/put

In the case of Open Flow 1.2, which is currently the only
time that OXM is be used, there is a 4 byte header before
the match which needs to be taken into account when calculating
the pad length. This complicates nx_match pull and put somewhat.

This patch takes an approach suggested by Ben Pfaff to separate the
encoding of the match and the adding of padding and, in the case of OXM,
a header.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 893566da4b0daf66e38e15e0908280e7ecce7924..16c1674ac87c7d95a9d52441c7f1a29bb37b238f 100644 (file)
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -91,12 +91,11 @@ nx_entry_ok(const void *p, unsigned int match_len)
 }
 
 static enum ofperr
-nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict,
-                uint16_t priority, struct cls_rule *rule,
-                ovs_be64 *cookie, ovs_be64 *cookie_mask)
+nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict,
+            uint16_t priority, struct cls_rule *rule,
+            ovs_be64 *cookie, ovs_be64 *cookie_mask)
 {
     uint32_t header;
-    uint8_t *p;
 
     assert((cookie != NULL) == (cookie_mask != NULL));
 
@@ -108,14 +107,6 @@ nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict,
         return 0;
     }
 
-    p = ofpbuf_try_pull(b, ROUND_UP(match_len, 8));
-    if (!p) {
-        VLOG_DBG_RL(&rl, "nx_match length %u, rounded up to a "
-                    "multiple of 8, is longer than space in message (max "
-                    "length %zu)", match_len, b->size);
-        return OFPERR_OFPBMC_BAD_LEN;
-    }
-
     for (;
          (header = nx_entry_ok(p, match_len)) != 0;
          p += 4 + NXM_LENGTH(header), match_len -= 4 + NXM_LENGTH(header)) {
@@ -198,6 +189,27 @@ nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict,
     return match_len ? OFPERR_OFPBMC_BAD_LEN : 0;
 }
 
+static enum ofperr
+nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict,
+                uint16_t priority, struct cls_rule *rule,
+                ovs_be64 *cookie, ovs_be64 *cookie_mask)
+{
+    uint8_t *p = NULL;
+
+    if (match_len) {
+        p = ofpbuf_try_pull(b, ROUND_UP(match_len, 8));
+        if (!p) {
+            VLOG_DBG_RL(&rl, "nx_match length %u, rounded up to a "
+                        "multiple of 8, is longer than space in message (max "
+                        "length %zu)", match_len, b->size);
+            return OFPERR_OFPBMC_BAD_LEN;
+        }
+    }
+
+    return nx_pull_raw(p, match_len, strict, priority, rule,
+                       cookie, cookie_mask);
+}
+
 /* Parses the nx_match formatted match description in 'b' with length
  * 'match_len'.  The results are stored in 'rule', which is initialized with
  * 'priority'.  If 'cookie' and 'cookie_mask' contain valid pointers, then the
@@ -226,6 +238,61 @@ nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len,
     return nx_pull_match__(b, match_len, false, priority, rule, cookie,
                            cookie_mask);
 }
+
+static enum ofperr
+oxm_pull_match__(struct ofpbuf *b, bool strict,
+                 uint16_t priority, struct cls_rule *rule)
+{
+    struct ofp11_match_header *omh = b->data;
+    uint8_t *p;
+    uint16_t match_len;
+
+    if (b->size < sizeof *omh) {
+        return OFPERR_OFPBMC_BAD_LEN;
+    }
+
+    match_len = ntohs(omh->length);
+    if (match_len < sizeof *omh) {
+        return OFPERR_OFPBMC_BAD_LEN;
+    }
+
+    if (omh->type != htons(OFPMT_OXM)) {
+        return OFPERR_OFPBMC_BAD_TYPE;
+    }
+
+    p = ofpbuf_try_pull(b, ROUND_UP(match_len, 8));
+    if (!p) {
+        VLOG_DBG_RL(&rl, "oxm length %u, rounded up to a "
+                    "multiple of 8, is longer than space in message (max "
+                    "length %zu)", match_len, b->size);
+        return OFPERR_OFPBMC_BAD_LEN;
+    }
+
+    return nx_pull_raw(p + sizeof *omh, match_len - sizeof *omh,
+                       strict, priority, rule, NULL, NULL);
+}
+
+/* Parses the oxm formatted match description preceeded by a struct
+ * ofp11_match in 'b' with length 'match_len'.  The results are stored in
+ * 'rule', which is initialized with 'priority'.
+ *
+ * Fails with an error when encountering unknown OXM headers.
+ *
+ * Returns 0 if successful, otherwise an OpenFlow error code. */
+enum ofperr
+oxm_pull_match(struct ofpbuf *b, uint16_t priority, struct cls_rule *rule)
+{
+    return oxm_pull_match__(b, true, priority, rule);
+}
+
+/* Behaves the same as oxm_pull_match() with one exception.  Skips over unknown
+ * PXM headers instead of failing with an error when they are encountered. */
+enum ofperr
+oxm_pull_match_loose(struct ofpbuf *b, uint16_t priority,
+                     struct cls_rule *rule)
+{
+    return oxm_pull_match__(b, false, priority, rule);
+}
 \f
 /* nx_put_match() and helpers.
  *
@@ -472,10 +539,9 @@ nxm_put_ip(struct ofpbuf *b, const struct cls_rule *cr,
 }
 
 /* Appends to 'b' the nx_match format that expresses 'cr' (except for
- * 'cr->priority', because priority is not part of nx_match), plus enough
- * zero bytes to pad the nx_match out to a multiple of 8.  For Flow Mod
- * and Flow Stats Requests messages, a 'cookie' and 'cookie_mask' may be
- * supplied.  Otherwise, 'cookie_mask' should be zero.
+ * 'cr->priority', because priority is not part of nx_match).  For Flow Mod and
+ * Flow Stats Requests messages, a 'cookie' and 'cookie_mask' may be supplied.
+ * Otherwise, 'cookie_mask' should be zero.
  *
  * This function can cause 'b''s data to be reallocated.
  *
@@ -483,9 +549,9 @@ nxm_put_ip(struct ofpbuf *b, const struct cls_rule *cr,
  *
  * If 'cr' is a catch-all rule that matches every packet, then this function
  * appends nothing to 'b' and returns 0. */
-int
-nx_put_match(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
-             ovs_be64 cookie, ovs_be64 cookie_mask)
+static int
+nx_put_raw(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
+           ovs_be64 cookie, ovs_be64 cookie_mask)
 {
     const flow_wildcards_t wc = cr->wc.wildcards;
     const struct flow *flow = &cr->flow;
@@ -603,7 +669,56 @@ nx_put_match(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
     nxm_put_64m(b, NXM_NX_COOKIE, cookie, cookie_mask);
 
     match_len = b->size - start_len;
+    return match_len;
+}
+
+/* Appends to 'b' the nx_match format that expresses 'cr' (except for
+ * 'cr->priority', because priority is not part of nx_match), plus enough zero
+ * bytes to pad the nx_match out to a multiple of 8.  For Flow Mod and Flow
+ * Stats Requests messages, a 'cookie' and 'cookie_mask' may be supplied.
+ * Otherwise, 'cookie_mask' should be zero.
+ *
+ * This function can cause 'b''s data to be reallocated.
+ *
+ * Returns the number of bytes appended to 'b', excluding padding.  The return
+ * value can be zero if it appended nothing at all to 'b' (which happens if
+ * 'cr' is a catch-all rule that matches every packet). */
+int
+nx_put_match(struct ofpbuf *b, const struct cls_rule *cr,
+             ovs_be64 cookie, ovs_be64 cookie_mask)
+{
+    int match_len = nx_put_raw(b, false, cr, cookie, cookie_mask);
+
+    ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len);
+    return match_len;
+}
+
+
+/* Appends to 'b' an struct ofp11_match_header followed by the oxm format that
+ * expresses 'cr' (except for 'cr->priority', because priority is not part of
+ * nx_match), plus enough zero bytes to pad the data appended out to a multiple
+ * of 8.
+ *
+ * This function can cause 'b''s data to be reallocated.
+ *
+ * Returns the number of bytes appended to 'b', excluding the padding.  Never
+ * returns zero. */
+int
+oxm_put_match(struct ofpbuf *b, const struct cls_rule *cr)
+{
+    int match_len;
+    struct ofp11_match_header *omh;
+    size_t start_len = b->size;
+    ovs_be64 cookie = htonll(0), cookie_mask = htonll(0);
+
+    ofpbuf_put_uninit(b, sizeof *omh);
+    match_len = nx_put_raw(b, true, cr, cookie, cookie_mask) + sizeof *omh;
     ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len);
+
+    omh = (struct ofp11_match_header *)((char *)b->data + start_len);
+    omh->type = htons(OFPMT_OXM);
+    omh->length = htons(match_len);
+
     return match_len;
 }
 \f
@@ -662,6 +777,43 @@ nx_match_to_string(const uint8_t *p, unsigned int match_len)
     return ds_steal_cstr(&s);
 }
 
+char *
+oxm_match_to_string(const uint8_t *p, unsigned int match_len)
+{
+    const struct ofp11_match_header *omh = (struct ofp11_match_header *)p;
+    uint16_t match_len_;
+    struct ds s;
+
+    ds_init(&s);
+
+    if (match_len < sizeof *omh) {
+        ds_put_format(&s, "<match too short: %u>", match_len);
+        goto err;
+    }
+
+    if (omh->type != htons(OFPMT_OXM)) {
+        ds_put_format(&s, "<bad match type field: %u>", ntohs(omh->type));
+        goto err;
+    }
+
+    match_len_ = ntohs(omh->length);
+    if (match_len_ < sizeof *omh) {
+        ds_put_format(&s, "<match length field too short: %u>", match_len_);
+        goto err;
+    }
+
+    if (match_len_ != match_len) {
+        ds_put_format(&s, "<match length field incorrect: %u != %u>",
+                      match_len_, match_len);
+        goto err;
+    }
+
+    return nx_match_to_string(p + sizeof *omh, match_len - sizeof *omh);
+
+err:
+    return ds_steal_cstr(&s);
+}
+
 static void
 format_nxm_field_name(struct ds *s, uint32_t header)
 {
@@ -738,12 +890,11 @@ parse_nxm_field_name(const char *name, int name_len)
 \f
 /* nx_match_from_string(). */
 
-int
-nx_match_from_string(const char *s, struct ofpbuf *b)
+static int
+nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 {
     const char *full_s = s;
     const size_t start_len = b->size;
-    int match_len;
 
     if (!strcmp(s, "<any>")) {
         /* Ensure that 'b->data' isn't actually null. */
@@ -795,8 +946,32 @@ nx_match_from_string(const char *s, struct ofpbuf *b)
         s++;
     }
 
-    match_len = b->size - start_len;
+    return b->size - start_len;
+}
+
+int
+nx_match_from_string(const char *s, struct ofpbuf *b)
+{
+    int match_len = nx_match_from_string_raw(s, b);
+    ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len);
+    return match_len;
+}
+
+int
+oxm_match_from_string(const char *s, struct ofpbuf *b)
+{
+    int match_len;
+    struct ofp11_match_header *omh;
+    size_t start_len = b->size;
+
+    ofpbuf_put_uninit(b, sizeof *omh);
+    match_len = nx_match_from_string_raw(s, b) + sizeof *omh;
     ofpbuf_put_zeros(b, ROUND_UP(match_len, 8) - match_len);
+
+    omh = (struct ofp11_match_header *)((char *)b->data + start_len);
+    omh->type = htons(OFPMT_OXM);
+    omh->length = htons(match_len);
+
     return match_len;
 }
 \f

diff --git a/lib/nx-match.h b/lib/nx-match.h
index 161733f7feadfce9122b14dde65cba6546d6f894..3bfeeb739b79a6f931326d20a63a67b7db0e62f0 100644 (file)
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -44,13 +44,21 @@ enum ofperr nx_pull_match(struct ofpbuf *, unsigned int match_len,
                           uint16_t priority, struct cls_rule *,
                           ovs_be64 *cookie, ovs_be64 *cookie_mask);
 enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int match_len,
-                                uint16_t priority, struct cls_rule *,
-                                ovs_be64 *cookie, ovs_be64 *cookie_mask);
-int nx_put_match(struct ofpbuf *, bool oxm, const struct cls_rule *,
+                                uint16_t priority,
+                                struct cls_rule *, ovs_be64 *cookie,
+                                ovs_be64 *cookie_mask);
+enum ofperr oxm_pull_match(struct ofpbuf *, uint16_t priority,
+                           struct cls_rule *);
+enum ofperr oxm_pull_match_loose(struct ofpbuf *, uint16_t priority,
+                                 struct cls_rule *);
+int nx_put_match(struct ofpbuf *, const struct cls_rule *,
                  ovs_be64 cookie, ovs_be64 cookie_mask);
+int oxm_put_match(struct ofpbuf *, const struct cls_rule *);
 
 char *nx_match_to_string(const uint8_t *, unsigned int match_len);
+char *oxm_match_to_string(const uint8_t *, unsigned int match_len);
 int nx_match_from_string(const char *, struct ofpbuf *);
+int oxm_match_from_string(const char *, struct ofpbuf *);
 
 void nxm_parse_reg_move(struct ofpact_reg_move *, const char *);
 void nxm_parse_reg_load(struct ofpact_reg_load *, const char *);

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 73644e23787280be71f4dcf2f2cf67fcf47e81ab..5d92e9f2fe3c646626dfc180b9e9282016b94687 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1302,8 +1302,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
         nfm = ofpbuf_put_zeros(msg, sizeof *nfm);
         nfm->command = htons(command);
         nfm->cookie = fm->new_cookie;
-        match_len = nx_put_match(msg, false, &fm->cr,
-                                 fm->cookie, fm->cookie_mask);
+        match_len = nx_put_match(msg, &fm->cr, fm->cookie, fm->cookie_mask);
         nfm = msg->l3;
         nfm->idle_timeout = htons(fm->idle_timeout);
         nfm->hard_timeout = htons(fm->hard_timeout);
@@ -1463,7 +1462,7 @@ ofputil_encode_flow_stats_request(const struct ofputil_flow_stats_request *fsr,
                : OFPRAW_NXST_FLOW_REQUEST);
         msg = ofpraw_alloc(raw, OFP10_VERSION, 0);
         ofpbuf_put_zeros(msg, sizeof *nfsr);
-        match_len = nx_put_match(msg, false, &fsr->match,
+        match_len = nx_put_match(msg, &fsr->match,
                                  fsr->cookie, fsr->cookie_mask);
 
         nfsr = msg->l3;
@@ -1675,7 +1674,7 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
         int match_len;
 
         ofpbuf_put_uninit(reply, sizeof *nfs);
-        match_len = nx_put_match(reply, false, &fs->rule, 0, 0);
+        match_len = nx_put_match(reply, &fs->rule, 0, 0);
         ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply);
 
         nfs = ofpbuf_at_assert(reply, start_ofs, sizeof *nfs);
@@ -1844,7 +1843,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr,
         msg = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_REMOVED, OFP10_VERSION,
                                htonl(0), NXM_TYPICAL_LEN);
         nfr = ofpbuf_put_zeros(msg, sizeof *nfr);
-        match_len = nx_put_match(msg, false, &fr->rule, 0, 0);
+        match_len = nx_put_match(msg, &fr->rule, 0, 0);
 
         nfr = msg->l3;
         nfr->cookie = fr->cookie;
@@ -1980,7 +1979,7 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
                                   htonl(0), (sizeof(struct flow_metadata) * 2
                                              + 2 + send_len));
         ofpbuf_put_zeros(packet, sizeof *npi);
-        match_len = nx_put_match(packet, false, &rule, 0, 0);
+        match_len = nx_put_match(packet, &rule, 0, 0);
         ofpbuf_put_zeros(packet, 2);
         ofpbuf_put(packet, pin->packet, send_len);
 
@@ -2752,7 +2751,7 @@ ofputil_append_flow_monitor_request(
 
     start_ofs = msg->size;
     ofpbuf_put_zeros(msg, sizeof *nfmr);
-    match_len = nx_put_match(msg, false, &rq->match, htonll(0), htonll(0));
+    match_len = nx_put_match(msg, &rq->match, htonll(0), htonll(0));
 
     nfmr = ofpbuf_at_assert(msg, start_ofs, sizeof *nfmr);
     nfmr->id = htonl(rq->id);
@@ -2920,8 +2919,7 @@ ofputil_append_flow_update(const struct ofputil_flow_update *update,
         int match_len;
 
         ofpbuf_put_zeros(msg, sizeof *nfuf);
-        match_len = nx_put_match(msg, false, update->match,
-                                 htonll(0), htonll(0));
+        match_len = nx_put_match(msg, update->match, htonll(0), htonll(0));
         ofpacts_put_openflow10(update->ofpacts, update->ofpacts_len, msg);
 
         nfuf = ofpbuf_at_assert(msg, start_ofs, sizeof *nfuf);

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 00ba06b18f05f45476a5374f12e70a567d82b9fa..61d02669146fb24037886c5b33a1b20d20f7fd6f 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2182,28 +2182,45 @@ ofctl_parse_nxm__(bool oxm)
 
         /* Convert string to nx_match. */
         ofpbuf_init(&nx_match, 0);
-        match_len = nx_match_from_string(ds_cstr(&in), &nx_match);
+        if (oxm) {
+            match_len = oxm_match_from_string(ds_cstr(&in), &nx_match);
+        } else {
+            match_len = nx_match_from_string(ds_cstr(&in), &nx_match);
+        }
 
         /* Convert nx_match to cls_rule. */
         if (strict) {
-            error = nx_pull_match(&nx_match, match_len, 0, &rule,
-                                  &cookie, &cookie_mask);
+            if (oxm) {
+                error = oxm_pull_match(&nx_match, 0, &rule);
+            } else {
+                error = nx_pull_match(&nx_match, match_len, 0, &rule,
+                                      &cookie, &cookie_mask);
+            }
         } else {
-            error = nx_pull_match_loose(&nx_match, match_len, 0, &rule,
-                                        &cookie, &cookie_mask);
+            if (oxm) {
+                error = oxm_pull_match_loose(&nx_match, 0, &rule);
+            } else {
+                error = nx_pull_match_loose(&nx_match, match_len, 0, &rule,
+                                            &cookie, &cookie_mask);
+            }
         }
 
+
         if (!error) {
             char *out;
 
             /* Convert cls_rule back to nx_match. */
             ofpbuf_uninit(&nx_match);
             ofpbuf_init(&nx_match, 0);
-            match_len = nx_put_match(&nx_match, oxm, &rule,
-                                     cookie, cookie_mask);
+            if (oxm) {
+                match_len = oxm_put_match(&nx_match, &rule);
+                out = oxm_match_to_string(nx_match.data, match_len);
+            } else {
+                match_len = nx_put_match(&nx_match, &rule,
+                                         cookie, cookie_mask);
+                out = nx_match_to_string(nx_match.data, match_len);
+            }
 
-            /* Convert nx_match to string. */
-            out = nx_match_to_string(nx_match.data, match_len);
             puts(out);
             free(out);
         } else {
@@ -2568,7 +2585,7 @@ ofctl_check_vlan(int argc OVS_UNUSED, char *argv[])
 
     /* Convert to and from NXM. */
     ofpbuf_init(&nxm, 0);
-    nxm_match_len = nx_put_match(&nxm, false, &rule, htonll(0), htonll(0));
+    nxm_match_len = nx_put_match(&nxm, &rule, htonll(0), htonll(0));
     nxm_s = nx_match_to_string(nxm.data, nxm_match_len);
     error = nx_pull_match(&nxm, nxm_match_len, 0, &nxm_rule, NULL, NULL);
     printf("NXM: %s -> ", nxm_s);
@@ -2584,9 +2601,9 @@ ofctl_check_vlan(int argc OVS_UNUSED, char *argv[])
 
     /* Convert to and from OXM. */
     ofpbuf_init(&nxm, 0);
-    nxm_match_len = nx_put_match(&nxm, true, &rule, htonll(0), htonll(0));
-    nxm_s = nx_match_to_string(nxm.data, nxm_match_len);
-    error = nx_pull_match(&nxm, nxm_match_len, 0, &nxm_rule, NULL, NULL);
+    nxm_match_len = oxm_put_match(&nxm, &rule);
+    nxm_s = oxm_match_to_string(nxm.data, nxm_match_len);
+    error = oxm_pull_match(&nxm, 0, &nxm_rule);
     printf("OXM: %s -> ", nxm_s);
     if (error) {
         printf("%s\n", ofperr_to_string(error));