[ovs-dev] [PATCH 15/17] nx-match: Add support for experimenter OXM.

Ben Pfaff blp at nicira.com
Wed Sep 17 05:57:08 UTC 2014


OpenFlow 1.2+ defines a means for vendors to define vendor-specific OXM
fields, called "experimenter OXM".  These OXM fields are expressed with a
64-bit OXM header instead of the 32-bit header used for standard OXM (and
NXM).  Until now, OVS has not implemented experimenter OXM, and indeed we
have had little need to do so because of a pair of special 32-bit OXM classes
grandfathered to OVS as part of the OpenFlow 1.2 standardization process.

However, I want to prototype a feature for OpenFlow 1.5 that uses an
experimenter OXM as part of the prototype, so to do this OVS needs to
support experimenter OXM.  This commit adds that support.

Most of this commit is a fairly straightforward change: it extends the type
used for OXM/NXM from 32 to 64 bits and adds code to encode and decode the
longer headers when necessary.  Some other changes are necessary because
experimenter OXMs have a funny idea of the division between "header" and
"body": the extra 32 bits for experimenter OXMs are counted as part of the body
rather than the header according to the OpenFlow standard (even though this
does not entirely make sense), so arithmetic in various places has to be
adjusted, which is the reason for the new functions nxm_experimenter_len(),
nxm_payload_len(), and nxm_header_len().

Another change that calls for explanation is the new function mf_nxm_header()
that has been split from mf_oxm_header().  This function is used in actions
where the space for an NXM or OXM header is fixed so that there is no room
for a 64-bit experimenter type.  An upcoming commit will add new variations
of these actions that can support experimenter OXM.

Testing experimenter OXM is tricky because I do not know of any in
widespread use.  Two ONF proposals use experimenter OXMs: EXT-256 and
EXT-233.  EXT-256 is not suitable to implement for testing because its use
of experimenter OXM is wrong and will be changed.  EXT-233 is not suitable
to implement for testing because it requires adding a new field to struct
flow and I am not yet convinced that that field and the feature that it
supports is worth having in Open vSwitch.  Thus, this commit assigns an
experimenter OXM code point to an existing OVS field that is currently
restricted from use by controllers, "dp_hash", and uses that for testing.
Because controllers cannot use it, this leaves future versions of OVS free
to drop the support for the experimenter OXM for this field without causing
backward compatibility problems.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 build-aux/extract-ofp-fields |  35 ++++--
 lib/meta-flow.h              |   8 +-
 lib/nx-match.c               | 247 ++++++++++++++++++++++++++++++++-----------
 lib/nx-match.h               |   2 +-
 lib/ofp-actions.c            |  22 ++--
 tests/ofp-print.at           |  14 +++
 tests/ovs-ofctl.at           |  87 ++++++++++++++-
 utilities/ovs-ofctl.c        |   5 +
 8 files changed, 338 insertions(+), 82 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 58f5234..590a27e 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -48,12 +48,24 @@ PREREQS = {"none": "MFP_NONE",
           "ND solicit": "MFP_ND_SOLICIT",
           "ND advert": "MFP_ND_ADVERT"}
 
-# Maps a name prefix to an oxm_class.
+# Maps a name prefix into an (experimenter ID, class) pair, so:
+#
+#      - Standard OXM classes are written as (0, <oxm_class>)
+#
+#      - Experimenter OXM classes are written as (<oxm_vender>, 0xffff)
+#
 # If a name matches more than one prefix, the longest one is used.
-OXM_CLASSES = {"NXM_OF_": 0x0000,
-               "NXM_NX_": 0x0001,
-               "OXM_OF_": 0x8000,
-               "OXM_OF_PKT_REG": 0x8001}
+OXM_CLASSES = {"NXM_OF_":        (0,          0x0000),
+               "NXM_NX_":        (0,          0x0001),
+               "OXM_OF_":        (0,          0x8000),
+               "OXM_OF_PKT_REG": (0,          0x8001),
+
+               # This is the experimenter OXM class for Nicira, which is the
+               # one that OVS would be using instead of NXM_OF_ and NXM_NX_
+               # if OVS didn't have those grandfathered in.  It is currently
+               # used only to test support for experimenter OXM, since there
+               # are barely any real uses of experimenter OXM in the wild.
+               "NXOXM_ET_":      (0x00002320, 0xffff)}
 def oxm_name_to_class(name):
     prefix = ''
     class_ = None
@@ -118,12 +130,21 @@ def parse_oxm(s, prefix, n_bytes):
     if not m:
         fatal("%s: syntax error parsing %s" % (s, prefix))
         
-    name, code, of_version, ovs_version = m.groups()
+    name, oxm_type, of_version, ovs_version = m.groups()
 
     class_ = oxm_name_to_class(name)
     if class_ is None:
         fatal("unknown OXM class for %s" % name)
-    header = ("NXM_HEADER(0x%04x,%s,0,%d)" % (class_, code, n_bytes))
+    oxm_vendor, oxm_class = class_
+
+    # Normally the oxm_length is the size of the field, but for experimenter
+    # OXMs oxm_length also includes the 4-byte experimenter ID.
+    oxm_length = n_bytes
+    if oxm_class == 0xffff:
+        oxm_length += 4
+
+    header = ("NXM_HEADER(0x%x,0x%x,0,%s,%d)"
+              % (oxm_vendor, oxm_class, oxm_type, oxm_length))
 
     if of_version:
         if of_version not in VERSION:
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 0195ea3..dc68826 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -263,13 +263,19 @@ enum OVS_PACKED_ENUM mf_field_id {
      * Flow hash computed in the datapath.  Internal use only, not programmable
      * from controller.
      *
+     * The OXM code point for this is an attempt to test OXM experimenter
+     * support, which is otherwise difficult to test due to the dearth of use
+     * out in the wild.  Because controllers can't add flows that match on
+     * dp_hash, this doesn't commit OVS to supporting this OXM experimenter
+     * code point in the future.
+     *
      * Type: be32.
      * Maskable: bitwise.
      * Formatting: hexadecimal.
      * Prerequisites: none.
      * Access: read-only.
      * NXM: NXM_NX_DP_HASH(35) since v2.2.
-     * OXM: none.
+     * OXM: NXOXM_ET_DP_HASH(0) since OF1.5 and v2.4.
      */
     MFF_DP_HASH,
 
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 5970e44..cefd72b 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -37,6 +37,46 @@
 
 VLOG_DEFINE_THIS_MODULE(nx_match);
 
+/* OXM headers.
+ *
+ *
+ * Standard OXM/NXM
+ * ================
+ *
+ * The header is 32 bits long.  It looks like this:
+ *
+ * |31                              16 15            9| 8 7                0
+ * +----------------------------------+---------------+--+------------------+
+ * |            oxm_class             |   oxm_field   |hm|    oxm_length    |
+ * +----------------------------------+---------------+--+------------------+
+ *
+ * where hm stands for oxm_hasmask.  It is followed by oxm_length bytes of
+ * payload.  When oxm_hasmask is 0, the payload is the value of the field
+ * identified by the header; when oxm_hasmask is 1, the payload is a value for
+ * the field followed by a mask of equal length.
+ *
+ * Internally, we represent a standard OXM header as a 64-bit integer with the
+ * above information in the most-significant bits.
+ *
+ *
+ * Experimenter OXM
+ * ================
+ *
+ * The header is 64 bits long.  It looks like the diagram above except that a
+ * 32-bit experimenter ID, which we call oxm_vendor and which identifies a
+ * vendor, is inserted just before the payload.  Experimenter OXMs are
+ * identified by an all-1-bits oxm_class (OFPXMC12_EXPERIMENTER).  The
+ * oxm_length value *includes* the experimenter ID, so that the real payload is
+ * only oxm_length - 4 bytes long.
+ *
+ * Internally, we represent an experimenter OXM header as a 64-bit integer with
+ * the standard header in the upper 32 bits and the experimenter ID in the
+ * lower 32 bits.  (It would be more convenient to swap the positions of the
+ * two 32-bit words, but this would be more error-prone because experimenter
+ * OXMs are very rarely used, so accidentally passing one through a 32-bit type
+ * somewhere in the OVS code would be hard to find.)
+ */
+
 /*
  * OXM Class IDs.
  * The high order bit differentiate reserved classes from member classes.
@@ -51,41 +91,90 @@ enum ofp12_oxm_class {
     OFPXMC12_EXPERIMENTER   = 0xffff, /* Experimenter class */
 };
 
-/* Functions for extracting fields from OXM/NXM headers. */
-static int nxm_class(uint32_t header) { return header >> 16; }
-static int nxm_field(uint32_t header) { return (header >> 9) & 0x7f; }
-static bool nxm_hasmask(uint32_t header) { return (header & 0x100) != 0; }
-static int nxm_length(uint32_t header) { return header & 0xff; }
+/* Functions for extracting raw field values from OXM/NXM headers. */
+static uint32_t nxm_vendor(uint64_t header) { return header; }
+static int nxm_class(uint64_t header) { return header >> 48; }
+static int nxm_field(uint64_t header) { return (header >> 41) & 0x7f; }
+static bool nxm_hasmask(uint64_t header) { return (header >> 40) & 1; }
+static int nxm_length(uint64_t header) { return (header >> 32) & 0xff; }
+
+static bool
+is_experimenter_oxm(uint64_t header)
+{
+    return nxm_class(header) == OFPXMC12_EXPERIMENTER;
+}
+
+/* The OXM header "length" field is somewhat tricky:
+ *
+ *     - For a standard OXM header, the length is the number of bytes of the
+ *       payload, and the payload consists of just the value (and mask, if
+ *       present).
+ *
+ *     - For an experimenter OXM header, the length is the number of bytes in
+ *       the payload plus 4 (the length of the experimenter ID).  That is, the
+ *       experimenter ID is included in oxm_length.
+ *
+ * This function returns the length of the experimenter ID field in 'header'.
+ * That is, for an experimenter OXM (when an experimenter ID is present), it
+ * returns 4, and for a standard OXM (when no experimenter ID is present), it
+ * returns 0. */
+static int
+nxm_experimenter_len(uint64_t header)
+{
+    return is_experimenter_oxm(header) ? 4 : 0;
+}
+
+/* Returns the number of bytes that follow the header for an NXM/OXM entry
+ * with the given 'header'. */
+static int
+nxm_payload_len(uint64_t header)
+{
+    return nxm_length(header) - nxm_experimenter_len(header);
+}
+
+/* Returns the number of bytes in the header for an NXM/OXM entry with the
+ * given 'header'. */
+static int
+nxm_header_len(uint64_t header)
+{
+    return 4 + nxm_experimenter_len(header);
+}
 
 /* Returns true if 'header' is a legacy NXM header, false if it is an OXM
  * header.*/
 static bool
-is_nxm_header(uint32_t header)
+is_nxm_header(uint64_t header)
 {
     return nxm_class(header) <= 1;
 }
 
-#define NXM_HEADER(CLASS, FIELD, HASMASK, LENGTH)                       \
-    (((CLASS) << 16) | ((FIELD) << 9) | ((HASMASK) << 8) | (LENGTH))
+#define NXM_HEADER(VENDOR, CLASS, HASMASK, FIELD, LENGTH)       \
+    (((uint64_t) (CLASS) << 48) |                               \
+     ((uint64_t) (FIELD) << 41) |                               \
+     ((uint64_t) (HASMASK) << 40) |                             \
+     ((uint64_t) (LENGTH) << 32) |                              \
+     (VENDOR))
 
-#define NXM_HEADER_FMT "%d:%d:%d:%d"
-#define NXM_HEADER_ARGS(HEADER)                 \
-    nxm_class(HEADER), nxm_field(HEADER),      \
+#define NXM_HEADER_FMT "%#"PRIx32":%d:%d:%d:%d"
+#define NXM_HEADER_ARGS(HEADER)                                 \
+    nxm_vendor(HEADER), nxm_class(HEADER), nxm_field(HEADER),   \
     nxm_hasmask(HEADER), nxm_length(HEADER)
 
 /* Functions for turning the "hasmask" bit on or off.  (This also requires
  * adjusting the length.) */
-static uint32_t
-nxm_make_exact_header(uint32_t header)
+static uint64_t
+nxm_make_exact_header(uint64_t header)
 {
-    return NXM_HEADER(nxm_class(header), nxm_field(header), 0,
-                      nxm_length(header) / 2);
+    int new_len = nxm_payload_len(header) / 2 + nxm_experimenter_len(header);
+    return NXM_HEADER(nxm_vendor(header), nxm_class(header), 0,
+                      nxm_field(header), new_len);
 }
-static uint32_t
-nxm_make_wild_header(uint32_t header)
+static uint64_t
+nxm_make_wild_header(uint64_t header)
 {
-    return NXM_HEADER(nxm_class(header), nxm_field(header), 1,
-                      nxm_length(header) * 2);
+    int new_len = nxm_payload_len(header) * 2 + nxm_experimenter_len(header);
+    return NXM_HEADER(nxm_vendor(header), nxm_class(header), 1,
+                      nxm_field(header), new_len);
 }
 
 /* Flow cookie.
@@ -95,23 +184,23 @@ nxm_make_wild_header(uint32_t header)
  * with specific cookies.  See the "nx_flow_mod" and "nx_flow_stats_request"
  * structure definitions for more details.  This match is otherwise not
  * allowed. */
-#define NXM_NX_COOKIE     NXM_HEADER  (0x0001, 30, 0, 8)
+#define NXM_NX_COOKIE     NXM_HEADER  (0, 0x0001, 0, 30, 8)
 #define NXM_NX_COOKIE_W   nxm_make_wild_header(NXM_NX_COOKIE)
 
 struct nxm_field {
-    uint32_t header;
+    uint64_t header;
     enum ofp_version version;
     const char *name;           /* e.g. "NXM_OF_IN_PORT". */
 
     enum mf_field_id id;
 };
 
-static const struct nxm_field *nxm_field_by_header(uint32_t header);
+static const struct nxm_field *nxm_field_by_header(uint64_t header);
 static const struct nxm_field *nxm_field_by_name(const char *name, size_t len);
 static const struct nxm_field *nxm_field_by_mf_id(enum mf_field_id);
 static const struct nxm_field *oxm_field_by_mf_id(enum mf_field_id);
 
-static void nx_put_header__(struct ofpbuf *, uint32_t header, bool masked);
+static void nx_put_header__(struct ofpbuf *, uint64_t header, bool masked);
 
 /* Rate limit for nx_match parse errors.  These always indicate a bug in the
  * peer and so there's not much point in showing a lot of them. */
@@ -132,28 +221,47 @@ nxm_field_from_mf_field(enum mf_field_id id, enum ofp_version version)
  * 'version'.  Specify 0 for 'version' if an NXM legacy header should be
  * preferred over any standardized OXM header.  Returns 0 if field 'id' cannot
  * be expressed in NXM or OXM. */
-uint32_t
+static uint64_t
 mf_oxm_header(enum mf_field_id id, enum ofp_version version)
 {
     const struct nxm_field *f = nxm_field_from_mf_field(id, version);
     return f ? f->header : 0;
 }
 
+/* Returns the 32-bit OXM or NXM header to use for field 'id', preferring an
+ * NXM legacy header over any standardized OXM header.  Returns 0 if field 'id'
+ * cannot be expressed with a 32-bit NXM or OXM header.
+ *
+ * Whenever possible, use nx_pull_header() instead of this function, because
+ * this function cannot support 64-bit experimenter OXM headers. */
+uint32_t
+mf_nxm_header(enum mf_field_id id)
+{
+    uint64_t oxm = mf_oxm_header(id, 0);
+    return is_experimenter_oxm(oxm) ? 0 : oxm >> 32;
+}
+
+static const struct mf_field *
+mf_from_oxm_header(uint64_t header)
+{
+    const struct nxm_field *f = nxm_field_by_header(header);
+    return f ? mf_from_id(f->id) : NULL;
+}
+
 /* Returns the "struct mf_field" that corresponds to NXM or OXM header
  * 'header', or NULL if 'header' doesn't correspond to any known field.  */
 const struct mf_field *
 mf_from_nxm_header(uint32_t header)
 {
-    const struct nxm_field *f = nxm_field_by_header(header);
-    return f ? mf_from_id(f->id) : NULL;
+    return mf_from_oxm_header((uint64_t) header << 32);
 }
 
 /* Returns the width of the data for a field with the given 'header', in
  * bytes. */
 static int
-nxm_field_bytes(uint32_t header)
+nxm_field_bytes(uint64_t header)
 {
-    unsigned int length = nxm_length(header);
+    unsigned int length = nxm_payload_len(header);
     return nxm_hasmask(header) ? length / 2 : length;
 }
 
@@ -172,7 +280,7 @@ mf_oxm_version(enum mf_field_id id)
  * for any 1-bit in the value where there is a 0-bit in the mask.  Returns 0 if
  * none, otherwise an error code. */
 static bool
-is_mask_consistent(uint32_t header, const uint8_t *value, const uint8_t *mask)
+is_mask_consistent(uint64_t header, const uint8_t *value, const uint8_t *mask)
 {
     unsigned int width = nxm_field_bytes(header);
     unsigned int i;
@@ -191,38 +299,49 @@ is_mask_consistent(uint32_t header, const uint8_t *value, const uint8_t *mask)
 }
 
 static bool
-is_cookie_pseudoheader(uint32_t header)
+is_cookie_pseudoheader(uint64_t header)
 {
     return header == NXM_NX_COOKIE || header == NXM_NX_COOKIE_W;
 }
 
 static enum ofperr
-nx_pull_header__(struct ofpbuf *b, bool allow_cookie, uint32_t *header,
+nx_pull_header__(struct ofpbuf *b, bool allow_cookie, uint64_t *header,
                  const struct mf_field **field)
 {
     if (ofpbuf_size(b) < 4) {
-        VLOG_DBG_RL(&rl, "encountered partial (%"PRIu32"-byte) OXM entry",
-                    ofpbuf_size(b));
-        goto error;
+        goto bad_len;
     }
-    *header = ntohl(get_unaligned_be32(ofpbuf_pull(b, 4)));
-    if (nxm_length(*header) == 0) {
-        VLOG_WARN_RL(&rl, "OXM header "NXM_HEADER_FMT" has zero length",
-                     NXM_HEADER_ARGS(*header));
+
+    *header = ((uint64_t) ntohl(get_unaligned_be32(ofpbuf_data(b)))) << 32;
+    if (is_experimenter_oxm(*header)) {
+        if (ofpbuf_size(b) < 8) {
+            goto bad_len;
+        }
+        *header = ntohll(get_unaligned_be64(ofpbuf_data(b)));
+    }
+    if (nxm_length(*header) <= nxm_experimenter_len(*header)) {
+        VLOG_WARN_RL(&rl, "OXM header "NXM_HEADER_FMT" has invalid length %d "
+                     "(minimum is %d)",
+                     NXM_HEADER_ARGS(*header), nxm_length(*header),
+                     nxm_header_len(*header) + 1);
         goto error;
     }
+
     if (field) {
-        *field = mf_from_nxm_header(*header);
+        *field = mf_from_oxm_header(*header);
         if (!*field && !(allow_cookie && is_cookie_pseudoheader(*header))) {
             VLOG_DBG_RL(&rl, "OXM header "NXM_HEADER_FMT" is unknown",
                         NXM_HEADER_ARGS(*header));
-            *header = 0;
             return OFPERR_OFPBMC_BAD_FIELD;
         }
     }
 
+    ofpbuf_pull(b, nxm_header_len(*header));
     return 0;
 
+bad_len:
+    VLOG_DBG_RL(&rl, "encountered partial (%"PRIu32"-byte) OXM entry",
+                ofpbuf_size(b));
 error:
     *header = 0;
     *field = NULL;
@@ -230,7 +349,7 @@ error:
 }
 
 static enum ofperr
-nx_pull_entry__(struct ofpbuf *b, bool allow_cookie, uint32_t *header,
+nx_pull_entry__(struct ofpbuf *b, bool allow_cookie, uint64_t *header,
                 const struct mf_field **field,
                 union mf_value *value, union mf_value *mask)
 {
@@ -244,7 +363,7 @@ nx_pull_entry__(struct ofpbuf *b, bool allow_cookie, uint32_t *header,
         return error;
     }
 
-    payload_len = nxm_length(*header);
+    payload_len = nxm_payload_len(*header);
     payload = ofpbuf_try_pull(b, payload_len);
     if (!payload) {
         VLOG_DBG_RL(&rl, "OXM header "NXM_HEADER_FMT" calls for %u-byte "
@@ -290,7 +409,7 @@ enum ofperr
 nx_pull_entry(struct ofpbuf *b, const struct mf_field **field,
               union mf_value *value, union mf_value *mask)
 {
-    uint32_t header;
+    uint64_t header;
 
     return nx_pull_entry__(b, false, &header, field, value, mask);
 }
@@ -309,7 +428,7 @@ enum ofperr
 nx_pull_header(struct ofpbuf *b, const struct mf_field **field, bool *masked)
 {
     enum ofperr error;
-    uint32_t header;
+    uint64_t header;
 
     error = nx_pull_header__(b, false, &header, field);
     if (masked) {
@@ -326,7 +445,7 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie,
                     union mf_value *value, union mf_value *mask)
 {
     enum ofperr error;
-    uint32_t header;
+    uint64_t header;
 
     error = nx_pull_entry__(b, allow_cookie, &header, field, value, mask);
     if (error) {
@@ -911,12 +1030,12 @@ oxm_put_match(struct ofpbuf *b, const struct match *match,
 }
 
 static void
-nx_put_header__(struct ofpbuf *b, uint32_t header, bool masked)
+nx_put_header__(struct ofpbuf *b, uint64_t header, bool masked)
 {
-    uint32_t masked_header = masked ? nxm_make_wild_header(header) : header;
-    ovs_be32 network_header = htonl(masked_header);
+    uint64_t masked_header = masked ? nxm_make_wild_header(header) : header;
+    ovs_be64 network_header = htonll(masked_header);
 
-    ofpbuf_put(b, &network_header, sizeof network_header);
+    ofpbuf_put(b, &network_header, 4 + nxm_experimenter_len(header));
 }
 
 void
@@ -943,7 +1062,7 @@ nx_put_entry(struct ofpbuf *b,
 
 /* nx_match_to_string() and helpers. */
 
-static void format_nxm_field_name(struct ds *, uint32_t header);
+static void format_nxm_field_name(struct ds *, uint64_t header);
 
 char *
 nx_match_to_string(const uint8_t *p, unsigned int match_len)
@@ -961,7 +1080,7 @@ nx_match_to_string(const uint8_t *p, unsigned int match_len)
         union mf_value value;
         union mf_value mask;
         enum ofperr error;
-        uint32_t header;
+        uint64_t header;
         int value_len;
 
         error = nx_pull_entry__(&b, true, &header, NULL, &value, &mask);
@@ -1046,7 +1165,7 @@ nx_format_field_name(enum mf_field_id id, enum ofp_version version,
 }
 
 static void
-format_nxm_field_name(struct ds *s, uint32_t header)
+format_nxm_field_name(struct ds *s, uint64_t header)
 {
     const struct nxm_field *f = nxm_field_by_header(header);
     if (f) {
@@ -1069,7 +1188,7 @@ streq_len(const char *a, size_t a_len, const char *b)
     return strlen(b) == a_len && !memcmp(a, b, a_len);
 }
 
-static uint32_t
+static uint64_t
 parse_nxm_field_name(const char *name, int name_len)
 {
     const struct nxm_field *f;
@@ -1090,14 +1209,22 @@ parse_nxm_field_name(const char *name, int name_len)
         return NXM_NX_COOKIE_W;
     }
 
-    /* Check whether it's a 32-bit field header value as hex.
+    /* Check whether it's a field header value as hex.
      * (This isn't ordinarily useful except for testing error behavior.) */
     if (name_len == 8) {
-        uint32_t header;
+        uint64_t header;
+        bool ok;
+
+        header = hexits_value(name, name_len, &ok) << 32;
+        if (ok) {
+            return header;
+        }
+    } else if (name_len == 16) {
+        uint64_t header;
         bool ok;
 
         header = hexits_value(name, name_len, &ok);
-        if (!ok) {
+        if (ok && is_experimenter_oxm(header)) {
             return header;
         }
     }
@@ -1121,7 +1248,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 
     for (s += strspn(s, ", "); *s; s += strspn(s, ", ")) {
         const char *name;
-        uint32_t header;
+        uint64_t header;
         int name_len;
         size_t n;
 
@@ -1533,7 +1660,7 @@ oxm_bitmap_from_mf_bitmap(const struct mf_bitmap *fields,
     int i;
 
     BITMAP_FOR_EACH_1 (i, MFF_N_IDS, fields->bm) {
-        uint32_t oxm = mf_oxm_header(i, version);
+        uint64_t oxm = mf_oxm_header(i, version);
         uint32_t class = nxm_class(oxm);
         int field = nxm_field(oxm);
 
@@ -1554,7 +1681,7 @@ oxm_bitmap_to_mf_bitmap(ovs_be64 oxm_bitmap, enum ofp_version version)
 
     for (enum mf_field_id id = 0; id < MFF_N_IDS; id++) {
         if (version >= mf_oxm_version(id)) {
-            uint32_t oxm = mf_oxm_header(id, version);
+            uint64_t oxm = mf_oxm_header(id, version);
             uint32_t class = nxm_class(oxm);
             int field = nxm_field(oxm);
 
@@ -1653,7 +1780,7 @@ nxm_init(void)
 }
 
 static const struct nxm_field *
-nxm_field_by_header(uint32_t header)
+nxm_field_by_header(uint64_t header)
 {
     const struct nxm_field_index *nfi;
 
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 929b1db..62e73a0 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -76,7 +76,7 @@ void nx_put_header(struct ofpbuf *, enum mf_field_id, enum ofp_version,
  * nx_put_entry/header() for decoding and encoding OXM/NXM.  In those cases,
  * the nx_*() functions should be preferred because they can support the 64-bit
  * "experimenter" OXM format (even though it is not yet implemented). */
-uint32_t mf_oxm_header(enum mf_field_id, enum ofp_version oxm_version);
+uint32_t mf_nxm_header(enum mf_field_id);
 const struct mf_field *mf_from_nxm_header(uint32_t nxm_header);
 
 char *nx_match_to_string(const uint8_t *, unsigned int match_len);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 752f690..95200af 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -736,7 +736,7 @@ encode_OUTPUT_REG(const struct ofpact_output_reg *output_reg,
 
     naor->ofs_nbits = nxm_encode_ofs_nbits(output_reg->src.ofs,
                                            output_reg->src.n_bits);
-    naor->src = htonl(mf_oxm_header(output_reg->src.field->id, 0));
+    naor->src = htonl(mf_nxm_header(output_reg->src.field->id));
     naor->max_len = htons(output_reg->max_len);
 }
 
@@ -849,7 +849,7 @@ decode_bundle(bool load, const struct nx_action_bundle *nab,
     } else if (bundle->algorithm != NX_BD_ALG_HRW
                && bundle->algorithm != NX_BD_ALG_ACTIVE_BACKUP) {
         VLOG_WARN_RL(&rl, "unsupported algorithm %d", (int) bundle->algorithm);
-    } else if (slave_type != mf_oxm_header(MFF_IN_PORT, 0)) {
+    } else if (slave_type != mf_nxm_header(MFF_IN_PORT)) {
         VLOG_WARN_RL(&rl, "unsupported slave type %"PRIu16, slave_type);
     } else {
         error = 0;
@@ -930,12 +930,12 @@ encode_BUNDLE(const struct ofpact_bundle *bundle,
     nab->algorithm = htons(bundle->algorithm);
     nab->fields = htons(bundle->fields);
     nab->basis = htons(bundle->basis);
-    nab->slave_type = htonl(mf_oxm_header(MFF_IN_PORT, 0));
+    nab->slave_type = htonl(mf_nxm_header(MFF_IN_PORT));
     nab->n_slaves = htons(bundle->n_slaves);
     if (bundle->dst.field) {
         nab->ofs_nbits = nxm_encode_ofs_nbits(bundle->dst.ofs,
                                               bundle->dst.n_bits);
-        nab->dst = htonl(mf_oxm_header(bundle->dst.field->id, 0));
+        nab->dst = htonl(mf_nxm_header(bundle->dst.field->id));
     }
 
     slaves = ofpbuf_put_zeros(out, slaves_len);
@@ -1830,8 +1830,8 @@ encode_REG_MOVE(const struct ofpact_reg_move *move,
         narm->n_bits = htons(move->dst.n_bits);
         narm->src_ofs = htons(move->src.ofs);
         narm->dst_ofs = htons(move->dst.ofs);
-        narm->src = htonl(mf_oxm_header(move->src.field->id, 0));
-        narm->dst = htonl(mf_oxm_header(move->dst.field->id, 0));
+        narm->src = htonl(mf_nxm_header(move->src.field->id));
+        narm->dst = htonl(mf_nxm_header(move->dst.field->id));
     }
 }
 
@@ -2075,7 +2075,7 @@ set_field_to_nxast(const struct ofpact_set_field *sf, struct ofpbuf *openflow)
     while (next_load_segment(sf, &dst, &value)) {
         struct nx_action_reg_load *narl = put_NXAST_REG_LOAD(openflow);
         narl->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs, dst.n_bits);
-        narl->dst = htonl(mf_oxm_header(dst.field->id, 0));
+        narl->dst = htonl(mf_nxm_header(dst.field->id));
         narl->value = htonll(value);
     }
 }
@@ -2417,7 +2417,7 @@ encode_STACK_op(const struct ofpact_stack *stack_action,
 {
     nasp->offset = htons(stack_action->subfield.ofs);
     nasp->n_bits = htons(stack_action->subfield.n_bits);
-    nasp->field = htonl(mf_oxm_header(stack_action->subfield.field->id, 0));
+    nasp->field = htonl(mf_nxm_header(stack_action->subfield.field->id));
 }
 
 static void
@@ -3636,7 +3636,7 @@ encode_LEARN(const struct ofpact_learn *learn,
         put_u16(out, spec->n_bits | spec->dst_type | spec->src_type);
 
         if (spec->src_type == NX_LEARN_SRC_FIELD) {
-            put_u32(out, mf_oxm_header(spec->src.field->id, 0));
+            put_u32(out, mf_nxm_header(spec->src.field->id));
             put_u16(out, spec->src.ofs);
         } else {
             size_t n_dst_bytes = 2 * DIV_ROUND_UP(spec->n_bits, 16);
@@ -3648,7 +3648,7 @@ encode_LEARN(const struct ofpact_learn *learn,
 
         if (spec->dst_type == NX_LEARN_DST_MATCH ||
             spec->dst_type == NX_LEARN_DST_LOAD) {
-            put_u32(out, mf_oxm_header(spec->dst.field->id, 0));
+            put_u32(out, mf_nxm_header(spec->dst.field->id));
             put_u16(out, spec->dst.ofs);
         }
     }
@@ -3773,7 +3773,7 @@ encode_MULTIPATH(const struct ofpact_multipath *mp,
     nam->max_link = htons(mp->max_link);
     nam->arg = htonl(mp->arg);
     nam->ofs_nbits = nxm_encode_ofs_nbits(mp->dst.ofs, mp->dst.n_bits);
-    nam->dst = htonl(mf_oxm_header(mp->dst.field->id, 0));
+    nam->dst = htonl(mf_nxm_header(mp->dst.field->id));
 }
 
 static char * WARN_UNUSED_RESULT
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 29555ef..9e0ec54 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -890,6 +890,20 @@ OFPT_FLOW_MOD (OF1.2) (xid=0x52334507): ADD priority=255,sctp actions=set_field:
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_FLOW_MOD - OF1.2 - experimenter OXM])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
+03 0e 00 48 52 33 45 07 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff \
+ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \
+00 01 00 14 ff ff 01 0c 00 00 23 20 01 23 45 67 \
+0f ff ff ff 00 00 00 00
+" 2], [0], [dnl
+OFPT_FLOW_MOD (OF1.2) (xid=0x52334507): ADD priority=255,dp_hash=0x1234567/0xfffffff actions=drop
+], [dnl
+])
+AT_CLEANUP
+
 dnl This triggered a buggy "instructions out of order" message earlier.
 AT_SETUP([OFPT_FLOW_MOD - OF1.3 - meter])
 AT_KEYWORDS([ofp-print])
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index e3eb1c0..85706b9 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -797,9 +797,17 @@ NXM_NX_REG0_W(a0e0d050/f0f0f0f0)
 NXM_NX_REG0_W(a0e0d050/ffffffff)
 NXM_NX_REG0_W(00000000/00000000)
 
+# dp_hash (testing experimenter OXM).
+NXM_NX_DP_HASH(01234567)
+NXOXM_ET_DP_HASH(01234567)
+
 # Invalid field number.
 01020304(1111/2222)
 
+# Invalid field numbers (experimenter OXM).
+ffff020800002320(11112222)
+ffff030800002320(1111/3333)
+
 # Unimplemented registers.
 #
 # This test assumes that at least two registers, but fewer than 16,
@@ -1080,9 +1088,17 @@ NXM_NX_REG0_W(a0e0d050/f0f0f0f0)
 NXM_NX_REG0(a0e0d050)
 <any>
 
+# dp_hash (testing experimenter OXM).
+NXM_NX_DP_HASH(01234567)
+NXM_NX_DP_HASH(01234567)
+
 # Invalid field number.
 nx_pull_match() returned error OFPBMC_BAD_FIELD
 
+# Invalid field numbers (experimenter OXM).
+nx_pull_match() returned error OFPBMC_BAD_FIELD
+nx_pull_match() returned error OFPBMC_BAD_FIELD
+
 # Unimplemented registers.
 #
 # This test assumes that at least two registers, but fewer than 16,
@@ -1096,7 +1112,7 @@ nx_pull_match() returned error OFPBMC_BAD_FIELD
 # Check that at least the first warning made it.  (It's rate-limited
 # so a variable number could show up, especially under valgrind etc.)
 AT_CHECK([grep '1-bits in value' stderr | sed 1q], [0], [dnl
-nx_match|WARN|Rejecting NXM/OXM entry 0:1:1:12 with 1-bits in value for bits wildcarded by the mask.
+nx_match|WARN|Rejecting NXM/OXM entry 0:0:1:1:12 with 1-bits in value for bits wildcarded by the mask.
 ])
 
 # Check that there wasn't any other stderr output.
@@ -1648,14 +1664,20 @@ AT_SETUP([ovs-ofctl parse-nx-match loose])
 AT_KEYWORDS([nx-match])
 AT_DATA([nx-match.txt], [dnl
 NXM_OF_IN_PORT(0001), 01020304(1111/3333), NXM_OF_ETH_TYPE(0800)
+NXM_OF_IN_PORT(0001), ffff020800002320(11112222), NXM_OF_ETH_TYPE(0800)
+NXM_OF_IN_PORT(0001), ffff030800002320(1111/3333), NXM_OF_ETH_TYPE(0800)
 ])
 
 AT_CHECK([ovs-ofctl --strict parse-nx-match < nx-match.txt], [0], [dnl
 nx_pull_match() returned error OFPBMC_BAD_FIELD
+nx_pull_match() returned error OFPBMC_BAD_FIELD
+nx_pull_match() returned error OFPBMC_BAD_FIELD
 ])
 
 AT_CHECK([ovs-ofctl parse-nx-match < nx-match.txt], [0], [dnl
 NXM_OF_IN_PORT(0001), NXM_OF_ETH_TYPE(0800)
+NXM_OF_IN_PORT(0001), NXM_OF_ETH_TYPE(0800)
+NXM_OF_IN_PORT(0001), NXM_OF_ETH_TYPE(0800)
 ])
 AT_CLEANUP
 
@@ -1896,6 +1918,10 @@ OXM_OF_PKT_REG0_W(00000000a0e0d050/00000000f0f0f0f0), OXM_OF_PKT_REG1_W(a0e0d050
 
 # Invalid field number.
 01020304(1111/2222)
+
+# Invalid field numbers (experimenter OXM).
+ffff020800002320(11112222)
+ffff030800002320(1111/3333)
 ])
 AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' --strict parse-oxm OpenFlow12 < oxm.txt],
   [0], [dnl
@@ -2133,12 +2159,16 @@ NXM_NX_REG1_W(a0e0d050/f0f0f0f0), NXM_NX_REG2(a0e0d050)
 
 # Invalid field number.
 nx_pull_match() returned error OFPBMC_BAD_FIELD
+
+# Invalid field numbers (experimenter OXM).
+nx_pull_match() returned error OFPBMC_BAD_FIELD
+nx_pull_match() returned error OFPBMC_BAD_FIELD
 ], [stderr])
 
 # Check that at least the first warning made it.  (It's rate-limited
 # so a variable number could show up, especially under valgrind etc.)
 AT_CHECK([grep '1-bits in value' stderr | sed 1q], [0], [dnl
-nx_match|WARN|Rejecting NXM/OXM entry 32768:2:1:16 with 1-bits in value for bits wildcarded by the mask.
+nx_match|WARN|Rejecting NXM/OXM entry 0:32768:2:1:16 with 1-bits in value for bits wildcarded by the mask.
 ])
 
 # Check that there wasn't any other stderr output.
@@ -2198,14 +2228,67 @@ AT_SETUP([ovs-ofctl parse-oxm loose])
 AT_KEYWORDS([oxm])
 AT_DATA([oxm.txt], [dnl
 OXM_OF_IN_PORT(00000001), 01020304(1111/3333), OXM_OF_ETH_TYPE(0800)
+OXM_OF_IN_PORT(00000001), ffff020800002320(11112222), OXM_OF_ETH_TYPE(0800)
+OXM_OF_IN_PORT(00000001), ffff030800002320(1111/3333), OXM_OF_ETH_TYPE(0800)
 ])
 
 AT_CHECK([ovs-ofctl --strict parse-oxm OpenFlow12 < oxm.txt], [0], [dnl
 nx_pull_match() returned error OFPBMC_BAD_FIELD
+nx_pull_match() returned error OFPBMC_BAD_FIELD
+nx_pull_match() returned error OFPBMC_BAD_FIELD
 ])
 
 AT_CHECK([ovs-ofctl parse-oxm OpenFlow12 < oxm.txt], [0], [dnl
 OXM_OF_IN_PORT(00000001), OXM_OF_ETH_TYPE(0800)
+OXM_OF_IN_PORT(00000001), OXM_OF_ETH_TYPE(0800)
+OXM_OF_IN_PORT(00000001), OXM_OF_ETH_TYPE(0800)
+])
+AT_CLEANUP
+
+AT_SETUP([experimenter OXM encoding])
+AT_DATA([oxm.txt], [dnl
+NXM_NX_DP_HASH(01234567)
+NXOXM_ET_DP_HASH(01234567)
+
+NXM_NX_DP_HASH_W(01234567/0fffffff)
+NXOXM_ET_DP_HASH_W(01234567/0fffffff)
+])
+
+# To allow for testing experimenter OXM, which doesn't really have many
+# examples in the wild, we've defined a variant of NXM_NX_DP_HASH that uses
+# the experimenter OXM mechanism, called NXOXM_ET_DP_HASH.  We've defined
+# it as if it were introduced with OpenFlow 1.5, which gives us the
+# opportunity to see that both forms are accepted in all OpenFlow versions
+# but the experimenter form is used for encoding in OF1.5+.
+#
+# First verify that both forms are accepted and NXOXM_ET_DP_HASH is encoded
+# in OF1.5.
+AT_CHECK([ovs-ofctl -m --strict parse-oxm OpenFlow15 < oxm.txt], [0], [dnl
+NXOXM_ET_DP_HASH(01234567)
+00000000  00 01 00 10 ff ff 00 08-00 00 23 20 01 23 45 67 @&t@
+NXOXM_ET_DP_HASH(01234567)
+00000000  00 01 00 10 ff ff 00 08-00 00 23 20 01 23 45 67 @&t@
+
+NXOXM_ET_DP_HASH_W(01234567/0fffffff)
+00000000  00 01 00 14 ff ff 01 0c-00 00 23 20 01 23 45 67 @&t@
+00000010  0f ff ff ff 00 00 00 00-
+NXOXM_ET_DP_HASH_W(01234567/0fffffff)
+00000000  00 01 00 14 ff ff 01 0c-00 00 23 20 01 23 45 67 @&t@
+00000010  0f ff ff ff 00 00 00 00-
+])
+
+# Then verify that both forms are accepted and NXM_NX_DP_HASH is encoded
+# in OF1.2.
+AT_CHECK([ovs-ofctl -m --strict parse-oxm OpenFlow12 < oxm.txt], [0], [dnl
+NXM_NX_DP_HASH(01234567)
+00000000  00 01 00 0c 00 01 46 04-01 23 45 67 00 00 00 00 @&t@
+NXM_NX_DP_HASH(01234567)
+00000000  00 01 00 0c 00 01 46 04-01 23 45 67 00 00 00 00 @&t@
+
+NXM_NX_DP_HASH_W(01234567/0fffffff)
+00000000  00 01 00 10 00 01 47 08-01 23 45 67 0f ff ff ff @&t@
+NXM_NX_DP_HASH_W(01234567/0fffffff)
+00000000  00 01 00 10 00 01 47 08-01 23 45 67 0f ff ff ff @&t@
 ])
 AT_CLEANUP
 
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 4a90155..e70b0e5 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2879,6 +2879,11 @@ ofctl_parse_nxm__(bool oxm, enum ofp_version version)
 
             puts(out);
             free(out);
+
+            if (verbosity > 0) {
+                ovs_hex_dump(stdout, ofpbuf_data(&nx_match),
+                             ofpbuf_size(&nx_match), 0, false);
+            }
         } else {
             printf("nx_pull_match() returned error %s\n",
                    ofperr_get_name(error));
-- 
1.9.1




More information about the dev mailing list