From 0f032e95d89d24665e66fc31013588435c673fc1 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sat, 2 Nov 2013 08:43:14 -0700 Subject: [PATCH] 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 --- lib/dpif.c | 4 +++- ofproto/ofproto-dpif-xlate.c | 11 ++++++----- tests/ofproto-dpif.at | 8 ++++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/dpif.c b/lib/dpif.c index b284e13c1..ed84f5fcf 100644 --- 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 2a8217726..ae6f6380d 100644 --- 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 9e0ea4ba2..726ec99be 100644 --- 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]) -- 2.43.0