From 7eb4b1f1d70345f44ac7f3de89eb0340a5cafb71 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 7 Oct 2014 16:49:50 -0700 Subject: [PATCH] ofp-actions: Support OF1.5 (draft) masked Set-Field, merge with reg_load. OpenFlow 1.5 (draft) extends the OFPAT_SET_FIELD action originally introduced in OpenFlow 1.2 so that it can set not just entire fields but any subset of bits within a field as well. This commit adds support for that feature when OpenFlow 1.5 is used. With this feature, OFPAT_SET_FIELD becomes a superset of NXAST_REG_LOAD. Thus, this commit merges the implementations of the two actions into a single ofpact_set_field. ONF-JIRA: EXT-314 Signed-off-by: Ben Pfaff Acked-by: YAMAMOTO Takashi --- NEWS | 1 + lib/learn.c | 22 +-- lib/meta-flow.c | 30 ++++ lib/meta-flow.h | 4 + lib/nx-match.c | 81 +-------- lib/nx-match.h | 7 - lib/ofp-actions.c | 327 ++++++++++++++++++++++------------- lib/ofp-actions.h | 16 +- ofproto/ofproto-dpif-xlate.c | 10 +- tests/ofp-actions.at | 5 +- utilities/ovs-ofctl.8.in | 39 +++-- 11 files changed, 287 insertions(+), 255 deletions(-) diff --git a/NEWS b/NEWS index 5292d2585..3307754ee 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,7 @@ Post-v2.3.0 - OpenFlow: * OpenFlow 1.5 (draft) extended registers are now supported. * OpenFlow 1.5 (draft) Copy-Field action is now supported. + * OpenFlow 1.5 (draft) masked Set-Field action is now supported. * OpenFlow 1.3+ table features requests are now supported (read-only). * Nicira extension "move" actions may now be included in action sets. * "resubmit" actions may now be included in action sets. The resubmit diff --git a/lib/learn.c b/lib/learn.c index e1c73cb50..c04e1f3bb 100644 --- a/lib/learn.c +++ b/lib/learn.c @@ -120,8 +120,8 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow, } for (spec = learn->specs; spec < &learn->specs[learn->n_specs]; spec++) { + struct ofpact_set_field *sf; union mf_subvalue value; - int chunk, ofs; if (spec->src_type == NX_LEARN_SRC_FIELD) { mf_read_subfield(&spec->src, flow, &value); @@ -135,19 +135,13 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow, break; case NX_LEARN_DST_LOAD: - for (ofs = 0; ofs < spec->n_bits; ofs += chunk) { - struct ofpact_reg_load *load; - - chunk = MIN(spec->n_bits - ofs, 64); - - load = ofpact_put_REG_LOAD(ofpacts); - load->dst.field = spec->dst.field; - load->dst.ofs = spec->dst.ofs + ofs; - load->dst.n_bits = chunk; - bitwise_copy(&value, sizeof value, ofs, - &load->subvalue, sizeof load->subvalue, 0, - chunk); - } + sf = ofpact_put_reg_load(ofpacts); + sf->field = spec->dst.field; + bitwise_copy(&value, sizeof value, 0, + &sf->value, spec->dst.field->n_bytes, spec->dst.ofs, + spec->n_bits); + bitwise_one(&sf->mask, spec->dst.field->n_bytes, spec->dst.ofs, + spec->n_bits); break; case NX_LEARN_DST_OUTPUT: diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 5056be5ae..787154586 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -1076,6 +1076,36 @@ mf_set_flow_value(const struct mf_field *mf, } } +/* Consider each of 'src', 'mask', and 'dst' as if they were arrays of 8*n + * bits. Then, for each 0 <= i < 8 * n such that mask[i] == 1, sets dst[i] = + * src[i]. */ +static void +apply_mask(const uint8_t *src, const uint8_t *mask, uint8_t *dst, size_t n) +{ + size_t i; + + for (i = 0; i < n; i++) { + dst[i] = (src[i] & mask[i]) | (dst[i] & ~mask[i]); + } +} + +/* Sets 'flow' member field described by 'field' to 'value', except that bits + * for which 'mask' has a 0-bit keep their existing values. The caller is + * responsible for ensuring that 'flow' meets 'field''s prerequisites.*/ +void +mf_set_flow_value_masked(const struct mf_field *field, + const union mf_value *value, + const union mf_value *mask, + struct flow *flow) +{ + union mf_value tmp; + + mf_get_value(field, flow, &tmp); + apply_mask((const uint8_t *) value, (const uint8_t *) mask, + (uint8_t *) &tmp, field->n_bytes); + mf_set_flow_value(field, &tmp, flow); +} + /* Returns true if 'mf' has a zero value in 'flow', false if it is nonzero. * * The caller is responsible for ensuring that 'flow' meets 'mf''s diff --git a/lib/meta-flow.h b/lib/meta-flow.h index 7cdacca16..d74816b4d 100644 --- a/lib/meta-flow.h +++ b/lib/meta-flow.h @@ -1538,6 +1538,10 @@ void mf_set_value(const struct mf_field *, const union mf_value *value, struct match *); void mf_set_flow_value(const struct mf_field *, const union mf_value *value, struct flow *); +void mf_set_flow_value_masked(const struct mf_field *, + const union mf_value *value, + const union mf_value *mask, + struct flow *); bool mf_is_zero(const struct mf_field *, const struct flow *); void mf_mask_field(const struct mf_field *, struct flow *); diff --git a/lib/nx-match.c b/lib/nx-match.c index 38233dbdb..238bfdbd0 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -1295,39 +1295,8 @@ nxm_parse_reg_move(struct ofpact_reg_move *move, const char *s) } return NULL; } - -/* Parses 's' as a "load" action, in the form described in ovs-ofctl(8), into - * '*load'. - * - * Returns NULL if successful, otherwise a malloc()'d string describing the - * error. The caller is responsible for freeing the returned string. */ -char * WARN_UNUSED_RESULT -nxm_parse_reg_load(struct ofpact_reg_load *load, const char *s) -{ - const char *full_s = s; - uint64_t value = strtoull(s, (char **) &s, 0); - char *error; - - if (strncmp(s, "->", 2)) { - return xasprintf("%s: missing `->' following value", full_s); - } - s += 2; - error = mf_parse_subfield(&load->dst, s); - if (error) { - return error; - } - - if (load->dst.n_bits < 64 && (value >> load->dst.n_bits) != 0) { - return xasprintf("%s: value %"PRIu64" does not fit into %d bits", - full_s, value, load->dst.n_bits); - } - - load->subvalue.be64[0] = htonll(0); - load->subvalue.be64[1] = htonll(value); - return NULL; -} -/* nxm_format_reg_move(), nxm_format_reg_load(). */ +/* nxm_format_reg_move(). */ void nxm_format_reg_move(const struct ofpact_reg_move *move, struct ds *s) @@ -1338,14 +1307,6 @@ nxm_format_reg_move(const struct ofpact_reg_move *move, struct ds *s) mf_format_subfield(&move->dst, s); } -void -nxm_format_reg_load(const struct ofpact_reg_load *load, struct ds *s) -{ - ds_put_cstr(s, "load:"); - mf_format_subvalue(&load->subvalue, s); - ds_put_cstr(s, "->"); - mf_format_subfield(&load->dst, s); -} enum ofperr nxm_reg_move_check(const struct ofpact_reg_move *move, const struct flow *flow) @@ -1359,15 +1320,8 @@ nxm_reg_move_check(const struct ofpact_reg_move *move, const struct flow *flow) return mf_check_dst(&move->dst, NULL); } - -enum ofperr -nxm_reg_load_check(const struct ofpact_reg_load *load, const struct flow *flow) -{ - return mf_check_dst(&load->dst, flow); -} - -/* nxm_execute_reg_move(), nxm_execute_reg_load(). */ +/* nxm_execute_reg_move(). */ void nxm_execute_reg_move(const struct ofpact_reg_move *move, @@ -1387,37 +1341,6 @@ nxm_execute_reg_move(const struct ofpact_reg_move *move, mf_set_flow_value(move->dst.field, &dst_value, flow); } -void -nxm_execute_reg_load(const struct ofpact_reg_load *load, struct flow *flow, - struct flow_wildcards *wc) -{ - /* Since at the datapath interface we do not have set actions for - * individual fields, but larger sets of fields for a given protocol - * layer, the set action will in practice only ever apply to exactly - * matched flows for the given protocol layer. For example, if the - * reg_load changes the IP TTL, the corresponding datapath action will - * rewrite also the IP addresses and TOS byte. Since these other field - * values may not be explicitly set, they depend on the incoming flow field - * values, and are hence all of them are set in the wildcards masks, when - * the action is committed to the datapath. For the rare case, where the - * reg_load action does not actually change the value, and no other flow - * field values are set (or loaded), the datapath action is skipped, and - * no mask bits are set. Such a datapath flow should, however, be - * dependent on the specific field value, so the corresponding wildcard - * mask bits must be set, lest the datapath flow be applied to packets - * containing some other value in the field and the field value remain - * unchanged regardless of the incoming value. - * - * We set the masks here for the whole fields, and their prerequisities. - * Even if only the lower byte of a TCP destination port is set, - * we set the mask for the whole field, and also the ip_proto in the IP - * header, so that the kernel flow would not be applied on, e.g., a UDP - * packet, or any other IP protocol in addition to TCP packets. - */ - mf_mask_field_and_prereqs(load->dst.field, &wc->masks); - mf_write_subfield_flow(&load->dst, &load->subvalue, flow); -} - void nxm_reg_load(const struct mf_subfield *dst, uint64_t src_data, struct flow *flow, struct flow_wildcards *wc) diff --git a/lib/nx-match.h b/lib/nx-match.h index b3f9694ee..929b1db3c 100644 --- a/lib/nx-match.h +++ b/lib/nx-match.h @@ -88,21 +88,14 @@ void nx_format_field_name(enum mf_field_id, enum ofp_version, struct ds *); char *nxm_parse_reg_move(struct ofpact_reg_move *, const char *) WARN_UNUSED_RESULT; -char *nxm_parse_reg_load(struct ofpact_reg_load *, const char *) - WARN_UNUSED_RESULT; void nxm_format_reg_move(const struct ofpact_reg_move *, struct ds *); -void nxm_format_reg_load(const struct ofpact_reg_load *, struct ds *); enum ofperr nxm_reg_move_check(const struct ofpact_reg_move *, const struct flow *); -enum ofperr nxm_reg_load_check(const struct ofpact_reg_load *, - const struct flow *); void nxm_execute_reg_move(const struct ofpact_reg_move *, struct flow *, struct flow_wildcards *); -void nxm_execute_reg_load(const struct ofpact_reg_load *, struct flow *, - struct flow_wildcards *); void nxm_reg_load(const struct mf_subfield *, uint64_t src_data, struct flow *, struct flow_wildcards *); diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index f6818ca02..97acc896a 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -201,9 +201,14 @@ enum ofp_raw_action_type { /* NX1.0+(21): struct nx_action_cnt_ids, ... */ NXAST_RAW_DEC_TTL_CNT_IDS, - /* OF1.2+(25): struct ofp12_action_set_field, ... */ + /* OF1.2-1.4(25): struct ofp12_action_set_field, ... */ OFPAT_RAW12_SET_FIELD, - /* NX1.0+(7): struct nx_action_reg_load. */ + /* OF1.5+(25): struct ofp12_action_set_field, ... */ + OFPAT_RAW15_SET_FIELD, + /* NX1.0-1.4(7): struct nx_action_reg_load. + * + * [In OpenFlow 1.5, set_field is a superset of reg_load functionality, so + * we drop reg_load.] */ NXAST_RAW_REG_LOAD, /* OF1.5+(28): struct ofp15_action_copy_field, ... */ @@ -1865,6 +1870,20 @@ format_REG_MOVE(const struct ofpact_reg_move *a, struct ds *s) nxm_format_reg_move(a, s); } +/* Action structure for OFPAT12_SET_FIELD. */ +struct ofp12_action_set_field { + ovs_be16 type; /* OFPAT12_SET_FIELD. */ + ovs_be16 len; /* Length is padded to 64 bits. */ + + /* Followed by: + * - An OXM header, value, and (in OpenFlow 1.5+) optionally a mask. + * - Enough 0-bytes to pad out to a multiple of 64 bits. + * + * The "pad" member is the beginning of the above. */ + uint8_t pad[4]; +}; +OFP_ASSERT(sizeof(struct ofp12_action_set_field) == 8); + /* Action structure for NXAST_REG_LOAD. * * Copies value[0:n_bits] to dst[ofs:ofs+n_bits], where a[b:c] denotes the bits @@ -1903,68 +1922,8 @@ struct nx_action_reg_load { OFP_ASSERT(sizeof(struct nx_action_reg_load) == 24); static enum ofperr -decode_NXAST_RAW_REG_LOAD(const struct nx_action_reg_load *narl, - struct ofpbuf *out) -{ - struct ofpact_reg_load *load; - - load = ofpact_put_REG_LOAD(out); - load->dst.field = mf_from_nxm_header(ntohl(narl->dst)); - load->dst.ofs = nxm_decode_ofs(narl->ofs_nbits); - load->dst.n_bits = nxm_decode_n_bits(narl->ofs_nbits); - load->subvalue.be64[1] = narl->value; - - /* Reject 'narl' if a bit numbered 'n_bits' or higher is set to 1 in - * narl->value. */ - if (load->dst.n_bits < 64 && - ntohll(narl->value) >> load->dst.n_bits) { - return OFPERR_OFPBAC_BAD_ARGUMENT; - } - - return nxm_reg_load_check(load, NULL); -} - -static void -encode_REG_LOAD(const struct ofpact_reg_load *load, - enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *out) -{ - struct nx_action_reg_load *narl; - - narl = put_NXAST_REG_LOAD(out); - narl->ofs_nbits = nxm_encode_ofs_nbits(load->dst.ofs, load->dst.n_bits); - narl->dst = htonl(mf_oxm_header(load->dst.field->id, 0)); - narl->value = load->subvalue.be64[1]; -} - -static char * WARN_UNUSED_RESULT -parse_REG_LOAD(char *arg, struct ofpbuf *ofpacts, - enum ofputil_protocol *usable_protocols OVS_UNUSED) -{ - return nxm_parse_reg_load(ofpact_put_REG_LOAD(ofpacts), arg); -} - -static void -format_REG_LOAD(const struct ofpact_reg_load *a, struct ds *s) -{ - nxm_format_reg_load(a, s); -} - -/* Action structure for OFPAT12_SET_FIELD. */ -struct ofp12_action_set_field { - ovs_be16 type; /* OFPAT12_SET_FIELD. */ - ovs_be16 len; /* Length is padded to 64 bits. */ - /* Followed by: - * - An OXM header and value (no mask allowed). - * - Enough 0-bytes to pad out to a multiple of 64 bits. - * - * The "pad" member is the beginning of the above. */ - uint8_t pad[4]; -}; -OFP_ASSERT(sizeof(struct ofp12_action_set_field) == 8); - -static enum ofperr -decode_OFPAT_RAW12_SET_FIELD(const struct ofp12_action_set_field *oasf, - struct ofpbuf *ofpacts) +decode_ofpat_set_field(const struct ofp12_action_set_field *oasf, + bool may_mask, struct ofpbuf *ofpacts) { struct ofpact_set_field *sf; enum ofperr error; @@ -1973,14 +1932,17 @@ decode_OFPAT_RAW12_SET_FIELD(const struct ofp12_action_set_field *oasf, sf = ofpact_put_SET_FIELD(ofpacts); ofpbuf_use_const(&b, oasf, ntohs(oasf->len)); - ofpbuf_pull(&b, 4); - error = nx_pull_entry(&b, &sf->field, &sf->value, NULL); + ofpbuf_pull(&b, OBJECT_OFFSETOF(oasf, pad)); + error = nx_pull_entry(&b, &sf->field, &sf->value, + may_mask ? &sf->mask : NULL); if (error) { - /* OF1.5 specifically says to use OFPBAC_BAD_SET_MASK in this case. */ return (error == OFPERR_OFPBMC_BAD_MASK ? OFPERR_OFPBAC_BAD_SET_MASK : error); } + if (!may_mask) { + memset(&sf->mask, 0xff, sf->field->n_bytes); + } if (!is_all_zeros(ofpbuf_data(&b), ofpbuf_size(&b))) { return OFPERR_OFPBAC_BAD_SET_ARGUMENT; @@ -1999,11 +1961,13 @@ decode_OFPAT_RAW12_SET_FIELD(const struct ofp12_action_set_field *oasf, return OFPERR_OFPBAC_BAD_SET_ARGUMENT; } - /* The value must be valid for match and must have the OFPVID_PRESENT bit - * on for OXM_OF_VLAN_VID. */ + /* The value must be valid for match. The OpenFlow 1.5 draft also says, + * "In an OXM_OF_VLAN_VID set-field action, the OFPVID_PRESENT bit must be + * a 1-bit in oxm_value and in oxm_mask." */ if (!mf_is_value_valid(sf->field, &sf->value) || (sf->field->id == MFF_VLAN_VID - && !(sf->value.be16 & htons(OFPVID12_PRESENT)))) { + && (!(sf->mask.be16 & htons(OFPVID12_PRESENT) + || !(sf->value.be16 & htons(OFPVID12_PRESENT)))))) { struct ds ds = DS_EMPTY_INITIALIZER; mf_format(sf->field, &sf->value, NULL, &ds); VLOG_WARN_RL(&rl, "Invalid value for set field %s: %s", @@ -2015,56 +1979,110 @@ decode_OFPAT_RAW12_SET_FIELD(const struct ofp12_action_set_field *oasf, return 0; } +static enum ofperr +decode_OFPAT_RAW12_SET_FIELD(const struct ofp12_action_set_field *oasf, + struct ofpbuf *ofpacts) +{ + return decode_ofpat_set_field(oasf, false, ofpacts); +} + +static enum ofperr +decode_OFPAT_RAW15_SET_FIELD(const struct ofp12_action_set_field *oasf, + struct ofpbuf *ofpacts) +{ + return decode_ofpat_set_field(oasf, true, ofpacts); +} + +static enum ofperr +decode_NXAST_RAW_REG_LOAD(const struct nx_action_reg_load *narl, + struct ofpbuf *out) +{ + struct ofpact_set_field *sf = ofpact_put_reg_load(out); + struct mf_subfield dst; + enum ofperr error; + + sf->ofpact.raw = NXAST_RAW_REG_LOAD; + + dst.field = mf_from_nxm_header(ntohl(narl->dst)); + dst.ofs = nxm_decode_ofs(narl->ofs_nbits); + dst.n_bits = nxm_decode_n_bits(narl->ofs_nbits); + error = mf_check_dst(&dst, NULL); + if (error) { + return error; + } + + /* Reject 'narl' if a bit numbered 'n_bits' or higher is set to 1 in + * narl->value. */ + if (dst.n_bits < 64 && ntohll(narl->value) >> dst.n_bits) { + return OFPERR_OFPBAC_BAD_ARGUMENT; + } + + sf->field = dst.field; + bitwise_put(ntohll(narl->value), + &sf->value, dst.field->n_bytes, dst.ofs, + dst.n_bits); + bitwise_put(UINT64_MAX, + &sf->mask, dst.field->n_bytes, dst.ofs, + dst.n_bits); + + return 0; +} + static void ofpact_put_set_field(struct ofpbuf *openflow, enum ofp_version ofp_version, enum mf_field_id field, uint64_t value_) { + struct ofp12_action_set_field *oasf OVS_UNUSED; int n_bytes = mf_from_id(field)->n_bytes; size_t start_ofs = ofpbuf_size(openflow); union mf_value value; value.be64 = htonll(value_ << (8 * (8 - n_bytes))); - put_OFPAT12_SET_FIELD(openflow); - ofpbuf_set_size(openflow, ofpbuf_size(openflow) - 4); + oasf = put_OFPAT12_SET_FIELD(openflow); + ofpbuf_set_size(openflow, ofpbuf_size(openflow) - sizeof oasf->pad); nx_put_entry(openflow, field, ofp_version, &value, NULL); pad_ofpat(openflow, start_ofs); } +static bool +next_load_segment(const struct ofpact_set_field *sf, + struct mf_subfield *dst, uint64_t *value) +{ + int w = sf->field->n_bytes; + int start = dst->ofs + dst->n_bits; + + if (start < 8 * w) { + dst->field = sf->field; + dst->ofs = bitwise_scan(&sf->mask, w, 1, start, 8 * w); + if (dst->ofs < 8 * w) { + dst->n_bits = bitwise_scan(&sf->mask, w, 0, dst->ofs + 1, + MIN(dst->ofs + 64, 8 * w)) - dst->ofs; + *value = bitwise_get(&sf->value, w, dst->ofs, dst->n_bits); + return true; + } + } + return false; +} -/* Convert 'sf' to one or two REG_LOADs. */ +/* Convert 'sf' to a series of REG_LOADs. */ static void set_field_to_nxast(const struct ofpact_set_field *sf, struct ofpbuf *openflow) { - const struct mf_field *mf = sf->field; - ovs_be32 header = htonl(mf_oxm_header(mf->id, 0)); - struct nx_action_reg_load *narl; - - if (mf->n_bits > 64) { - ovs_assert(mf->n_bytes == 16); /* IPv6 addr. */ - /* Split into 64bit chunks */ - /* Lower bits first. */ - narl = put_NXAST_REG_LOAD(openflow); - narl->ofs_nbits = nxm_encode_ofs_nbits(0, 64); - narl->dst = header; - memcpy(&narl->value, &sf->value.ipv6.s6_addr[8], sizeof narl->value); - /* Higher bits next. */ - narl = put_NXAST_REG_LOAD(openflow); - narl->ofs_nbits = nxm_encode_ofs_nbits(64, mf->n_bits - 64); - narl->dst = header; - memcpy(&narl->value, &sf->value.ipv6.s6_addr[0], sizeof narl->value); - } else { - narl = put_NXAST_REG_LOAD(openflow); - narl->ofs_nbits = nxm_encode_ofs_nbits(0, mf->n_bits); - narl->dst = header; - memset(&narl->value, 0, 8 - mf->n_bytes); - memcpy((char*)&narl->value + (8 - mf->n_bytes), - &sf->value, mf->n_bytes); + struct mf_subfield dst; + uint64_t value; + + dst.ofs = dst.n_bits = 0; + while (next_load_segment(sf, &dst, &value)) { + struct nx_action_reg_load *narl = put_NXAST_REG_LOAD(openflow); + narl->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs, dst.n_bits); + narl->dst = htonl(mf_oxm_header(dst.field->id, 0)); + narl->value = htonll(value); } } -/* Convert 'sf' to standard OpenFlow 1.0/1.1 actions, if we can, falling back - * to Nicira extensions if we must. +/* Convert 'sf', which must set an entire field, to standard OpenFlow 1.0/1.1 + * actions, if we can, falling back to Nicira extensions if we must. * * We check only meta-flow types that can appear within set field actions and * that have a mapping to compatible action types. These struct mf_field @@ -2184,20 +2202,41 @@ set_field_to_legacy_openflow(const struct ofpact_set_field *sf, } } +static void +set_field_to_set_field(const struct ofpact_set_field *sf, + enum ofp_version ofp_version, struct ofpbuf *out) +{ + struct ofp12_action_set_field *oasf OVS_UNUSED; + size_t start_ofs = ofpbuf_size(out); + + oasf = put_OFPAT12_SET_FIELD(out); + ofpbuf_set_size(out, ofpbuf_size(out) - sizeof oasf->pad); + nx_put_entry(out, sf->field->id, ofp_version, &sf->value, &sf->mask); + pad_ofpat(out, start_ofs); +} + static void encode_SET_FIELD(const struct ofpact_set_field *sf, enum ofp_version ofp_version, struct ofpbuf *out) { - if (ofp_version < OFP12_VERSION) { + if (ofp_version >= OFP15_VERSION) { + /* OF1.5+ only has Set-Field (we drop NXAST_REG_LOAD entirely). */ + set_field_to_set_field(sf, ofp_version, out); + } else if (sf->ofpact.raw == NXAST_RAW_REG_LOAD) { + /* It came in as NXAST_REG_LOAD, send it out the same way. */ + set_field_to_nxast(sf, out); + } else if (ofp_version < OFP12_VERSION) { + /* OpenFlow 1.0 and 1.1 don't have Set-Field. */ set_field_to_legacy_openflow(sf, ofp_version, out); + } else if (is_all_ones((const uint8_t *) &sf->mask, sf->field->n_bytes)) { + /* We're encoding to OpenFlow 1.2, 1.3, or 1.4. The action sets an + * entire field, so encode it as OFPAT_SET_FIELD. */ + set_field_to_set_field(sf, ofp_version, out); } else { - /* Use Set-Field. */ - size_t start_ofs = ofpbuf_size(out); - - put_OFPAT12_SET_FIELD(out); - ofpbuf_set_size(out, ofpbuf_size(out) - 4); - nx_put_entry(out, sf->field->id, ofp_version, &sf->value, NULL); - pad_ofpat(out, start_ofs); + /* We're encoding to OpenFlow 1.2, 1.3, or 1.4. The action cannot be + * encoded as OFPAT_SET_FIELD because it does not set an entire field, + * so encode it as reg_load. */ + set_field_to_nxast(sf, out); } } @@ -2236,7 +2275,7 @@ set_field_parse__(char *arg, struct ofpbuf *ofpacts, } sf->field = mf; delim[0] = '\0'; - error = mf_parse_value(mf, value, &sf->value); + error = mf_parse(mf, value, &sf->value, &sf->mask); if (error) { return error; } @@ -2264,12 +2303,69 @@ parse_SET_FIELD(const char *arg, struct ofpbuf *ofpacts, return error; } +static char * WARN_UNUSED_RESULT +parse_reg_load(char *arg, struct ofpbuf *ofpacts) +{ + struct ofpact_set_field *sf = ofpact_put_reg_load(ofpacts); + const char *full_arg = arg; + uint64_t value = strtoull(arg, (char **) &arg, 0); + struct mf_subfield dst; + char *error; + + if (strncmp(arg, "->", 2)) { + return xasprintf("%s: missing `->' following value", full_arg); + } + arg += 2; + error = mf_parse_subfield(&dst, arg); + if (error) { + return error; + } + + if (dst.n_bits < 64 && (value >> dst.n_bits) != 0) { + return xasprintf("%s: value %"PRIu64" does not fit into %d bits", + full_arg, value, dst.n_bits); + } + + sf->field = dst.field; + memset(&sf->value, 0, sizeof sf->value); + bitwise_put(value, &sf->value, dst.field->n_bytes, dst.ofs, dst.n_bits); + bitwise_put(UINT64_MAX, &sf->mask, + dst.field->n_bytes, dst.ofs, dst.n_bits); + return NULL; +} + static void format_SET_FIELD(const struct ofpact_set_field *a, struct ds *s) { - ds_put_format(s, "set_field:"); - mf_format(a->field, &a->value, NULL, s); - ds_put_format(s, "->%s", a->field->name); + if (a->ofpact.raw == NXAST_RAW_REG_LOAD) { + struct mf_subfield dst; + uint64_t value; + + dst.ofs = dst.n_bits = 0; + while (next_load_segment(a, &dst, &value)) { + ds_put_format(s, "load:%#"PRIx64"->", value); + mf_format_subfield(&dst, s); + ds_put_char(s, ','); + } + ds_chomp(s, ','); + } else { + ds_put_cstr(s, "set_field:"); + mf_format(a->field, &a->value, &a->mask, s); + ds_put_format(s, "->%s", a->field->name); + } +} + +/* Appends an OFPACT_SET_FIELD ofpact to 'ofpacts' and returns it. The ofpact + * is marked such that, if possible, it will be translated to OpenFlow as + * NXAST_REG_LOAD extension actions rather than OFPAT_SET_FIELD, either because + * that was the way that the action was expressed when it came into OVS or for + * backward compatibility. */ +struct ofpact_set_field * +ofpact_put_reg_load(struct ofpbuf *ofpacts) +{ + struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts); + sf->ofpact.raw = NXAST_RAW_REG_LOAD; + return sf; } /* Action structure for NXAST_STACK_PUSH and NXAST_STACK_POP. @@ -4291,7 +4387,6 @@ ofpact_is_set_or_move_action(const struct ofpact *a) { switch (a->type) { case OFPACT_SET_FIELD: - case OFPACT_REG_LOAD: case OFPACT_REG_MOVE: case OFPACT_SET_ETH_DST: case OFPACT_SET_ETH_SRC: @@ -4356,7 +4451,6 @@ ofpact_is_allowed_in_actions_set(const struct ofpact *a) case OFPACT_POP_MPLS: case OFPACT_PUSH_MPLS: case OFPACT_PUSH_VLAN: - case OFPACT_REG_LOAD: case OFPACT_REG_MOVE: case OFPACT_SET_FIELD: case OFPACT_SET_ETH_DST: @@ -4587,7 +4681,6 @@ ovs_instruction_type_from_ofpact_type(enum ofpact_type type) case OFPACT_SET_L4_SRC_PORT: case OFPACT_SET_L4_DST_PORT: case OFPACT_REG_MOVE: - case OFPACT_REG_LOAD: case OFPACT_SET_FIELD: case OFPACT_STACK_PUSH: case OFPACT_STACK_POP: @@ -5091,9 +5184,6 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a, case OFPACT_REG_MOVE: return nxm_reg_move_check(ofpact_get_REG_MOVE(a), flow); - case OFPACT_REG_LOAD: - return nxm_reg_load_check(ofpact_get_REG_LOAD(a), flow); - case OFPACT_SET_FIELD: mf = ofpact_get_SET_FIELD(a)->field; /* Require OXM_OF_VLAN_VID to have an existing VLAN header. */ @@ -5561,7 +5651,6 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port) case OFPACT_SET_L4_SRC_PORT: case OFPACT_SET_L4_DST_PORT: case OFPACT_REG_MOVE: - case OFPACT_REG_LOAD: case OFPACT_SET_FIELD: case OFPACT_STACK_PUSH: case OFPACT_STACK_POP: @@ -5820,6 +5909,8 @@ ofpacts_parse__(char *str, struct ofpbuf *ofpacts, } else if (!strcasecmp(key, "set_tunnel64")) { error = parse_set_tunnel(value, ofpacts, NXAST_RAW_SET_TUNNEL64); + } else if (!strcasecmp(key, "load")) { + error = parse_reg_load(value, ofpacts); } else if (!strcasecmp(key, "bundle_load")) { error = parse_bundle_load(value, ofpacts); } else if (!strcasecmp(key, "drop")) { diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h index 5436f2422..e863cdc59 100644 --- a/lib/ofp-actions.h +++ b/lib/ofp-actions.h @@ -77,7 +77,6 @@ OFPACT(SET_L4_SRC_PORT, ofpact_l4_port, ofpact, "mod_tp_src") \ OFPACT(SET_L4_DST_PORT, ofpact_l4_port, ofpact, "mod_tp_dst") \ OFPACT(REG_MOVE, ofpact_reg_move, ofpact, "move") \ - OFPACT(REG_LOAD, ofpact_reg_load, ofpact, "load") \ OFPACT(STACK_PUSH, ofpact_stack, ofpact, "push") \ OFPACT(STACK_POP, ofpact_stack, ofpact, "pop") \ OFPACT(DEC_TTL, ofpact_cnt_ids, cnt_ids, "dec_ttl") \ @@ -383,23 +382,15 @@ struct ofpact_stack { struct mf_subfield subfield; }; -/* OFPACT_REG_LOAD. - * - * Used for NXAST_REG_LOAD. */ -struct ofpact_reg_load { - struct ofpact ofpact; - struct mf_subfield dst; - union mf_subvalue subvalue; /* Least-significant bits are used. */ -}; - /* OFPACT_SET_FIELD. * - * Used for OFPAT12_SET_FIELD. */ + * Used for NXAST_REG_LOAD and OFPAT12_SET_FIELD. */ struct ofpact_set_field { struct ofpact ofpact; const struct mf_field *field; bool flow_has_vlan; /* VLAN present at action validation time. */ union mf_value value; + union mf_value mask; }; /* OFPACT_PUSH_VLAN/MPLS/PBB @@ -844,6 +835,9 @@ OFPACTS void ofpact_update_len(struct ofpbuf *, struct ofpact *); void ofpact_pad(struct ofpbuf *); +/* Additional functions for composing ofpacts. */ +struct ofpact_set_field *ofpact_put_reg_load(struct ofpbuf *ofpacts); + /* OpenFlow 1.1 instructions. * The order is sorted in execution order. Not in the value of OFPIT11_xxx. * It is enforced on parser from text string. diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 851b9465d..a881dfb1a 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3610,9 +3610,6 @@ ofpact_needs_recirculation_after_mpls(const struct xlate_ctx *ctx, return (mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->dst.field) || mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->src.field)); - case OFPACT_REG_LOAD: - return mf_is_l3_or_higher(ofpact_get_REG_LOAD(a)->dst.field); - case OFPACT_SET_FIELD: return mf_is_l3_or_higher(ofpact_get_SET_FIELD(a)->field); @@ -3797,10 +3794,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, nxm_execute_reg_move(ofpact_get_REG_MOVE(a), flow, wc); break; - case OFPACT_REG_LOAD: - nxm_execute_reg_load(ofpact_get_REG_LOAD(a), flow, wc); - break; - case OFPACT_SET_FIELD: set_field = ofpact_get_SET_FIELD(a); mf = set_field->field; @@ -3819,7 +3812,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, } mf_mask_field_and_prereqs(mf, &wc->masks); - mf_set_flow_value(mf, &set_field->value, flow); + mf_set_flow_value_masked(mf, &set_field->value, &set_field->mask, + flow); break; case OFPACT_STACK_PUSH: diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 5b5a500c7..64b4bc2bc 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -539,10 +539,7 @@ AT_DATA([test-data], [dnl # actions=move:NXM_OF_IN_PORT[]->NXM_OF_VLAN_TCI[] 001c 0018 0010 0000 0000 0008 00000002 00000802 00000000 -# bad OpenFlow15 actions: OFPBAC_BAD_SET_MASK -& ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_SET_MASK): -& 00000000 00 19 00 18 80 00 09 0c-00 00 00 00 12 34 00 00 -& 00000010 00 00 ff ff 00 00 00 00- +# actions=set_field:00:00:00:00:12:34/00:00:00:00:ff:ff->eth_src 0019 0018 8000090c 000000001234 00000000ffff 00000000 ]) diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index ac7c9dc84..bcc3f33f4 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1424,12 +1424,31 @@ In OpenFlow 1.0 through 1.4, \fBmove\fR uses an Open vSwitch extension to OpenFlow. In OpenFlow 1.5, \fBmove\fR uses the OpenFlow 1.5 (draft) standard \fBcopy_field\fR action. . -.IP "\fBload:\fIvalue\fB\->\fIdst\fB[\fIstart\fB..\fIend\fB]" -Writes \fIvalue\fR to bits \fIstart\fR through \fIend\fR, inclusive, -in field \fIdst\fR. -.IP -Example: \fBload:55\->NXM_NX_REG2[0..5]\fR loads value 55 (bit pattern -\fB110111\fR) into bits 0 through 5, inclusive, in register 2. +.IP "\fBset_field:\fIvalue\fR[/\fImask\fR]\fB\->\fIdst" +.IQ "\fBload:\fIvalue\fB\->\fIdst\fB[\fIstart\fB..\fIend\fB]" +Loads a literal value into a field or part of a field. With +\fBset_field\fR, \fBvalue\fR and the optional \fBmask\fR are given in +the customary syntax for field \fIdst\fR, which is expressed as a +field name. For example, \fBset_field:00:11:22:33:44:55->eth_src\fR +sets the Ethernet source address to 00:11:22:33:44:55. With +\fBload\fR, \fIvalue\fR must be an integer value (in decimal or +prefixed by \fB0x\fR for hexadecimal) and \fIdst\fR is the NXM or OXM +name for the field. For example, +\fBload:0x001122334455->OXM_OF_ETH_DST[]\fR has the same effect as the +prior \fBset_field\fR example. +.IP +The two forms exist for historical reasons. Open vSwitch 1.1 +introduced \fBNXAST_REG_LOAD\fR as a Nicira extension to OpenFlow 1.0 +and used \fBload\fR to express it. Later, OpenFlow 1.2 introduced a +standard \fBOFPAT_SET_FIELD\fR action that was restricted to loading +entire fields, so Open vSwitch added the form \fBset_field\fR with +this restriction. OpenFlow 1.5 extended \fBOFPAT_SET_FIELD\fR to the +point that it became a superset of \fBNXAST_REG_LOAD\fR. Open vSwitch +translates either syntax as necessary for the OpenFlow version in use: +in OpenFlow 1.0 and 1.1, \fBNXAST_REG_LOAD\fR; in OpenFlow 1.2, 1.3, +and 1.4, \fBNXAST_REG_LOAD\fR for \fBload\fR or for loading a +subfield, \fBOFPAT_SET_FIELD\fR otherwise; and OpenFlow 1.5 and later, +\fBOFPAT_SET_FIELD\fR. . .IP "\fBpush:\fIsrc\fB[\fIstart\fB..\fIend\fB]" Pushes \fIstart\fR to \fIend\fR bits inclusive, in fields @@ -1448,14 +1467,6 @@ Example: \fBpop:NXM_NX_REG2[0..5]\fR pops the value from top of the stack. Set register 2 bits 0 through 5, inclusive, based on bits 0 through 5 from the value just popped. . -.IP "\fBset_field:\fIvalue\fB\->\fIdst" -Writes the literal \fIvalue\fR into the field \fIdst\fR, which should -be specified as a name used for matching. (This is similar to -\fBload\fR but more closely matches the set-field action defined in -OpenFlow 1.2 and above.) -. -.IP -Example: \fBset_field:00:11:22:33:44:55->eth_src\fR. . .IP "\fBmultipath(\fIfields\fB, \fIbasis\fB, \fIalgorithm\fB, \fIn_links\fB, \fIarg\fB, \fIdst\fB[\fIstart\fB..\fIend\fB])\fR" Hashes \fIfields\fR using \fIbasis\fR as a universal hash parameter, -- 2.20.1