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

[Joe Stringer]

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>
---
 lib/dpif.c |   26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/lib/dpif.c b/lib/dpif.c
index afbab35..850b4db 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1005,9 +1005,6 @@ dpif_flow_dump_next(struct dpif_flow_dump *dump, struct ofpbuf *buffer,
                                                  key, key_len,
                                                  mask, mask_len,
                                                  stats);
-        if (error) {
-            dpif->dpif_class->flow_dump_done(dpif, dump->state);
-        }
     }
     if (error) {
         if (key) {
@@ -1019,17 +1016,14 @@ dpif_flow_dump_next(struct dpif_flow_dump *dump, struct ofpbuf *buffer,
             *mask_len = 0;
         }
     }
-    if (!dump->error) {
-        if (error == EOF) {
-            VLOG_DBG_RL(&dpmsg_rl, "%s: dumped all flows", dpif_name(dpif));
-        } else if (should_log_flow_message(error)) {
-            log_flow_message(dpif, error, "flow_dump",
-                             key ? *key : NULL, key ? *key_len : 0,
-                             mask ? *mask : NULL, mask ? *mask_len : 0,
-                             stats ? stats : NULL, NULL, 0);
-        }
+    if (error == EOF) {
+        VLOG_DBG_RL(&dpmsg_rl, "%s: dumped all flows", dpif_name(dpif));
+    } else if (should_log_flow_message(error)) {
+        log_flow_message(dpif, error, "flow_dump",
+                         key ? *key : NULL, key ? *key_len : 0,
+                         mask ? *mask : NULL, mask ? *mask_len : 0,
+                         stats ? stats : NULL, NULL, 0);
     }
-    dump->error = error;
     return !error;
 }
 
@@ -1040,10 +1034,8 @@ int
 dpif_flow_dump_done(struct dpif_flow_dump *dump)
 {
     const struct dpif *dpif = dump->dpif;
-    if (!dump->error) {
-        dump->error = dpif->dpif_class->flow_dump_done(dpif, dump->state);
-        log_operation(dpif, "flow_dump_done", dump->error);
-    }
+    dump->error = dpif->dpif_class->flow_dump_done(dpif, dump->state);
+    log_operation(dpif, "flow_dump_done", dump->error);
     return dump->error == EOF ? 0 : dump->error;
 }
 
-- 
1.7.9.5