Message ID | 20170315230141.32414-4-joe@ovn.org |
---|---|
State | Accepted |
Headers | show |
Acked-by: Jarno Rajahalme <jarno@ovn.org> with two notes: 1. Maybe patch 4 should be applied before this one to avoid creating a potential memory leak in the history? 2. taking new references before releasing old ones in modify_flows_start__() would seem better. Since the table holds a reference this does not matter in practice. Jarno > On Mar 15, 2017, at 4:01 PM, Joe Stringer <joe@ovn.org> wrote: > > From: Yi-Hung Wei <yihung.wei@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@ovn.org> > Suggested-by: Joe Stringer <joe@ovn.org> > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > Signed-off-by: Joe Stringer <joe@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 >
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;