ofp-actions: Include OFPACT_REG_MOVE in action set

Treating OFPACT_REG_MOVE as a "set" action preserves the order of loads
and moves and allows a load to overwrite a previous move to the same
register.

This makes the following work:

add-group br0 group_id=1234,type=all, \
              bucket=output:10,move:NXM_NX_REG1[]->NXM_OF_IP_SRC[], \
			  bucket=output:11
add-flow br0 ip actions=load:0xffffffff->NXM_NX_REG1[],group:1234

Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/NEWS b/NEWS
index 7085efa..1ea7bda 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,7 @@ Post-v2.3.0
      * OpenFlow 1.5 (draft) extended registers are now supported.
      * OpenFlow 1.5 (draft) Copy-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.
 
 
 v2.3.0 - 14 Aug 2014

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ed30ec2..61ee5dc 100644 (file)
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -4276,13 +4276,15 @@ ofpacts_pull_openflow_actions(struct ofpbuf *openflow,
 
 /* True if an action sets the value of a field
  * in a way that is compatibile with the action set.
+ * The field can be set via either a set or a move action.
  * False otherwise. */
 static bool
-ofpact_is_set_action(const struct ofpact *a)
+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:
     case OFPACT_SET_IP_DSCP:
@@ -4320,7 +4322,6 @@ ofpact_is_set_action(const struct ofpact *a)
     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:
@@ -4348,6 +4349,7 @@ ofpact_is_allowed_in_actions_set(const struct ofpact *a)
     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:
     case OFPACT_SET_ETH_SRC:
@@ -4382,7 +4384,6 @@ ofpact_is_allowed_in_actions_set(const struct ofpact *a)
     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:
@@ -4471,7 +4472,7 @@ ofpacts_execute_action_set(struct ofpbuf *action_list,
     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_all(action_list, action_set, ofpact_is_set_or_move_action);
     ofpacts_copy_last(action_list, action_set, OFPACT_SET_QUEUE);
 
     /* If both OFPACT_GROUP and OFPACT_OUTPUT are present, OpenFlow says that

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index eda6939..17fe974 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -431,6 +431,19 @@ AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - load and move order])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10], [11])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0 'group_id=1234,type=all,bucket=output:10,move:NXM_NX_REG1[[]]->NXM_OF_IP_SRC[[]],bucket=output:11'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=write_actions(load:0xffffffff->NXM_NX_REG1[[]],move:NXM_NX_REG1[[]]->NXM_NX_REG2[[]],group:1234)'])
+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 -2 stdout], [0],
+  [Megaflow: recirc_id=0,skb_priority=0,icmp,in_port=1,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=128
+Datapath actions: set(ipv4(src=255.255.255.255,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),10,set(ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no)),11
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - push-pop])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [20], [21], [22], [33], [90])

diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index aafda23..d31c173 100644 (file)
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1605,6 +1605,8 @@ the action set, the one written later replaces the earlier action:
 .IP 5.
 \fBload\fR
 .IQ
+\fBmove\fR
+.IQ
 \fBmod_dl_dst\fR
 .IQ
 \fBmod_dl_src\fR
@@ -1634,9 +1636,10 @@ the action set, the one written later replaces the earlier action:
 \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
+cumulative effect. They will be applied in the order as added.
+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.