This seems the same that I already reviewed by itself:
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Jan 27, 2016, at 9:51 AM, Ben Pfaff <b...@ovn.org> wrote:
>
> 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.
>
> Now consider this flow table (as seen in the "MPLS handling" test in
> ofproto-dpif.at):
>
> table=0 actions=pop_mpls(0x0806),resubmit(,1)
> table=1 ip,nw_dst=1.2.3.4 actions=controller
>
> Here, we do want to add the "resubmit" to the actions to execute
> post-recirculation, since the "resubmit" cannot be processed until after
> recirculation makes the nw_dst field available.
>
> This commit fixes the problem in both cases.
>
> Found when testing a feature based on recirculation.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> ofproto/ofproto-dpif-xlate.c | 24 +++++++++++++++++++++++-
> tests/ofproto-dpif.at | 24 ++++++++++++++++++++++++
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 4b25bf4..6e928da 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4575,8 +4575,30 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
> ofpacts_len,
> break;
>
> case OFPACT_RESUBMIT:
> + /* Recirculation complicates resubmit. There are two cases:
> + *
> + * - If mpls_pop has been executed, then the flow table
> lookup
> + * as part of resubmit might depend on fields that can only
> + * be obtained via recirculation, so the resubmit itself
> + * triggers recirculation and we need to make sure that the
> + * resubmit is executed again after recirculation.
> + * Therefore, in this case we trigger recirculation and let
> + * the code following this "switch" append the resubmit to
> + * the post-recirculation actions.
> + *
> + * - Otherwise, some action in the flow entry found by
> resubmit
> + * might trigger recirculation. If that happens, then we
> do
> + * not want to execute the resubmit again after
> + * recirculation, so we want to skip back to the head of
> the
> + * loop to avoid that, only adding any actions that follow
> + * the resubmit to the post-recirculation actions.
> + */
> + if (ctx->was_mpls) {
> + ctx_trigger_recirculation(ctx);
> + break;
> + }
> xlate_ofpact_resubmit(ctx, ofpact_get_RESUBMIT(a));
> - break;
> + continue;
>
> case OFPACT_SET_TUNNEL:
> flow->tunnel.tun_id = htonll(ofpact_get_SET_TUNNEL(a)->tun_id);
> 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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
