Acked-by: Jarno Rajahalme <[email protected]> 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 <[email protected]> wrote: > > From: Yi-Hung Wei <[email protected]> > > 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 <[email protected]> > Suggested-by: Joe Stringer <[email protected]> > Signed-off-by: Yi-Hung Wei <[email protected]> > Signed-off-by: Joe Stringer <[email protected]> > --- > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
