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