[ovs-dev] [PATCH 3/6] ofproto: Allow xlate_actions() to fail.

Joe Stringer joestringer at nicira.com
Wed Nov 4 19:31:07 UTC 2015


On 28 October 2015 at 20:07, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> Sometimes xlate_actions() fails due to too deep recursion, too many
> MPLS labels, or missing recirculation context.  Make xlate_actions()
> fail in these circumstances, so that we can:
>
>  - not install a datapath flow if xlate_actions() fails, and
>
>  - delete an existing datapath flow, rather than replacing it with an
>    incomplete flow, when the revalidation fails due to failing
>    xlate_actions.
>
> Before this action it was possible that the revalidation installed a
> flow with a recirculation ID with an invalid recirc ID (== 0), due to
> the introduction of in-place modification in commit 43b2f131a229
> (ofproto: Allow in-place modifications of datapath flows).
>
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>

Could you also describe how this affects upcall translation? (Does it
just fail to install the flow, or will it install a new flow drops the
traffic which hits the offending parts of translation?)


> @@ -1066,9 +1067,14 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
>
>      upcall->dump_seq = seq_read(udpif->dump_seq);
>      upcall->reval_seq = seq_read(udpif->reval_seq);
> -    xlate_actions(&xin, &upcall->xout);
> +    error = xlate_actions(&xin, &upcall->xout);
>      upcall->xout_initialized = true;
>
> +    if (error) {
> +        ofpbuf_init(&upcall->put_actions, 0);
> +        return error;
> +    }
> +
>      /* Special case for fail-open mode.
>       *
>       * If we are in fail-open mode, but we are connected to a controller too,

Isn't upcall->put_actions initialized in upcall_receive()?


> @@ -4988,7 +4991,8 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
>                struct ds *ds)
>  {
>      struct trace_ctx trace;
> -
> +    int error;
> +

Whitespace.


> @@ -5007,29 +5011,32 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
>      trace.xin.resubmit_hook = trace_resubmit;
>      trace.xin.report_hook = trace_report_valist;
>
> -    xlate_actions(&trace.xin, &trace.xout);
> -
> -    ds_put_char(ds, '\n');
> -    trace_format_flow(ds, 0, "Final flow", &trace);
> -    trace_format_megaflow(ds, 0, "Megaflow", &trace);
> +    error = xlate_actions(&trace.xin, &trace.xout);
> +    if (error) {
> +        ds_put_format(ds, "\nTranslation failed with errno %d!", error);
> +    } else {
> +        ds_put_char(ds, '\n');
> +        trace_format_flow(ds, 0, "Final flow", &trace);
> +        trace_format_megaflow(ds, 0, "Megaflow", &trace);

Does this completely lose the context about where the translation
failed? Is it possible/worthwhile to still try to print as much info
as we can, up until the point that translation failed?

The errno could also be translated using ovs_strerror().



More information about the dev mailing list