Verify that a flow isn't adding more than 64kB in actions when an action
uses multiple check_pkt_larger statements. Failure to do so can cause
ovs-vswitchd to throw signal 6. Also add a depth check to clone and
check_pkt_larger.

Signed-off-by: Mike Pattrick <[email protected]>
---
 ofproto/ofproto-dpif-xlate.c | 26 ++++++++++++++++++
 tests/ofproto-dpif.at        | 52 ++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5a770f171..50357e6bb 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5806,7 +5806,9 @@ clone_xlate_actions(const struct ofpact *actions, size_t 
actions_len,
 
     if (reversible_actions(actions, actions_len) || is_last_action) {
         old_flow = ctx->xin->flow;
+        ctx->depth++;
         do_xlate_actions(actions, actions_len, ctx, is_last_action, false);
+        ctx->depth--;
         if (!ctx->freezing) {
             xlate_action_set(ctx);
         }
@@ -5830,7 +5832,9 @@ clone_xlate_actions(const struct ofpact *actions, size_t 
actions_len,
     if (ctx->xbridge->support.clone) { /* Use clone action */
         /* Use clone action as datapath clone. */
         offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CLONE);
+        ctx->depth++;
         do_xlate_actions(actions, actions_len, ctx, true, false);
+        ctx->depth--;
         if (!ctx->freezing) {
             xlate_action_set(ctx);
         }
@@ -5846,7 +5850,9 @@ clone_xlate_actions(const struct ofpact *actions, size_t 
actions_len,
         offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_SAMPLE);
         ac_offset = nl_msg_start_nested(ctx->odp_actions,
                                         OVS_SAMPLE_ATTR_ACTIONS);
+        ctx->depth++;
         do_xlate_actions(actions, actions_len, ctx, true, false);
+        ctx->depth--;
         if (!ctx->freezing) {
             xlate_action_set(ctx);
         }
@@ -6412,7 +6418,18 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx,
         ctx->odp_actions, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER);
     value.u8_val = 1;
     mf_write_subfield_flow(&check_pkt_larger->dst, &value, &ctx->xin->flow);
+    ctx->depth++;
     do_xlate_actions(remaining_acts, remaining_acts_len, ctx, true, false);
+    ctx->depth--;
+
+    if (!xlate_resubmit_resource_check(ctx)) {
+        if (!ctx->error) {
+            /* If we don't set an error here, we end up with corrupted
+             * odp actions in the slow path. */
+            ctx->error = XLATE_RECURSION_TOO_DEEP;
+        }
+        return;
+    }
     if (!ctx->freezing) {
         xlate_action_set(ctx);
     }
@@ -6437,7 +6454,16 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx,
         ctx->odp_actions, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL);
     value.u8_val = 0;
     mf_write_subfield_flow(&check_pkt_larger->dst, &value, &ctx->xin->flow);
+    ctx->depth++;
     do_xlate_actions(remaining_acts, remaining_acts_len, ctx, true, false);
+    ctx->depth--;
+
+    if (!xlate_resubmit_resource_check(ctx)) {
+        if (!ctx->error) {
+            ctx->error = XLATE_RECURSION_TOO_DEEP;
+        }
+        return;
+    }
     if (!ctx->freezing) {
         xlate_action_set(ctx);
     }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index dbb3b6dda..3f70e732a 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -11640,3 +11640,55 @@ AT_CHECK([test 1 = `ovs-ofctl parse-pcap p2-tx.pcap | 
wc -l`])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - discard large flows])
+OVS_VSWITCHD_START
+add_of_ports --pcap br0 `seq 1 2`
+
+AT_DATA([flows.txt], [dnl
+table=0,in_port=1 
actions=clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl Over 64 
recursions
+                          
clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl
+                          
clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl
+                          
clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl
+                          
clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl
+                          
clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl
+                          
clone(clone(clone(clone(clone(clone(clone(clone(clone( dnl
+                          clone(resubmit(,2))                                  
  dnl
+                          )))))))))                                            
  dnl
+                          )))))))))                                            
  dnl
+                          )))))))))                                            
  dnl
+                          )))))))))                                            
  dnl
+                          )))))))))                                            
  dnl
+                          )))))))))                                            
  dnl
+                          )))))))))
+table=0,in_port=2 actions=check_pkt_larger(20)->NXM_NX_REG0[[1]],              
  dnl Over 64 kB of actions
+                          check_pkt_larger(40)->NXM_NX_REG1[[1]],              
  dnl
+                          check_pkt_larger(60)->NXM_NX_REG2[[1]],              
  dnl
+                          check_pkt_larger(80)->NXM_NX_REG3[[1]],              
  dnl
+                          check_pkt_larger(100)->NXM_NX_REG4[[1]],             
  dnl
+                          check_pkt_larger(110)->NXM_NX_REG5[[1]],             
  dnl
+                          check_pkt_larger(120)->NXM_NX_REG6[[1]],             
  dnl
+                          check_pkt_larger(140)->NXM_NX_REG7[[1]],             
  dnl
+                          check_pkt_larger(160)->NXM_NX_REG8[[1]],             
  dnl
+                          check_pkt_larger(180)->NXM_NX_REG9[[1]],             
  dnl
+                          check_pkt_larger(200)->NXM_NX_REG10[[1]],            
  dnl
+                          check_pkt_larger(210)->NXM_NX_REG11[[1]],resubmit(,2)
+table=2,reg0=0x0 actions=normal
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(2),eth(src=50:54:00:00:00:0a,dst=50:54:00:00:00:09),dnl
+eth_type(0x0800),ipv4(src=10.10.10.1,dst=10.10.10.2,proto=1,dnl
+tos=1,ttl=128,frag=no),icmp(type=8,code=0)' | grep -c ">>>> resubmits yielded 
over 64 kB of actions <<<<"], , [dnl
+13
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),dnl
+eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,dnl
+tos=1,ttl=128,frag=no),icmp(type=8,code=0)' | grep -c ">>>> over max 
translation depth 64 <<<<"], , [dnl
+1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
-- 
2.27.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to