Some actions require an MPLS dl_type while others require a non-MPLS
dl_type. Enforce this for actions handled in ofpact_check__().

Update the following actions to require an non-MPLS dl_type:

set_ipv4_src
set_ipv4_dst
set_ipv4_dscp
set_l4_src_port
set_l4_dst_port
dec_ttl

Update the following actions to require an MPLS dl_type:

set_mpls_ttl
dec_mpls_ttl
pop_mpls

Update handling of the following actions to use the dl_type set by MPLS
push and pop actions if it differs from the orginal dl_type. This allows
their existing checks to encorce dl_type pre-requisites correctly.

        output_reg
        bundle
        reg_move
        stack_push
        stack_pop
        learn
        multipath

Signed-off-by: Simon Horman <ho...@verge.net.au>
---
 lib/ofp-actions.c     | 85 +++++++++++++++++++++++++++++++++++++--------------
 tests/ofproto-dpif.at |  4 +--
 2 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 068699f..2576f20 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1136,6 +1136,18 @@ exit:
     return error;
 }
 
+static const struct flow *update_flow(const struct flow *flow,
+                                       struct flow *flow_storage,
+                                       ovs_be16 dl_type)
+{
+    if (flow->dl_type == dl_type) {
+        return flow;
+    }
+    *flow_storage = *flow;
+    flow_storage->dl_type = dl_type;
+    return flow_storage;
+}
+
 static enum ofperr
 ofpact_check__(const struct ofpact *a, const struct flow *flow, int max_ports,
                ovs_be16 *dl_type)
@@ -1158,11 +1170,17 @@ ofpact_check__(const struct ofpact *a, const struct 
flow *flow, int max_ports,
         }
         return 0;
 
-    case OFPACT_OUTPUT_REG:
-        return mf_check_src(&ofpact_get_OUTPUT_REG(a)->src, flow);
+    case OFPACT_OUTPUT_REG: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return mf_check_src(&ofpact_get_OUTPUT_REG(a)->src, updated_flow);
+    }
 
-    case OFPACT_BUNDLE:
-        return bundle_check(ofpact_get_BUNDLE(a), max_ports, flow);
+    case OFPACT_BUNDLE: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return bundle_check(ofpact_get_BUNDLE(a), max_ports, updated_flow);
+    }
 
     case OFPACT_SET_VLAN_VID:
     case OFPACT_SET_VLAN_PCP:
@@ -1170,34 +1188,46 @@ ofpact_check__(const struct ofpact *a, const struct 
flow *flow, int max_ports,
     case OFPACT_PUSH_VLAN:
     case OFPACT_SET_ETH_SRC:
     case OFPACT_SET_ETH_DST:
+        return 0;
+
     case OFPACT_SET_IPV4_SRC:
     case OFPACT_SET_IPV4_DST:
     case OFPACT_SET_IPV4_DSCP:
     case OFPACT_SET_L4_SRC_PORT:
     case OFPACT_SET_L4_DST_PORT:
-        return 0;
+        return eth_type_mpls(*dl_type) ? OFPERR_OFPBAC_BAD_ARGUMENT : 0;
 
-    case OFPACT_REG_MOVE:
-        return nxm_reg_move_check(ofpact_get_REG_MOVE(a), flow);
+    case OFPACT_REG_MOVE: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return nxm_reg_move_check(ofpact_get_REG_MOVE(a), updated_flow);
+    }
 
-    case OFPACT_REG_LOAD:
-        if (*dl_type != flow->dl_type) {
-            struct flow updated_flow = *flow;
-            updated_flow.dl_type = *dl_type;
-            return nxm_reg_load_check(ofpact_get_REG_LOAD(a), &updated_flow);
-        } else {
-            return nxm_reg_load_check(ofpact_get_REG_LOAD(a), flow);
-        }
+    case OFPACT_REG_LOAD: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return nxm_reg_load_check(ofpact_get_REG_LOAD(a), updated_flow);
+    }
 
-    case OFPACT_STACK_PUSH:
-        return nxm_stack_push_check(ofpact_get_STACK_PUSH(a), flow);
+    case OFPACT_STACK_PUSH: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return nxm_stack_push_check(ofpact_get_STACK_PUSH(a), updated_flow);
+    }
 
-    case OFPACT_STACK_POP:
-        return nxm_stack_pop_check(ofpact_get_STACK_POP(a), flow);
+    case OFPACT_STACK_POP: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return nxm_stack_pop_check(ofpact_get_STACK_POP(a), updated_flow);
+    }
 
     case OFPACT_DEC_TTL:
+        return eth_type_mpls(*dl_type) ? OFPERR_OFPBAC_BAD_ARGUMENT : 0;
+
     case OFPACT_SET_MPLS_TTL:
     case OFPACT_DEC_MPLS_TTL:
+        return eth_type_mpls(*dl_type) ? 0 : OFPERR_OFPBAC_BAD_ARGUMENT;
+
     case OFPACT_SET_TUNNEL:
     case OFPACT_SET_QUEUE:
     case OFPACT_POP_QUEUE:
@@ -1205,11 +1235,17 @@ ofpact_check__(const struct ofpact *a, const struct 
flow *flow, int max_ports,
     case OFPACT_RESUBMIT:
         return 0;
 
-    case OFPACT_LEARN:
-        return learn_check(ofpact_get_LEARN(a), flow);
+    case OFPACT_LEARN: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return learn_check(ofpact_get_LEARN(a), updated_flow);
+    }
 
-    case OFPACT_MULTIPATH:
-        return multipath_check(ofpact_get_MULTIPATH(a), flow);
+    case OFPACT_MULTIPATH: {
+        struct flow storage;
+        const struct flow *updated_flow = update_flow(flow, &storage, 
*dl_type);
+        return multipath_check(ofpact_get_MULTIPATH(a), updated_flow);
+    }
 
     case OFPACT_NOTE:
     case OFPACT_EXIT:
@@ -1220,6 +1256,9 @@ ofpact_check__(const struct ofpact *a, const struct flow 
*flow, int max_ports,
         return 0;
 
     case OFPACT_POP_MPLS:
+        if (!eth_type_mpls(*dl_type)) {
+            return OFPERR_OFPBAC_BAD_ARGUMENT;
+        }
         *dl_type = ofpact_get_POP_MPLS(a)->ethertype;
         return 0;
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 5903191..3d0f949 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -272,7 +272,7 @@ cookie=0xa dl_src=40:44:44:44:44:46 
actions=push_mpls:0x8847,load:10->OXM_OF_MPL
 cookie=0xa dl_src=40:44:44:44:44:47 
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],dec_mpls_ttl,set_mpls_ttl(10),controller
 cookie=0xa dl_src=40:44:44:44:44:48 
actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),dec_mpls_ttl,controller
 cookie=0xb dl_src=50:55:55:55:55:55 dl_type=0x8847 
actions=load:1000->OXM_OF_MPLS_LABEL[[]],controller
-cookie=0xd dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,controller
+cookie=0xd dl_src=60:66:66:66:66:66 dl_type=0x8847 
actions=pop_mpls:0x0800,controller
 cookie=0xc dl_src=70:77:77:77:77:77 
actions=push_mpls:0x8848,load:1000->OXM_OF_MPLS_LABEL[[]],load:7->OXM_OF_MPLS_TC[[]],controller
 ])
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
@@ -628,7 +628,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], 
[0], [dnl
  cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:48 
actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),dec_mpls_ttl,CONTROLLER:65535
  cookie=0xb, n_packets=3, n_bytes=180, mpls,dl_src=50:55:55:55:55:55 
actions=load:0x3e8->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
  cookie=0xc, n_packets=3, n_bytes=180, dl_src=70:77:77:77:77:77 
actions=push_mpls:0x8848,load:0x3e8->OXM_OF_MPLS_LABEL[[]],load:0x7->OXM_OF_MPLS_TC[[]],CONTROLLER:65535
- cookie=0xd, n_packets=3, n_bytes=186, dl_src=60:66:66:66:66:66 
actions=pop_mpls:0x0800,CONTROLLER:65535
+ cookie=0xd, n_packets=3, n_bytes=186, mpls,dl_src=60:66:66:66:66:66 
actions=pop_mpls:0x0800,CONTROLLER:65535
  n_packets=3, n_bytes=180, dl_src=10:11:11:11:11:11 actions=CONTROLLER:65535
 NXST_FLOW reply:
 ])
-- 
1.8.2.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to