[ovs-dev] [PATCH 09/10] dpif: Don't synchronize flow_dump_next() status.

Joe Stringer joestringer at nicira.com
Tue Jan 14 18:41:30 UTC 2014


On 13 January 2014 15:25, Ben Pfaff <blp at nicira.com> wrote:

> On Fri, Jan 10, 2014 at 11:43:14AM -0800, Joe Stringer wrote:
> > In an earlier incarnation of dpif, the interface specified that if
> > flow_dump_next() returned an error code, then the function must not be
> > called again in the same dump operation. As such, the dpif itself needed
> > to cache the error status to ensure that it did not make such calls.
> > Recent changes have relaxed this restriction, making it the
> > responsibility of the dpif implementation to track flow dump error
> > status.
> >
> > This patch ensures that dpif_flow_dump_next() does not attempt to finish
> > the flow dump, as it will be the responsibility of just one thread to do
> > this. Furthermore, we no longer update 'dump->status' in this function,
> > so that even if one thread finishes processing flows in its buffer (and
> > handles the final flow), it will not prevent other callers from
> > processing the remaining flows in their buffers.
> >
> > After this patch, the error code that dpif_flow_dump_next() returns is
> > only significant for the current state and buffer. As before, the status
> > of the entire flow dump operation can be obtained by calling
> > dpif_flow_dump_done().
> >
> > Signed-off-by: Joe Stringer <joestringer at nicira.com>
>
> Doesn't think cause errors generated by flow_dump_next implementations
> to be lost?  e.g. dpif_linux_flow_dump_next() can return errors
> produced by dpif_linux_flow_from_ofpbuf().
>

Yes, I missed this. My intention was to push error status tracking down to
the dpif implementation. In this case, dpif_flow_dump_next() only
determines whether to continue at the moment, while dpif_flow_dump_done()
provides the error status for the entire operation by calling the
flow_dump_done() implementation. This means that dpif-linux should be
storing this error condition to return at the end of the entire flow_dump
(and potentially logging the error when it occurs). Do you have a
preference on this?

By the way, somewhere along the line we ended up with a really silly
> loop in dpif_linux_flow_from_ofpbuf() (the "while" condition can never
> be true):
>
>     do {
>         if (!nl_dump_next(&state->dump, &buf, buffer)) {
>             return EOF;
>         }
>
>         error = dpif_linux_flow_from_ofpbuf(&flow, &buf);
>         if (error) {
>             return error;
>         }
>     } while (error);
>

Right. The loop was there to handle failures when fetching actions, but
I've removed the actions. I'll make sure to tidy this up when I go back
over the actions patch.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140114/efe9de0d/attachment-0003.html>


More information about the dev mailing list