Add support for write-actions
authorSimon Horman <horms@verge.net.au>
Fri, 11 Oct 2013 04:23:29 +0000 (13:23 +0900)
committerBen Pfaff <blp@nicira.com>
Mon, 14 Oct 2013 22:20:03 +0000 (15:20 -0700)
Implementation note:

All actions which modify a field are added to the action set
at the point where "set" actions should be added. In general
modifying a field many times is the same as only modifying it
the last time so the implementation simply adds all set actions to
the action set in the order they are specified. However, this breaks
down if two actions modify different portions of the same field.

Some examples.

1. load acting a subfield
2. mod_vlan_vid, mod_vlan_pcp

If this is considered to be a problem one possible solution would be to
either disallow all set actions other than set_field in write_actions.
Another possible solution is prohibit problematic the actions listed above
in write actions.

Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com simplified and edited the code]
Signed-off-by: Ben Pfaff <blp@nicira.com>
NEWS
OPENFLOW-1.1+
lib/ofp-actions.c
lib/ofp-actions.h
lib/ofp-parse.c
ofproto/ofproto-dpif-xlate.c
tests/ofp-actions.at
tests/ofproto-dpif.at
utilities/ovs-ofctl.8.in

diff --git a/NEWS b/NEWS
index 94e0da9..891c3fb 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,8 @@ Post-v2.0.0
      IANA-assigned numbers in a future release.  Consider updating
      your installations to specify port numbers instead of using the
      defaults.
+   - OpenFlow:
+     * The OpenFlow 1.1+ "Write-Actions" instruction is now supported.
 
 
 v2.0.0 - xx xxx xxxx
index 90f811f..07b2660 100644 (file)
@@ -54,9 +54,6 @@ OpenFlow 1.1
 The list of remaining work items for OpenFlow 1.1 is below.  It is
 probably incomplete.
 
-    * Implement Write-Actions instruction.
-      [required for 1.1+]
-
     * The new in_phy_port field in OFPT_PACKET_IN needs some kind of
       implementation.  It has a sensible interpretation for tunnels
       but in general the physical port is not in the datapath for OVS
index 1d0321a..06f9f6b 100644 (file)
@@ -880,6 +880,233 @@ ofpacts_from_openflow11(const union ofp_action *in, size_t n_in,
 {
     return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow11);
 }
+
+/* True if an action sets the value of a field
+ * in a way that is compatibile with the action set.
+ * False otherwise. */
+static bool
+ofpact_is_set_action(const struct ofpact *a)
+{
+    switch (a->type) {
+    case OFPACT_REG_LOAD:
+    case OFPACT_SET_ETH_DST:
+    case OFPACT_SET_ETH_SRC:
+    case OFPACT_SET_IPV4_DSCP:
+    case OFPACT_SET_IPV4_DST:
+    case OFPACT_SET_IPV4_SRC:
+    case OFPACT_SET_L4_DST_PORT:
+    case OFPACT_SET_L4_SRC_PORT:
+    case OFPACT_SET_MPLS_TTL:
+    case OFPACT_SET_QUEUE:
+    case OFPACT_SET_TUNNEL:
+    case OFPACT_SET_VLAN_PCP:
+    case OFPACT_SET_VLAN_VID:
+        return true;
+    case OFPACT_BUNDLE:
+    case OFPACT_CLEAR_ACTIONS:
+    case OFPACT_CONTROLLER:
+    case OFPACT_DEC_MPLS_TTL:
+    case OFPACT_DEC_TTL:
+    case OFPACT_ENQUEUE:
+    case OFPACT_EXIT:
+    case OFPACT_FIN_TIMEOUT:
+    case OFPACT_GOTO_TABLE:
+    case OFPACT_GROUP:
+    case OFPACT_LEARN:
+    case OFPACT_METER:
+    case OFPACT_MULTIPATH:
+    case OFPACT_NOTE:
+    case OFPACT_OUTPUT:
+    case OFPACT_OUTPUT_REG:
+    case OFPACT_POP_MPLS:
+    case OFPACT_POP_QUEUE:
+    case OFPACT_PUSH_MPLS:
+    case OFPACT_PUSH_VLAN:
+    case OFPACT_REG_MOVE:
+    case OFPACT_RESUBMIT:
+    case OFPACT_SAMPLE:
+    case OFPACT_STACK_POP:
+    case OFPACT_STACK_PUSH:
+    case OFPACT_STRIP_VLAN:
+    case OFPACT_WRITE_ACTIONS:
+    case OFPACT_WRITE_METADATA:
+        return false;
+    default:
+        NOT_REACHED();
+    }
+}
+
+/* True if an action is allowed in the action set.
+ * False otherwise. */
+static bool
+ofpact_is_allowed_in_actions_set(const struct ofpact *a)
+{
+    switch (a->type) {
+    case OFPACT_DEC_MPLS_TTL:
+    case OFPACT_DEC_TTL:
+    case OFPACT_GROUP:
+    case OFPACT_OUTPUT:
+    case OFPACT_POP_MPLS:
+    case OFPACT_PUSH_MPLS:
+    case OFPACT_PUSH_VLAN:
+    case OFPACT_REG_LOAD:
+    case OFPACT_SET_ETH_DST:
+    case OFPACT_SET_ETH_SRC:
+    case OFPACT_SET_IPV4_DSCP:
+    case OFPACT_SET_IPV4_DST:
+    case OFPACT_SET_IPV4_SRC:
+    case OFPACT_SET_L4_DST_PORT:
+    case OFPACT_SET_L4_SRC_PORT:
+    case OFPACT_SET_MPLS_TTL:
+    case OFPACT_SET_QUEUE:
+    case OFPACT_SET_TUNNEL:
+    case OFPACT_SET_VLAN_PCP:
+    case OFPACT_SET_VLAN_VID:
+    case OFPACT_STRIP_VLAN:
+        return true;
+
+    /* In general these actions are excluded because they are not part of
+     * the OpenFlow specification nor map to actions that are defined in
+     * the specification.  Thus the order in which they should be applied
+     * in the action set is undefined. */
+    case OFPACT_BUNDLE:
+    case OFPACT_CONTROLLER:
+    case OFPACT_ENQUEUE:
+    case OFPACT_EXIT:
+    case OFPACT_FIN_TIMEOUT:
+    case OFPACT_LEARN:
+    case OFPACT_MULTIPATH:
+    case OFPACT_NOTE:
+    case OFPACT_OUTPUT_REG:
+    case OFPACT_POP_QUEUE:
+    case OFPACT_REG_MOVE:
+    case OFPACT_RESUBMIT:
+    case OFPACT_SAMPLE:
+    case OFPACT_STACK_POP:
+    case OFPACT_STACK_PUSH:
+
+    /* The action set may only include actions and thus
+     * may not include any instructions */
+    case OFPACT_CLEAR_ACTIONS:
+    case OFPACT_GOTO_TABLE:
+    case OFPACT_METER:
+    case OFPACT_WRITE_ACTIONS:
+    case OFPACT_WRITE_METADATA:
+        return false;
+    default:
+        NOT_REACHED();
+    }
+}
+
+/* Append ofpact 'a' onto the tail of 'out' */
+static void
+ofpact_copy(struct ofpbuf *out, const struct ofpact *a)
+{
+    ofpbuf_put(out, a, OFPACT_ALIGN(a->len));
+}
+
+/* Copies the last ofpact whose type is 'filter' from 'in' to 'out'. */
+static bool
+ofpacts_copy_last(struct ofpbuf *out, const struct ofpbuf *in,
+                  enum ofpact_type filter)
+{
+    const struct ofpact *target;
+    const struct ofpact *a;
+
+    target = NULL;
+    OFPACT_FOR_EACH (a, in->data, in->size) {
+        if (a->type == filter) {
+            target = a;
+        }
+    }
+    if (target) {
+        ofpact_copy(out, target);
+    }
+    return target != NULL;
+}
+
+/* Append all ofpacts, for which 'filter' returns true, from 'in' to 'out'.
+ * The order of appended ofpacts is preserved between 'in' and 'out' */
+static void
+ofpacts_copy_all(struct ofpbuf *out, const struct ofpbuf *in,
+                 bool (*filter)(const struct ofpact *))
+{
+    const struct ofpact *a;
+
+    OFPACT_FOR_EACH (a, in->data, in->size) {
+        if (filter(a)) {
+            ofpact_copy(out, a);
+        }
+    }
+}
+
+/* Reads 'action_set', which contains ofpacts accumulated by
+ * OFPACT_WRITE_ACTIONS instructions, and writes equivalent actions to be
+ * executed directly into 'action_list'.  (These names correspond to the
+ * "Action Set" and "Action List" terms used in OpenFlow 1.1+.)
+ *
+ * In general this involves appending the last instance of each action that is
+ * adimissible in the action set in the order described in the OpenFlow
+ * specification.
+ *
+ * Exceptions:
+ * + output action is only appended if no group action was present in 'in'.
+ * + As a simplification all set actions are copied in the order the are
+ *   provided in 'in' as many set actions applied to a field has the same
+ *   affect as only applying the last action that sets a field and
+ *   duplicates are removed by do_xlate_actions().
+ *   This has an unwanted side-effect of compsoting multiple
+ *   LOAD_REG actions that touch different regions of the same field. */
+void
+ofpacts_execute_action_set(struct ofpbuf *action_list,
+                           const struct ofpbuf *action_set)
+{
+    /* The OpenFlow spec "Action Set" section specifies this order. */
+    ofpacts_copy_last(action_list, action_set, OFPACT_STRIP_VLAN);
+    ofpacts_copy_last(action_list, action_set, OFPACT_POP_MPLS);
+    ofpacts_copy_last(action_list, action_set, OFPACT_PUSH_MPLS);
+    ofpacts_copy_last(action_list, action_set, OFPACT_PUSH_VLAN);
+    ofpacts_copy_last(action_list, action_set, OFPACT_DEC_TTL);
+    ofpacts_copy_last(action_list, action_set, OFPACT_DEC_MPLS_TTL);
+    ofpacts_copy_all(action_list, action_set, ofpact_is_set_action);
+    ofpacts_copy_last(action_list, action_set, OFPACT_SET_QUEUE);
+
+    /* If both OFPACT_GROUP and OFPACT_OUTPUT are present, OpenFlow says that
+     * we should execute only OFPACT_GROUP.
+     *
+     * If neither OFPACT_GROUP nor OFPACT_OUTPUT is present, then we can drop
+     * all the actions because there's no point in modifying a packet that will
+     * not be sent anywhere. */
+    if (!ofpacts_copy_last(action_list, action_set, OFPACT_GROUP) &&
+        !ofpacts_copy_last(action_list, action_set, OFPACT_OUTPUT)) {
+        ofpbuf_clear(action_list);
+    }
+}
+
+
+static enum ofperr
+ofpacts_from_openflow11_for_action_set(const union ofp_action *in,
+                                       size_t n_in, struct ofpbuf *out)
+{
+    enum ofperr error;
+    struct ofpact *a;
+    size_t start = out->size;
+
+    error = ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow11);
+    if (error) {
+        return error;
+    }
+
+    OFPACT_FOR_EACH (a, ofpact_end(out->data, start), out->size - start) {
+        if (!ofpact_is_allowed_in_actions_set(a)) {
+            VLOG_WARN_RL(&rl, "disallowed action in action set");
+            return OFPERR_OFPBAC_BAD_TYPE;
+        }
+    }
+
+    return 0;
+}
+
 \f
 /* OpenFlow 1.1 instructions. */
 
@@ -946,6 +1173,8 @@ ovs_instruction_type_from_ofpact_type(enum ofpact_type type)
         return OVSINST_OFPIT13_METER;
     case OFPACT_CLEAR_ACTIONS:
         return OVSINST_OFPIT11_CLEAR_ACTIONS;
+    case OFPACT_WRITE_ACTIONS:
+        return OVSINST_OFPIT11_WRITE_ACTIONS;
     case OFPACT_WRITE_METADATA:
         return OVSINST_OFPIT11_WRITE_METADATA;
     case OFPACT_GOTO_TABLE:
@@ -1170,7 +1399,23 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
             insts[OVSINST_OFPIT11_CLEAR_ACTIONS]);
         ofpact_put_CLEAR_ACTIONS(ofpacts);
     }
-    /* XXX Write-Actions */
+    if (insts[OVSINST_OFPIT11_WRITE_ACTIONS]) {
+        struct ofpact_nest *on;
+        const union ofp_action *actions;
+        size_t n_actions;
+        size_t start = ofpacts->size;
+
+        on = ofpact_put(ofpacts, OFPACT_WRITE_ACTIONS,
+                        offsetof(struct ofpact_nest, actions));
+        get_actions_from_instruction(insts[OVSINST_OFPIT11_WRITE_ACTIONS],
+                                     &actions, &n_actions);
+        error = ofpacts_from_openflow11_for_action_set(actions, n_actions,
+                                                       ofpacts);
+        if (error) {
+            goto exit;
+        }
+        on->ofpact.len = ofpacts->size - start;
+    }
     if (insts[OVSINST_OFPIT11_WRITE_METADATA]) {
         const struct ofp11_instruction_write_metadata *oiwm;
         struct ofpact_metadata *om;
@@ -1192,11 +1437,6 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
         ogt->table_id = oigt->table_id;
     }
 
-    if (insts[OVSINST_OFPIT11_WRITE_ACTIONS]) {
-        error = OFPERR_OFPBIC_UNSUP_INST;
-        goto exit;
-    }
-
     error = ofpacts_verify(ofpacts->data, ofpacts->size);
 exit:
     if (error) {
@@ -1292,6 +1532,14 @@ ofpact_check__(const struct ofpact *a, struct flow *flow, ofp_port_t max_ports,
         return 0;
 
     case OFPACT_CLEAR_ACTIONS:
+        return 0;
+
+    case OFPACT_WRITE_ACTIONS: {
+        struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
+        return ofpacts_check(on->actions, ofpact_nest_get_action_len(on),
+                             flow, max_ports, table_id);
+    }
+
     case OFPACT_WRITE_METADATA:
         return 0;
 
@@ -1353,7 +1601,9 @@ ofpacts_verify(const struct ofpact ofpacts[], size_t ofpacts_len)
         enum ovs_instruction_type next;
 
         next = ovs_instruction_type_from_ofpact_type(a->type);
-        if (inst != OVSINST_OFPIT11_APPLY_ACTIONS && next <= inst) {
+        if (inst == OVSINST_OFPIT11_APPLY_ACTIONS
+            ? next < inst
+            : next <= inst) {
             const char *name = ovs_instruction_name_from_type(inst);
             const char *next_name = ovs_instruction_name_from_type(next);
 
@@ -1622,6 +1872,7 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf *out)
     case OFPACT_SET_IPV4_DSCP:
     case OFPACT_SET_L4_SRC_PORT:
     case OFPACT_SET_L4_DST_PORT:
+    case OFPACT_WRITE_ACTIONS:
     case OFPACT_CLEAR_ACTIONS:
     case OFPACT_GOTO_TABLE:
     case OFPACT_METER:
@@ -1716,6 +1967,7 @@ ofpact_to_openflow10(const struct ofpact *a, struct ofpbuf *out)
 
     case OFPACT_PUSH_VLAN:
     case OFPACT_CLEAR_ACTIONS:
+    case OFPACT_WRITE_ACTIONS:
     case OFPACT_GOTO_TABLE:
     case OFPACT_METER:
         /* XXX */
@@ -1892,6 +2144,7 @@ ofpact_to_openflow11(const struct ofpact *a, struct ofpbuf *out)
         break;
 
     case OFPACT_CLEAR_ACTIONS:
+    case OFPACT_WRITE_ACTIONS:
     case OFPACT_GOTO_TABLE:
     case OFPACT_METER:
         NOT_REACHED();
@@ -2016,8 +2269,19 @@ ofpacts_put_openflow11_instructions(const struct ofpact ofpacts[],
             break;
         }
 
-        case OVSINST_OFPIT11_WRITE_ACTIONS:
-            NOT_REACHED();
+        case OVSINST_OFPIT11_WRITE_ACTIONS: {
+            const size_t ofs = openflow->size;
+            const struct ofpact_nest *on;
+
+            on = ofpact_get_WRITE_ACTIONS(a);
+            instruction_put_OFPIT11_WRITE_ACTIONS(openflow);
+            ofpacts_put_openflow11_actions(on->actions,
+                                           ofpact_nest_get_action_len(on),
+                                           openflow);
+            ofpacts_update_instruction_actions(openflow, ofs);
+
+            break;
+        }
         }
     }
 }
@@ -2068,6 +2332,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port)
     case OFPACT_POP_MPLS:
     case OFPACT_SAMPLE:
     case OFPACT_CLEAR_ACTIONS:
+    case OFPACT_WRITE_ACTIONS:
     case OFPACT_GOTO_TABLE:
     case OFPACT_METER:
     case OFPACT_GROUP:
@@ -2416,6 +2681,16 @@ ofpact_format(const struct ofpact *a, struct ds *s)
             sample->obs_domain_id, sample->obs_point_id);
         break;
 
+    case OFPACT_WRITE_ACTIONS: {
+        struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
+        ds_put_format(s, "%s(",
+                      ovs_instruction_name_from_type(
+                          OVSINST_OFPIT11_WRITE_ACTIONS));
+        ofpacts_format(on->actions, ofpact_nest_get_action_len(on), s);
+        ds_put_char(s, ')');
+        break;
+    }
+
     case OFPACT_CLEAR_ACTIONS:
         ds_put_format(s, "%s",
                       ovs_instruction_name_from_type(
index 0876ed7..5996651 100644 (file)
@@ -17,6 +17,7 @@
 #ifndef OFP_ACTIONS_H
 #define OFP_ACTIONS_H 1
 
+#include <stddef.h>
 #include <stdint.h>
 #include "meta-flow.h"
 #include "ofp-errors.h"
                                                                     \
     /* Instructions */                                              \
     DEFINE_OFPACT(METER,           ofpact_meter,         ofpact)    \
-    /* XXX Write-Actions */                                         \
     DEFINE_OFPACT(CLEAR_ACTIONS,   ofpact_null,          ofpact)    \
+    DEFINE_OFPACT(WRITE_ACTIONS,   ofpact_nest,          ofpact)    \
     DEFINE_OFPACT(WRITE_METADATA,  ofpact_metadata,      ofpact)    \
     DEFINE_OFPACT(GOTO_TABLE,      ofpact_goto_table,    ofpact)
 
@@ -384,6 +385,25 @@ struct ofpact_meter {
     uint32_t meter_id;
 };
 
+/* OFPACT_WRITE_ACTIONS.
+ *
+ * Used for OFPIT11_WRITE_ACTIONS. */
+struct ofpact_nest {
+    struct ofpact ofpact;
+    uint8_t pad[OFPACT_ALIGN(sizeof(struct ofpact)) - sizeof(struct ofpact)];
+    struct ofpact actions[];
+};
+BUILD_ASSERT_DECL(offsetof(struct ofpact_nest, actions) == OFPACT_ALIGNTO);
+
+static inline size_t
+ofpact_nest_get_action_len(const struct ofpact_nest *on)
+{
+    return on->ofpact.len - offsetof(struct ofpact_nest, actions);
+}
+
+void ofpacts_execute_action_set(struct ofpbuf *action_list,
+                                const struct ofpbuf *action_set);
+
 /* OFPACT_RESUBMIT.
  *
  * Used for NXAST_RESUBMIT, NXAST_RESUBMIT_TABLE. */
@@ -665,5 +685,4 @@ enum ovs_instruction_type ovs_instruction_type_from_ofpact_type(
 
 void ofpact_set_field_init(struct ofpact_reg_load *load,
                            const struct mf_field *mf, const void *src);
-
 #endif /* ofp-actions.h */
index 7ca7305..f55eb3f 100644 (file)
@@ -879,12 +879,11 @@ str_to_ofpact__(char *pos, char *act, char *arg,
  * Returns NULL if successful, otherwise a malloc()'d string describing the
  * error.  The caller is responsible for freeing the returned string. */
 static char * WARN_UNUSED_RESULT
-str_to_ofpacts(char *str, struct ofpbuf *ofpacts,
-               enum ofputil_protocol *usable_protocols)
+str_to_ofpacts__(char *str, struct ofpbuf *ofpacts,
+                 enum ofputil_protocol *usable_protocols)
 {
     size_t orig_size = ofpacts->size;
     char *pos, *act, *arg;
-    enum ofperr error;
     int n_actions;
 
     pos = str;
@@ -899,13 +898,34 @@ str_to_ofpacts(char *str, struct ofpbuf *ofpacts,
         n_actions++;
     }
 
+    ofpact_pad(ofpacts);
+    return NULL;
+}
+
+
+/* Parses 'str' as a series of actions, and appends them to 'ofpacts'.
+ *
+ * Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error.  The caller is responsible for freeing the returned string. */
+static char * WARN_UNUSED_RESULT
+str_to_ofpacts(char *str, struct ofpbuf *ofpacts,
+               enum ofputil_protocol *usable_protocols)
+{
+    size_t orig_size = ofpacts->size;
+    char *error_s;
+    enum ofperr error;
+
+    error_s = str_to_ofpacts__(str, ofpacts, usable_protocols);
+    if (error_s) {
+        return error_s;
+    }
+
     error = ofpacts_verify(ofpacts->data, ofpacts->size);
     if (error) {
         ofpacts->size = orig_size;
         return xstrdup("Incorrect action ordering");
     }
 
-    ofpact_pad(ofpacts);
     return NULL;
 }
 
@@ -929,10 +949,22 @@ parse_named_instruction(enum ovs_instruction_type type,
         NOT_REACHED();  /* This case is handled by str_to_inst_ofpacts() */
         break;
 
-    case OVSINST_OFPIT11_WRITE_ACTIONS:
-        /* XXX */
-        error_s = xstrdup("instruction write-actions is not supported yet");
+    case OVSINST_OFPIT11_WRITE_ACTIONS: {
+        struct ofpact_nest *on;
+        size_t ofs = ofpacts->size;
+
+        on = ofpact_put(ofpacts, OFPACT_WRITE_ACTIONS,
+                        offsetof(struct ofpact_nest, actions));
+        error_s = str_to_ofpacts__(arg, ofpacts, usable_protocols);
+
+        on = ofpbuf_at_assert(ofpacts, ofs, sizeof *on);
+        on->ofpact.len = ofpacts->size - ofs;
+
+        if (error_s) {
+            ofpacts->size = ofs;
+        }
         break;
+    }
 
     case OVSINST_OFPIT11_CLEAR_ACTIONS:
         ofpact_put_CLEAR_ACTIONS(ofpacts);
index efa5cbe..9313a7f 100644 (file)
@@ -179,6 +179,15 @@ struct xlate_ctx {
     odp_port_t sflow_odp_port;  /* Output port for composing sFlow action. */
     uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
     bool exit;                  /* No further actions should be processed. */
+
+    /* OpenFlow 1.1+ action set.
+     *
+     * 'action_set' accumulates "struct ofpact"s added by OFPACT_WRITE_ACTIONS.
+     * When translation is otherwise complete, ofpacts_execute_action_set()
+     * converts it to a set of "struct ofpact"s that can be translated into
+     * datapath actions.   */
+    struct ofpbuf action_set;   /* Action set. */
+    uint64_t action_set_stub[1024 / 8];
 };
 
 /* A controller may use OFPP_NONE as the ingress port to indicate that
@@ -2246,6 +2255,26 @@ may_receive(const struct xport *xport, struct xlate_ctx *ctx)
     return true;
 }
 
+static void
+xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a)
+{
+    struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
+    ofpbuf_put(&ctx->action_set, on->actions, ofpact_nest_get_action_len(on));
+    ofpact_pad(&ctx->action_set);
+}
+
+static void
+xlate_action_set(struct xlate_ctx *ctx)
+{
+    uint64_t action_list_stub[1024 / 64];
+    struct ofpbuf action_list;
+
+    ofpbuf_use_stub(&action_list, action_list_stub, sizeof action_list_stub);
+    ofpacts_execute_action_set(&action_list, &ctx->action_set);
+    do_xlate_actions(action_list.data, action_list.size, ctx);
+    ofpbuf_uninit(&action_list);
+}
+
 static void
 do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
                  struct xlate_ctx *ctx)
@@ -2457,11 +2486,11 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_CLEAR_ACTIONS:
-            /* XXX
-             * Nothing to do because writa-actions is not supported for now.
-             * When writa-actions is supported, clear-actions also must
-             * be supported at the same time.
-             */
+            ofpbuf_clear(&ctx->action_set);
+            break;
+
+        case OFPACT_WRITE_ACTIONS:
+            xlate_write_actions(ctx, a);
             break;
 
         case OFPACT_WRITE_METADATA:
@@ -2759,6 +2788,8 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     }
 
     ofpbuf_use_stub(&ctx.stack, ctx.init_stack, sizeof ctx.init_stack);
+    ofpbuf_use_stub(&ctx.action_set,
+                    ctx.action_set_stub, sizeof ctx.action_set_stub);
 
     if (mbridge_has_mirrors(ctx.xbridge->mbridge)) {
         /* Do this conditionally because the copy is expensive enough that it
@@ -2817,6 +2848,10 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
             }
         }
 
+        if (ctx.action_set.size) {
+            xlate_action_set(&ctx);
+        }
+
         if (ctx.xbridge->has_in_band
             && in_band_must_output_to_local_port(flow)
             && !actions_output_to_local_port(&ctx)) {
@@ -2840,6 +2875,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     }
 
     ofpbuf_uninit(&ctx.stack);
+    ofpbuf_uninit(&ctx.action_set);
 
     /* Clear the metadata and register wildcard masks, because we won't
      * use non-header fields as part of the cache. */
index ebad040..0909ce1 100644 (file)
@@ -416,9 +416,29 @@ dnl and OVS reorders it to the canonical order)
 # 31: ff -> 00
 0001 0008 01 000000 0002 0018 00000000 fedcba9876543210 ffffffffffffffff
 
-dnl Write-Actions not supported yet.
-# bad OF1.1 instructions: OFPBIC_UNSUP_INST
-0003 0008 01 000000
+dnl empty Write-Actions non-zero padding
+# actions=write_actions(drop)
+#  0: 00 -> (none)
+#  1: 03 -> (none)
+#  2: 00 -> (none)
+#  3: 08 -> (none)
+#  4: 00 -> (none)
+#  5: 00 -> (none)
+#  6: 00 -> (none)
+#  7: 01 -> (none)
+0003 0008 00000001
+
+dnl Check that an empty Write-Actions instruction gets dropped.
+# actions=write_actions(drop)
+#  0: 00 -> (none)
+#  1: 03 -> (none)
+#  2: 00 -> (none)
+#  3: 08 -> (none)
+#  4: 00 -> (none)
+#  5: 00 -> (none)
+#  6: 00 -> (none)
+#  7: 00 -> (none)
+0003 0008 00000000
 
 dnl Clear-Actions too-long
 # bad OF1.1 instructions: OFPBIC_BAD_LEN
index 5316ce9..3630cda 100644 (file)
@@ -34,6 +34,37 @@ AT_CHECK([tail -1 stdout], [0],
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - write actions])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10], [11], [12], [13])
+AT_DATA([flows.txt], [dnl
+table=0 in_port=1,ip actions=output(10),write_actions(set_field:192.168.3.90->ip_src,output(12)),goto_table(1)
+table=1 ip actions=write_actions(output(13)),goto_table(2)
+table=2 ip actions=set_field:192.168.3.91->ip_src,output(11)
+])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flows br0 flows.txt])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 10,set(ipv4(src=192.168.3.91,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),11,set(ipv4(src=192.168.3.90,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),13
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - clear actions])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10], [11], [12])
+AT_DATA([flows.txt], [dnl
+table=0 in_port=1,ip actions=output(10),write_actions(set_field:192.168.3.90->ip_src,output(12)),goto_table(1)
+table=1 ip actions=set_field:192.168.3.91->ip_src,output(11),clear_actions
+])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flows br0 flows.txt])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 10,set(ipv4(src=192.168.3.91,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),11
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - registers])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [20], [21], [22], [33], [90])
index c43b48c..b2bc8d7 100644 (file)
@@ -1346,6 +1346,87 @@ to \fBactions=\fR field.
 .IP \fBclear_actions\fR
 Clears all the actions in the action set immediately.
 .
+.IP \fBwrite_actions(\fR[\fIaction\fR][\fB,\fIaction\fR...]\fB)
+Add the specific actions to the action set.  The syntax of
+\fIactions\fR is the same as in the \fBactions=\fR field.  The action
+set is carried between flow tables and then executed at the end of the
+pipeline.
+.
+.IP
+The actions in the action set are applied in the following order, as
+required by the OpenFlow specification, regardless of the order in
+which they were added to the action set.  Except as specified
+otherwise below, the action set only holds at most a single action of
+each type.  When more than one action of a single type is written to
+the action set, the one written later replaces the earlier action:
+.
+.RS
+.IP 1.
+\fBstrip_vlan\fR
+.IQ
+\fBpop_mpls\fR
+.
+.IP 2.
+\fBpush_mpls\fR
+.
+.IP 3.
+\fBpush_vlan\fR
+.
+.IP 4.
+\fBdec_ttl\fR
+.IQ
+\fBdec_mpls_ttl\fR
+.
+.IP 5.
+\fBload\fR
+.IQ
+\fBmod_dl_dst\fR
+.IQ
+\fBmod_dl_src\fR
+.IQ
+\fBmod_nw_dst\fR
+.IQ
+\fBmod_nw_src\fR
+.IQ
+\fBmod_nw_tos\fR
+.IQ
+\fBmod_tp_dst\fR
+.IQ
+\fBmod_tp_src\fR
+.IQ
+\fBmod_vlan_pcp\fR
+.IQ
+\fBmod_vlan_vid\fR
+.IQ
+\fBset_field\fR
+.IQ
+\fBset_tunnel\fR
+.IQ
+\fBset_tunnel64\fR
+.IQ
+The action set can contain any number of these actions, with
+cumulative effect.  That is, when multiple actions modify the same
+part of a field, the later modification takes effect, and when they
+modify different parts of a field (or different fields), then both
+modifications are applied.
+.
+.IP 6.
+\fBset_queue\fR
+.
+.IP 7.
+\fBgroup\fR
+.IQ
+\fBoutput\fR
+.IQ
+If both actions are present, then \fBgroup\fR is executed and
+\fBoutput\fR is ignored, regardless of the order in which they were
+added to the action set.  (If neither action is present, the action
+set has no real effect, because the modified packet is not sent
+anywhere and thus the modifications are not visible.)
+.RE
+.IP
+Only the actions listed above may be written to the action set.
+.
 .IP \fBwrite_metadata\fB:\fIvalue\fR[/\fImask\fR]
 Updates the metadata field for the flow. If \fImask\fR is omitted, the
 metadata field is set exactly to \fIvalue\fR; if \fImask\fR is specified, then
@@ -1409,10 +1490,12 @@ configuring sample collector sets.
 This action was added in Open vSwitch 1.10.90.
 .
 .IP "\fBexit\fR"
-This action causes Open vSwitch to immediately halt execution of further
-actions.  Those actions which have already been executed are unaffected.  Any
-further actions, including those which may be in other tables, or different
-levels of the \fBresubmit\fR call stack, are ignored.
+This action causes Open vSwitch to immediately halt execution of
+further actions.  Those actions which have already been executed are
+unaffected.  Any further actions, including those which may be in
+other tables, or different levels of the \fBresubmit\fR call stack,
+are ignored.  Actions in the action set is still executed (specify
+\fBclear_actions\fR before \fBexit\fR to discard them).
 .RE
 .
 .PP