New version of this is included in the NAT series, as I recall it depends on this.
Jarno > On Nov 4, 2015, at 11:31 AM, Joe Stringer <joestrin...@nicira.com> wrote: > > On 28 October 2015 at 20:07, Jarno Rajahalme <jrajaha...@nicira.com > <mailto: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
