Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
On Sep 30, 2014, at 5:47 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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 <b...@nicira.com>
> ---
> v1->v2: Fix handling of unknown fields when parsing flows loosely. This
> caused unit test failures that somehow I missed in v1.
> ---
> build-aux/extract-ofp-fields | 35 ++++--
> lib/meta-flow.h | 8 +-
> lib/nx-match.c | 246 ++++++++++++++++++++++++++++++++----------
> lib/nx-match.h | 2 +-
> lib/ofp-actions.c | 22 ++--
> tests/ofp-print.at | 14 +++
> tests/ovs-ofctl.at | 90 +++++++++++++++-
> utilities/ovs-ofctl.c | 5 +
> 8 files changed, 339 insertions(+), 83 deletions(-)
>
> diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
> index a82606b..88eb6d2 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 63f06dd..93bb866 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,28 +299,37 @@ 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;
> }
> + ofpbuf_pull(b, nxm_header_len(*header));
> +
> 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));
> @@ -222,6 +339,9 @@ nx_pull_header__(struct ofpbuf *b, bool allow_cookie,
> uint32_t *header,
>
> return 0;
>
> +bad_len:
> + VLOG_DBG_RL(&rl, "encountered partial (%"PRIu32"-byte) OXM entry",
> + ofpbuf_size(b));
> error:
> *header = 0;
> *field = NULL;
> @@ -229,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)
> {
> @@ -243,7 +363,7 @@ nx_pull_entry__(struct ofpbuf *b, bool allow_cookie,
> uint32_t *header,
> return header_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 "
> @@ -289,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);
> }
> @@ -308,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) {
> @@ -325,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) {
> @@ -907,12 +1027,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
> @@ -939,7 +1059,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)
> @@ -957,7 +1077,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);
> @@ -1042,7 +1162,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) {
> @@ -1065,7 +1185,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;
> @@ -1086,16 +1206,24 @@ 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);
> + 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 && is_experimenter_oxm(header)) {
> + return header;
> + }
> }
>
> return 0;
> @@ -1117,7 +1245,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;
>
> @@ -1529,7 +1657,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);
>
> @@ -1550,7 +1678,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);
>
> @@ -1649,7 +1777,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 ca4dac9..a02dd22 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 e5c0e98..d29b4de 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/3333)
>
> +# 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,15 +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 '-vPATTERN:console:%c|%p|%m' -vnx_match parse-nx-match <
> nx-match.txt], [0], [dnl
> +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)
> -], [nx_match|DBG|OXM header 258:1:1:4 is unknown
> ])
> AT_CLEANUP
>
> @@ -1897,6 +1918,10 @@ OXM_OF_PKT_REG0_W(00000000a0e0d050/00000000f0f0f0f0),
> OXM_OF_PKT_REG1_W(a0e0d050
>
> # Invalid field number.
> 01020304(1111/3333)
> +
> +# 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
> @@ -2134,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.
> @@ -2199,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 8dcd72c..ae8d59d 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.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
