[ovs-dev] [PATCH] ofp-actions: Fix tunnel meatdata subfield length in flow action

Yi-Hung Wei yihung.wei at gmail.com
Tue Jan 10 02:09:33 UTC 2017


Previously, if a flow action that involves a tunnel metadata subfield is
dumpped from vswitchd, the replied subfield length in the OXM header is
filled with the maximum possible field length, instead of the length
configured in the tunnel TLV mapping table.

In order to derive the correct length of a tun_metadata subfield, this patch
passes the tunnel TLV mapping table (struct tun_table) to where we decode
the flow actions. extract-ofp-actions is updated to pass the tunnel TLV mapping
table to the flow action decoding functions. Also, a new error code is
introduced to identify a flow with an invalid tun_metadata ID. Moreover, a
testcase is added to prevent future regressions.

Currently, this patch only addresses the usage of tun_metaata for REG_MOVE action.
Other actions that may use the tun_metadata will be addressed in subsequent patches.

VMWare-BZ: #1768370
Reported-by: Harold Lim <haroldl at vmware.com>
Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
---
 build-aux/extract-ofp-actions     | 46 ++++++++++++++++++---------
 include/openvswitch/meta-flow.h   |  1 +
 include/openvswitch/ofp-actions.h | 10 +++---
 include/openvswitch/ofp-errors.h  |  4 +++
 lib/nx-match.c                    | 12 +++++++
 lib/nx-match.h                    |  2 ++
 lib/ofp-actions.c                 | 67 ++++++++++++++++++++++++++++-----------
 lib/ofp-util.c                    |  6 ++--
 lib/tun-metadata.c                | 23 +++++++++++++-
 lib/tun-metadata.h                |  4 +++
 ovn/lib/expr.c                    |  2 +-
 tests/ofproto.at                  | 26 +++++++++++++++
 utilities/ovs-ofctl.c             | 12 ++++---
 13 files changed, 168 insertions(+), 47 deletions(-)

diff --git a/build-aux/extract-ofp-actions b/build-aux/extract-ofp-actions
index 3a72349..2ee8549 100755
--- a/build-aux/extract-ofp-actions
+++ b/build-aux/extract-ofp-actions
@@ -123,7 +123,13 @@ def extract_ofp_actions(fn, definitions):
             fatal("unexpected syntax between actions")
 
         dsts = m.group(1)
-        argtype = m.group(2).strip().replace('.', '', 1)
+        argtypes = m.group(2).strip().replace('.', '', 1)
+
+        if 'TTMT' in argtypes:
+            arg_tun_table = True
+        else:
+            arg_tun_table = False
+        argtype = argtypes.replace('TTMT', '', 1).rstrip()
 
         get_line()
         m = re.match(r'\s+(([A-Z]+)_RAW([0-9]*)_([A-Z0-9_]+)),?', line)
@@ -210,17 +216,18 @@ def extract_ofp_actions(fn, definitions):
                     else:
                         max_length = min_length
 
-                    info = {"enum": enum,                 # 0
-                            "deprecation": deprecation,   # 1
-                            "file_name": file_name,       # 2
-                            "line_number": line_number,   # 3
-                            "min_length": min_length,     # 4
-                            "max_length": max_length,     # 5
-                            "arg_ofs": arg_ofs,           # 6
-                            "arg_len": arg_len,           # 7
-                            "base_argtype": base_argtype, # 8
-                            "version": version,           # 9
-                            "type": type_}                # 10
+                    info = {"enum": enum,                   # 0
+                            "deprecation": deprecation,     # 1
+                            "file_name": file_name,         # 2
+                            "line_number": line_number,     # 3
+                            "min_length": min_length,       # 4
+                            "max_length": max_length,       # 5
+                            "arg_ofs": arg_ofs,             # 6
+                            "arg_len": arg_len,             # 7
+                            "base_argtype": base_argtype,   # 8
+                            "arg_tun_table": arg_tun_table, # 9
+                            "version": version,             # 10
+                            "type": type_}                  # 11
                     domain[vendor][type_][version] = info
 
                     enums.setdefault(enum, [])
@@ -314,7 +321,8 @@ def extract_ofp_actions(fn, definitions):
         print """\
 static enum ofperr
 ofpact_decode(const struct ofp_action_header *a, enum ofp_raw_action_type raw,
-              enum ofp_version version, uint64_t arg, struct ofpbuf *out)
+              enum ofp_version version, uint64_t arg, const struct tun_table *tun_table,
+              struct ofpbuf *out)
 {
     switch (raw) {\
 """
@@ -322,6 +330,7 @@ ofpact_decode(const struct ofp_action_header *a, enum ofp_raw_action_type raw,
             enum = versions[0]["enum"]
             print "    case %s:" % enum
             base_argtype = versions[0]["base_argtype"]
+            arg_tun_table = versions[0]["arg_tun_table"]
             if base_argtype == 'void':
                 print "        return decode_%s(out);" % enum
             else:
@@ -333,7 +342,10 @@ ofpact_decode(const struct ofp_action_header *a, enum ofp_raw_action_type raw,
                         arg = "%s(arg)" % hton
                     else:
                         arg = "arg"
-                print "        return decode_%s(%s, version, out);" % (enum, arg)
+                if arg_tun_table:
+                    print "        return decode_%s(%s, version, tun_table, out);" % (enum, arg)
+                else:
+                    print "        return decode_%s(%s, version, out);" % (enum, arg)
             print
         print """\
     default:
@@ -346,11 +358,14 @@ ofpact_decode(const struct ofp_action_header *a, enum ofp_raw_action_type raw,
             enum = versions[0]["enum"]
             prototype = "static enum ofperr decode_%s(" % enum
             base_argtype = versions[0]["base_argtype"]
+            arg_tun_table = versions[0]["arg_tun_table"]
             if base_argtype != 'void':
                 if base_argtype.startswith('struct'):
                     prototype += "const %s *, enum ofp_version, " % base_argtype
                 else:
                     prototype += "%s, enum ofp_version, " % base_argtype
+                if arg_tun_table:
+                    prototype += 'const struct tun_table *, '
             prototype += "struct ofpbuf *);"
             print prototype
 
@@ -358,7 +373,8 @@ ofpact_decode(const struct ofp_action_header *a, enum ofp_raw_action_type raw,
 static enum ofperr ofpact_decode(const struct ofp_action_header *,
                                  enum ofp_raw_action_type raw,
                                  enum ofp_version version,
-                                 uint64_t arg, struct ofpbuf *out);
+                                 uint64_t arg, const struct tun_table *tun_table,
+                                 struct ofpbuf *out);
 """
 
 if __name__ == '__main__':
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 9f123ef..8f60224 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -1988,6 +1988,7 @@ struct mf_subfield {
     const struct mf_field *field;
     unsigned int ofs;           /* Bit offset. */
     unsigned int n_bits;        /* Number of bits. */
+    unsigned int n_bytes;       /* Length of the subfield in bytes. */
 };
 
 /* Data for some part of an mf_field.
diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index df9025c..77a547a 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -943,10 +943,12 @@ enum ofperr ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
                                           unsigned int actions_len,
                                           enum ofp_version version,
                                           struct ofpbuf *ofpacts);
-enum ofperr ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
-                                               unsigned int instructions_len,
-                                               enum ofp_version version,
-                                               struct ofpbuf *ofpacts);
+enum ofperr
+ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
+                                   unsigned int instructions_len,
+                                   enum ofp_version version,
+                                   const struct tun_table *tun_table,
+                                   struct ofpbuf *ofpacts);
 enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
                           struct flow *, ofp_port_t max_ports,
                           uint8_t table_id, uint8_t n_tables,
diff --git a/include/openvswitch/ofp-errors.h b/include/openvswitch/ofp-errors.h
index a378909..a25081e 100644
--- a/include/openvswitch/ofp-errors.h
+++ b/include/openvswitch/ofp-errors.h
@@ -395,6 +395,10 @@ enum ofperr {
      * nxt_flow_mod_table_id extension is enabled. */
     OFPERR_NXFMFC_BAD_TABLE_ID,
 
+    /* NX1.0-1.1(1,536), NX1.2+(37).  Attempted to add a flow with an invalid
+     * tun_metadata ID that is not mapped in the tunnel TLV mapping table. */
+    OFPERR_NXFMFC_INVALID_TUN_METADATA_ID,
+
 /* ## ---------------------- ## */
 /* ## OFPET_GROUP_MOD_FAILED ## */
 /* ## ---------------------- ## */
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 2fd31b9..42ce25f 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1267,6 +1267,18 @@ nx_put_header(struct ofpbuf *b, enum mf_field_id field,
     nx_put_header__(b, mf_oxm_header(field, version), masked);
 }
 
+/* Appends to 'b' the NXM header that specifies subfield 'sf'. Sets the length
+ * of the subfield to its default length if it not determined (equal to 0). */
+void nx_put_mf_subfield_header(struct ofpbuf *b, const struct mf_subfield *sf,
+                               enum ofp_version version, bool masked)
+{
+    if (sf->n_bytes == 0) {
+        nx_put_header(b, sf->field->id, version, masked);
+    } else {
+        nx_put_header_len(b, sf->field->id, version, masked, sf->n_bytes);
+    }
+}
+
 static void
 nx_put_header_len(struct ofpbuf *b, enum mf_field_id field,
                   enum ofp_version version, bool masked, size_t n_bytes)
diff --git a/lib/nx-match.h b/lib/nx-match.h
index a86cceb..cdd51bc 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -87,6 +87,8 @@ 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,
                    bool masked);
+void nx_put_mf_subfield_header(struct ofpbuf *, const struct mf_subfield *,
+                               enum ofp_version, bool);
 
 /* NXM and OXM protocol headers values.
  *
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index f29673f..8ff0108 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -114,6 +114,10 @@ OFP_ASSERT(sizeof(struct ext_action_header) == 16);
  *         # "void": The action is just a (standard or vendor extension)
  *           header.
  *
+ *         # Optionally, one may add "TTMT" in the end of the second part if
+ *           the tunnel TLV mapping table (struct tun_table) is required while
+ *           decoding the Openflow action.
+ *
  *   - Optional additional text enclosed in square brackets is commentary for
  *     the human reader.
  */
@@ -245,7 +249,7 @@ enum ofp_raw_action_type {
     OFPAT_RAW15_COPY_FIELD,
     /* ONF1.3-1.4(3200): struct onf_action_copy_field, ... */
     ONFACT_RAW13_COPY_FIELD,
-    /* NX1.0-1.4(6): struct nx_action_reg_move, ... */
+    /* NX1.0-1.4(6): struct nx_action_reg_move, ... TTMT */
     NXAST_RAW_REG_MOVE,
 
 /* ## ------------------------- ## */
@@ -394,7 +398,8 @@ static char *OVS_WARN_UNUSED_RESULT ofpacts_parse(
 static enum ofperr ofpacts_pull_openflow_actions__(
     struct ofpbuf *openflow, unsigned int actions_len,
     enum ofp_version version, uint32_t allowed_ovsinsts,
-    struct ofpbuf *ofpacts, enum ofpact_type outer_action);
+    struct ofpbuf *ofpacts, enum ofpact_type outer_action,
+    const struct tun_table *tun_table);
 static char * OVS_WARN_UNUSED_RESULT ofpacts_parse_copy(
     const char *s_, struct ofpbuf *ofpacts,
     enum ofputil_protocol *usable_protocols,
@@ -2309,6 +2314,7 @@ decode_ONFACT_RAW13_COPY_FIELD(const struct onf_action_copy_field *oacf,
 static enum ofperr
 decode_NXAST_RAW_REG_MOVE(const struct nx_action_reg_move *narm,
                           enum ofp_version ofp_version OVS_UNUSED,
+                          const struct tun_table *tun_table,
                           struct ofpbuf *ofpacts)
 {
     struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts);
@@ -2325,10 +2331,28 @@ decode_NXAST_RAW_REG_MOVE(const struct nx_action_reg_move *narm,
     if (error) {
         return error;
     }
+    move->src.n_bytes = move->src.field->n_bytes;
+    if (mf_is_tun_metadata(move->src.field)) {
+        error = tun_metadata_len(move->src.field, tun_table,
+                &move->src.n_bytes);
+        if (error) {
+            return error;
+        }
+    }
+
     error = nx_pull_header(&b, &move->dst.field, NULL);
     if (error) {
         return error;
     }
+    move->dst.n_bytes = move->dst.field->n_bytes;
+    if (mf_is_tun_metadata(move->dst.field)) {
+        error = tun_metadata_len(move->dst.field, tun_table,
+                &move->dst.n_bytes);
+        if (error) {
+            return error;
+        }
+    }
+
     if (!is_all_zeros(b.data, b.size)) {
         return OFPERR_NXBRC_MUST_BE_ZERO;
     }
@@ -2353,8 +2377,8 @@ encode_REG_MOVE(const struct ofpact_reg_move *move,
         copy->src_offset = htons(move->src.ofs);
         copy->dst_offset = htons(move->dst.ofs);
         out->size = out->size - sizeof copy->pad2;
-        nx_put_header(out, move->src.field->id, ofp_version, false);
-        nx_put_header(out, move->dst.field->id, ofp_version, false);
+        nx_put_mf_subfield_header(out, &move->src, ofp_version, false);
+        nx_put_mf_subfield_header(out, &move->dst, ofp_version, false);
     } else if (ofp_version == OFP13_VERSION
                && move->ofpact.raw == ONFACT_RAW13_COPY_FIELD) {
         struct onf_action_copy_field *copy = put_ONFACT13_COPY_FIELD(out);
@@ -2362,15 +2386,15 @@ encode_REG_MOVE(const struct ofpact_reg_move *move,
         copy->src_offset = htons(move->src.ofs);
         copy->dst_offset = htons(move->dst.ofs);
         out->size = out->size - sizeof copy->pad3;
-        nx_put_header(out, move->src.field->id, ofp_version, false);
-        nx_put_header(out, move->dst.field->id, ofp_version, false);
+        nx_put_mf_subfield_header(out, &move->src, ofp_version, false);
+        nx_put_mf_subfield_header(out, &move->dst, ofp_version, false);
     } else {
         struct nx_action_reg_move *narm = put_NXAST_REG_MOVE(out);
         narm->n_bits = htons(move->dst.n_bits);
         narm->src_ofs = htons(move->src.ofs);
         narm->dst_ofs = htons(move->dst.ofs);
-        nx_put_header(out, move->src.field->id, 0, false);
-        nx_put_header(out, move->dst.field->id, 0, false);
+        nx_put_mf_subfield_header(out, &move->src, 0, false);
+        nx_put_mf_subfield_header(out, &move->dst, 0, false);
     }
     pad_ofpat(out, start_ofs);
 }
@@ -4808,7 +4832,7 @@ decode_NXAST_RAW_CLONE(const struct ext_action_header *eah,
     error = ofpacts_pull_openflow_actions__(&openflow, openflow.size,
                                             ofp_version,
                                             1u << OVSINST_OFPIT11_APPLY_ACTIONS,
-                                            out, 0);
+                                            out, 0, NULL);
     clone = ofpbuf_push_uninit(out, sizeof *clone);
     out->header = &clone->ofpact;
     ofpact_finish_CLONE(out, &clone);
@@ -5302,7 +5326,7 @@ decode_NXAST_RAW_CT(const struct nx_action_conntrack *nac,
     error = ofpacts_pull_openflow_actions__(&openflow, openflow.size,
                                             ofp_version,
                                             1u << OVSINST_OFPIT11_APPLY_ACTIONS,
-                                            out, OFPACT_CT);
+                                            out, OFPACT_CT, NULL);
     if (error) {
         goto out;
     }
@@ -6157,7 +6181,8 @@ log_bad_action(const struct ofp_action_header *actions, size_t actions_len,
 
 static enum ofperr
 ofpacts_decode(const void *actions, size_t actions_len,
-               enum ofp_version ofp_version, struct ofpbuf *ofpacts)
+               enum ofp_version ofp_version, const struct tun_table *tun_table,
+               struct ofpbuf *ofpacts)
 {
     struct ofpbuf openflow = ofpbuf_const_initializer(actions, actions_len);
     while (openflow.size) {
@@ -6168,7 +6193,7 @@ ofpacts_decode(const void *actions, size_t actions_len,
 
         error = ofpact_pull_raw(&openflow, ofp_version, &raw, &arg);
         if (!error) {
-            error = ofpact_decode(action, raw, ofp_version, arg, ofpacts);
+            error = ofpact_decode(action, raw, ofp_version, arg, tun_table, ofpacts);
         }
 
         if (error) {
@@ -6185,7 +6210,8 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
                                 enum ofp_version version,
                                 uint32_t allowed_ovsinsts,
                                 struct ofpbuf *ofpacts,
-                                enum ofpact_type outer_action)
+                                enum ofpact_type outer_action,
+                                const struct tun_table *tun_table)
 {
     const struct ofp_action_header *actions;
     size_t orig_size = ofpacts->size;
@@ -6205,7 +6231,7 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
         return OFPERR_OFPBRC_BAD_LEN;
     }
 
-    error = ofpacts_decode(actions, actions_len, version, ofpacts);
+    error = ofpacts_decode(actions, actions_len, version, tun_table, ofpacts);
     if (error) {
         ofpacts->size = orig_size;
         return error;
@@ -6244,7 +6270,7 @@ ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
 {
     return ofpacts_pull_openflow_actions__(openflow, actions_len, version,
                                            1u << OVSINST_OFPIT11_APPLY_ACTIONS,
-                                           ofpacts, 0);
+                                           ofpacts, 0, NULL);
 }
 
 /* OpenFlow 1.1 actions. */
@@ -6481,13 +6507,14 @@ ofpacts_execute_action_set(struct ofpbuf *action_list,
 static enum ofperr
 ofpacts_decode_for_action_set(const struct ofp_action_header *in,
                               size_t n_in, enum ofp_version version,
+                              const struct tun_table *tun_table,
                               struct ofpbuf *out)
 {
     enum ofperr error;
     struct ofpact *a;
     size_t start = out->size;
 
-    error = ofpacts_decode(in, n_in, version, out);
+    error = ofpacts_decode(in, n_in, version, tun_table, out);
 
     if (error) {
         return error;
@@ -6784,6 +6811,7 @@ enum ofperr
 ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
                                    unsigned int instructions_len,
                                    enum ofp_version version,
+                                   const struct tun_table *tun_table,
                                    struct ofpbuf *ofpacts)
 {
     const struct ofp11_instruction *instructions;
@@ -6795,7 +6823,7 @@ ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
         return ofpacts_pull_openflow_actions__(openflow, instructions_len,
                                                version,
                                                (1u << N_OVS_INSTRUCTIONS) - 1,
-                                               ofpacts, 0);
+                                               ofpacts, 0, tun_table);
     }
 
     if (instructions_len % OFP11_INSTRUCTION_ALIGN != 0) {
@@ -6838,7 +6866,8 @@ ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
 
         get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS],
                                      &actions, &actions_len);
-        error = ofpacts_decode(actions, actions_len, version, ofpacts);
+        error = ofpacts_decode(actions, actions_len, version, tun_table,
+                               ofpacts);
         if (error) {
             goto exit;
         }
@@ -6858,7 +6887,7 @@ ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
         get_actions_from_instruction(insts[OVSINST_OFPIT11_WRITE_ACTIONS],
                                      &actions, &actions_len);
         error = ofpacts_decode_for_action_set(actions, actions_len,
-                                              version, ofpacts);
+                                              version, tun_table, ofpacts);
         if (error) {
             goto exit;
         }
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 156d8d2..33c824e 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1723,8 +1723,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
         return OFPERR_OFPFMFC_BAD_COMMAND;
     }
 
-    error = ofpacts_pull_openflow_instructions(&b, b.size,
-                                               oh->version, ofpacts);
+    error = ofpacts_pull_openflow_instructions(&b, b.size, oh->version,
+                                               tun_table, ofpacts);
     if (error) {
         return error;
     }
@@ -2996,7 +2996,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
     }
 
     if (ofpacts_pull_openflow_instructions(msg, instructions_len, oh->version,
-                                           ofpacts)) {
+                                           NULL, ofpacts)) {
         VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
         return EINVAL;
     }
diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
index 92643b3..a53b131 100644
--- a/lib/tun-metadata.c
+++ b/lib/tun-metadata.c
@@ -32,7 +32,7 @@
 
 struct tun_meta_entry {
     struct hmap_node node;      /* In struct tun_table's key_hmap. */
-    uint32_t key;               /* (class << 16) | type. */
+    uint32_t key;               /* (class << 8) | type. */
     struct tun_metadata_loc loc;
     bool valid;                 /* True if allocated to a class and type. */
 };
@@ -79,6 +79,27 @@ tun_key_type(uint32_t key)
     return key & 0xff;
 }
 
+/* Derives the length of a tun_metadata field 'mf' from tunnel TLV mapping
+ * table 'tun_table'. If 'tun_table' is NULL, set 'n_bytes' to be the default
+ * size of the tun_metadata field. */
+enum ofperr
+tun_metadata_len(const struct mf_field *mf, const struct tun_table *tun_table,
+                 unsigned int *n_bytes) {
+    unsigned int idx = mf->id - MFF_TUN_METADATA0;
+
+    if (!tun_table) {
+        *n_bytes = mf->n_bytes;
+    } else {
+        if (!tun_table->entries[idx].valid) {
+            return OFPERR_NXFMFC_INVALID_TUN_METADATA_ID;
+        } else {
+            *n_bytes = tun_table->entries[idx].loc.len;
+        }
+    }
+
+    return 0;
+}
+
 /* Returns a newly allocated tun_table.  If 'old_map' is nonnull then the new
  * tun_table is a deep copy of the old one. */
 struct tun_table *
diff --git a/lib/tun-metadata.h b/lib/tun-metadata.h
index 7dad950..ab1c798 100644
--- a/lib/tun-metadata.h
+++ b/lib/tun-metadata.h
@@ -33,6 +33,10 @@ struct ofputil_tlv_table_mod;
 struct ofputil_tlv_table_reply;
 struct tun_table;
 
+enum ofperr tun_metadata_len(const struct mf_field *mf,
+                             const struct tun_table *tun_table,
+                             unsigned int *n_bytes);
+
 struct tun_table *tun_metadata_alloc(const struct tun_table *old_map);
 void tun_metadata_free(struct tun_table *);
 void tun_metadata_postpone_free(struct tun_table *);
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 5399985..7d980c8 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -3006,7 +3006,7 @@ expr_resolve_field(const struct expr_field *f)
     }
 
     int n_bits = symbol->width ? f->n_bits : symbol->field->n_bits;
-    return (struct mf_subfield) { symbol->field, ofs, n_bits };
+    return (struct mf_subfield) { symbol->field, ofs, n_bits, 0 };
 }
 
 static struct expr *
diff --git a/tests/ofproto.at b/tests/ofproto.at
index ad8df28..0b28c1a 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -5685,3 +5685,29 @@ NXST_FLOW reply:
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto - flow mod with tunnel metadata])
+AT_KEYWORDS([ofp-actions])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-ofctl add-tlv-map br0 "{class=0xffff,type=0,len=4}->tun_metadata0"])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=1,actions=move:NXM_NX_TUN_METADATA0[[0..31]]->NXM_NX_REG0[[]]"])
+
+AT_CHECK([ovs-ofctl dump-flows br0 -mmmm], [0], [stdout])
+dnl Ignore the first 0x50 bytes of hex dump from the reply msg since the
+dnl NXM header that describes the tunnel metadata starts at offset 0x50.
+AT_CHECK([sed -e 's/duration=[[0-9.]]*s/duration=?s/' -e '/^000000[[0-4]]0 / d' stdout], [0], [dnl
+NXST_FLOW reply (xid=0x4):
+ cookie=0x0, duration=?s, table=0, n_packets=0, n_bytes=0, idle_age=0, in_port=1 actions=move:NXM_NX_TUN_METADATA0[[0..31]]->NXM_NX_REG0[[]]
+00000050  ff ff 00 18 00 00 23 20-00 06 00 20 00 00 00 00 |......# ... ....|
+00000060  00 01 50 04 00 01 00 04-                        |..P.....        |
+])
+
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=2,actions=move:NXM_NX_TUN_METADATA1[[0..31]]->NXM_NX_REG0[[]]"], [1], [], [stderr])
+AT_CHECK([strip_xids < stderr | sed '/truncated/,$d'], [0], [dnl
+OFPT_ERROR: NXFMFC_INVALID_TUN_METADATA_ID
+OFPT_FLOW_MOD:
+])
+
+OVS_VSWITCHD_STOP(["/NXFMFC_INVALID_TUN_METADATA_ID/d"])
+AT_CLEANUP
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index e4935d5..1081c8f 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -3868,10 +3868,14 @@ ofctl_parse_actions__(const char *version_s, bool instructions)
         /* Convert to ofpacts. */
         ofpbuf_init(&ofpacts, 0);
         size = of_in.size;
-        error = (instructions
-                 ? ofpacts_pull_openflow_instructions
-                 : ofpacts_pull_openflow_actions)(
-                     &of_in, of_in.size, version, &ofpacts);
+        if (instructions) {
+            error = ofpacts_pull_openflow_instructions(&of_in, of_in.size,
+                                                       version, NULL,
+                                                        &ofpacts);
+        } else {
+            error = ofpacts_pull_openflow_actions(&of_in, of_in.size, version,
+                                                  &ofpacts);
+        }
         if (!error && instructions) {
             /* Verify actions, enforce consistency. */
             enum ofputil_protocol protocol;
-- 
2.7.4



More information about the dev mailing list