[ovs-dev] [PATCHv4 3/7] dpif: Separate local and shared flow dump state.

Joe Stringer joestringer at nicira.com
Thu Feb 27 22:13:07 UTC 2014


This patch separates the structures for thread-local flow dump state
("state") from the shared flow dump state ("iter") in dpif-linux and
dpif-netdev. Future patches will make use of this to allow multiple
threads to dump flows from the same flow dump operation.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
v3: Rebase.
v2: First post.
---
 lib/dpif-linux.c    |   57 ++++++++++++++++++++++++++++++++++++---------------
 lib/dpif-netdev.c   |   51 +++++++++++++++++++++++++++++++++------------
 lib/dpif-provider.h |   25 ++++++++++++++--------
 lib/dpif.c          |   25 ++++++++++++++++++----
 lib/dpif.h          |    4 +++-
 5 files changed, 120 insertions(+), 42 deletions(-)

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 723de23..dd0ab4b 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -992,22 +992,46 @@ dpif_linux_flow_del(struct dpif *dpif_, const struct dpif_flow_del *del)
 }
 
 struct dpif_linux_flow_state {
-    struct nl_dump dump;
     struct dpif_linux_flow flow;
     struct dpif_flow_stats stats;
     struct ofpbuf buffer;         /* Always used to store flows. */
     struct ofpbuf *tmp;           /* Used if kernel does not supply actions. */
 };
 
+struct dpif_linux_flow_iter {
+    struct nl_dump dump;
+    void *state;
+};
+
+static void
+dpif_linux_flow_dump_state_init(void **statep)
+{
+    struct dpif_linux_flow_state *state;
+
+    *statep = state = xmalloc(sizeof *state);
+    ofpbuf_init(&state->buffer, NL_DUMP_BUFSIZE);
+    state->tmp = NULL;
+}
+
+static void
+dpif_linux_flow_dump_state_uninit(void *state_)
+{
+    struct dpif_linux_flow_state *state = state_;
+
+    ofpbuf_uninit(&state->buffer);
+    ofpbuf_delete(state->tmp);
+    free(state);
+}
+
 static int
-dpif_linux_flow_dump_start(const struct dpif *dpif_, void **statep)
+dpif_linux_flow_dump_start(const struct dpif *dpif_, void **iterp)
 {
     const struct dpif_linux *dpif = dpif_linux_cast(dpif_);
-    struct dpif_linux_flow_state *state;
+    struct dpif_linux_flow_iter *iter;
     struct dpif_linux_flow request;
     struct ofpbuf *buf;
 
-    *statep = state = xmalloc(sizeof *state);
+    *iterp = iter = xmalloc(sizeof *iter);
 
     dpif_linux_flow_init(&request);
     request.cmd = OVS_FLOW_CMD_GET;
@@ -1015,23 +1039,23 @@ dpif_linux_flow_dump_start(const struct dpif *dpif_, void **statep)
 
     buf = ofpbuf_new(1024);
     dpif_linux_flow_to_ofpbuf(&request, buf);
-    nl_dump_start(&state->dump, NETLINK_GENERIC, buf);
+    nl_dump_start(&iter->dump, NETLINK_GENERIC, buf);
     ofpbuf_delete(buf);
 
-    ofpbuf_init(&state->buffer, NL_DUMP_BUFSIZE);
-    state->tmp = NULL;
+    dpif_linux_flow_dump_state_init(&iter->state);
 
     return 0;
 }
 
 static int
-dpif_linux_flow_dump_next(const struct dpif *dpif_, void *state_,
+dpif_linux_flow_dump_next(const struct dpif *dpif_, void *iter_,
                           const struct nlattr **key, size_t *key_len,
                           const struct nlattr **mask, size_t *mask_len,
                           const struct nlattr **actions, size_t *actions_len,
                           const struct dpif_flow_stats **stats)
 {
-    struct dpif_linux_flow_state *state = state_;
+    struct dpif_linux_flow_iter *iter = iter_;
+    struct dpif_linux_flow_state *state = iter->state;
     struct ofpbuf buf;
     int error;
 
@@ -1039,7 +1063,7 @@ dpif_linux_flow_dump_next(const struct dpif *dpif_, void *state_,
         ofpbuf_delete(state->tmp);
         state->tmp = NULL;
 
-        if (!nl_dump_next(&state->dump, &buf, &state->buffer)) {
+        if (!nl_dump_next(&iter->dump, &buf, &state->buffer)) {
             return EOF;
         }
 
@@ -1081,13 +1105,12 @@ dpif_linux_flow_dump_next(const struct dpif *dpif_, void *state_,
 }
 
 static int
-dpif_linux_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *state_)
+dpif_linux_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *iter_)
 {
-    struct dpif_linux_flow_state *state = state_;
-    int error = nl_dump_done(&state->dump);
-    ofpbuf_uninit(&state->buffer);
-    ofpbuf_delete(state->tmp);
-    free(state);
+    struct dpif_linux_flow_iter *iter = iter_;
+    int error = nl_dump_done(&iter->dump);
+    dpif_linux_flow_dump_state_uninit(iter->state);
+    free(iter);
     return error;
 }
 
@@ -1640,9 +1663,11 @@ const struct dpif_class dpif_linux_class = {
     dpif_linux_flow_put,
     dpif_linux_flow_del,
     dpif_linux_flow_flush,
+    dpif_linux_flow_dump_state_init,
     dpif_linux_flow_dump_start,
     dpif_linux_flow_dump_next,
     dpif_linux_flow_dump_done,
+    dpif_linux_flow_dump_state_uninit,
     dpif_linux_execute,
     dpif_linux_operate,
     dpif_linux_recv_set,
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c4ba646..3bb31e6 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1306,40 +1306,63 @@ dpif_netdev_flow_del(struct dpif *dpif, const struct dpif_flow_del *del)
 }
 
 struct dp_netdev_flow_state {
-    uint32_t bucket;
-    uint32_t offset;
     struct dp_netdev_actions *actions;
     struct odputil_keybuf keybuf;
     struct odputil_keybuf maskbuf;
     struct dpif_flow_stats stats;
 };
 
-static int
-dpif_netdev_flow_dump_start(const struct dpif *dpif OVS_UNUSED, void **statep)
+struct dp_netdev_flow_iter {
+    uint32_t bucket;
+    uint32_t offset;
+    void *state;
+};
+
+static void
+dpif_netdev_flow_dump_state_init(void **statep)
 {
     struct dp_netdev_flow_state *state;
 
     *statep = state = xmalloc(sizeof *state);
-    state->bucket = 0;
-    state->offset = 0;
     state->actions = NULL;
+}
+
+static void
+dpif_netdev_flow_dump_state_uninit(void *state_)
+{
+    struct dp_netdev_flow_state *state = state_;
+
+    dp_netdev_actions_unref(state->actions);
+    free(state);
+}
+
+static int
+dpif_netdev_flow_dump_start(const struct dpif *dpif OVS_UNUSED, void **iterp)
+{
+    struct dp_netdev_flow_iter *iter;
+
+    *iterp = iter = xmalloc(sizeof *iter);
+    iter->bucket = 0;
+    iter->offset = 0;
+    dpif_netdev_flow_dump_state_init(&iter->state);
     return 0;
 }
 
 static int
-dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_,
+dpif_netdev_flow_dump_next(const struct dpif *dpif, void *iter_,
                            const struct nlattr **key, size_t *key_len,
                            const struct nlattr **mask, size_t *mask_len,
                            const struct nlattr **actions, size_t *actions_len,
                            const struct dpif_flow_stats **stats)
 {
-    struct dp_netdev_flow_state *state = state_;
+    struct dp_netdev_flow_iter *iter = iter_;
+    struct dp_netdev_flow_state *state = iter->state;
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *netdev_flow;
     struct hmap_node *node;
 
     fat_rwlock_rdlock(&dp->cls.rwlock);
-    node = hmap_at_position(&dp->flow_table, &state->bucket, &state->offset);
+    node = hmap_at_position(&dp->flow_table, &iter->bucket, &iter->offset);
     if (node) {
         netdev_flow = CONTAINER_OF(node, struct dp_netdev_flow, node);
         dp_netdev_flow_ref(netdev_flow);
@@ -1397,12 +1420,12 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_,
 }
 
 static int
-dpif_netdev_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *state_)
+dpif_netdev_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *iter_)
 {
-    struct dp_netdev_flow_state *state = state_;
+    struct dp_netdev_flow_iter *iter = iter_;
 
-    dp_netdev_actions_unref(state->actions);
-    free(state);
+    dpif_netdev_flow_dump_state_uninit(iter->state);
+    free(iter);
     return 0;
 }
 
@@ -1854,9 +1877,11 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_flow_put,
     dpif_netdev_flow_del,
     dpif_netdev_flow_flush,
+    dpif_netdev_flow_dump_state_init,
     dpif_netdev_flow_dump_start,
     dpif_netdev_flow_dump_next,
     dpif_netdev_flow_dump_done,
+    dpif_netdev_flow_dump_state_uninit,
     dpif_netdev_execute,
     NULL,                       /* operate */
     dpif_netdev_recv_set,
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index adc5242..57b37b0 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -262,12 +262,17 @@ struct dpif_class {
      * packets. */
     int (*flow_flush)(struct dpif *dpif);
 
+    /* Allocates thread-local state for use with the function 'flow_dump_next'.
+     * On return, initializes '*statep' with any private data needed for
+     * iteration. */
+    void (*flow_dump_state_init)(void **statep);
+
     /* Attempts to begin dumping the flows in a dpif.  On success, returns 0
-     * and initializes '*statep' with any data needed for iteration.  On
-     * failure, returns a positive errno value. */
-    int (*flow_dump_start)(const struct dpif *dpif, void **statep);
+     * and initializes '*iterp' with any shared data needed for iteration.
+     * On failure, returns a positive errno value. */
+    int (*flow_dump_start)(const struct dpif *dpif, void **iterp);
 
-    /* Attempts to retrieve another flow from 'dpif' for 'state', which was
+    /* Attempts to retrieve another flow from 'dpif' for 'iter', which was
      * initialized by a successful call to the 'flow_dump_start' function for
      * 'dpif'.  On success, updates the output parameters as described below
      * and returns 0.  Returns EOF if the end of the flow table has been
@@ -295,16 +300,20 @@ struct dpif_class {
      * All of the returned data is owned by 'dpif', not by the caller, and the
      * caller must not modify or free it.  'dpif' must guarantee that it
      * remains accessible and unchanging until at least the next call to
-     * 'flow_dump_next' or 'flow_dump_done' for 'state'. */
-    int (*flow_dump_next)(const struct dpif *dpif, void *state,
+     * 'flow_dump_next' or 'flow_dump_done' for 'iter'. */
+    int (*flow_dump_next)(const struct dpif *dpif, void *iter,
                           const struct nlattr **key, size_t *key_len,
                           const struct nlattr **mask, size_t *mask_len,
                           const struct nlattr **actions, size_t *actions_len,
                           const struct dpif_flow_stats **stats);
 
-    /* Releases resources from 'dpif' for 'state', which was initialized by a
+    /* Releases resources from 'dpif' for 'iter', which was initialized by a
      * successful call to the 'flow_dump_start' function for 'dpif'.  */
-    int (*flow_dump_done)(const struct dpif *dpif, void *state);
+    int (*flow_dump_done)(const struct dpif *dpif, void *iter);
+
+    /* Releases 'state' which was initialized by a call to the
+     * 'flow_dump_state_init' function for this 'dpif'. */
+    void (*flow_dump_state_uninit)(void *statep);
 
     /* Performs the 'execute->actions_len' bytes of actions in
      * 'execute->actions' on the Ethernet frame in 'execute->packet'
diff --git a/lib/dpif.c b/lib/dpif.c
index 2b79a6e..7126571 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -961,6 +961,23 @@ dpif_flow_del(struct dpif *dpif,
     return dpif_flow_del__(dpif, &del);
 }
 
+/* Allocates thread-local state for use with the 'flow_dump_next' function for
+ * 'dpif'. On return, initializes '*statep' with any private data needed for
+ * iteration. */
+void
+dpif_flow_dump_state_init(const struct dpif *dpif, void **statep)
+{
+    dpif->dpif_class->flow_dump_state_init(statep);
+}
+
+/* Releases 'state' which was initialized by a call to the
+ * 'flow_dump_state_init' function for 'dpif'. */
+void
+dpif_flow_dump_state_uninit(const struct dpif *dpif, void *state)
+{
+    dpif->dpif_class->flow_dump_state_uninit(state);
+}
+
 /* Initializes 'dump' to begin dumping the flows in a dpif.
  *
  * This function provides no status indication.  An error status for the entire
@@ -971,7 +988,7 @@ void
 dpif_flow_dump_start(struct dpif_flow_dump *dump, const struct dpif *dpif)
 {
     dump->dpif = dpif;
-    dump->error = dpif->dpif_class->flow_dump_start(dpif, &dump->state);
+    dump->error = dpif->dpif_class->flow_dump_start(dpif, &dump->iter);
     log_operation(dpif, "flow_dump_start", dump->error);
 }
 
@@ -1004,13 +1021,13 @@ dpif_flow_dump_next(struct dpif_flow_dump *dump,
     int error = dump->error;
 
     if (!error) {
-        error = dpif->dpif_class->flow_dump_next(dpif, dump->state,
+        error = dpif->dpif_class->flow_dump_next(dpif, dump->iter,
                                                  key, key_len,
                                                  mask, mask_len,
                                                  actions, actions_len,
                                                  stats);
         if (error) {
-            dpif->dpif_class->flow_dump_done(dpif, dump->state);
+            dpif->dpif_class->flow_dump_done(dpif, dump->iter);
         }
     }
     if (error) {
@@ -1053,7 +1070,7 @@ 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);
+        dump->error = dpif->dpif_class->flow_dump_done(dpif, dump->iter);
         log_operation(dpif, "flow_dump_done", dump->error);
     }
     return dump->error == EOF ? 0 : dump->error;
diff --git a/lib/dpif.h b/lib/dpif.h
index 7f986f9..e4f75b1 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -507,8 +507,9 @@ int dpif_flow_get(const struct dpif *,
 struct dpif_flow_dump {
     const struct dpif *dpif;
     int error;
-    void *state;
+    void *iter;
 };
+void dpif_flow_dump_state_init(const struct dpif *, void **statep);;
 void dpif_flow_dump_start(struct dpif_flow_dump *, const struct dpif *);
 bool dpif_flow_dump_next(struct dpif_flow_dump *,
                          const struct nlattr **key, size_t *key_len,
@@ -516,6 +517,7 @@ bool dpif_flow_dump_next(struct dpif_flow_dump *,
                          const struct nlattr **actions, size_t *actions_len,
                          const struct dpif_flow_stats **);
 int dpif_flow_dump_done(struct dpif_flow_dump *);
+void dpif_flow_dump_state_uninit(const struct dpif *, void *state);
 
 /* Operation batching interface.
  *
-- 
1.7.9.5




More information about the dev mailing list