ofp-actions: Support OF1.5 (draft) masked Set-Field, merge with reg_load.
authorBen Pfaff <blp@nicira.com>
Tue, 7 Oct 2014 23:49:50 +0000 (16:49 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 8 Oct 2014 20:56:25 +0000 (13:56 -0700)
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 <blp@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
NEWS
lib/learn.c
lib/meta-flow.c
lib/meta-flow.h
lib/nx-match.c
lib/nx-match.h
lib/ofp-actions.c
lib/ofp-actions.h
ofproto/ofproto-dpif-xlate.c
tests/ofp-actions.at
utilities/ovs-ofctl.8.in

diff --git a/NEWS b/NEWS
index 5292d25..3307754 100644 (file)
--- 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
index e1c73cb..c04e1f3 100644 (file)
@@ -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:
index 5056be5..7871545 100644 (file)
@@ -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
index 7cdacca..d74816b 100644 (file)
@@ -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 *);
 
index 38233db..238bfdb 100644 (file)
@@ -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;
-}
 \f
-/* 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);
-}
 \f
 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);
-}
 \f
-\f
-/* 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)
index b3f9694..929b1db 100644 (file)
@@ -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 *);
 
index f6818ca..97acc89 100644 (file)
@@ -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);
 }
 \f
+/* 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);
-}
-\f
-/* 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;
 }
 \f
 /* 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")) {
index 5436f24..e863cdc 100644 (file)
@@ -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.
index 851b946..a881dfb 100644 (file)
@@ -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:
index 5b5a500..64b4bc2 100644 (file)
@@ -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
 
 ])
index ac7c9dc..bcc3f33 100644 (file)
@@ -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,