On 9 November 2015 at 15:33, Jarno Rajahalme <[email protected]> wrote:
>
> On Nov 9, 2015, at 12:50 PM, Joe Stringer <[email protected]> wrote:
>
> On 9 November 2015 at 10:56, Joe Stringer <[email protected]> wrote:
>
> If conntrack recirculates, it should not stop processing the current
> pipeline. The cloned packet will begin with a fresh action set in the
> table specified with the current metadata; The current copy of the
> packet will continue processing, including to return back to prior
> resubmit() calls.
>
> Reported-by: Russell Bryant <[email protected]>
> Signed-off-by: Joe Stringer <[email protected]>
>
>
> FWIW, the main change in this commit is actually to avoid the
> following instruction in ctx_trigger_recirculation():
>
> ctx->exit = true;
>
> ofproto/ofproto-dpif-xlate.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 325e308e3340..bc21fa894682 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3591,6 +3591,16 @@ compose_recirculate_action(struct xlate_ctx *ctx)
>     compose_recirculate_action__(ctx, 0);
> }
>
> +/* Fork the pipeline here. The recirculated packet will continue processing
> in
> + * 'table' with an empty action set, and the current packet will continue
> + * processing the current action list. */
> +static void
> +compose_recirculate_and_fork(struct xlate_ctx *ctx, uint8_t table)
> +{
> +    ctx->recirc_action_offset = ctx->action_set.size;
>
>
> This is actually indicating that the current action set should be
> preserved for the recirculated packet. At minimum the
> documentation/commit message should be updated to reflect this. I also
> welcome discussion on the implications of preserving the action set
> across recirculation.
>
>
> For implicit recirculation, like for pop mpls and then goto/resubmit to
> another table to match the inner header, we have no other option than to
> preserve the action set across the recirculation, otherwise we would break
> behavior standardized for the action set. So the discussion here should
> pertain only to the explicit recirculation we do with the conntrack action,
> i.e., should the recirculated packet start with an empty action set, or
> inherit it from the recirculating flow (at the point of recirculation), as
> currently happens. The registers and other metadata is inherited in the same
> fashion, and that can be very useful in that the pipeline processing need
> not start from an empty state after recirculation. I would say that
> preserving the action set serves the same purpose, and the flows in the
> recirculated to table can always clear the action set (clear_actions) if
> that is desired.

Thanks for the feedback, that sounds like a fair approach.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to