ofproto-dpif-xlate: Handle oversized actions more gracefully.

If the datapath actions exceed the maximum size of a Netlink attribute
(about 64 kB), then previously we would assert-fail (before commit
542024c4c3d36 "ofproto-dpif-xlate: Suppress oversize datapath actions.")
or just drop all of them (after that commit).  This commit makes OVS cope
by slow-pathing the flow and executing all of its actions in userspace.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/dpif.c b/lib/dpif.c
index b284e13..ed84f5f 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1183,6 +1183,8 @@ dpif_execute__(struct dpif *dpif, const struct dpif_execute *execute)
  * OVS_ACTION_ATTR_USERSPACE actions it passes the packet through to the dpif
  * implementation.
  *
+ * This works even if 'actions_len' is too long for a Netlink attribute.
+ *
  * Returns 0 if successful, otherwise a positive errno value. */
 int
 dpif_execute(struct dpif *dpif,
@@ -1198,7 +1200,7 @@ dpif_execute(struct dpif *dpif,
     execute.actions = actions;
     execute.actions_len = actions_len;
     execute.packet = buf;
-    execute.needs_help = needs_help;
+    execute.needs_help = needs_help || nl_attr_oversized(actions_len);
     return dpif_execute__(dpif, &execute);
 }
 

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2a82177..ae6f638 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -49,6 +49,7 @@
 #include "vlog.h"
 
 COVERAGE_DEFINE(xlate_actions);
+COVERAGE_DEFINE(xlate_actions_oversize);
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate);
 
@@ -2969,11 +2970,11 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
 
     if (nl_attr_oversized(ctx.xout->odp_actions.size)) {
         /* These datapath actions are too big for a Netlink attribute, so we
-         * can't execute them. */
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-
-        VLOG_ERR_RL(&rl, "discarding oversize datapath actions");
-        ofpbuf_clear(&ctx.xout->odp_actions);
+         * can't hand them to the kernel directly.  dpif_execute() can execute
+         * them one by one with help, so just mark the result as SLOW_ACTION to
+         * prevent the flow from being installed. */
+        COVERAGE_INC(xlate_actions_oversize);
+        ctx.xout->slow |= SLOW_ACTION;
     }
 
     ofpbuf_uninit(&ctx.stack);

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 9e0ea4b..726ec99 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -3048,12 +3048,12 @@ ADD_OF_PORTS([br0], 1)
  echo "in_port=13, actions=local,local,local,local,local,local,local,local") > flows
 AT_CHECK([ovs-ofctl add-flows br0 flows])
 AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1'], [0], [stdout])
-AT_CHECK([grep -c 'resubmits yielded over 64 kB of actions' ovs-vswitchd.log], [0], [1
+AT_CHECK([grep -c -e '- Uses action(s) not supported by datapath' stdout],
+  [0], [1
 ])
-AT_CHECK([grep -c 'discarding oversize datapath actions' ovs-vswitchd.log], [0], [1
+AT_CHECK([grep -c 'resubmits yielded over 64 kB of actions' ovs-vswitchd.log], [0], [1
 ])
-OVS_VSWITCHD_STOP(["/resubmits yielded over 64 kB of actions/d
-/discarding oversize datapath actions/d"])
+OVS_VSWITCHD_STOP(["/resubmits yielded over 64 kB of actions/d"])
 AT_CLEANUP
 
 AT_SETUP([ofproto-dpif - stack too deep])