[ovs-dev] [PATCH] tunnel: Support matching on the presence of Geneve options.

Jesse Gross jesse at nicira.com
Fri Aug 28 16:39:47 UTC 2015


Sometimes it is useful to match only on whether a Geneve option
is present even if the specific value is unimportant. A special
case of this is zero length options where there is no value at all
and the only information conveyed is whether the option was included
in the packet.

This operation was partially supported before but it was not consistent -
in particular, options were never serialized through NXM/OXM unless
they had a non-zero mask. Furthermore, zero length options were rejected
altogether when they were installed through the Geneve map OpenFlow
command.

This adds support for these types of matches by making any NXM/OXM for
tunnel metadata force a match on that field. In the case of a zero length
option, both the value and mask of the NXM are ignored.

Signed-off-by: Jesse Gross <jesse at nicira.com>
---
 lib/bitmap.h                 |  2 ++
 lib/meta-flow.c              | 46 ++++++++++++++++++++--------
 lib/meta-flow.h              |  3 +-
 lib/nx-match.c               | 43 +++++++++++++-------------
 lib/nx-match.h               |  6 ++--
 lib/odp-util.c               |  3 +-
 lib/ofp-parse.c              | 21 ++++++++++---
 lib/ofp-util.c               |  3 +-
 lib/tun-metadata.c           | 72 ++++++++++++++++++++++++++++++--------------
 lib/tun-metadata.h           | 11 ++++++-
 ofproto/ofproto-dpif-xlate.c |  5 +++
 tests/ovs-ofctl.at           |  1 +
 tests/tunnel.at              | 42 +++++++++++++++++++++++++-
 utilities/ovs-ofctl.8.in     |  7 ++++-
 14 files changed, 193 insertions(+), 72 deletions(-)

diff --git a/lib/bitmap.h b/lib/bitmap.h
index cf5d3f0..35f033e 100644
--- a/lib/bitmap.h
+++ b/lib/bitmap.h
@@ -278,4 +278,6 @@ bitmap_is_all_zeros(const unsigned long *bitmap, size_t n)
 #define ULLONG_SET0(MAP, OFFSET) ((MAP) &= ~(1ULL << (OFFSET)))
 #define ULLONG_SET1(MAP, OFFSET) ((MAP) |= 1ULL << (OFFSET))
 
+#define ULLONG_GET(MAP, OFFSET) ((MAP) & (1ULL << (OFFSET)))
+
 #endif /* bitmap.h */
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 5a46ce4..971565d 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -202,12 +202,9 @@ mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
         return !wc->masks.tunnel.gbp_id;
     case MFF_TUN_GBP_FLAGS:
         return !wc->masks.tunnel.gbp_flags;
-    CASE_MFF_TUN_METADATA: {
-        union mf_value value;
-
-        tun_metadata_read(&wc->masks.tunnel, mf, &value);
-        return is_all_zeros(&value.tun_metadata, mf->n_bytes);
-    }
+    CASE_MFF_TUN_METADATA:
+        return !ULLONG_GET(wc->masks.tunnel.metadata.present.map,
+                           mf->id - MFF_TUN_METADATA0);
     case MFF_METADATA:
         return !wc->masks.metadata;
     case MFF_IN_PORT:
@@ -1063,19 +1060,28 @@ field_len(const struct mf_field *mf, const union mf_value *value_)
 /* Returns the effective length of the field. For fixed length fields,
  * this is just the defined length. For variable length fields, it is
  * the minimum size encoding that retains the same meaning (i.e.
- * discarding leading zeros). */
+ * discarding leading zeros).
+ *
+ * 'is_masked' returns (if non-NULL) whether the original contained
+ * a mask. Otherwise, a mask that is the same length as the value
+ * might be misinterpreted as an exact match. */
 int
 mf_field_len(const struct mf_field *mf, const union mf_value *value,
-             const union mf_value *mask)
+             const union mf_value *mask, bool *is_masked_)
 {
     int len, mask_len;
+    bool is_masked = mask && !is_all_ones(mask, mf->n_bytes);
 
     len = field_len(mf, value);
-    if (mask && !is_all_ones(mask, mf->n_bytes)) {
+    if (is_masked) {
         mask_len = field_len(mf, mask);
         len = MAX(len, mask_len);
     }
 
+    if (is_masked_) {
+        *is_masked_ = is_masked;
+    }
+
     return len;
 }
 
@@ -1329,6 +1335,13 @@ mf_set_flow_value_masked(const struct mf_field *field,
     mf_set_flow_value(field, &tmp, flow);
 }
 
+bool
+mf_is_tun_metadata(const struct mf_field *mf)
+{
+    return mf->id >= MFF_TUN_METADATA0 &&
+           mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
+}
+
 /* Returns true if 'mf' has a zero value in 'flow', false if it is nonzero.
  *
  * The caller is responsible for ensuring that 'flow' meets 'mf''s
@@ -1336,10 +1349,15 @@ mf_set_flow_value_masked(const struct mf_field *field,
 bool
 mf_is_zero(const struct mf_field *mf, const struct flow *flow)
 {
-    union mf_value value;
+    if (!mf_is_tun_metadata(mf)) {
+        union mf_value value;
 
-    mf_get_value(mf, flow, &value);
-    return is_all_zeros(&value, mf->n_bytes);
+        mf_get_value(mf, flow, &value);
+        return is_all_zeros(&value, mf->n_bytes);
+    } else {
+        return !ULLONG_GET(flow->tunnel.metadata.present.map,
+                           mf->id - MFF_TUN_METADATA0);
+    }
 }
 
 /* Makes 'match' wildcard field 'mf'.
@@ -1585,7 +1603,9 @@ mf_set(const struct mf_field *mf,
     if (!mask || is_all_ones(mask, mf->n_bytes)) {
         mf_set_value(mf, value, match);
         return mf->usable_protocols_exact;
-    } else if (is_all_zeros(mask, mf->n_bytes)) {
+    } else if (is_all_zeros(mask, mf->n_bytes) && !mf_is_tun_metadata(mf)) {
+        /* Tunnel metadata matches on the existence of the field itself, so
+         * it still needs to be encoded even if the value is wildcarded. */
         mf_set_wild(mf, match);
         return OFPUTIL_P_ANY;
     }
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 323da02..b274d72 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1851,10 +1851,11 @@ void mf_set_flow_value_masked(const struct mf_field *,
                               const union mf_value *value,
                               const union mf_value *mask,
                               struct flow *);
+bool mf_is_tun_metadata(const struct mf_field *);
 bool mf_is_zero(const struct mf_field *, const struct flow *);
 void mf_mask_field(const struct mf_field *, struct flow *);
 int mf_field_len(const struct mf_field *, const union mf_value *value,
-                 const union mf_value *mask);
+                 const union mf_value *mask, bool *is_masked);
 
 void mf_get(const struct mf_field *, const struct match *,
             union mf_value *value, union mf_value *mask);
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 54645df..08f0078 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -299,11 +299,11 @@ nx_pull_header__(struct ofpbuf *b, bool allow_cookie, uint64_t *header,
         }
         *header = ntohll(get_unaligned_be64(b->data));
     }
-    if (nxm_length(*header) <= nxm_experimenter_len(*header)) {
+    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);
+                     nxm_header_len(*header));
         goto error;
     }
     ofpbuf_pull(b, nxm_header_len(*header));
@@ -674,26 +674,25 @@ oxm_pull_field_array(const void *fields_data, size_t fields_len,
  * 'put' functions whose names end in 'm' add a field that might be wildcarded.
  * Other 'put' functions add exact-match fields.
  */
-
-static void
-nxm_put_unmasked(struct ofpbuf *b, enum mf_field_id field,
-                 enum ofp_version version, const void *value, size_t n_bytes)
+void
+nxm_put__(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
+          const void *value, const void *mask, size_t n_bytes)
 {
-    nx_put_header_len(b, field, version, false, n_bytes);
+    nx_put_header_len(b, field, version, !!mask, n_bytes);
     ofpbuf_put(b, value, n_bytes);
+    if (mask) {
+        ofpbuf_put(b, mask, n_bytes);
+    }
+
 }
 
-void
+static void
 nxm_put(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
         const void *value, const void *mask, size_t n_bytes)
 {
     if (!is_all_zeros(mask, n_bytes)) {
         bool masked = !is_all_ones(mask, n_bytes);
-        nx_put_header_len(b, field, version, masked, n_bytes);
-        ofpbuf_put(b, value, n_bytes);
-        if (masked) {
-            ofpbuf_put(b, mask, n_bytes);
-        }
+        nxm_put__(b, field, version, value, masked ? mask : NULL, n_bytes);
     }
 }
 
@@ -708,7 +707,7 @@ static void
 nxm_put_8(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
           uint8_t value)
 {
-    nxm_put_unmasked(b, field, version, &value, sizeof value);
+    nxm_put__(b, field, version, &value, NULL, sizeof value);
 }
 
 static void
@@ -722,7 +721,7 @@ static void
 nxm_put_16(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
            ovs_be16 value)
 {
-    nxm_put_unmasked(b, field, version, &value, sizeof value);
+    nxm_put__(b, field, version, &value, NULL, sizeof value);
 }
 
 static void
@@ -736,7 +735,7 @@ static void
 nxm_put_32(struct ofpbuf *b, enum mf_field_id field, enum ofp_version version,
            ovs_be32 value)
 {
-    nxm_put_unmasked(b, field, version, &value, sizeof value);
+    nxm_put__(b, field, version, &value, NULL, sizeof value);
 }
 
 static void
@@ -1172,10 +1171,10 @@ oxm_put_field_array(struct ofpbuf *b, const struct field_array *fa,
 
     for (i = 0; i < MFF_N_IDS; i++) {
         if (bitmap_is_set(fa->used.bm, i)) {
-            int len = mf_field_len(mf_from_id(i), &fa->value[i], NULL);
-            nxm_put_unmasked(b, i, version,
-                             &fa->value[i].u8 + mf_from_id(i)->n_bytes - len,
-                             len);
+            int len = mf_field_len(mf_from_id(i), &fa->value[i], NULL, NULL);
+            nxm_put__(b, i, version,
+                      &fa->value[i].u8 + mf_from_id(i)->n_bytes - len, NULL,
+                      len);
         }
     }
 
@@ -1217,10 +1216,10 @@ nx_put_entry(struct ofpbuf *b,
              const union mf_value *value, const union mf_value *mask)
 {
     const struct mf_field *mf = mf_from_id(field);
-    bool masked = mask && !is_all_ones(mask, mf->n_bytes);
+    bool masked;
     int len, offset;
 
-    len = mf_field_len(mf, value, mask);
+    len = mf_field_len(mf, value, mask, &masked);
     offset = mf->n_bytes - len;
 
     nx_put_header_len(b, field, version, masked, len);
diff --git a/lib/nx-match.h b/lib/nx-match.h
index db20987..2625c63 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -70,9 +70,9 @@ enum ofperr nx_pull_entry(struct ofpbuf *, const struct mf_field **,
                           union mf_value *value, union mf_value *mask);
 enum ofperr nx_pull_header(struct ofpbuf *, const struct mf_field **,
                            bool *masked);
-void nxm_put(struct ofpbuf *b, enum mf_field_id field,
-             enum ofp_version version, const void *value,
-             const void *mask, size_t n_bytes);
+void nxm_put__(struct ofpbuf *b, enum mf_field_id field,
+               enum ofp_version version, const void *value,
+               const void *mask, size_t n_bytes);
 void nx_put_entry(struct ofpbuf *, enum mf_field_id, enum ofp_version,
                   const union mf_value *value, const union mf_value *mask);
 void nx_put_header(struct ofpbuf *, enum mf_field_id, enum ofp_version,
diff --git a/lib/odp-util.c b/lib/odp-util.c
index b50d87b..3ff537e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1845,7 +1845,8 @@ format_geneve_opts(const struct geneve_opt *opt,
                     verbose);
         format_u8x(ds, "type", opt->type, MASK(mask, type), verbose);
         format_u8u(ds, "len", data_len, mask ? &data_len_mask : NULL, verbose);
-        if (verbose || !mask || !is_all_zeros(mask + 1, data_len)) {
+        if (data_len &&
+            (verbose || !mask || !is_all_zeros(mask + 1, data_len))) {
             ds_put_hex(ds, opt + 1, data_len);
             if (mask && !is_all_ones(mask + 1, data_len)) {
                 ds_put_char(ds, '/');
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 372d72f..d3d76f2 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -365,6 +365,21 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
         } else if (!strcmp(name, "no_readonly_table")
                    || !strcmp(name, "allow_hidden_fields")) {
              /* ignore these fields. */
+        } else if (mf_from_name(name)) {
+            char *value;
+
+            value = strtok_r(NULL, ", \t\r\n", &save_ptr);
+            if (!value) {
+                /* If there's no value, we're just trying to match on the
+                 * existence of the field, so use a no-op value. */
+                value = "0/0";
+            }
+
+            error = parse_field(mf_from_name(name), value, &fm->match,
+                                usable_protocols);
+            if (error) {
+                return error;
+            }
         } else {
             char *value;
 
@@ -424,9 +439,6 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
                     error = str_to_be64(value, &fm->new_cookie);
                     fm->modify_cookie = true;
                 }
-            } else if (mf_from_name(name)) {
-                error = parse_field(mf_from_name(name), value, &fm->match,
-                                    usable_protocols);
             } else if (!strcmp(name, "duration")
                        || !strcmp(name, "n_packets")
                        || !strcmp(name, "n_bytes")
@@ -1243,7 +1255,8 @@ parse_select_group_field(char *s, struct field_array *fa,
                 }
 
                 /* The mask cannot be all-zeros */
-                if (is_all_zeros(&value, mf->n_bytes)) {
+                if (!mf_is_tun_metadata(mf) &&
+                    is_all_zeros(&value, mf->n_bytes)) {
                     return xasprintf("%s: values are wildcards here "
                                      "and must not be all-zeros", s);
                 }
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 0a5232d..8559621 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9199,8 +9199,7 @@ decode_geneve_table_mappings(struct ofpbuf *msg, unsigned int max_fields,
         map->option_type = nx_map->option_type;
 
         map->option_len = nx_map->option_len;
-        if (map->option_len == 0 || map->option_len % 4 ||
-            map->option_len > GENEVE_MAX_OPT_SIZE) {
+        if (map->option_len % 4 || map->option_len > GENEVE_MAX_OPT_SIZE) {
             VLOG_WARN_RL(&bad_ofmsg_rl,
                          "geneve table option length (%u) is not a valid option size",
                          map->option_len);
diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
index acaacff..bef4641 100644
--- a/lib/tun-metadata.c
+++ b/lib/tun-metadata.c
@@ -280,7 +280,7 @@ tun_metadata_write(struct flow_tnl *tnl,
 
 static const struct tun_metadata_loc *
 metadata_loc_from_match(struct tun_table *map, struct match *match,
-                        unsigned int idx, unsigned int field_len)
+                        unsigned int idx, unsigned int field_len, bool masked)
 {
     ovs_assert(idx < TUN_METADATA_NUM_OPTS);
 
@@ -293,18 +293,19 @@ metadata_loc_from_match(struct tun_table *map, struct match *match,
     }
 
     if (match->tun_md.alloc_offset + field_len >= TUN_METADATA_TOT_OPT_SIZE ||
-        match->tun_md.loc[idx].len) {
+        match->wc.masks.tunnel.metadata.present.map & idx) {
         return NULL;
     }
 
-    match->tun_md.loc[idx].len = field_len;
-    match->tun_md.loc[idx].c.offset = match->tun_md.alloc_offset;
-    match->tun_md.loc[idx].c.len = field_len;
-    match->tun_md.loc[idx].c.next = NULL;
+    match->tun_md.entry[idx].loc.len = field_len;
+    match->tun_md.entry[idx].loc.c.offset = match->tun_md.alloc_offset;
+    match->tun_md.entry[idx].loc.c.len = field_len;
+    match->tun_md.entry[idx].loc.c.next = NULL;
+    match->tun_md.entry[idx].masked = masked;
     match->tun_md.alloc_offset += field_len;
     match->tun_md.valid = true;
 
-    return &match->tun_md.loc[idx];
+    return &match->tun_md.entry[idx].loc;
 }
 
 /* Makes 'match' match 'value'/'mask' on field 'mf'.
@@ -330,13 +331,14 @@ tun_metadata_set_match(const struct mf_field *mf, const union mf_value *value,
     const struct tun_metadata_loc *loc;
     unsigned int idx = mf->id - MFF_TUN_METADATA0;
     unsigned int field_len;
+    bool is_masked;
     unsigned int data_offset;
     union mf_value data;
 
     ovs_assert(!(match->flow.tunnel.flags & FLOW_TNL_F_UDPIF));
 
-    field_len = mf_field_len(mf, value, mask);
-    loc = metadata_loc_from_match(map, match, idx, field_len);
+    field_len = mf_field_len(mf, value, mask, &is_masked);
+    loc = metadata_loc_from_match(map, match, idx, field_len, is_masked);
     if (!loc) {
         return;
     }
@@ -422,7 +424,8 @@ tun_metadata_get_fmd(const struct flow_tnl *tnl, struct match *flow_metadata)
         const struct tun_metadata_loc *old_loc = &flow.metadata.tab->entries[i].loc;
         const struct tun_metadata_loc *new_loc;
 
-        new_loc = metadata_loc_from_match(NULL, flow_metadata, i, old_loc->len);
+        new_loc = metadata_loc_from_match(NULL, flow_metadata, i,
+                                          old_loc->len, false);
 
         memcpy_from_metadata(opts.tun_metadata, &flow.metadata, old_loc);
         memcpy_to_metadata(&flow_metadata->flow.tunnel.metadata,
@@ -950,12 +953,22 @@ tun_metadata_to_geneve_udpif_mask(const struct flow_tnl *flow_src,
 
 static const struct tun_metadata_loc *
 metadata_loc_from_match_read(struct tun_table *map, const struct match *match,
-                             unsigned int idx)
+                             unsigned int idx, struct flow_tnl *mask,
+                             bool *is_masked)
 {
+    union mf_value mask_opts;
+
     if (match->tun_md.valid) {
-        return &match->tun_md.loc[idx];
+        *is_masked = match->tun_md.entry[idx].masked;
+        return &match->tun_md.entry[idx].loc;
     }
 
+    memcpy_from_metadata(mask_opts.tun_metadata, &mask->metadata,
+                         &map->entries[idx].loc);
+
+    *is_masked = map->entries[idx].loc.len == 0 ||
+                 !is_all_ones(mask_opts.tun_metadata,
+                              map->entries[idx].loc.len);
     return &map->entries[idx].loc;
 }
 
@@ -973,14 +986,16 @@ tun_metadata_to_nx_match(struct ofpbuf *b, enum ofp_version oxm,
 
     ULLONG_FOR_EACH_1 (i, mask.metadata.present.map) {
         const struct tun_metadata_loc *loc;
+        bool is_masked;
         union mf_value opts;
         union mf_value mask_opts;
 
-        loc = metadata_loc_from_match_read(flow.metadata.tab, match, i);
+        loc = metadata_loc_from_match_read(flow.metadata.tab, match, i,
+                                           &mask, &is_masked);
         memcpy_from_metadata(opts.tun_metadata, &flow.metadata, loc);
         memcpy_from_metadata(mask_opts.tun_metadata, &mask.metadata, loc);
-        nxm_put(b, MFF_TUN_METADATA0 + i, oxm, opts.tun_metadata,
-                mask_opts.tun_metadata, loc->len);
+        nxm_put__(b, MFF_TUN_METADATA0 + i, oxm, opts.tun_metadata,
+                  is_masked ? mask_opts.tun_metadata : NULL, loc->len);
     }
 }
 
@@ -997,18 +1012,29 @@ tun_metadata_match_format(struct ds *s, const struct match *match)
 
     ULLONG_FOR_EACH_1 (i, mask.metadata.present.map) {
         const struct tun_metadata_loc *loc;
-        union mf_value opts;
+        bool is_masked;
+        union mf_value opts, mask_opts;
 
-        loc = metadata_loc_from_match_read(flow.metadata.tab, match, i);
+        loc = metadata_loc_from_match_read(flow.metadata.tab, match, i,
+                                           &mask, &is_masked);
 
-        ds_put_format(s, "tun_metadata%u=", i);
-        memcpy_from_metadata(opts.tun_metadata, &flow.metadata, loc);
-        ds_put_hex(s, opts.tun_metadata, loc->len);
+        ds_put_format(s, "tun_metadata%u", i);
+        memcpy_from_metadata(mask_opts.tun_metadata, &mask.metadata, loc);
+
+        if (!ULLONG_GET(flow.metadata.present.map, i)) {
+            /* Indicate that we are matching on the field being not present. */
+            ds_put_cstr(s, "=NP");
+        } else if (!(is_masked &&
+                     is_all_zeros(mask_opts.tun_metadata, loc->len))) {
+            ds_put_char(s, '=');
 
-        memcpy_from_metadata(opts.tun_metadata, &mask.metadata, loc);
-        if (!is_all_ones(opts.tun_metadata, loc->len)) {
-            ds_put_char(s, '/');
+            memcpy_from_metadata(opts.tun_metadata, &flow.metadata, loc);
             ds_put_hex(s, opts.tun_metadata, loc->len);
+
+            if (!is_all_ones(mask_opts.tun_metadata, loc->len)) {
+                ds_put_char(s, '/');
+                ds_put_hex(s, mask_opts.tun_metadata, loc->len);
+            }
         }
         ds_put_char(s, ',');
     }
diff --git a/lib/tun-metadata.h b/lib/tun-metadata.h
index 49db511..85652ad 100644
--- a/lib/tun-metadata.h
+++ b/lib/tun-metadata.h
@@ -86,13 +86,22 @@ struct tun_metadata_loc {
     struct tun_metadata_loc_chain c;
 };
 
+/* Bookkeeping information to keep track of an option that was allocated
+ * inside struct match. */
+struct tun_metadata_match_entry {
+    struct tun_metadata_loc loc; /* Allocated position. */
+    bool masked; /* Source value had a mask. Otherwise we can't tell if the
+                  * entire field was exact matched or only the portion that
+                  * is the same size as the value. */
+};
+
 /* Allocation of options inside struct match.  This is important if we don't
  * have access to a global allocation table - either because there isn't one
  * (ovs-ofctl) or if we need to keep the allocation outside of packet
  * processing context (Packet-In). These structures never have dynamically
  * allocated memory because the address space is never fragmented. */
 struct tun_metadata_allocation {
-    struct tun_metadata_loc loc[TUN_METADATA_NUM_OPTS];
+    struct tun_metadata_match_entry entry[TUN_METADATA_NUM_OPTS];
     uint8_t alloc_offset;       /* Byte offset into 'opts', multiple of 4.  */
     bool valid;                 /* Set to true after any allocation occurs. */
 };
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6b9713f..b5ed032 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3326,6 +3326,11 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
             }
             basis = hash_bytes(&value, mf->n_bytes, basis);
 
+            /* For tunnels, hash in whether the field is present. */
+            if (mf_is_tun_metadata(mf)) {
+                basis = hash_boolean(mf_is_zero(mf, &ctx->xin->flow), basis);
+            }
+
             mf_mask_field(mf, &ctx->wc->masks);
         }
     }
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index ad08b31..c6f6bca 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -19,6 +19,7 @@ for test_case in \
     'tun_gbp_flags=0/0x1                         NXM,OXM' \
     'tun_metadata0=0                             NXM,OXM' \
     'tun_metadata0=0/0x1                         NXM,OXM' \
+    'tun_metadata0                               NXM,OXM' \
     'metadata=0                                  NXM,OXM,OpenFlow11' \
     'metadata=1/1                                NXM,OXM,OpenFlow11' \
     'in_port=1                                   any' \
diff --git a/tests/tunnel.at b/tests/tunnel.at
index bec79dc..93e4493 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -448,6 +448,7 @@ Datapath actions: 2
 ])
 
 dnl Check mapping table constraints
+AT_CHECK([ovs-ofctl del-flows br0])
 AT_CHECK([ovs-ofctl add-geneve-map br0 "{class=0xffff,type=2,len=124}->tun_metadata2,{class=0xffff,type=3,len=124}->tun_metadata3"], [1], [ignore],
 [OFPT_ERROR (xid=0x4): NXGTMFC_TABLE_FULL
 NXT_GENEVE_TABLE_MOD (xid=0x4):
@@ -468,7 +469,7 @@ AT_CHECK([ovs-ofctl add-geneve-map br0 "{class=0xffff,type=3,len=8}->tun_metadat
 AT_CHECK([ovs-ofctl add-flow br0 tun_metadata3=0x1234567890abcdef,actions=2])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=3,len=8,0x1234567890abcdef}),flags(df|key)),in_port(6081),skb_mark(0),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
-  [Megaflow: pkt_mark=0,recirc_id=0,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_ttl=64,tun_flags=+df-csum+key,tun_metadata0=0/0xf,tun_metadata3=0x1234567890abcdef,in_port=1,nw_frag=no
+  [Megaflow: pkt_mark=0,recirc_id=0,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_ttl=64,tun_flags=+df-csum+key,tun_metadata3=0x1234567890abcdef,in_port=1,nw_frag=no
 Datapath actions: 2
 ])
 
@@ -476,3 +477,42 @@ AT_CHECK([ovs-ofctl del-geneve-map br0])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([tunnel - Geneve option present])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=geneve \
+                    options:remote_ip=1.1.1.1 ofport_request=1 \
+                    -- add-port br0 p2 -- set Interface p2 type=dummy \
+                    ofport_request=2 ofport_request=2])
+OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP
+
+AT_CHECK([ovs-ofctl add-geneve-map br0 "{class=0xffff,type=0,len=4}->tun_metadata0,{class=0xffff,type=1,len=0}->tun_metadata1,{class=0xffff,type=2,len=4}->tun_metadata2"])
+
+AT_DATA([flows.txt], [dnl
+priority=1,tun_metadata0,actions=2
+priority=2,tun_metadata1=0,actions=IN_PORT
+priority=3,tun_metadata2=0,actions=drop
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort],
+[0], [dnl
+ priority=1,tun_metadata0 actions=output:2
+ priority=2,tun_metadata1 actions=IN_PORT
+ priority=3,tun_metadata2=0 actions=drop
+NXST_FLOW reply:
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=0,len=4,0x12345678}),flags(df|key)),in_port(6081),skb_mark(0),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([tail -2 stdout], [0],
+  [Megaflow: pkt_mark=0,recirc_id=0,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_ttl=64,tun_flags=+df-csum+key,tun_metadata0,tun_metadata1=NP,tun_metadata2=NP,in_port=1,nw_frag=no
+Datapath actions: 2
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=1,len=0}),flags(df|key)),in_port(6081),skb_mark(0),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([tail -2 stdout], [0],
+  [Megaflow: pkt_mark=0,recirc_id=0,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_ttl=64,tun_flags=+df-csum+key,tun_metadata1,tun_metadata2=NP,in_port=1,nw_ecn=0,nw_frag=no
+Datapath actions: set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,geneve({class=0xffff,type=0x1,len=0}),flags(df|key))),6081
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index f4a11f3..8bb3715 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1236,9 +1236,14 @@ set.
 For more information, please see the corresponding IETF draft:
 https://tools.ietf.org/html/draft-smith-vxlan-group-policy
 .
-.IP "\fBtun_metadata\fIidx\fB=\fIvalue\fR[\fB/\fImask\fR]"
+.IP "\fBtun_metadata\fIidx\fR[\fB=\fIvalue\fR[\fB/\fImask\fR]]"
 Matches \fIvalue\fR either exactly or with optional \fImask\fR in
 tunnel metadata field number \fIidx\fR (numbered from 0 to 63).
+The act of specifying a field implies a match on the existence
+of that field in the packet in addition to the masked value. As
+a shorthand, it is possible to specify only the field name to
+simply match on an option being present.
+.IP
 Tunnel metadata fields can be dynamically assigned onto the data
 contained in the options of Geneve packets using the commands
 described in the section \fBOpenFlow Switch Geneve Option Table
-- 
2.1.4




More information about the dev mailing list