On 28 October 2015 at 20:07, Jarno Rajahalme <jrajaha...@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 <jrajaha...@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().
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
