Currently, a controller may potentially trigger a segmentation fault if it
accidentally removes a TLV mapping that is still used by an active flow.
To resolve this issue, in this patch, we maintain reference counting for each
dynamically allocated variable length mf_fields, so that vswitchd can use this
information to properly remove a TLV mapping, and to return an error if the
controller tries to remove a TLV mapping that is still used by any active flow.
To keep track of the usage of tun_metadata for each flow, two 'uint64_t'
bitmaps are introduce for the flow match and flow action respectively. We use
'uint64_t' as a bitmap since the 64 geneve TLV tunnel metadata are the only
available variable length mf_fields for now. We shall adopt general bitmap when
more variable length mf_fields are introduced. The bitmaps are configured
during the flow decoding process, and vswitchd use these bitmaps to increase or
decrease the ref counting when the flow is created or deleted.
VMWare-BZ: #1768370
Fixes: 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs.")
Suggested-by: Jarno Rajahalme <ja...@ovn.org>
Suggested-by: Joe Stringer <j...@ovn.org>
Signed-off-by: Yi-Hung Wei <yihung....@gmail.com>
---
build-aux/extract-ofp-actions | 9 +-
include/openvswitch/ofp-actions.h | 3 +-
include/openvswitch/ofp-errors.h | 4 +
include/openvswitch/ofp-util.h | 1 +
lib/learn.c | 6 ++
lib/meta-flow.c | 196 +++++++++++++++++++++++++++++++-------
lib/ofp-actions.c | 156 ++++++++++++++++++------------
lib/ofp-util.c | 21 ++--
lib/vl-mff-map.h | 10 +-
ofproto/ofproto-provider.h | 4 +
ofproto/ofproto.c | 39 ++++++--
ovn/controller/pinctrl.c | 6 +-
tests/tunnel.at | 76 ++++++++++++++-
utilities/ovs-ofctl.c | 2 +-
14 files changed, 404 insertions(+), 129 deletions(-)
diff --git a/build-aux/extract-ofp-actions b/build-aux/extract-ofp-actions
index 184447b99422..0062ab881dd5 100755
--- a/build-aux/extract-ofp-actions
+++ b/build-aux/extract-ofp-actions
@@ -322,7 +322,8 @@ def extract_ofp_actions(fn, definitions):
static enum ofperr
ofpact_decode(const struct ofp_action_header *a, enum ofp_raw_action_type raw,
enum ofp_version version, uint64_t arg,
- const struct vl_mff_map *vl_mff_map, struct ofpbuf *out)
+ const struct vl_mff_map *vl_mff_map,
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
switch (raw) {\
"""
@@ -343,7 +344,7 @@ ofpact_decode(const struct ofp_action_header *a, enum
ofp_raw_action_type raw,
else:
arg = "arg"
if arg_vl_mff_map:
- print " return decode_%s(%s, version, vl_mff_map,
out);" % (enum, arg)
+ print " return decode_%s(%s, version, vl_mff_map,
tlv_bitmap, out);" % (enum, arg)
else:
print " return decode_%s(%s, version, out);" %
(enum, arg)
print
@@ -365,7 +366,7 @@ ofpact_decode(const struct ofp_action_header *a, enum
ofp_raw_action_type raw,
else:
prototype += "%s, enum ofp_version, " % base_argtype
if arg_vl_mff_map:
- prototype += 'const struct vl_mff_map *, '
+ prototype += 'const struct vl_mff_map *, uint64_t *, '
prototype += "struct ofpbuf *);"
print prototype
@@ -374,7 +375,7 @@ static enum ofperr ofpact_decode(const struct
ofp_action_header *,
enum ofp_raw_action_type raw,
enum ofp_version version,
uint64_t arg, const struct vl_mff_map
*vl_mff_map,
- struct ofpbuf *out);
+ uint64_t *tlv_bitmap, struct ofpbuf *out);
"""
if __name__ == '__main__':
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index 88f573dcd74e..808715e9b1a4 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -946,13 +946,14 @@ enum ofperr ofpacts_pull_openflow_actions(struct ofpbuf
*openflow,
unsigned int actions_len,
enum ofp_version version,
const struct vl_mff_map *,
+ uint64_t *,
struct ofpbuf *ofpacts);
enum ofperr
ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
unsigned int instructions_len,
enum ofp_version version,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts);
+ uint64_t *, 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 81825817e843..a5bba8619bcb 100644
--- a/include/openvswitch/ofp-errors.h
+++ b/include/openvswitch/ofp-errors.h
@@ -772,6 +772,10 @@ enum ofperr {
* to be mapped is the same as one assigned to a different field. */
OFPERR_NXTTMFC_DUP_ENTRY,
+ /* NX1.0-1.1(1,537), NX1.2+(38). Attempted to delete a TLV mapping that
+ * is used by any active flow. */
+ OFPERR_NXTTMFC_INVALID_TLV_DEL,
+
/* ## ---------- ## */
/* ## NXT_RESUME ## */
/* ## ---------- ## */
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index e73a942a3e15..7cb9e7fd32bd 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -326,6 +326,7 @@ struct ofputil_flow_mod {
uint16_t importance; /* Eviction precedence. */
struct ofpact *ofpacts; /* Series of "struct ofpact"s. */
size_t ofpacts_len; /* Length of ofpacts, in bytes. */
+ uint64_t ofpacts_tlv_bitmap; /* 1-bit for each present TLV in 'ofpacts'. */
};
enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod *,
diff --git a/lib/learn.c b/lib/learn.c
index ce52c35f297a..0e86f1629d74 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -29,6 +29,7 @@
#include "openvswitch/ofp-errors.h"
#include "openvswitch/ofp-util.h"
#include "openvswitch/ofpbuf.h"
+#include "vl-mff-map.h"
#include "unaligned.h"
@@ -107,6 +108,8 @@ learn_execute(const struct ofpact_learn *learn, const
struct flow *flow,
fm->importance = 0;
fm->buffer_id = UINT32_MAX;
fm->out_port = OFPP_NONE;
+ fm->match.flow.tunnel.metadata.present.map = 0;
+ fm->ofpacts_tlv_bitmap = 0;
fm->flags = 0;
if (learn->flags & NX_LEARN_F_SEND_FLOW_REM) {
fm->flags |= OFPUTIL_FF_SEND_FLOW_REM;
@@ -136,6 +139,8 @@ learn_execute(const struct ofpact_learn *learn, const
struct flow *flow,
switch (spec->dst_type) {
case NX_LEARN_DST_MATCH:
mf_write_subfield(&spec->dst, &value, &fm->match);
+ mf_vl_mff_set_tlv_bitmap(
+ spec->dst.field, &fm->match.flow.tunnel.metadata.present.map);
break;
case NX_LEARN_DST_LOAD:
@@ -145,6 +150,7 @@ learn_execute(const struct ofpact_learn *learn, const
struct flow *flow,
spec->n_bits);
bitwise_one(ofpact_set_field_mask(sf), spec->dst.field->n_bytes,
spec->dst.ofs, spec->n_bits);
+ mf_vl_mff_set_tlv_bitmap(spec->dst.field, &fm->ofpacts_tlv_bitmap);
break;
case NX_LEARN_DST_OUTPUT:
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 40704e628aaa..7807b3055f5b 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -27,6 +27,8 @@
#include "openvswitch/dynamic-string.h"
#include "nx-match.h"
#include "openvswitch/ofp-util.h"
+#include "ofproto/ofproto-provider.h"
+#include "ovs-atomic.h"
#include "ovs-rcu.h"
#include "ovs-thread.h"
#include "packets.h"
@@ -2646,6 +2648,7 @@ field_array_set(enum mf_field_id id, const union mf_value
*value,
* struct vl_mff_map.*/
struct vl_mf_field {
struct mf_field mf;
+ struct ovs_refcount ref_cnt;
struct cmap_node cmap_node; /* In ofproto->vl_mff_map->cmap. */
};
@@ -2655,17 +2658,25 @@ mf_field_hash(uint32_t key)
return hash_int(key, 0);
}
-void
-mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
+enum ofperr
+mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map, bool clear)
OVS_REQUIRES(vl_mff_map->mutex)
{
struct vl_mf_field *vmf;
CMAP_FOR_EACH (vmf, cmap_node, &vl_mff_map->cmap) {
- cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
- mf_field_hash(vmf->mf.id));
- ovsrcu_postpone(free, vmf);
+ if (clear) {
+ cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
+ mf_field_hash(vmf->mf.id));
+ ovsrcu_postpone(free, vmf);
+ } else {
+ if (ovs_refcount_read(&vmf->ref_cnt) != 1) {
+ return OFPERR_NXTTMFC_INVALID_TLV_DEL;
+ }
+ }
}
+
+ return 0;
}
static struct vl_mf_field *
@@ -2697,53 +2708,100 @@ mf_get_vl_mff(const struct mf_field *mff,
return NULL;
}
-/* Updates the tun_metadata mf_field in 'vl_mff_map' according to 'ttm'.
- * This function is supposed to be invoked after tun_metadata_table_mod(). */
-enum ofperr
-mf_vl_mff_map_mod_from_tun_metadata(struct vl_mff_map *vl_mff_map,
- const struct ofputil_tlv_table_mod *ttm)
+static enum ofperr
+mf_vl_mff_map_del(struct vl_mff_map *vl_mff_map,
+ const struct ofputil_tlv_table_mod *ttm, bool del)
OVS_REQUIRES(vl_mff_map->mutex)
{
struct ofputil_tlv_map *tlv_map;
-
- if (ttm->command == NXTTMC_CLEAR) {
- mf_vl_mff_map_clear(vl_mff_map);
- return 0;
- }
+ struct vl_mf_field *vmf;
+ unsigned int idx;
LIST_FOR_EACH (tlv_map, list_node, &ttm->mappings) {
- unsigned int idx = MFF_TUN_METADATA0 + tlv_map->index;
- struct vl_mf_field *vmf;
-
+ idx = MFF_TUN_METADATA0 + tlv_map->index;
if (idx >= MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS) {
return OFPERR_NXTTMFC_BAD_FIELD_IDX;
}
- switch (ttm->command) {
- case NXTTMC_ADD:
- vmf = xmalloc(sizeof *vmf);
- vmf->mf = mf_fields[idx];
- vmf->mf.n_bytes = tlv_map->option_len;
- vmf->mf.n_bits = tlv_map->option_len * 8;
- vmf->mf.mapped = true;
-
- cmap_insert(&vl_mff_map->cmap, &vmf->cmap_node,
- mf_field_hash(idx));
- break;
-
- case NXTTMC_DELETE:
- vmf = mf_get_vl_mff__(idx, vl_mff_map);
- if (vmf) {
+ vmf = mf_get_vl_mff__(idx, vl_mff_map);
+ if (vmf) {
+ if (del == true) {
cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
mf_field_hash(idx));
ovsrcu_postpone(free, vmf);
+ } else {
+ if (ovs_refcount_read(&vmf->ref_cnt) != 1) {
+ return OFPERR_NXTTMFC_INVALID_TLV_DEL;
+ }
}
- break;
+ }
+ }
+
+ return 0;
+}
+
+static enum ofperr
+mf_vl_mff_map_add(struct vl_mff_map *vl_mff_map,
+ const struct ofputil_tlv_table_mod *ttm)
+ OVS_REQUIRES(vl_mff_map->mutex)
+{
+ struct ofputil_tlv_map *tlv_map;
+ struct vl_mf_field *vmf;
+ unsigned int idx;
+
+ LIST_FOR_EACH (tlv_map, list_node, &ttm->mappings) {
+ idx = MFF_TUN_METADATA0 + tlv_map->index;
+ if (idx >= MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS) {
+ return OFPERR_NXTTMFC_BAD_FIELD_IDX;
+ }
+
+ vmf = xmalloc(sizeof *vmf);
+ vmf->mf = mf_fields[idx];
+ vmf->mf.n_bytes = tlv_map->option_len;
+ vmf->mf.n_bits = tlv_map->option_len * 8;
+ vmf->mf.mapped = true;
+ ovs_refcount_init(&vmf->ref_cnt);
+
+ cmap_insert(&vl_mff_map->cmap, &vmf->cmap_node,
+ mf_field_hash(idx));
+ }
+
+ return 0;
+}
- case NXTTMC_CLEAR:
- default:
- OVS_NOT_REACHED();
+/* Updates the tun_metadata mf_field in 'vl_mff_map' according to 'ttm'.
+ * This function is supposed to be invoked after tun_metadata_table_mod().
+ * Returns OFPERR_NXTTMFC_BAD_FIELD_IDX, if the index for the vl_mf_field is
+ * invalid.
+ * Returns OFPERR_NXTTMFC_INVALID_TLV_DEL, if 'ttm' tries to delete an
+ * vl_mf_field that is still used by any active flow.*/
+enum ofperr
+mf_vl_mff_map_mod_from_tun_metadata(struct vl_mff_map *vl_mff_map,
+ const struct ofputil_tlv_table_mod *ttm)
+ OVS_REQUIRES(vl_mff_map->mutex)
+{
+ enum ofperr error;
+
+ switch (ttm->command) {
+ case NXTTMC_ADD:
+ return mf_vl_mff_map_add(vl_mff_map, ttm);
+
+ case NXTTMC_DELETE:
+ error = mf_vl_mff_map_del(vl_mff_map, ttm, false);
+ if (error) {
+ return error;
}
+ return mf_vl_mff_map_del(vl_mff_map, ttm, true);
+
+ case NXTTMC_CLEAR:
+ error = mf_vl_mff_map_clear(vl_mff_map, false);
+ if (error) {
+ return error;
+ }
+ return mf_vl_mff_map_clear(vl_mff_map, true);
+
+ default:
+ OVS_NOT_REACHED();
}
return 0;
@@ -2756,3 +2814,67 @@ mf_vl_mff_invalid(const struct mf_field *mff, const
struct vl_mff_map *map)
{
return map && mff && mff->variable_len && !mff->mapped;
}
+
+void
+mf_vl_mff_set_tlv_bitmap(const struct mf_field *mff, uint64_t *tlv_bitmap)
+{
+ if (mff && mff->mapped) {
+ ULLONG_SET1(*tlv_bitmap, mff->id - MFF_TUN_METADATA0);
+ }
+}
+
+/* If the 'mff' is a valid variable length mf_field, udpates the 'tlv_bitmap'.
+ * Returns OFPERR_NXFMFC_INVALID_TLV_FIELD if the variable length mf_field
+ * is invalid. */
+enum ofperr
+mf_check_vl_mff(const struct mf_field *mff, const struct vl_mff_map *map,
+ uint64_t *tlv_bitmap)
+{
+ if (mf_vl_mff_invalid(mff, map)) {
+ return OFPERR_NXFMFC_INVALID_TLV_FIELD;
+ }
+ mf_vl_mff_set_tlv_bitmap(mff, tlv_bitmap);
+
+ return 0;
+}
+
+static void
+mf_vl_mff_ref_cnt_mod(const struct vl_mff_map *map, uint64_t tlv_bitmap,
+ bool ref)
+{
+ struct vl_mf_field *vmf;
+ int i;
+
+ if (map) {
+ ULLONG_FOR_EACH_1 (i, tlv_bitmap) {
+ vmf = mf_get_vl_mff__(i + MFF_TUN_METADATA0, map);
+ if (vmf) {
+ if (ref) {
+ ovs_refcount_ref(&vmf->ref_cnt);
+ } else {
+ ovs_refcount_unref(&vmf->ref_cnt);
+ }
+ } else {
+ VLOG_WARN("Invalid TLV index %d.", i);
+ }
+ }
+ }
+}
+
+void
+mf_vl_mff_add_ref_cnt_from_rule(const struct rule *rule)
+{
+ mf_vl_mff_ref_cnt_mod(&rule->ofproto->vl_mff_map, rule->match_tlv_bitmap,
+ true);
+ mf_vl_mff_ref_cnt_mod(&rule->ofproto->vl_mff_map, rule->ofpacts_tlv_bitmap,
+ true);
+}
+
+void
+mf_vl_mff_remove_ref_cnt_from_rule(const struct rule *rule)
+{
+ mf_vl_mff_ref_cnt_mod(&rule->ofproto->vl_mff_map, rule->match_tlv_bitmap,
+ false);
+ mf_vl_mff_ref_cnt_mod(&rule->ofproto->vl_mff_map, rule->ofpacts_tlv_bitmap,
+ false);
+}
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ce80f57e8df3..584e5cd4e847 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -405,7 +405,7 @@ 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,
- const struct vl_mff_map *vl_mff_map);
+ const struct vl_mff_map *vl_mff_map, uint64_t *ofpacts_tlv_bitmap);
static char * OVS_WARN_UNUSED_RESULT ofpacts_parse_copy(
const char *s_, struct ofpbuf *ofpacts,
enum ofputil_protocol *usable_protocols,
@@ -1117,13 +1117,20 @@ struct nx_action_output_reg2 {
};
OFP_ASSERT(sizeof(struct nx_action_output_reg2) == 24);
+#define CHECK_VL_MFF(FIELD, VL_MFF_MAP, BITMAP, ERR) \
+ERR = mf_check_vl_mff(FIELD, VL_MFF_MAP, BITMAP); \
+if (ERR) { \
+ return ERR; \
+}
+
static enum ofperr
decode_NXAST_RAW_OUTPUT_REG(const struct nx_action_output_reg *naor,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
struct ofpact_output_reg *output_reg;
+ enum ofperr error;
if (!is_all_zeros(naor->zero, sizeof naor->zero)) {
return OFPERR_OFPBAC_BAD_ARGUMENT;
@@ -1136,9 +1143,7 @@ decode_NXAST_RAW_OUTPUT_REG(const struct
nx_action_output_reg *naor,
output_reg->src.n_bits = nxm_decode_n_bits(naor->ofs_nbits);
output_reg->max_len = ntohs(naor->max_len);
- if (mf_vl_mff_invalid(output_reg->src.field, vl_mff_map)) {
- return OFPERR_NXFMFC_INVALID_TLV_FIELD;
- }
+ CHECK_VL_MFF(output_reg->src.field, vl_mff_map, tlv_bitmap, error);
return mf_check_src(&output_reg->src, NULL);
}
@@ -1147,7 +1152,7 @@ static enum ofperr
decode_NXAST_RAW_OUTPUT_REG2(const struct nx_action_output_reg2 *naor,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
struct ofpact_output_reg *output_reg;
output_reg = ofpact_put_OUTPUT_REG(out);
@@ -1164,6 +1169,8 @@ decode_NXAST_RAW_OUTPUT_REG2(const struct
nx_action_output_reg2 *naor,
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(output_reg->src.field, tlv_bitmap);
+
if (!is_all_zeros(b.data, b.size)) {
return OFPERR_NXBRC_MUST_BE_ZERO;
}
@@ -1286,7 +1293,8 @@ OFP_ASSERT(sizeof(struct nx_action_bundle) == 32);
static enum ofperr
decode_bundle(bool load, const struct nx_action_bundle *nab,
- const struct vl_mff_map *vl_mff_map, struct ofpbuf *ofpacts)
+ const struct vl_mff_map *vl_mff_map, uint64_t *tlv_bitmap,
+ struct ofpbuf *ofpacts)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
struct ofpact_bundle *bundle;
@@ -1326,9 +1334,7 @@ decode_bundle(bool load, const struct nx_action_bundle
*nab,
bundle->dst.field = mf_from_nxm_header(ntohl(nab->dst), vl_mff_map);
bundle->dst.ofs = nxm_decode_ofs(nab->ofs_nbits);
bundle->dst.n_bits = nxm_decode_n_bits(nab->ofs_nbits);
- if (mf_vl_mff_invalid(bundle->dst.field, vl_mff_map)) {
- return OFPERR_NXFMFC_INVALID_TLV_FIELD;
- }
+ CHECK_VL_MFF(bundle->dst.field, vl_mff_map, tlv_bitmap, error);
if (bundle->dst.n_bits < 16) {
VLOG_WARN_RL(&rl, "bundle_load action requires at least 16 bit "
@@ -1369,16 +1375,16 @@ decode_NXAST_RAW_BUNDLE(const struct nx_action_bundle
*nab,
enum ofp_version ofp_version OVS_UNUSED,
struct ofpbuf *out)
{
- return decode_bundle(false, nab, NULL, out);
+ return decode_bundle(false, nab, NULL, NULL, out);
}
static enum ofperr
decode_NXAST_RAW_BUNDLE_LOAD(const struct nx_action_bundle *nab,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
- return decode_bundle(true, nab, vl_mff_map, out);
+ return decode_bundle(true, nab, vl_mff_map, tlv_bitmap, out);
}
static void
@@ -2282,7 +2288,7 @@ static enum ofperr
decode_copy_field__(ovs_be16 src_offset, ovs_be16 dst_offset, ovs_be16 n_bits,
const void *action, ovs_be16 action_len, size_t oxm_offset,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts);
move->ofpact.raw = ONFACT_RAW13_COPY_FIELD;
@@ -2298,10 +2304,13 @@ decode_copy_field__(ovs_be16 src_offset, ovs_be16
dst_offset, ovs_be16 n_bits,
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(move->src.field, tlv_bitmap);
+
error = nx_pull_header(&b, vl_mff_map, &move->dst.field, NULL);
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(move->dst.field, tlv_bitmap);
if (!is_all_zeros(b.data, b.size)) {
return OFPERR_NXBRC_MUST_BE_ZERO;
@@ -2314,31 +2323,31 @@ static enum ofperr
decode_OFPAT_RAW15_COPY_FIELD(const struct ofp15_action_copy_field *oacf,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
return decode_copy_field__(oacf->src_offset, oacf->dst_offset,
oacf->n_bits, oacf, oacf->len,
OBJECT_OFFSETOF(oacf, pad2), vl_mff_map,
- ofpacts);
+ tlv_bitmap, ofpacts);
}
static enum ofperr
decode_ONFACT_RAW13_COPY_FIELD(const struct onf_action_copy_field *oacf,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
return decode_copy_field__(oacf->src_offset, oacf->dst_offset,
oacf->n_bits, oacf, oacf->len,
OBJECT_OFFSETOF(oacf, pad3), vl_mff_map,
- ofpacts);
+ tlv_bitmap, ofpacts);
}
static enum ofperr
decode_NXAST_RAW_REG_MOVE(const struct nx_action_reg_move *narm,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts);
move->ofpact.raw = NXAST_RAW_REG_MOVE;
@@ -2354,10 +2363,13 @@ decode_NXAST_RAW_REG_MOVE(const struct
nx_action_reg_move *narm,
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(move->src.field, tlv_bitmap);
+
error = nx_pull_header(&b, vl_mff_map, &move->dst.field, NULL);
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(move->dst.field, tlv_bitmap);
if (!is_all_zeros(b.data, b.size)) {
return OFPERR_NXBRC_MUST_BE_ZERO;
@@ -2481,7 +2493,7 @@ OFP_ASSERT(sizeof(struct nx_action_reg_load) == 24);
static enum ofperr
decode_ofpat_set_field(const struct ofp12_action_set_field *oasf,
bool may_mask, const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpbuf b = ofpbuf_const_initializer(oasf, ntohs(oasf->len));
ofpbuf_pull(&b, OBJECT_OFFSETOF(oasf, pad));
@@ -2495,6 +2507,8 @@ decode_ofpat_set_field(const struct
ofp12_action_set_field *oasf,
? OFPERR_OFPBAC_BAD_SET_MASK
: error);
}
+ mf_vl_mff_set_tlv_bitmap(field, tlv_bitmap);
+
if (!may_mask) {
memset(&mask, 0xff, field->n_bytes);
}
@@ -2540,25 +2554,26 @@ static enum ofperr
decode_OFPAT_RAW12_SET_FIELD(const struct ofp12_action_set_field *oasf,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
- return decode_ofpat_set_field(oasf, false, vl_mff_map, ofpacts);
+ return decode_ofpat_set_field(oasf, false, vl_mff_map, tlv_bitmap,
+ ofpacts);
}
static enum ofperr
decode_OFPAT_RAW15_SET_FIELD(const struct ofp12_action_set_field *oasf,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
- return decode_ofpat_set_field(oasf, true, vl_mff_map, ofpacts);
+ return decode_ofpat_set_field(oasf, true, vl_mff_map, tlv_bitmap, ofpacts);
}
static enum ofperr
decode_NXAST_RAW_REG_LOAD(const struct nx_action_reg_load *narl,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
struct mf_subfield dst;
enum ofperr error;
@@ -2566,9 +2581,7 @@ decode_NXAST_RAW_REG_LOAD(const struct nx_action_reg_load
*narl,
dst.field = mf_from_nxm_header(ntohl(narl->dst), vl_mff_map);
dst.ofs = nxm_decode_ofs(narl->ofs_nbits);
dst.n_bits = nxm_decode_n_bits(narl->ofs_nbits);
- if (mf_vl_mff_invalid(dst.field, vl_mff_map)) {
- return OFPERR_NXFMFC_INVALID_TLV_FIELD;
- }
+ CHECK_VL_MFF(dst.field, vl_mff_map, tlv_bitmap, error);
error = mf_check_dst(&dst, NULL);
if (error) {
@@ -2596,7 +2609,7 @@ static enum ofperr
decode_NXAST_RAW_REG_LOAD2(const struct ext_action_header *eah,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
struct ofpbuf b = ofpbuf_const_initializer(eah, ntohs(eah->len));
ofpbuf_pull(&b, OBJECT_OFFSETOF(eah, pad));
@@ -2607,6 +2620,8 @@ decode_NXAST_RAW_REG_LOAD2(const struct ext_action_header
*eah,
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(field, tlv_bitmap);
+
if (!is_all_zeros(b.data, b.size)) {
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
@@ -3109,7 +3124,7 @@ OFP_ASSERT(sizeof(struct nx_action_stack) == 24);
static enum ofperr
decode_stack_action(const struct nx_action_stack *nasp,
- const struct vl_mff_map *vl_mff_map,
+ const struct vl_mff_map *vl_mff_map, uint64_t *tlv_bitmap,
struct ofpact_stack *stack_action)
{
stack_action->subfield.ofs = ntohs(nasp->offset);
@@ -3121,6 +3136,8 @@ decode_stack_action(const struct nx_action_stack *nasp,
if (error) {
return error;
}
+ mf_vl_mff_set_tlv_bitmap(stack_action->subfield.field, tlv_bitmap);
+
stack_action->subfield.n_bits = ntohs(*(const ovs_be16 *) b.data);
ofpbuf_pull(&b, 2);
if (!is_all_zeros(b.data, b.size)) {
@@ -3134,10 +3151,11 @@ static enum ofperr
decode_NXAST_RAW_STACK_PUSH(const struct nx_action_stack *nasp,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpact_stack *push = ofpact_put_STACK_PUSH(ofpacts);
- enum ofperr error = decode_stack_action(nasp, vl_mff_map, push);
+ enum ofperr error = decode_stack_action(nasp, vl_mff_map, tlv_bitmap,
+ push);
return error ? error : nxm_stack_push_check(push, NULL);
}
@@ -3145,10 +3163,11 @@ static enum ofperr
decode_NXAST_RAW_STACK_POP(const struct nx_action_stack *nasp,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpact_stack *pop = ofpact_put_STACK_POP(ofpacts);
- enum ofperr error = decode_stack_action(nasp, vl_mff_map, pop);
+ enum ofperr error = decode_stack_action(nasp, vl_mff_map, tlv_bitmap,
+ pop);
return error ? error : nxm_stack_pop_check(pop, NULL);
}
@@ -4279,14 +4298,14 @@ get_be32(const void **pp)
static enum ofperr
get_subfield(int n_bits, const void **p, struct mf_subfield *sf,
- const struct vl_mff_map *vl_mff_map)
+ const struct vl_mff_map *vl_mff_map, uint64_t *tlv_bitmap)
{
+ enum ofperr error;
+
sf->field = mf_from_nxm_header(ntohl(get_be32(p)), vl_mff_map);
sf->ofs = ntohs(get_be16(p));
sf->n_bits = n_bits;
- if (mf_vl_mff_invalid(sf->field, vl_mff_map)) {
- return OFPERR_NXFMFC_INVALID_TLV_FIELD;
- }
+ CHECK_VL_MFF(sf->field, vl_mff_map, tlv_bitmap, error);
return 0;
}
@@ -4319,7 +4338,7 @@ static enum ofperr
decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *ofpacts)
+ uint64_t *tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpact_learn *learn;
const void *p, *end;
@@ -4384,7 +4403,8 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
unsigned int imm_bytes = 0;
enum ofperr error;
if (spec->src_type == NX_LEARN_SRC_FIELD) {
- error = get_subfield(spec->n_bits, &p, &spec->src, vl_mff_map);
+ error = get_subfield(spec->n_bits, &p, &spec->src, vl_mff_map,
+ tlv_bitmap);
if (error) {
return error;
}
@@ -4399,7 +4419,8 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
/* Get the destination. */
if (spec->dst_type == NX_LEARN_DST_MATCH ||
spec->dst_type == NX_LEARN_DST_LOAD) {
- error = get_subfield(spec->n_bits, &p, &spec->dst, vl_mff_map);
+ error = get_subfield(spec->n_bits, &p, &spec->dst, vl_mff_map,
+ tlv_bitmap);
if (error) {
return error;
}
@@ -4649,11 +4670,12 @@ static enum ofperr
decode_NXAST_RAW_MULTIPATH(const struct nx_action_multipath *nam,
enum ofp_version ofp_version OVS_UNUSED,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
uint32_t n_links = ntohs(nam->max_link) + 1;
size_t min_n_bits = log_2_ceil(n_links);
struct ofpact_multipath *mp;
+ enum ofperr error;
mp = ofpact_put_MULTIPATH(out);
mp->fields = ntohs(nam->fields);
@@ -4665,9 +4687,7 @@ decode_NXAST_RAW_MULTIPATH(const struct
nx_action_multipath *nam,
mp->dst.ofs = nxm_decode_ofs(nam->ofs_nbits);
mp->dst.n_bits = nxm_decode_n_bits(nam->ofs_nbits);
- if (mf_vl_mff_invalid(mp->dst.field, vl_mff_map)) {
- return OFPERR_NXFMFC_INVALID_TLV_FIELD;
- }
+ CHECK_VL_MFF(mp->dst.field, vl_mff_map, tlv_bitmap, error);
if (!flow_hash_fields_valid(mp->fields)) {
VLOG_WARN_RL(&rl, "unsupported fields %d", (int) mp->fields);
@@ -4855,7 +4875,7 @@ static enum ofperr
decode_NXAST_RAW_CLONE(const struct ext_action_header *eah,
enum ofp_version ofp_version,
const struct vl_mff_map *vl_mff_map,
- struct ofpbuf *out)
+ uint64_t *tlv_bitmap, struct ofpbuf *out)
{
int error;
struct ofpbuf openflow;
@@ -4869,7 +4889,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, vl_mff_map);
+ out, 0, vl_mff_map, tlv_bitmap);
clone = ofpbuf_push_uninit(out, sizeof *clone);
out->header = &clone->ofpact;
ofpact_finish_CLONE(out, &clone);
@@ -5316,7 +5336,7 @@ OFP_ASSERT(sizeof(struct nx_action_conntrack) == 24);
static enum ofperr
decode_ct_zone(const struct nx_action_conntrack *nac,
struct ofpact_conntrack *out,
- const struct vl_mff_map *vl_mff_map)
+ const struct vl_mff_map *vl_mff_map, uint64_t *tlv_bitmap)
{
if (nac->zone_src) {
enum ofperr error;
@@ -5325,9 +5345,7 @@ decode_ct_zone(const struct nx_action_conntrack *nac,
vl_mff_map);
out->zone_src.ofs = nxm_decode_ofs(nac->zone_ofs_nbits);
out->zone_src.n_bits = nxm_decode_n_bits(nac->zone_ofs_nbits);
- if (mf_vl_mff_invalid(out->zone_src.field, vl_mff_map)) {
- return OFPERR_NXFMFC_INVALID_TLV_FIELD;
- }
+ CHECK_VL_MFF(out->zone_src.field, vl_mff_map, tlv_bitmap, error);
error = mf_check_src(&out->zone_src, NULL);
if (error) {
@@ -5350,13 +5368,14 @@ decode_ct_zone(const struct nx_action_conntrack *nac,
static enum ofperr
decode_NXAST_RAW_CT(const struct nx_action_conntrack *nac,
enum ofp_version ofp_version,
- const struct vl_mff_map *vl_mff_map, struct ofpbuf *out)
+ const struct vl_mff_map *vl_mff_map, uint64_t *tlv_bitmap,
+ struct ofpbuf *out)
{
const size_t ct_offset = ofpacts_pull(out);
struct ofpact_conntrack *conntrack = ofpact_put_CT(out);
conntrack->flags = ntohs(nac->flags);
- int error = decode_ct_zone(nac, conntrack, vl_mff_map);
+ int error = decode_ct_zone(nac, conntrack, vl_mff_map, tlv_bitmap);
if (error) {
goto out;
}
@@ -5370,7 +5389,8 @@ 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, vl_mff_map);
+ out, OFPACT_CT, vl_mff_map,
+ tlv_bitmap);
if (error) {
goto out;
}
@@ -6256,7 +6276,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,
- const struct vl_mff_map *vl_mff_map, struct ofpbuf *ofpacts)
+ const struct vl_mff_map *vl_mff_map,
+ uint64_t *ofpacts_tlv_bitmap, struct ofpbuf *ofpacts)
{
struct ofpbuf openflow = ofpbuf_const_initializer(actions, actions_len);
while (openflow.size) {
@@ -6268,7 +6289,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, vl_mff_map,
- ofpacts);
+ ofpacts_tlv_bitmap, ofpacts);
}
if (error) {
@@ -6286,7 +6307,8 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
uint32_t allowed_ovsinsts,
struct ofpbuf *ofpacts,
enum ofpact_type outer_action,
- const struct vl_mff_map *vl_mff_map)
+ const struct vl_mff_map *vl_mff_map,
+ uint64_t *ofpacts_tlv_bitmap)
{
const struct ofp_action_header *actions;
size_t orig_size = ofpacts->size;
@@ -6306,7 +6328,8 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
return OFPERR_OFPBRC_BAD_LEN;
}
- error = ofpacts_decode(actions, actions_len, version, vl_mff_map, ofpacts);
+ error = ofpacts_decode(actions, actions_len, version, vl_mff_map,
+ ofpacts_tlv_bitmap, ofpacts);
if (error) {
ofpacts->size = orig_size;
return error;
@@ -6342,11 +6365,13 @@ ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
unsigned int actions_len,
enum ofp_version version,
const struct vl_mff_map *vl_mff_map,
+ uint64_t *ofpacts_tlv_bitmap,
struct ofpbuf *ofpacts)
{
return ofpacts_pull_openflow_actions__(openflow, actions_len, version,
1u << OVSINST_OFPIT11_APPLY_ACTIONS,
- ofpacts, 0, vl_mff_map);
+ ofpacts, 0, vl_mff_map,
+ ofpacts_tlv_bitmap);
}
/* OpenFlow 1.1 actions. */
@@ -6586,13 +6611,15 @@ static enum ofperr
ofpacts_decode_for_action_set(const struct ofp_action_header *in,
size_t n_in, enum ofp_version version,
const struct vl_mff_map *vl_mff_map,
+ uint64_t *ofpacts_tlv_bitmap,
struct ofpbuf *out)
{
enum ofperr error;
struct ofpact *a;
size_t start = out->size;
- error = ofpacts_decode(in, n_in, version, vl_mff_map, out);
+ error = ofpacts_decode(in, n_in, version, vl_mff_map, ofpacts_tlv_bitmap,
+ out);
if (error) {
return error;
@@ -6891,6 +6918,7 @@ ofpacts_pull_openflow_instructions(struct ofpbuf
*openflow,
unsigned int instructions_len,
enum ofp_version version,
const struct vl_mff_map *vl_mff_map,
+ uint64_t *ofpacts_tlv_bitmap,
struct ofpbuf *ofpacts)
{
const struct ofp11_instruction *instructions;
@@ -6902,7 +6930,8 @@ ofpacts_pull_openflow_instructions(struct ofpbuf
*openflow,
return ofpacts_pull_openflow_actions__(openflow, instructions_len,
version,
(1u << N_OVS_INSTRUCTIONS) - 1,
- ofpacts, 0, vl_mff_map);
+ ofpacts, 0, vl_mff_map,
+ ofpacts_tlv_bitmap);
}
if (instructions_len % OFP11_INSTRUCTION_ALIGN != 0) {
@@ -6946,7 +6975,7 @@ 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, vl_mff_map,
- ofpacts);
+ ofpacts_tlv_bitmap, ofpacts);
if (error) {
goto exit;
}
@@ -6966,7 +6995,8 @@ 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, vl_mff_map, ofpacts);
+ version, vl_mff_map,
+ ofpacts_tlv_bitmap, ofpacts);
if (error) {
goto exit;
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index f7da90b276f9..96b4f8d22333 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1726,8 +1726,11 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
return OFPERR_OFPFMFC_BAD_COMMAND;
}
+ fm->ofpacts_tlv_bitmap = 0;
error = ofpacts_pull_openflow_instructions(&b, b.size, oh->version,
- vl_mff_map, ofpacts);
+ vl_mff_map,
+ &fm->ofpacts_tlv_bitmap,
+ ofpacts);
if (error) {
return error;
}
@@ -3009,7 +3012,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats
*fs,
}
if (ofpacts_pull_openflow_instructions(msg, instructions_len, oh->version,
- NULL, ofpacts)) {
+ NULL, NULL, ofpacts)) {
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
return EINVAL;
}
@@ -4034,7 +4037,7 @@ parse_actions_property(struct ofpbuf *property, enum
ofp_version version,
}
return ofpacts_pull_openflow_actions(property, property->size,
- version, NULL, ofpacts);
+ version, NULL, NULL, ofpacts);
}
/* This is like ofputil_decode_packet_in(), except that it decodes the
@@ -4189,7 +4192,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po,
}
error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len),
- oh->version, NULL, ofpacts);
+ oh->version, NULL, NULL,
+ ofpacts);
if (error) {
return error;
}
@@ -4201,7 +4205,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po,
po->in_port = u16_to_ofp(ntohs(opo->in_port));
error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len),
- oh->version, NULL, ofpacts);
+ oh->version, NULL, NULL,
+ ofpacts);
if (error) {
return error;
}
@@ -6743,7 +6748,7 @@ ofputil_decode_flow_update(struct ofputil_flow_update
*update,
actions_len = length - sizeof *nfuf - ROUND_UP(match_len, 8);
error = ofpacts_pull_openflow_actions(msg, actions_len, oh->version,
- NULL, ofpacts);
+ NULL, NULL, ofpacts);
if (error) {
return error;
}
@@ -8727,7 +8732,7 @@ ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t
buckets_length,
ofpbuf_init(&ofpacts, 0);
error = ofpacts_pull_openflow_actions(msg, ob_len - sizeof *ob,
- version, NULL, &ofpacts);
+ version, NULL, NULL, &ofpacts);
if (error) {
ofpbuf_uninit(&ofpacts);
ofputil_bucket_list_destroy(buckets);
@@ -8801,7 +8806,7 @@ ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t
buckets_length,
buckets_length -= ob_len;
err = ofpacts_pull_openflow_actions(msg, actions_len, version,
- NULL, &ofpacts);
+ NULL, NULL, &ofpacts);
if (err) {
goto err;
}
diff --git a/lib/vl-mff-map.h b/lib/vl-mff-map.h
index 1c29385782ec..a47b8aa8a6f9 100644
--- a/lib/vl-mff-map.h
+++ b/lib/vl-mff-map.h
@@ -20,6 +20,8 @@
#include "cmap.h"
#include "openvswitch/thread.h"
+struct rule;
+
/* Variable length mf_fields mapping map. This is a single writer,
* multiple-reader hash table that a writer must hold the following mutex
* to access this map. */
@@ -29,7 +31,7 @@ struct vl_mff_map {
};
/* Variable length fields. */
-void mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
+enum ofperr mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map, bool)
OVS_REQUIRES(vl_mff_map->mutex);
enum ofperr mf_vl_mff_map_mod_from_tun_metadata(
struct vl_mff_map *vl_mff_map, const struct ofputil_tlv_table_mod *)
@@ -37,5 +39,9 @@ enum ofperr mf_vl_mff_map_mod_from_tun_metadata(
const struct mf_field * mf_get_vl_mff(const struct mf_field *,
const struct vl_mff_map *);
bool mf_vl_mff_invalid(const struct mf_field *, const struct vl_mff_map *);
-
+void mf_vl_mff_set_tlv_bitmap(const struct mf_field *, uint64_t *);
+enum ofperr mf_check_vl_mff(const struct mf_field *, const struct vl_mff_map *,
+ uint64_t *);
+void mf_vl_mff_add_ref_cnt_from_rule(const struct rule *);
+void mf_vl_mff_remove_ref_cnt_from_rule(const struct rule *);
#endif /* vl-mff-map.h */
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7d3e929f21e3..dab74e3a195f 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -423,6 +423,10 @@ struct rule {
/* Must hold 'mutex' for both read/write, 'ofproto_mutex' not needed. */
long long int modified OVS_GUARDED; /* Time of last modification. */
+
+ /* 1-bit for each present TLV in flow match / action. */
+ uint64_t match_tlv_bitmap;
+ uint64_t ofpacts_tlv_bitmap;
};
void ofproto_rule_ref(struct rule *);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 699c37cd5c3e..cf4db291e12a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -228,6 +228,8 @@ static enum ofperr ofproto_rule_create(struct ofproto *,
struct cls_rule *,
uint16_t importance,
const struct ofpact *ofpacts,
size_t ofpacts_len,
+ uint64_t match_tlv_bitmap,
+ uint64_t ofpacts_tlv_bitmap,
struct rule **new_rule)
OVS_NO_THREAD_SAFETY_ANALYSIS;
@@ -1580,12 +1582,6 @@ ofproto_destroy__(struct ofproto *ofproto)
tun_metadata_free(ovsrcu_get_protected(struct tun_table *,
&ofproto->metadata_tab));
- ovs_mutex_lock(&ofproto->vl_mff_map.mutex);
- mf_vl_mff_map_clear(&ofproto->vl_mff_map);
- ovs_mutex_unlock(&ofproto->vl_mff_map.mutex);
- cmap_destroy(&ofproto->vl_mff_map.cmap);
- ovs_mutex_destroy(&ofproto->vl_mff_map.mutex);
-
free(ofproto->name);
free(ofproto->type);
free(ofproto->mfr_desc);
@@ -1603,6 +1599,12 @@ ofproto_destroy__(struct ofproto *ofproto)
}
free(ofproto->tables);
+ ovs_mutex_lock(&ofproto->vl_mff_map.mutex);
+ mf_vl_mff_map_clear(&ofproto->vl_mff_map, true);
+ ovs_mutex_unlock(&ofproto->vl_mff_map.mutex);
+ cmap_destroy(&ofproto->vl_mff_map.cmap);
+ ovs_mutex_destroy(&ofproto->vl_mff_map.mutex);
+
ovs_assert(hindex_is_empty(&ofproto->cookies));
hindex_destroy(&ofproto->cookies);
@@ -2829,6 +2831,7 @@ rule_destroy_cb(struct rule *rule)
ofproto_rule_send_removed(rule);
}
rule->ofproto->ofproto_class->rule_destruct(rule);
+ mf_vl_mff_remove_ref_cnt_from_rule(rule);
ofproto_rule_destroy__(rule);
}
@@ -4741,7 +4744,9 @@ add_flow_init(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
fm->new_cookie, fm->idle_timeout,
fm->hard_timeout, fm->flags,
fm->importance, fm->ofpacts,
- fm->ofpacts_len, &ofm->temp_rule);
+ fm->ofpacts_len,
+ fm->match.flow.tunnel.metadata.present.map,
+ fm->ofpacts_tlv_bitmap, &ofm->temp_rule);
if (error) {
return error;
}
@@ -4856,6 +4861,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct
cls_rule *cr,
uint16_t idle_timeout, uint16_t hard_timeout,
enum ofputil_flow_mod_flags flags, uint16_t importance,
const struct ofpact *ofpacts, size_t ofpacts_len,
+ uint64_t match_tlv_bitmap, uint64_t ofpacts_tlv_bitmap,
struct rule **new_rule)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
@@ -4906,6 +4912,9 @@ ofproto_rule_create(struct ofproto *ofproto, struct
cls_rule *cr,
}
rule->state = RULE_INITIALIZED;
+ rule->match_tlv_bitmap = match_tlv_bitmap;
+ rule->ofpacts_tlv_bitmap = ofpacts_tlv_bitmap;
+ mf_vl_mff_add_ref_cnt_from_rule(rule);
*new_rule = rule;
return 0;
@@ -5003,6 +5012,8 @@ ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod
*ofm)
rule->importance,
rule->actions->ofpacts,
rule->actions->ofpacts_len,
+ rule->match_tlv_bitmap,
+ rule->ofpacts_tlv_bitmap,
&ofm->temp_rule);
ovs_mutex_unlock(&rule->mutex);
if (!error) {
@@ -5263,6 +5274,11 @@ modify_flows_start__(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm)
cls_rule_destroy(CONST_CAST(struct cls_rule *, &temp->cr));
cls_rule_clone(CONST_CAST(struct cls_rule *, &temp->cr),
&old_rule->cr);
+ if (temp->match_tlv_bitmap != old_rule->match_tlv_bitmap) {
+ mf_vl_mff_remove_ref_cnt_from_rule(temp);
+ temp->match_tlv_bitmap = old_rule->match_tlv_bitmap;
+ mf_vl_mff_add_ref_cnt_from_rule(temp);
+ }
*CONST_CAST(uint8_t *, &temp->table_id) = old_rule->table_id;
rule_collection_add(new_rules, temp);
first = false;
@@ -5276,6 +5292,8 @@ modify_flows_start__(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm)
temp->importance,
temp->actions->ofpacts,
temp->actions->ofpacts_len,
+ old_rule->match_tlv_bitmap,
+ temp->ofpacts_tlv_bitmap,
&new_rule);
if (!error) {
rule_collection_add(new_rules, new_rule);
@@ -7784,14 +7802,15 @@ handle_tlv_table_mod(struct ofconn *ofconn, const
struct ofp_header *oh)
old_tab = ovsrcu_get_protected(struct tun_table *, &ofproto->metadata_tab);
error = tun_metadata_table_mod(&ttm, old_tab, &new_tab);
if (!error) {
- ovsrcu_set(&ofproto->metadata_tab, new_tab);
- tun_metadata_postpone_free(old_tab);
-
ovs_mutex_lock(&ofproto->vl_mff_map.mutex);
error = mf_vl_mff_map_mod_from_tun_metadata(&ofproto->vl_mff_map,
&ttm);
ovs_mutex_unlock(&ofproto->vl_mff_map.mutex);
}
+ if (!error) {
+ ovsrcu_set(&ofproto->metadata_tab, new_tab);
+ tun_metadata_postpone_free(old_tab);
+ }
ofputil_uninit_tlv_table(&ttm.mappings);
return error;
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 0380c8481ecf..890b47fe0d4f 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -168,7 +168,8 @@ pinctrl_handle_arp(const struct flow *ip_flow, const struct
match *md,
reload_metadata(&ofpacts, md);
enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
- version, NULL, &ofpacts);
+ version, NULL, NULL,
+ &ofpacts);
if (error) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
VLOG_WARN_RL(&rl, "failed to parse arp actions (%s)",
@@ -1398,7 +1399,8 @@ pinctrl_handle_nd_na(const struct flow *ip_flow, const
struct match *md,
reload_metadata(&ofpacts, md);
enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
- version, NULL, &ofpacts);
+ version, NULL, NULL,
+ &ofpacts);
if (error) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
VLOG_WARN_RL(&rl, "failed to parse actions for 'na' (%s)",
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 1ba209dd4ce7..b9e9e21bfb3f 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -535,7 +535,81 @@ AT_CHECK([tail -2 stdout], [0],
Datapath actions: 2
])
-AT_CHECK([ovs-ofctl del-tlv-map br0])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip],
+[0], [dnl
+NXST_FLOW reply:
+ tun_metadata3=0x1234567890abcdef actions=output:2
+])
+
+dnl A TLV mapping should not be removed if any active flow uses the mapping.
+AT_CHECK([ovs-ofctl del-tlv-map br0], [1], [], [dnl
+OFPT_ERROR (xid=0x4): NXTTMFC_INVALID_TLV_DEL
+NXT_TLV_TABLE_MOD (xid=0x4):
+ CLEAR
+])
+
+AT_CHECK([ovs-ofctl del-flows br0], [0])
+AT_CHECK([ovs-ofctl del-tlv-map br0], [0])
+
+dnl Flow modification
+AT_CHECK([ovs-ofctl add-tlv-map br0
"{class=0xffff,type=1,len=4}->tun_metadata0"])
+AT_CHECK([ovs-ofctl add-tlv-map br0
"{class=0xffff,type=2,len=4}->tun_metadata1"])
+AT_CHECK([ovs-ofctl add-tlv-map br0
"{class=0xffff,type=3,len=4}->tun_metadata2"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=1
actions=multipath(eth_src,50,modulo_n,1,0,tun_metadata0[[0..31]])"])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=2
actions=push:tun_metadata1[[0..31]],clone(move:tun_metadata2[[0..31]]->reg0[[0..31]])"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=1 actions=output:4"])
+AT_CHECK([ovs-ofctl del-tlv-map br0
"{class=0xffff,type=3,len=4}->tun_metadata0"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=2
actions=push:tun_metadata2[[0..31]]"])
+AT_CHECK([ovs-ofctl del-tlv-map br0
"{class=0xffff,type=2,len=4}->tun_metadata1"])
+AT_CHECK([ovs-ofctl del-tlv-map br0
"{class=0xffff,type=3,len=4}->tun_metadata2"], [1], [], [dnl
+OFPT_ERROR (xid=0x4): NXTTMFC_INVALID_TLV_DEL
+NXT_TLV_TABLE_MOD (xid=0x4):
+ DEL mapping table:
+ class type length match field
+ ----- ---- ------ -----------
+ 0xffff 0x3 4 tun_metadata2
+])
+
+AT_CHECK([ovs-ofctl del-flows br0], [0])
+AT_CHECK([ovs-ofctl del-tlv-map br0], [0])
+
+dnl Learn action
+AT_CHECK([ovs-ofctl add-tlv-map br0
"{class=0xffff,type=1,len=4}->tun_metadata1"])
+AT_CHECK([ovs-ofctl add-tlv-map br0
"{class=0xffff,type=2,len=4}->tun_metadata2"])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=2, eth_src=00:00:00:00:00:01
actions=learn(tun_metadata1[[0..31]]=reg1, output:NXM_OF_IN_PORT[[]])"])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=2, eth_src=00:00:00:00:00:02
actions=learn(reg1[[0..31]]=0xFF, load:reg1[[0..31]]->tun_metadata2[[0..31]])"])
+flow1="in_port(2),eth(src=00:00:00:00:00:01,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0800)"
+flow2="in_port(2),eth(src=00:00:00:00:00:02,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0800)"
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow1" -generate], [0],
[stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow2" -generate], [0],
[stdout])
+
+dnl Delete flows with learn action
+AT_CHECK([ovs-ofctl del-flows br0 "in_port=2"])
+
+AT_CHECK([ovs-ofctl del-tlv-map br0
"{class=0xffff,type=1,len=4}->tun_metadata1"], [1], [], [dnl
+OFPT_ERROR (xid=0x4): NXTTMFC_INVALID_TLV_DEL
+NXT_TLV_TABLE_MOD (xid=0x4):
+ DEL mapping table:
+ class type length match field
+ ----- ---- ------ -----------
+ 0xffff 0x1 4 tun_metadata1
+])
+AT_CHECK([ovs-ofctl del-flows br0 "tun_metadata1"])
+AT_CHECK([ovs-ofctl del-tlv-map br0
"{class=0xffff,type=1,len=4}->tun_metadata1"])
+
+AT_CHECK([ovs-ofctl del-tlv-map br0
"{class=0xffff,type=2,len=4}->tun_metadata2"], [1], [], [dnl
+OFPT_ERROR (xid=0x4): NXTTMFC_INVALID_TLV_DEL
+NXT_TLV_TABLE_MOD (xid=0x4):
+ DEL mapping table:
+ class type length match field
+ ----- ---- ------ -----------
+ 0xffff 0x2 4 tun_metadata2
+])
+AT_CHECK([ovs-ofctl del-flows br0 "reg1=0xFF"])
+AT_CHECK([ovs-ofctl del-tlv-map br0], [0])
OVS_VSWITCHD_STOP
AT_CLEANUP
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 7b214ebbaacd..6f684d28c9d1 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -3876,7 +3876,7 @@ ofctl_parse_actions__(const char *version_s, bool
instructions)
error = (instructions
? ofpacts_pull_openflow_instructions
: ofpacts_pull_openflow_actions)(
- &of_in, of_in.size, version, NULL, &ofpacts);
+ &of_in, of_in.size, version, NULL, NULL, &ofpacts);
if (!error && instructions) {
/* Verify actions, enforce consistency. */
enum ofputil_protocol protocol;
--
2.7.4
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
