[ovs-dev] [branch-2.7 3/4] ofproto: Add ref counting for variable length mf_fields.
Joe Stringer
joe at ovn.org
Wed Mar 15 23:01:40 UTC 2017
From: Yi-Hung Wei <yihung.wei at gmail.com>
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 <jarno at ovn.org>
Suggested-by: Joe Stringer <joe at ovn.org>
Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
Signed-off-by: Joe Stringer <joe at ovn.org>
---
build-aux/extract-ofp-actions | 9 +-
include/openvswitch/ofp-actions.h | 2 +
include/openvswitch/ofp-errors.h | 4 +
include/openvswitch/ofp-util.h | 1 +
lib/learn.c | 5 +
lib/meta-flow.c | 228 ++++++++++++++++++++++++++++++++------
lib/ofp-actions.c | 208 +++++++++++++++++++++-------------
lib/ofp-util.c | 21 ++--
lib/vl-mff-map.h | 17 ++-
ofproto/ofproto-provider.h | 4 +
ofproto/ofproto.c | 33 +++++-
ovn/controller/pinctrl.c | 6 +-
tests/tunnel.at | 76 ++++++++++++-
utilities/ovs-ofctl.c | 2 +-
14 files changed, 479 insertions(+), 137 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..214dfed3f3bd 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -946,12 +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 *ofpacts_tlv_bitmap,
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,
+ uint64_t *ofpacts_tlv_bitmap,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
struct flow *, ofp_port_t max_ports,
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..edc5feb43d7c 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,7 @@ 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->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 +138,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 +149,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..016627556c58 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -27,6 +27,7 @@
#include "openvswitch/dynamic-string.h"
#include "nx-match.h"
#include "openvswitch/ofp-util.h"
+#include "ovs-atomic.h"
#include "ovs-rcu.h"
#include "ovs-thread.h"
#include "packets.h"
@@ -2646,6 +2647,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 +2657,41 @@ mf_field_hash(uint32_t key)
return hash_int(key, 0);
}
-void
-mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
+static void
+vmf_delete(struct vl_mf_field *vmf)
+{
+ if (ovs_refcount_unref(&vmf->ref_cnt) == 1) {
+ /* Postpone as this function is typically called immediately
+ * after removing from cmap. */
+ ovsrcu_postpone(free, vmf);
+ } else {
+ VLOG_WARN_RL(&rl,
+ "Attempted to delete VMF %s but refcount is nonzero!",
+ vmf->mf.name);
+ }
+}
+
+enum ofperr
+mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map, bool force)
OVS_REQUIRES(vl_mff_map->mutex)
{
struct vl_mf_field *vmf;
+ if (!force) {
+ CMAP_FOR_EACH (vmf, cmap_node, &vl_mff_map->cmap) {
+ if (ovs_refcount_read(&vmf->ref_cnt) != 1) {
+ return OFPERR_NXTTMFC_INVALID_TLV_DEL;
+ }
+ }
+ }
+
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);
+ vmf_delete(vmf);
}
+
+ return 0;
}
static struct vl_mf_field *
@@ -2697,53 +2723,98 @@ 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 force)
OVS_REQUIRES(vl_mff_map->mutex)
{
struct ofputil_tlv_map *tlv_map;
+ struct vl_mf_field *vmf;
+ unsigned int idx;
- if (ttm->command == NXTTMC_CLEAR) {
- mf_vl_mff_map_clear(vl_mff_map);
- return 0;
+ if (!force) {
+ 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 = mf_get_vl_mff__(idx, vl_mff_map);
+ if (vmf && ovs_refcount_read(&vmf->ref_cnt) != 1) {
+ return OFPERR_NXTTMFC_INVALID_TLV_DEL;
+ }
+ }
}
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,
+ vmf = mf_get_vl_mff__(idx, vl_mff_map);
+ if (vmf) {
+ cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
mf_field_hash(idx));
- break;
+ vmf_delete(vmf);
+ }
+ }
- case NXTTMC_DELETE:
- vmf = mf_get_vl_mff__(idx, vl_mff_map);
- if (vmf) {
- cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
- mf_field_hash(idx));
- ovsrcu_postpone(free, vmf);
- }
- break;
+ return 0;
+}
- case NXTTMC_CLEAR:
- default:
- OVS_NOT_REACHED();
+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;
+}
+
+/* Updates the tun_metadata mf_field in 'vl_mff_map' according to 'ttm'.
+ * This function must 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)
+{
+ switch (ttm->command) {
+ case NXTTMC_ADD:
+ return mf_vl_mff_map_add(vl_mff_map, ttm);
+
+ case NXTTMC_DELETE:
+ return mf_vl_mff_map_del(vl_mff_map, ttm, false);
+
+ case NXTTMC_CLEAR:
+ return mf_vl_mff_map_clear(vl_mff_map, false);
+
+ default:
+ OVS_NOT_REACHED();
}
return 0;
@@ -2756,3 +2827,90 @@ 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) {
+ ovs_assert(mf_is_tun_metadata(mff));
+ ULLONG_SET1(*tlv_bitmap, mff->id - MFF_TUN_METADATA0);
+ }
+}
+
+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_ref(const struct vl_mff_map *map, uint64_t tlv_bitmap)
+{
+ mf_vl_mff_ref_cnt_mod(map, tlv_bitmap, true);
+}
+
+void
+mf_vl_mff_unref(const struct vl_mff_map *map, uint64_t tlv_bitmap)
+{
+ mf_vl_mff_ref_cnt_mod(map, tlv_bitmap, false);
+}
+
+enum ofperr
+mf_vl_mff_nx_pull_header(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map,
+ const struct mf_field **field, bool *masked,
+ uint64_t *tlv_bitmap)
+{
+ enum ofperr error = nx_pull_header(b, vl_mff_map, field, masked);
+ if (error) {
+ return error;
+ }
+
+ mf_vl_mff_set_tlv_bitmap(*field, tlv_bitmap);
+ return 0;
+}
+
+enum ofperr
+mf_vl_mff_nx_pull_entry(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map,
+ const struct mf_field **field, union mf_value *value,
+ union mf_value *mask, uint64_t *tlv_bitmap)
+{
+ enum ofperr error = nx_pull_entry(b, vl_mff_map, field, value, mask);
+ if (error) {
+ return error;
+ }
+
+ mf_vl_mff_set_tlv_bitmap(*field, tlv_bitmap);
+ return 0;
+}
+
+enum ofperr
+mf_vl_mff_mf_from_nxm_header(uint32_t header,
+ const struct vl_mff_map *vl_mff_map,
+ const struct mf_field **field,
+ uint64_t *tlv_bitmap)
+{
+ *field = mf_from_nxm_header(header, vl_mff_map);
+ if (mf_vl_mff_invalid(*field, vl_mff_map)) {
+ return OFPERR_NXFMFC_INVALID_TLV_FIELD;
+ }
+
+ mf_vl_mff_set_tlv_bitmap(*field, tlv_bitmap);
+ return 0;
+}
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 5ff132370131..b358dcc10b98 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,
@@ -1121,9 +1121,10 @@ 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;
@@ -1131,13 +1132,13 @@ decode_NXAST_RAW_OUTPUT_REG(const struct nx_action_output_reg *naor,
output_reg = ofpact_put_OUTPUT_REG(out);
output_reg->ofpact.raw = NXAST_RAW_OUTPUT_REG;
- output_reg->src.field = mf_from_nxm_header(ntohl(naor->src), vl_mff_map);
output_reg->src.ofs = nxm_decode_ofs(naor->ofs_nbits);
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;
+ error = mf_vl_mff_mf_from_nxm_header(ntohl(naor->src), vl_mff_map,
+ &output_reg->src.field, tlv_bitmap);
+ if (error) {
+ return error;
}
return mf_check_src(&output_reg->src, NULL);
@@ -1147,9 +1148,11 @@ 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;
+ enum ofperr error;
+
output_reg = ofpact_put_OUTPUT_REG(out);
output_reg->ofpact.raw = NXAST_RAW_OUTPUT_REG2;
output_reg->src.ofs = nxm_decode_ofs(naor->ofs_nbits);
@@ -1159,11 +1162,12 @@ decode_NXAST_RAW_OUTPUT_REG2(const struct nx_action_output_reg2 *naor,
struct ofpbuf b = ofpbuf_const_initializer(naor, ntohs(naor->len));
ofpbuf_pull(&b, OBJECT_OFFSETOF(naor, pad));
- enum ofperr error = nx_pull_header(&b, vl_mff_map, &output_reg->src.field,
- NULL);
+ error = mf_vl_mff_nx_pull_header(&b, vl_mff_map, &output_reg->src.field,
+ NULL, tlv_bitmap);
if (error) {
return error;
}
+
if (!is_all_zeros(b.data, b.size)) {
return OFPERR_NXBRC_MUST_BE_ZERO;
}
@@ -1286,7 +1290,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;
@@ -1323,11 +1328,12 @@ decode_bundle(bool load, const struct nx_action_bundle *nab,
}
if (load) {
- 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;
+ error = mf_vl_mff_mf_from_nxm_header(ntohl(nab->dst), vl_mff_map,
+ &bundle->dst.field, tlv_bitmap);
+ if (error) {
+ return error;
}
if (bundle->dst.n_bits < 16) {
@@ -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,9 +2288,11 @@ 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);
+ enum ofperr error;
+
move->ofpact.raw = ONFACT_RAW13_COPY_FIELD;
move->src.ofs = ntohs(src_offset);
move->src.n_bits = ntohs(n_bits);
@@ -2294,11 +2302,13 @@ decode_copy_field__(ovs_be16 src_offset, ovs_be16 dst_offset, ovs_be16 n_bits,
struct ofpbuf b = ofpbuf_const_initializer(action, ntohs(action_len));
ofpbuf_pull(&b, oxm_offset);
- enum ofperr error = nx_pull_header(&b, vl_mff_map, &move->src.field, NULL);
+ error = mf_vl_mff_nx_pull_header(&b, vl_mff_map, &move->src.field, NULL,
+ tlv_bitmap);
if (error) {
return error;
}
- error = nx_pull_header(&b, vl_mff_map, &move->dst.field, NULL);
+ error = mf_vl_mff_nx_pull_header(&b, vl_mff_map, &move->dst.field, NULL,
+ tlv_bitmap);
if (error) {
return error;
}
@@ -2314,33 +2324,35 @@ 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);
+ enum ofperr error;
+
move->ofpact.raw = NXAST_RAW_REG_MOVE;
move->src.ofs = ntohs(narm->src_ofs);
move->src.n_bits = ntohs(narm->n_bits);
@@ -2350,11 +2362,14 @@ decode_NXAST_RAW_REG_MOVE(const struct nx_action_reg_move *narm,
struct ofpbuf b = ofpbuf_const_initializer(narm, ntohs(narm->len));
ofpbuf_pull(&b, sizeof *narm);
- enum ofperr error = nx_pull_header(&b, vl_mff_map, &move->src.field, NULL);
+ error = mf_vl_mff_nx_pull_header(&b, vl_mff_map, &move->src.field, NULL,
+ tlv_bitmap);
if (error) {
return error;
}
- error = nx_pull_header(&b, vl_mff_map, &move->dst.field, NULL);
+
+ error = mf_vl_mff_nx_pull_header(&b, vl_mff_map, &move->dst.field, NULL,
+ tlv_bitmap);
if (error) {
return error;
}
@@ -2481,20 +2496,22 @@ 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));
union mf_value value, mask;
const struct mf_field *field;
- enum ofperr error = nx_pull_entry(&b, vl_mff_map, &field, &value,
- may_mask ? &mask : NULL);
+ enum ofperr error;
+ error = mf_vl_mff_nx_pull_entry(&b, vl_mff_map, &field, &value,
+ may_mask ? &mask : NULL, tlv_bitmap);
if (error) {
return (error == OFPERR_OFPBMC_BAD_MASK
? OFPERR_OFPBAC_BAD_SET_MASK
: error);
}
+
if (!may_mask) {
memset(&mask, 0xff, field->n_bytes);
}
@@ -2540,34 +2557,36 @@ 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;
- 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;
+ error = mf_vl_mff_mf_from_nxm_header(ntohl(narl->dst), vl_mff_map,
+ &dst.field, tlv_bitmap);
+ if (error) {
+ return error;
}
error = mf_check_dst(&dst, NULL);
@@ -2596,17 +2615,20 @@ 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));
union mf_value value, mask;
const struct mf_field *field;
- enum ofperr error = nx_pull_entry(&b, vl_mff_map, &field, &value, &mask);
+ enum ofperr error;
+ error = mf_vl_mff_nx_pull_entry(&b, vl_mff_map, &field, &value, &mask,
+ tlv_bitmap);
if (error) {
return error;
}
+
if (!is_all_zeros(b.data, b.size)) {
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
@@ -3111,18 +3133,21 @@ 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)
{
+ enum ofperr error;
stack_action->subfield.ofs = ntohs(nasp->offset);
struct ofpbuf b = ofpbuf_const_initializer(nasp, sizeof *nasp);
ofpbuf_pull(&b, OBJECT_OFFSETOF(nasp, pad));
- enum ofperr error = nx_pull_header(&b, vl_mff_map,
- &stack_action->subfield.field, NULL);
+ error = mf_vl_mff_nx_pull_header(&b, vl_mff_map,
+ &stack_action->subfield.field, NULL,
+ tlv_bitmap);
if (error) {
return error;
}
+
stack_action->subfield.n_bits = ntohs(*(const ovs_be16 *) b.data);
ofpbuf_pull(&b, 2);
if (!is_all_zeros(b.data, b.size)) {
@@ -3136,10 +3161,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);
}
@@ -3147,10 +3173,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);
}
@@ -4281,15 +4308,15 @@ 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)
{
- sf->field = mf_from_nxm_header(ntohl(get_be32(p)), vl_mff_map);
+ enum ofperr error;
+
+ error = mf_vl_mff_mf_from_nxm_header(ntohl(get_be32(p)), vl_mff_map,
+ &sf->field, tlv_bitmap);
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;
- }
- return 0;
+ return error;
}
static unsigned int
@@ -4321,7 +4348,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;
@@ -4386,7 +4413,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;
}
@@ -4401,7 +4429,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;
}
@@ -4651,11 +4680,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);
@@ -4663,12 +4693,12 @@ decode_NXAST_RAW_MULTIPATH(const struct nx_action_multipath *nam,
mp->algorithm = ntohs(nam->algorithm);
mp->max_link = ntohs(nam->max_link);
mp->arg = ntohl(nam->arg);
- mp->dst.field = mf_from_nxm_header(ntohl(nam->dst), vl_mff_map);
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;
+ error = mf_vl_mff_mf_from_nxm_header(ntohl(nam->dst), vl_mff_map,
+ &mp->dst.field, tlv_bitmap);
+ if (error) {
+ return error;
}
if (!flow_hash_fields_valid(mp->fields)) {
@@ -4857,7 +4887,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;
@@ -4871,7 +4901,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);
@@ -5318,17 +5348,18 @@ 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;
- out->zone_src.field = mf_from_nxm_header(ntohl(nac->zone_src),
- 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;
+ error = mf_vl_mff_mf_from_nxm_header(ntohl(nac->zone_src),
+ vl_mff_map, &out->zone_src.field,
+ tlv_bitmap);
+ if (error) {
+ return error;
}
error = mf_check_src(&out->zone_src, NULL);
@@ -5352,13 +5383,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;
}
@@ -5372,7 +5404,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;
}
@@ -6258,7 +6291,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) {
@@ -6270,7 +6304,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) {
@@ -6288,7 +6322,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;
@@ -6308,7 +6343,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;
@@ -6334,6 +6370,13 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
* you should call ofpacts_pull_openflow_instructions() instead of this
* function.
*
+ * 'vl_mff_map' and 'ofpacts_tlv_bitmap' are optional. If 'vl_mff_map' is
+ * provided, it is used to get variable length mf_fields with configured
+ * length in the actions. If an action uses a variable length mf_field,
+ * 'ofpacts_tlv_bitmap' is updated accordingly for ref counting. If
+ * 'vl_mff_map' is not provided, the default mf_fields with maximum length
+ * will be used.
+ *
* The parsed actions are valid generically, but they may not be valid in a
* specific context. For example, port numbers up to OFPP_MAX are valid
* generically, but specific datapaths may only support port numbers in a
@@ -6344,11 +6387,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. */
@@ -6588,13 +6633,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;
@@ -6893,6 +6940,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;
@@ -6904,7 +6952,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) {
@@ -6948,7 +6997,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;
}
@@ -6968,7 +7017,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 c48081fe3e7f..db27abf8bcd9 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;
}
@@ -3013,7 +3016,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;
}
@@ -4041,7 +4044,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
@@ -4200,7 +4203,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;
}
@@ -4212,7 +4216,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;
}
@@ -6754,7 +6759,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;
}
@@ -8738,7 +8743,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);
@@ -8812,7 +8817,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..136492cb1c02 100644
--- a/lib/vl-mff-map.h
+++ b/lib/vl-mff-map.h
@@ -29,7 +29,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 +37,18 @@ 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 *tlv_bitmap);
+void mf_vl_mff_ref(const struct vl_mff_map *, uint64_t tlv_bitmap);
+void mf_vl_mff_unref(const struct vl_mff_map *, uint64_t tlv_bitmap);
+enum ofperr mf_vl_mff_nx_pull_header(struct ofpbuf *,
+ const struct vl_mff_map *,
+ const struct mf_field **, bool *masked,
+ uint64_t *tlv_bitmap);
+enum ofperr mf_vl_mff_nx_pull_entry(struct ofpbuf *, const struct vl_mff_map *,
+ const struct mf_field **, union mf_value *,
+ union mf_value *, uint64_t *tlv_bitmap);
+enum ofperr mf_vl_mff_mf_from_nxm_header(uint32_t header,
+ const struct vl_mff_map *,
+ const struct mf_field **,
+ uint64_t *tlv_bitmap);
#endif /* vl-mff-map.h */
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index bb4fd6f52b83..12e0cfb99d37 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -422,6 +422,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 04712004dc86..b00a4af5e5c7 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;
@@ -1576,7 +1578,7 @@ ofproto_destroy__(struct ofproto *ofproto)
&ofproto->metadata_tab));
ovs_mutex_lock(&ofproto->vl_mff_map.mutex);
- mf_vl_mff_map_clear(&ofproto->vl_mff_map);
+ 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);
@@ -2824,6 +2826,8 @@ rule_destroy_cb(struct rule *rule)
ofproto_rule_send_removed(rule);
}
rule->ofproto->ofproto_class->rule_destruct(rule);
+ mf_vl_mff_unref(&rule->ofproto->vl_mff_map, rule->match_tlv_bitmap);
+ mf_vl_mff_unref(&rule->ofproto->vl_mff_map, rule->ofpacts_tlv_bitmap);
ofproto_rule_destroy__(rule);
}
@@ -4736,7 +4740,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;
}
@@ -4851,6 +4857,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
{
@@ -4901,6 +4908,10 @@ 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_ref(&rule->ofproto->vl_mff_map, match_tlv_bitmap);
+ mf_vl_mff_ref(&rule->ofproto->vl_mff_map, ofpacts_tlv_bitmap);
*new_rule = rule;
return 0;
@@ -4998,6 +5009,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) {
@@ -5258,6 +5271,13 @@ 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_unref(&temp->ofproto->vl_mff_map,
+ temp->match_tlv_bitmap);
+ temp->match_tlv_bitmap = old_rule->match_tlv_bitmap;
+ mf_vl_mff_ref(&temp->ofproto->vl_mff_map,
+ temp->match_tlv_bitmap);
+ }
*CONST_CAST(uint8_t *, &temp->table_id) = old_rule->table_id;
rule_collection_add(new_rules, temp);
first = false;
@@ -5271,6 +5291,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);
@@ -7779,13 +7801,14 @@ 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);
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 426e2fbc6a1f..ec130d7256cf 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.11.1
More information about the dev
mailing list