Consider the following flow table:
table=0 actions=resubmit(,1),2
table=1 actions=debug_recirc
When debug_recirc triggers recirculation and we later resume processing,
only the output to port 2 should be executed, because the effects of
"resubmit" have already taken place. However, until now, the "resubmit"
was added to the actions to execute post-recirculation, resulting in an
infinite loop.
Found when testing a feature based on recirculation.
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
ofproto/ofproto-dpif-xlate.c | 22 +++++++++++++++++++---
tests/ofproto-dpif.at | 24 ++++++++++++++++++++++++
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4b25bf4..786df14 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4792,13 +4792,29 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
ofpacts_len,
case OFPACT_DEBUG_RECIRC:
ctx_trigger_recirculation(ctx);
- a = ofpact_next(a);
break;
}
- /* Check if need to store this and the remaining actions for later
- * execution. */
+ /* Check if need to store remaining actions for later execution. */
if (!ctx->error && ctx->exit && ctx_first_recirculation_action(ctx)) {
+ /* Most commonly, we recirculate because we cannot execute the
+ * current action with the information that we have now. Instead,
+ * we need the datapath to get us more information and then we can
+ * try again. Thus, in these situations we include the current
+ * action in the list.
+ *
+ * However, in other situations, the action we're executing has
+ * already executed (OFPACT_RESUBMIT) or intentionally triggers
+ * recirculation (OFPACT_DEBUG_RECIRC). In either case, including
+ * the current action in the list would cause it to be executed a
+ * second time when we resume later (possibly causing an infinite
+ * loop). In these cases, we omit the action from the list.
+ */
+ if (a->type == OFPACT_RESUBMIT ||
+ a->type == OFPACT_DEBUG_RECIRC) {
+ a = ofpact_next(a);
+ }
+
recirc_unroll_actions(a, OFPACT_ALIGN(ofpacts_len -
((uint8_t *)a -
(uint8_t *)ofpacts)),
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bfb1b56..aa2f1bb 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -4203,6 +4203,30 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath
actions: 3
OVS_VSWITCHD_STOP
AT_CLEANUP
+# This test verifies that "resubmit", when it triggers recirculation
+# indirectly through the flow that it recursively invokes, is not
+# re-executed when execution continues later post-recirculation.
+AT_SETUP([ofproto-dpif - recirculation after resubmit])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2])
+
+AT_DATA([flows.txt], [dnl
+table=0 in_port=1 actions=resubmit(,1),2
+table=1 in_port=1 actions=debug_recirc
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+flow="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)"
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow" -generate], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: recirc(0x1)
+])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow,recirc_id(1)" -generate],
[0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: 2
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
# Two testcases below are for the ofproto/trace command
# The first one tests all correct syntax:
# ofproto/trace [dp_name] odp_flow [-generate|packet]
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
