[ovs-dev] [netlink v5 35/61] dpif: Eliminate "struct odp_flow" from client-visible interface.

Ben Pfaff blp at nicira.com
Thu Jan 27 00:23:18 UTC 2011


Following this commit, "struct odp_flow" and related data structures are
only used in Linux-specific parts of OVS userspace code.  This allows the
actual Linux datapath interface to evolve more freely.

Reviewed by Justin Pettit.
---
 lib/dpif-linux.c      |  123 ++++++++++++++++++--
 lib/dpif-netdev.c     |  148 +++++++++++++++---------
 lib/dpif-provider.h   |  113 +++++++++++--------
 lib/dpif.c            |  300 ++++++++++++++++++++++++-------------------------
 lib/dpif.h            |   18 +++-
 lib/odp-util.c        |   10 --
 lib/odp-util.h        |    3 +-
 ofproto/ofproto.c     |  124 +++++++--------------
 utilities/ovs-dpctl.c |   29 ++---
 9 files changed, 487 insertions(+), 381 deletions(-)

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index e0619b5..fb682f2 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -38,6 +38,7 @@
 #include "netdev.h"
 #include "netdev-vport.h"
 #include "netlink.h"
+#include "odp-util.h"
 #include "ofpbuf.h"
 #include "openvswitch/tunnel.h"
 #include "packets.h"
@@ -459,40 +460,136 @@ dpif_linux_port_poll_wait(const struct dpif *dpif_)
 }
 
 static int
-dpif_linux_flow_get(const struct dpif *dpif_, struct odp_flow *flow)
+dpif_linux_flow_get(const struct dpif *dpif_, int flags,
+                    const struct nlattr *key, size_t key_len,
+                    struct ofpbuf **actionsp, struct odp_flow_stats *stats)
 {
-    return do_ioctl(dpif_, ODP_FLOW_GET, flow);
+    struct ofpbuf *actions = NULL;
+    struct odp_flow odp_flow;
+    int error;
+
+    memset(&odp_flow, 0, sizeof odp_flow);
+    odp_flow.key = (struct nlattr *) key;
+    odp_flow.key_len = key_len;
+    if (actionsp) {
+        actions = *actionsp = ofpbuf_new(65536);
+        odp_flow.actions = actions->base;
+        odp_flow.actions_len = actions->allocated;
+    }
+    odp_flow.flags = flags;
+
+    error = do_ioctl(dpif_, ODP_FLOW_GET, &odp_flow);
+    if (!error) {
+        if (stats) {
+            *stats = odp_flow.stats;
+        }
+        if (actions) {
+            actions->size = odp_flow.actions_len;
+            ofpbuf_trim(actions);
+        }
+    } else {
+        if (actions) {
+            ofpbuf_delete(actions);
+        }
+    }
+    return error;
 }
 
 static int
-dpif_linux_flow_put(struct dpif *dpif_, struct odp_flow_put *put)
+dpif_linux_flow_put(struct dpif *dpif_, int flags,
+                    const struct nlattr *key, size_t key_len,
+                    const struct nlattr *actions, size_t actions_len,
+                    struct odp_flow_stats *stats)
 {
-    return do_ioctl(dpif_, ODP_FLOW_PUT, put);
+    struct odp_flow_put put;
+    int error;
+
+    memset(&put, 0, sizeof put);
+    put.flow.key = (struct nlattr *) key;
+    put.flow.key_len = key_len;
+    put.flow.actions = (struct nlattr *) actions;
+    put.flow.actions_len = actions_len;
+    put.flow.flags = 0;
+    put.flags = flags;
+    error = do_ioctl(dpif_, ODP_FLOW_PUT, &put);
+    if (!error && stats) {
+        *stats = put.flow.stats;
+    }
+    return error;
 }
 
 static int
-dpif_linux_flow_del(struct dpif *dpif_, struct odp_flow *flow)
+dpif_linux_flow_del(struct dpif *dpif_,
+                    const struct nlattr *key, size_t key_len,
+                    struct odp_flow_stats *stats)
 {
-    return do_ioctl(dpif_, ODP_FLOW_DEL, flow);
+    struct odp_flow odp_flow;
+    int error;
+
+    memset(&odp_flow, 0, sizeof odp_flow);
+    odp_flow.key = (struct nlattr *) key;
+    odp_flow.key_len = key_len;
+    error = do_ioctl(dpif_, ODP_FLOW_DEL, &odp_flow);
+    if (!error && stats) {
+        *stats = odp_flow.stats;
+    }
+    return error;
 }
 
+struct dpif_linux_flow_state {
+    struct odp_flow_dump dump;
+    struct odp_flow flow;
+    uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
+    uint32_t actionsbuf[65536 / sizeof(uint32_t)];
+};
+
 static int
 dpif_linux_flow_dump_start(const struct dpif *dpif OVS_UNUSED, void **statep)
 {
-    *statep = xzalloc(sizeof(struct odp_flow_dump));
+    struct dpif_linux_flow_state *state;
+
+    *statep = state = xmalloc(sizeof *state);
+    state->dump.state[0] = 0;
+    state->dump.state[1] = 0;
+    state->dump.flow = &state->flow;
     return 0;
 }
 
 static int
-dpif_linux_flow_dump_next(const struct dpif *dpif, void *state,
-                          struct odp_flow *flow)
+dpif_linux_flow_dump_next(const struct dpif *dpif, void *state_,
+                          const struct nlattr **key, size_t *key_len,
+                          const struct nlattr **actions, size_t *actions_len,
+                          const struct odp_flow_stats **stats)
 {
-    struct odp_flow_dump *dump = state;
+    struct dpif_linux_flow_state *state = state_;
     int error;
 
-    dump->flow = flow;
-    error = do_ioctl(dpif, ODP_FLOW_DUMP, dump);
-    return error ? error : flow->flags & ODPFF_EOF ? EOF : 0;
+    memset(&state->flow, 0, sizeof state->flow);
+    state->flow.key = (struct nlattr *) state->keybuf;
+    state->flow.key_len = sizeof state->keybuf;
+    if (actions) {
+        state->flow.actions = (struct nlattr *) state->actionsbuf;
+        state->flow.actions_len = sizeof state->actionsbuf;
+    }
+
+    error = do_ioctl(dpif, ODP_FLOW_DUMP, &state->dump);
+    if (!error) {
+        if (state->flow.flags & ODPFF_EOF) {
+            return EOF;
+        }
+        if (key) {
+            *key = (const struct nlattr *) state->keybuf;
+            *key_len = state->flow.key_len;
+        }
+        if (actions) {
+            *actions = (const struct nlattr *) state->actionsbuf;
+            *actions_len = state->flow.actions_len;
+        }
+        if (stats) {
+            *stats = &state->flow.stats;
+        }
+    }
+    return error;
 }
 
 static int
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8bb0ea4..80b890f 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -620,26 +620,15 @@ dp_netdev_lookup_flow(const struct dp_netdev *dp, const struct flow *key)
     return NULL;
 }
 
-/* The caller must fill in odp_flow->key itself. */
 static void
-answer_flow_query(struct dp_netdev_flow *flow, uint32_t query_flags,
-                  struct odp_flow *odp_flow)
-{
-    odp_flow->stats.n_packets = flow->packet_count;
-    odp_flow->stats.n_bytes = flow->byte_count;
-    odp_flow->stats.used_sec = flow->used.tv_sec;
-    odp_flow->stats.used_nsec = flow->used.tv_nsec;
-    odp_flow->stats.tcp_flags = TCP_FLAGS(flow->tcp_ctl);
-    odp_flow->stats.reserved = 0;
-    if (odp_flow->actions_len > 0) {
-        memcpy(odp_flow->actions, flow->actions,
-               MIN(odp_flow->actions_len, flow->actions_len));
-        odp_flow->actions_len = flow->actions_len;
-    }
-
-    if (query_flags & ODPFF_ZERO_TCP_FLAGS) {
-        flow->tcp_ctl = 0;
-    }
+get_odp_flow_stats(struct dp_netdev_flow *flow, struct odp_flow_stats *stats)
+{
+    stats->n_packets = flow->packet_count;
+    stats->n_bytes = flow->byte_count;
+    stats->used_sec = flow->used.tv_sec;
+    stats->used_nsec = flow->used.tv_nsec;
+    stats->tcp_flags = TCP_FLAGS(flow->tcp_ctl);
+    stats->reserved = 0;
 }
 
 static int
@@ -669,15 +658,16 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len,
 }
 
 static int
-dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow *odp_flow)
+dpif_netdev_flow_get(const struct dpif *dpif, int flags,
+                     const struct nlattr *nl_key, size_t nl_key_len,
+                     struct ofpbuf **actionsp, struct odp_flow_stats *stats)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
     struct flow key;
     int error;
 
-    error = dpif_netdev_flow_from_nlattrs(odp_flow->key, odp_flow->key_len,
-                                          &key);
+    error = dpif_netdev_flow_from_nlattrs(nl_key, nl_key_len, &key);
     if (error) {
         return error;
     }
@@ -687,7 +677,15 @@ dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow *odp_flow)
         return ENOENT;
     }
 
-    answer_flow_query(flow, odp_flow->flags, odp_flow);
+    if (stats) {
+        get_odp_flow_stats(flow, stats);
+    }
+    if (actionsp) {
+        *actionsp = ofpbuf_clone_data(flow->actions, flow->actions_len);
+    }
+    if (flags & ODPFF_ZERO_TCP_FLAGS) {
+        flow->tcp_ctl = 0;
+    }
     return 0;
 }
 
@@ -753,25 +751,26 @@ dpif_netdev_validate_actions(const struct nlattr *actions,
 }
 
 static int
-set_flow_actions(struct dp_netdev_flow *flow, struct odp_flow *odp_flow)
+set_flow_actions(struct dp_netdev_flow *flow,
+                 const struct nlattr *actions, size_t actions_len)
 {
     bool mutates;
     int error;
 
-    error = dpif_netdev_validate_actions(odp_flow->actions,
-                                         odp_flow->actions_len, &mutates);
+    error = dpif_netdev_validate_actions(actions, actions_len, &mutates);
     if (error) {
         return error;
     }
 
-    flow->actions = xrealloc(flow->actions, odp_flow->actions_len);
-    flow->actions_len = odp_flow->actions_len;
-    memcpy(flow->actions, odp_flow->actions, odp_flow->actions_len);
+    flow->actions = xrealloc(flow->actions, actions_len);
+    flow->actions_len = actions_len;
+    memcpy(flow->actions, actions, actions_len);
     return 0;
 }
 
 static int
-add_flow(struct dpif *dpif, const struct flow *key, struct odp_flow *odp_flow)
+add_flow(struct dpif *dpif, const struct flow *key,
+         const struct nlattr *actions, size_t actions_len)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
@@ -780,7 +779,7 @@ add_flow(struct dpif *dpif, const struct flow *key, struct odp_flow *odp_flow)
     flow = xzalloc(sizeof *flow);
     flow->key = *key;
 
-    error = set_flow_actions(flow, odp_flow);
+    error = set_flow_actions(flow, actions, actions_len);
     if (error) {
         free(flow);
         return error;
@@ -801,24 +800,29 @@ clear_stats(struct dp_netdev_flow *flow)
 }
 
 static int
-dpif_netdev_flow_put(struct dpif *dpif, struct odp_flow_put *put)
+dpif_netdev_flow_put(struct dpif *dpif, int flags,
+                    const struct nlattr *nl_key, size_t nl_key_len,
+                    const struct nlattr *actions, size_t actions_len,
+                    struct odp_flow_stats *stats)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
     struct flow key;
     int error;
 
-    error = dpif_netdev_flow_from_nlattrs(put->flow.key, put->flow.key_len,
-                                          &key);
+    error = dpif_netdev_flow_from_nlattrs(nl_key, nl_key_len, &key);
     if (error) {
         return error;
     }
 
     flow = dp_netdev_lookup_flow(dp, &key);
     if (!flow) {
-        if (put->flags & ODPPF_CREATE) {
+        if (flags & ODPPF_CREATE) {
             if (hmap_count(&dp->flow_table) < MAX_FLOWS) {
-                return add_flow(dpif, &key, &put->flow);
+                if (stats) {
+                    memset(stats, 0, sizeof *stats);
+                }
+                return add_flow(dpif, &key, actions, actions_len);
             } else {
                 return EFBIG;
             }
@@ -826,10 +830,15 @@ dpif_netdev_flow_put(struct dpif *dpif, struct odp_flow_put *put)
             return ENOENT;
         }
     } else {
-        if (put->flags & ODPPF_MODIFY) {
-            int error = set_flow_actions(flow, &put->flow);
-            if (!error && put->flags & ODPPF_ZERO_STATS) {
-                clear_stats(flow);
+        if (flags & ODPPF_MODIFY) {
+            int error = set_flow_actions(flow, actions, actions_len);
+            if (!error) {
+                if (stats) {
+                    get_odp_flow_stats(flow, stats);
+                }
+                if (flags & ODPPF_ZERO_STATS) {
+                    clear_stats(flow);
+                }
             }
             return error;
         } else {
@@ -838,24 +847,26 @@ dpif_netdev_flow_put(struct dpif *dpif, struct odp_flow_put *put)
     }
 }
 
-
 static int
-dpif_netdev_flow_del(struct dpif *dpif, struct odp_flow *odp_flow)
+dpif_netdev_flow_del(struct dpif *dpif,
+                     const struct nlattr *nl_key, size_t nl_key_len,
+                     struct odp_flow_stats *stats)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
     struct flow key;
     int error;
 
-    error = dpif_netdev_flow_from_nlattrs(odp_flow->key, odp_flow->key_len,
-                                          &key);
+    error = dpif_netdev_flow_from_nlattrs(nl_key, nl_key_len, &key);
     if (error) {
         return error;
     }
 
     flow = dp_netdev_lookup_flow(dp, &key);
     if (flow) {
-        answer_flow_query(flow, 0, odp_flow);
+        if (stats) {
+            get_odp_flow_stats(flow, stats);
+        }
         dp_netdev_free_flow(dp, flow);
         return 0;
     } else {
@@ -866,24 +877,33 @@ dpif_netdev_flow_del(struct dpif *dpif, struct odp_flow *odp_flow)
 struct dp_netdev_flow_state {
     uint32_t bucket;
     uint32_t offset;
+    struct nlattr *actions;
+    uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
+    struct odp_flow_stats stats;
 };
 
 static int
 dpif_netdev_flow_dump_start(const struct dpif *dpif OVS_UNUSED, void **statep)
 {
-    *statep = xzalloc(sizeof(struct dp_netdev_flow_state));
+    struct dp_netdev_flow_state *state;
+
+    *statep = state = xmalloc(sizeof *state);
+    state->bucket = 0;
+    state->offset = 0;
+    state->actions = NULL;
     return 0;
 }
 
 static int
 dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_,
-                           struct odp_flow *odp_flow)
+                           const struct nlattr **key, size_t *key_len,
+                           const struct nlattr **actions, size_t *actions_len,
+                           const struct odp_flow_stats **stats)
 {
     struct dp_netdev_flow_state *state = state_;
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
     struct hmap_node *node;
-    struct ofpbuf key;
 
     node = hmap_at_position(&dp->flow_table, &state->bucket, &state->offset);
     if (!node) {
@@ -892,19 +912,39 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_,
 
     flow = CONTAINER_OF(node, struct dp_netdev_flow, node);
 
-    ofpbuf_use_stack(&key, odp_flow->key, odp_flow->key_len);
-    odp_flow_key_from_flow(&key, &flow->key);
-    odp_flow->key_len = key.size;
-    ofpbuf_uninit(&key);
+    if (key) {
+        struct ofpbuf buf;
+
+        ofpbuf_use_stack(&buf, state->keybuf, sizeof state->keybuf);
+        odp_flow_key_from_flow(&buf, &flow->key);
+        assert(buf.base == state->keybuf);
+
+        *key = buf.data;
+        *key_len = buf.size;
+    }
 
-    answer_flow_query(flow, 0, odp_flow);
+    if (actions) {
+        free(state->actions);
+        state->actions = xmemdup(flow->actions, flow->actions_len);
+
+        *actions = state->actions;
+        *actions_len = flow->actions_len;
+    }
+
+    if (stats) {
+        get_odp_flow_stats(flow, &state->stats);
+        *stats = &state->stats;
+    }
 
     return 0;
 }
 
 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 *state_)
 {
+    struct dp_netdev_flow_state *state = state_;
+
+    free(state->actions);
     free(state);
     return 0;
 }
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 2218c11..bded290 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -207,45 +207,59 @@ struct dpif_class {
      * value other than EAGAIN. */
     void (*port_poll_wait)(const struct dpif *dpif);
 
-    /* Queries 'dpif' for a flow entry matching 'flow->key'.
+    /* Queries 'dpif' for a flow entry.  The flow is specified by the Netlink
+     * attributes with types ODP_KEY_ATTR_* in the 'key_len' bytes starting at
+     * 'key'.
      *
-     * If a flow matching 'flow->key' exists in 'dpif', stores statistics for
-     * the flow into 'flow->stats'.  If 'flow->actions_len' is zero, then
-     * 'flow->actions' is ignored.  If 'flow->actions_len' is nonzero, then
-     * 'flow->actions' should point to an array of the specified number of
-     * bytes.  At most that many bytes of the flow's actions will be copied
-     * into that array.  'flow->actions_len' will be updated to the number of
-     * bytes of actions actually present in the flow, which may be greater than
-     * the amount stored if the flow has more actions than space available in
-     * the array.
+     * Returns 0 if successful.  If no flow matches, returns ENOENT.  On other
+     * failure, returns a positive errno value.
      *
-     * If no flow matching 'flow->key' exists in 'dpif', returns ENOENT.  On
-     * other failure, returns a positive errno value. */
-    int (*flow_get)(const struct dpif *dpif, struct odp_flow *flow);
-
-    /* Adds or modifies a flow in 'dpif' as specified in 'put':
+     * If 'actionsp' is nonnull, then on success '*actionsp' must be set to an
+     * ofpbuf owned by the caller that contains the Netlink attributes for the
+     * flow's actions.  The caller must free the ofpbuf (with ofpbuf_delete())
+     * when it is no longer needed.
+     *
+     * If 'stats' is nonnull, then on success it must be updated with the
+     * flow's statistics. */
+    int (*flow_get)(const struct dpif *dpif, int flags,
+                    const struct nlattr *key, size_t key_len,
+                    struct ofpbuf **actionsp, struct odp_flow_stats *stats);
+
+    /* Adds or modifies a flow in 'dpif'.  The flow is specified by the Netlink
+     * attributes with types ODP_KEY_ATTR_* in the 'key_len' bytes starting at
+     * 'key'.  The associated actions are specified by the Netlink attributes
+     * with types ODPAT_* in the 'actions_len' bytes starting at 'actions'.
+     *
+     * - If the flow's key does not exist in 'dpif', then the flow will be
+     *   added if 'flags' includes ODPPF_CREATE.  Otherwise the operation will
+     *   fail with ENOENT.
+     *
+     *   If the operation succeeds, then 'stats', if nonnull, must be zeroed.
      *
-     * - If the flow specified in 'put->flow' does not exist in 'dpif', then
-     *   behavior depends on whether ODPPF_CREATE is specified in 'put->flags':
-     *   if it is, the flow will be added, otherwise the operation will fail
-     *   with ENOENT.
+     * - If the flow's key does exist in 'dpif', then the flow's actions will
+     *   be updated if 'flags' includes ODPPF_MODIFY.  Otherwise the operation
+     *   will fail with EEXIST.  If the flow's actions are updated, then its
+     *   statistics will be zeroed if 'flags' includes ODPPF_ZERO_STATS, and
+     *   left as-is otherwise.
      *
-     * - Otherwise, the flow specified in 'put->flow' does exist in 'dpif'.
-     *   Behavior in this case depends on whether ODPPF_MODIFY is specified in
-     *   'put->flags': if it is, the flow's actions will be updated, otherwise
-     *   the operation will fail with EEXIST.  If the flow's actions are
-     *   updated, then its statistics will be zeroed if ODPPF_ZERO_STATS is set
-     *   in 'put->flags', left as-is otherwise.
+     *   If the operation succeeds, then 'stats', if nonnull, must be set to
+     *   the flow's statistics before the update.
      */
-    int (*flow_put)(struct dpif *dpif, struct odp_flow_put *put);
-
-    /* Deletes a flow matching 'flow->key' from 'dpif' or returns ENOENT if
-     * 'dpif' does not contain such a flow.
+    int (*flow_put)(struct dpif *dpif, int flags,
+                    const struct nlattr *key, size_t key_len,
+                    const struct nlattr *actions, size_t actions_len,
+                    struct odp_flow_stats *stats);
+
+    /* Deletes a flow from 'dpif' and returns 0, or returns ENOENT if 'dpif'
+     * does not contain such a flow.  The flow is specified by the Netlink
+     * attributes with types ODP_KEY_ATTR_* in the 'key_len' bytes starting at
+     * 'key'.
      *
-     * If successful, updates 'flow->stats', 'flow->n_actions', and
-     * 'flow->actions' as described in more detail under the flow_get member
-     * function below. */
-    int (*flow_del)(struct dpif *dpif, struct odp_flow *flow);
+     * If the operation succeeds, then 'stats', if nonnull, must be set to the
+     * flow's statistics before its deletion. */
+    int (*flow_del)(struct dpif *dpif,
+                    const struct nlattr *key, size_t key_len,
+                    struct odp_flow_stats *stats);
 
     /* Deletes all flows from 'dpif' and clears all of its queues of received
      * packets. */
@@ -258,22 +272,27 @@ struct dpif_class {
 
     /* Attempts to retrieve another flow from 'dpif' for 'state', which was
      * initialized by a successful call to the 'flow_dump_start' function for
-     * 'dpif'.  On success, stores a new odp_flow into 'flow' and returns 0.
-     * Returns EOF if the end of the flow table has been reached, or a positive
-     * errno value on error.  This function will not be called again once it
-     * returns nonzero once for a given iteration (but the 'flow_dump_done'
-     * function will be called afterward).
+     * 'dpif'.  On success, updates the output parameters as described below
+     * and returns 0.  Returns EOF if the end of the flow table has been
+     * reached, or a positive errno value on error.  This function will not be
+     * called again once it returns nonzero within a given iteration (but the
+     * 'flow_dump_done' function will be called afterward).
+     *
+     * On success, if 'key' and 'key_len' are nonnull then '*key' and
+     * '*key_len' must be set to Netlink attributes with types ODP_KEY_ATTR_*
+     * representing the dumped flow's key.  If 'actions' and 'actions_len' are
+     * nonnull then they should be set to Netlink attributes with types ODPAT_*
+     * representing the dumped flow's actions.  If 'stats' is nonnull then it
+     * should be set to the dumped flow's statistics.
      *
-     * Dumping flow actions is optional.  If the caller does not want to dump
-     * actions it will initialize 'flow->actions' to NULL and
-     * 'flow->actions_len' to 0.  Otherwise, 'flow->actions' points to an array
-     * of struct nlattr and 'flow->actions_len' contains the number of bytes of
-     * Netlink attributes.  The implemention should fill in as many actions as
-     * will fit into the provided array and update 'flow->actions_len' with the
-     * number of bytes required (regardless of whether they fit in the provided
-     * space). */
+     * 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,
-                          struct odp_flow *flow);
+                          const struct nlattr **key, size_t *key_len,
+                          const struct nlattr **actions, size_t *actions_len,
+                          const struct odp_flow_stats **stats);
 
     /* Releases resources from 'dpif' for 'state', which was initialized by a
      * successful call to the 'flow_dump_start' function for 'dpif'.  */
diff --git a/lib/dpif.c b/lib/dpif.c
index 463d4e7..487b60a 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -76,15 +76,14 @@ static struct vlog_rate_limit dpmsg_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 /* Not really much point in logging many dpif errors. */
 static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
 
+static void log_flow_message(const struct dpif *dpif, int error,
+                             const char *operation,
+                             const struct nlattr *key, size_t key_len,
+                             const struct odp_flow_stats *stats,
+                             const struct nlattr *actions, size_t actions_len);
 static void log_operation(const struct dpif *, const char *operation,
                           int error);
-static void log_flow_operation(const struct dpif *, const char *operation,
-                               int error, struct odp_flow *flow);
-static void log_flow_put(struct dpif *, int error,
-                         const struct odp_flow_put *);
 static bool should_log_flow_message(int error);
-static void check_rw_flow_actions(struct odp_flow *);
-static void check_rw_flow_key(struct odp_flow *);
 
 static void
 dp_initialize(void)
@@ -702,87 +701,138 @@ dpif_flow_flush(struct dpif *dpif)
     return error;
 }
 
-/* Queries 'dpif' for a flow entry matching 'flow->key'.
+/* Queries 'dpif' for a flow entry.  The flow is specified by the Netlink
+ * attributes with types ODP_KEY_ATTR_* in the 'key_len' bytes starting at
+ * 'key'.
  *
- * If a flow matching 'flow->key' exists in 'dpif', stores statistics for the
- * flow into 'flow->stats'.  If 'flow->actions_len' is zero, then
- * 'flow->actions' is ignored.  If 'flow->actions_len' is nonzero, then
- * 'flow->actions' should point to an array of the specified number of bytes.
- * At most that many bytes of the flow's actions will be copied into that
- * array.  'flow->actions_len' will be updated to the number of bytes of
- * actions actually present in the flow, which may be greater than the amount
- * stored if the flow has more actions than space available in the array.
+ * Returns 0 if successful.  If no flow matches, returns ENOENT.  On other
+ * failure, returns a positive errno value.
  *
- * If no flow matching 'flow->key' exists in 'dpif', returns ENOENT.  On other
- * failure, returns a positive errno value. */
+ * If 'actionsp' is nonnull, then on success '*actionsp' will be set to an
+ * ofpbuf owned by the caller that contains the Netlink attributes for the
+ * flow's actions.  The caller must free the ofpbuf (with ofpbuf_delete()) when
+ * it is no longer needed.
+ *
+ * If 'stats' is nonnull, then on success it will be updated with the flow's
+ * statistics. */
 int
-dpif_flow_get(const struct dpif *dpif, struct odp_flow *flow)
+dpif_flow_get(const struct dpif *dpif, int flags,
+              const struct nlattr *key, size_t key_len,
+              struct ofpbuf **actionsp, struct odp_flow_stats *stats)
 {
     int error;
 
     COVERAGE_INC(dpif_flow_get);
 
-    check_rw_flow_actions(flow);
-    error = dpif->dpif_class->flow_get(dpif, flow);
+    error = dpif->dpif_class->flow_get(dpif, flags, key, key_len, actionsp,
+                                       stats);
     if (error) {
-        /* Make the results predictable on error. */
-        memset(&flow->stats, 0, sizeof flow->stats);
-        flow->actions_len = 0;
+        if (actionsp) {
+            *actionsp = NULL;
+        }
+        if (stats) {
+            memset(stats, 0, sizeof *stats);
+        }
     }
     if (should_log_flow_message(error)) {
-        log_flow_operation(dpif, "flow_get", error, flow);
+        const struct nlattr *actions;
+        size_t actions_len;
+
+        if (!error && actionsp) {
+            actions = (*actionsp)->data;
+            actions_len = (*actionsp)->size;
+        } else {
+            actions = NULL;
+            actions_len = 0;
+        }
+        log_flow_message(dpif, error, "flow_get", key, key_len, stats,
+                         actions, actions_len);
     }
     return error;
 }
 
-/* Adds or modifies a flow in 'dpif' as specified in 'put':
+/* Adds or modifies a flow in 'dpif'.  The flow is specified by the Netlink
+ * attributes with types ODP_KEY_ATTR_* in the 'key_len' bytes starting at
+ * 'key'.  The associated actions are specified by the Netlink attributes with
+ * types ODPAT_* in the 'actions_len' bytes starting at 'actions'.
  *
- * - If the flow specified in 'put->flow' does not exist in 'dpif', then
- *   behavior depends on whether ODPPF_CREATE is specified in 'put->flags': if
- *   it is, the flow will be added, otherwise the operation will fail with
+ * - If the flow's key does not exist in 'dpif', then the flow will be added if
+ *   'flags' includes ODPPF_CREATE.  Otherwise the operation will fail with
  *   ENOENT.
  *
- * - Otherwise, the flow specified in 'put->flow' does exist in 'dpif'.
- *   Behavior in this case depends on whether ODPPF_MODIFY is specified in
- *   'put->flags': if it is, the flow's actions will be updated, otherwise the
- *   operation will fail with EEXIST.  If the flow's actions are updated, then
- *   its statistics will be zeroed if ODPPF_ZERO_STATS is set in 'put->flags',
- *   left as-is otherwise.
+ *   If the operation succeeds, then 'stats', if nonnull, will be zeroed.
  *
- * Returns 0 if successful, otherwise a positive errno value.
+ * - If the flow's key does exist in 'dpif', then the flow's actions will be
+ *   updated if 'flags' includes ODPPF_MODIFY.  Otherwise the operation will
+ *   fail with EEXIST.  If the flow's actions are updated, then its statistics
+ *   will be zeroed if 'flags' includes ODPPF_ZERO_STATS, and left as-is
+ *   otherwise.
+ *
+ *   If the operation succeeds, then 'stats', if nonnull, will be set to the
+ *   flow's statistics before the update.
  */
 int
-dpif_flow_put(struct dpif *dpif, struct odp_flow_put *put)
+dpif_flow_put(struct dpif *dpif, int flags,
+              const struct nlattr *key, size_t key_len,
+              const struct nlattr *actions, size_t actions_len,
+              struct odp_flow_stats *stats)
 {
     int error;
 
     COVERAGE_INC(dpif_flow_put);
 
-    error = dpif->dpif_class->flow_put(dpif, put);
+    error = dpif->dpif_class->flow_put(dpif, flags, key, key_len,
+                                       actions, actions_len, stats);
+    if (error && stats) {
+        memset(stats, 0, sizeof *stats);
+    }
     if (should_log_flow_message(error)) {
-        log_flow_put(dpif, error, put);
+        enum { ODPPF_ALL = ODPPF_CREATE | ODPPF_MODIFY | ODPPF_ZERO_STATS };
+        struct ds s;
+
+        ds_init(&s);
+        ds_put_cstr(&s, "put");
+        if (flags & ODPPF_CREATE) {
+            ds_put_cstr(&s, "[create]");
+        }
+        if (flags & ODPPF_MODIFY) {
+            ds_put_cstr(&s, "[modify]");
+        }
+        if (flags & ODPPF_ZERO_STATS) {
+            ds_put_cstr(&s, "[zero]");
+        }
+        if (flags & ~ODPPF_ALL) {
+            ds_put_format(&s, "[%x]", flags & ~ODPPF_ALL);
+        }
+        log_flow_message(dpif, error, ds_cstr(&s), key, key_len, stats,
+                         actions, actions_len);
+        ds_destroy(&s);
     }
     return error;
 }
 
-/* Deletes a flow matching 'flow->key' from 'dpif' or returns ENOENT if 'dpif'
- * does not contain such a flow.
+/* Deletes a flow from 'dpif' and returns 0, or returns ENOENT if 'dpif' does
+ * not contain such a flow.  The flow is specified by the Netlink attributes
+ * with types ODP_KEY_ATTR_* in the 'key_len' bytes starting at 'key'.
  *
- * If successful, updates 'flow->stats', 'flow->actions_len', and
- * 'flow->actions' as described for dpif_flow_get(). */
+ * If the operation succeeds, then 'stats', if nonnull, will be set to the
+ * flow's statistics before its deletion. */
 int
-dpif_flow_del(struct dpif *dpif, struct odp_flow *flow)
+dpif_flow_del(struct dpif *dpif,
+              const struct nlattr *key, size_t key_len,
+              struct odp_flow_stats *stats)
 {
     int error;
 
     COVERAGE_INC(dpif_flow_del);
 
-    check_rw_flow_actions(flow);
-    memset(&flow->stats, 0, sizeof flow->stats);
-
-    error = dpif->dpif_class->flow_del(dpif, flow);
+    error = dpif->dpif_class->flow_del(dpif, key, key_len, stats);
+    if (error && stats) {
+        memset(stats, 0, sizeof *stats);
+    }
     if (should_log_flow_message(error)) {
-        log_flow_operation(dpif, "delete flow", error, flow);
+        log_flow_message(dpif, error, "flow_del", key, key_len,
+                         !error ? stats : NULL, NULL, 0);
     }
     return error;
 }
@@ -802,47 +852,66 @@ dpif_flow_dump_start(struct dpif_flow_dump *dump, const struct dpif *dpif)
 }
 
 /* Attempts to retrieve another flow from 'dump', which must have been
- * initialized with dpif_flow_dump_start().  On success, stores a new odp_flow
- * into 'flow' and returns true.  Failure might indicate an actual error or
- * merely the end of the flow table.  An error status for the entire dump
- * operation is provided when it is completed by calling dpif_flow_dump_done().
+ * initialized with dpif_flow_dump_start().  On success, updates the output
+ * parameters as described below and returns true.  Otherwise, returns false.
+ * Failure might indicate an actual error or merely the end of the flow table.
+ * An error status for the entire dump operation is provided when it is
+ * completed by calling dpif_flow_dump_done().
+ *
+ * On success, if 'key' and 'key_len' are nonnull then '*key' and '*key_len'
+ * will be set to Netlink attributes with types ODP_KEY_ATTR_* representing the
+ * dumped flow's key.  If 'actions' and 'actions_len' are nonnull then they are
+ * set to Netlink attributes with types ODPAT_* representing the dumped flow's
+ * actions.  If 'stats' is nonnull then it will be set to the dumped flow's
+ * statistics.
  *
- * Dumping flow actions is optional.  To avoid dumping actions initialize
- * 'flow->actions' to NULL and 'flow->actions_len' to 0.  Otherwise, point
- * 'flow->actions' to an array of struct nlattr and initialize
- * 'flow->actions_len' with the number of bytes of Netlink attributes.
- * dpif_flow_dump_next() will fill in as many actions as will fit into the
- * provided array and update 'flow->actions_len' with the number of bytes
- * required (regardless of whether they fit in the provided space). */
+ * All of the returned data is owned by 'dpif', not by the caller, and the
+ * caller must not modify or free it.  'dpif' guarantees that it remains
+ * accessible and unchanging until at least the next call to 'flow_dump_next'
+ * or 'flow_dump_done' for 'dump'. */
 bool
-dpif_flow_dump_next(struct dpif_flow_dump *dump, struct odp_flow *flow)
+dpif_flow_dump_next(struct dpif_flow_dump *dump,
+                    const struct nlattr **key, size_t *key_len,
+                    const struct nlattr **actions, size_t *actions_len,
+                    const struct odp_flow_stats **stats)
 {
     const struct dpif *dpif = dump->dpif;
+    int error = dump->error;
 
-    check_rw_flow_actions(flow);
-    check_rw_flow_key(flow);
-
-    if (dump->error) {
-        return false;
+    if (!error) {
+        error = dpif->dpif_class->flow_dump_next(dpif, dump->state,
+                                                 key, key_len,
+                                                 actions, actions_len,
+                                                 stats);
+        if (error) {
+            dpif->dpif_class->flow_dump_done(dpif, dump->state);
+        }
     }
-
-    dump->error = dpif->dpif_class->flow_dump_next(dpif, dump->state, flow);
-    if (dump->error == EOF) {
-        VLOG_DBG_RL(&dpmsg_rl, "%s: dumped all flows", dpif_name(dpif));
-    } else {
-        if (dump->error) {
-            flow->key_len = 0;
+    if (error) {
+        if (key) {
+            *key = NULL;
+            *key_len = 0;
         }
-        if (should_log_flow_message(dump->error)) {
-            log_flow_operation(dpif, "flow_dump_next", dump->error, flow);
+        if (actions) {
+            *actions = NULL;
+            *actions_len = 0;
+        }
+        if (stats) {
+            *stats = NULL;
         }
     }
-
-    if (dump->error) {
-        dpif->dpif_class->flow_dump_done(dpif, dump->state);
-        return false;
+    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,
+                             stats ? *stats : NULL, actions ? *actions : NULL,
+                             actions ? *actions_len : 0);
+        }
     }
-    return true;
+    dump->error = error;
+    return !error;
 }
 
 /* Completes flow table dump operation 'dump', which must have been initialized
@@ -1124,76 +1193,3 @@ log_flow_message(const struct dpif *dpif, int error, const char *operation,
     vlog(THIS_MODULE, flow_message_log_level(error), "%s", ds_cstr(&ds));
     ds_destroy(&ds);
 }
-
-static void
-log_flow_operation(const struct dpif *dpif, const char *operation, int error,
-                   struct odp_flow *flow)
-{
-    if (error) {
-        flow->key_len = 0;
-        flow->actions_len = 0;
-    }
-    log_flow_message(dpif, error, operation,
-                     flow->key, flow->key_len, !error ? &flow->stats : NULL,
-                     flow->actions, flow->actions_len);
-}
-
-static void
-log_flow_put(struct dpif *dpif, int error, const struct odp_flow_put *put)
-{
-    enum { ODPPF_ALL = ODPPF_CREATE | ODPPF_MODIFY | ODPPF_ZERO_STATS };
-    struct ds s;
-
-    ds_init(&s);
-    ds_put_cstr(&s, "put");
-    if (put->flags & ODPPF_CREATE) {
-        ds_put_cstr(&s, "[create]");
-    }
-    if (put->flags & ODPPF_MODIFY) {
-        ds_put_cstr(&s, "[modify]");
-    }
-    if (put->flags & ODPPF_ZERO_STATS) {
-        ds_put_cstr(&s, "[zero]");
-    }
-    if (put->flags & ~ODPPF_ALL) {
-        ds_put_format(&s, "[%x]", put->flags & ~ODPPF_ALL);
-    }
-    log_flow_message(dpif, error, ds_cstr(&s),
-                     put->flow.key, put->flow.key_len,
-                     !error ? &put->flow.stats : NULL,
-                     put->flow.actions, put->flow.actions_len);
-    ds_destroy(&s);
-}
-
-/* There is a tendency to construct odp_flow objects on the stack and to
- * forget to properly initialize their "actions" and "actions_len" members.
- * When this happens, we get memory corruption because the kernel
- * writes through the random pointer that is in the "actions" member.
- *
- * This function attempts to combat the problem by:
- *
- *      - Forcing a segfault if "actions" points to an invalid region (instead
- *        of just getting back EFAULT, which can be easily missed in the log).
- *
- *      - Storing a distinctive value that is likely to cause an
- *        easy-to-identify error later if it is dereferenced, etc.
- *
- *      - Triggering a warning on uninitialized memory from Valgrind if
- *        "actions" or "actions_len" was not initialized.
- */
-static void
-check_rw_flow_actions(struct odp_flow *flow)
-{
-    if (flow->actions_len) {
-        memset(&flow->actions[0], 0xcc, sizeof flow->actions[0]);
-    }
-}
-
-/* Same as check_rw_flow_actions() but for flow->key. */
-static void
-check_rw_flow_key(struct odp_flow *flow)
-{
-    if (flow->key_len) {
-        memset(&flow->key[0], 0xcc, sizeof flow->key[0]);
-    }
-}
diff --git a/lib/dpif.h b/lib/dpif.h
index 3c376ec..22e3580 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -108,9 +108,16 @@ int dpif_port_poll(const struct dpif *, char **devnamep);
 void dpif_port_poll_wait(const struct dpif *);
 
 int dpif_flow_flush(struct dpif *);
-int dpif_flow_put(struct dpif *, struct odp_flow_put *);
-int dpif_flow_del(struct dpif *, struct odp_flow *);
-int dpif_flow_get(const struct dpif *, struct odp_flow *);
+int dpif_flow_put(struct dpif *, int flags,
+                  const struct nlattr *key, size_t key_len,
+                  const struct nlattr *actions, size_t actions_len,
+                  struct odp_flow_stats *);
+int dpif_flow_del(struct dpif *,
+                  const struct nlattr *key, size_t key_len,
+                  struct odp_flow_stats *);
+int dpif_flow_get(const struct dpif *, int flags,
+                  const struct nlattr *key, size_t key_len,
+                  struct ofpbuf **actionsp, struct odp_flow_stats *);
 
 struct dpif_flow_dump {
     const struct dpif *dpif;
@@ -118,7 +125,10 @@ struct dpif_flow_dump {
     void *state;
 };
 void dpif_flow_dump_start(struct dpif_flow_dump *, const struct dpif *);
-bool dpif_flow_dump_next(struct dpif_flow_dump *, struct odp_flow *);
+bool dpif_flow_dump_next(struct dpif_flow_dump *,
+                         const struct nlattr **key, size_t *key_len,
+                         const struct nlattr **actions, size_t *actions_len,
+                         const struct odp_flow_stats **);
 int dpif_flow_dump_done(struct dpif_flow_dump *);
 
 int dpif_execute(struct dpif *, const struct nlattr *actions,
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 013a7f1..60807ba 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -191,16 +191,6 @@ format_odp_flow_stats(struct ds *ds, const struct odp_flow_stats *s)
         ds_put_format(ds, "never");
     }
 }
-
-void
-format_odp_flow(struct ds *ds, const struct odp_flow *f)
-{
-    odp_flow_key_format(f->key, f->key_len, ds);
-    ds_put_cstr(ds, ", ");
-    format_odp_flow_stats(ds, &f->stats);
-    ds_put_cstr(ds, ", actions:");
-    format_odp_actions(ds, f->actions, f->actions_len);
-}
 
 /* Returns the correct length of the payload for a flow key attribute of the
  * specified 'type', or -1 if 'type' is unknown. */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 9486661..2062c06 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010 Nicira Networks.
+ * Copyright (c) 2009, 2010, 2011 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -61,7 +61,6 @@ void format_odp_action(struct ds *, const struct nlattr *);
 void format_odp_actions(struct ds *, const struct nlattr *odp_actions,
                         size_t actions_len);
 void format_odp_flow_stats(struct ds *, const struct odp_flow_stats *);
-void format_odp_flow(struct ds *, const struct odp_flow *);
 
 /* By my calculations currently the longest valid nlattr-formatted flow key is
  * 80 bytes long, so this leaves some safety margin.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index d928f68..ab945b0 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2306,8 +2306,7 @@ facet_make_actions(struct ofproto *p, struct facet *facet,
 }
 
 static int
-facet_put__(struct ofproto *ofproto, struct facet *facet, int flags,
-            struct odp_flow_put *put)
+facet_put__(struct ofproto *ofproto, struct facet *facet, int flags)
 {
     uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
     struct ofpbuf key;
@@ -2316,14 +2315,8 @@ facet_put__(struct ofproto *ofproto, struct facet *facet, int flags,
     odp_flow_key_from_flow(&key, &facet->flow);
     assert(key.base == keybuf);
 
-    memset(&put->flow.stats, 0, sizeof put->flow.stats);
-    put->flow.key = key.data;
-    put->flow.key_len = key.size;
-    put->flow.actions = facet->actions;
-    put->flow.actions_len = facet->actions_len;
-    put->flow.flags = 0;
-    put->flags = flags;
-    return dpif_flow_put(ofproto->dpif, put);
+    return dpif_flow_put(ofproto->dpif, flags, key.data, key.size,
+                         facet->actions, facet->actions_len, NULL);
 }
 
 /* If 'facet' is installable, inserts or re-inserts it into 'p''s datapath.  If
@@ -2333,14 +2326,11 @@ static void
 facet_install(struct ofproto *p, struct facet *facet, bool zero_stats)
 {
     if (facet->may_install) {
-        struct odp_flow_put put;
-        int flags;
-
-        flags = ODPPF_CREATE | ODPPF_MODIFY;
+        int flags = ODPPF_CREATE | ODPPF_MODIFY;
         if (zero_stats) {
             flags |= ODPPF_ZERO_STATS;
         }
-        if (!facet_put__(p, facet, flags, &put)) {
+        if (!facet_put__(p, facet, flags)) {
             facet->installed = true;
         }
     }
@@ -2370,20 +2360,15 @@ facet_uninstall(struct ofproto *p, struct facet *facet)
 {
     if (facet->installed) {
         uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
-        struct odp_flow odp_flow;
+        struct odp_flow_stats stats;
         struct ofpbuf key;
 
         ofpbuf_use_stack(&key, keybuf, sizeof keybuf);
         odp_flow_key_from_flow(&key, &facet->flow);
         assert(key.base == keybuf);
 
-        odp_flow.key = key.data;
-        odp_flow.key_len = key.size;
-        odp_flow.actions = NULL;
-        odp_flow.actions_len = 0;
-        odp_flow.flags = 0;
-        if (!dpif_flow_del(p->dpif, &odp_flow)) {
-            facet_update_stats(p, facet, &odp_flow.stats);
+        if (!dpif_flow_del(p->dpif, key.data, key.size, &stats)) {
+            facet_update_stats(p, facet, &stats);
         }
         facet->installed = false;
     }
@@ -2515,22 +2500,18 @@ facet_revalidate(struct ofproto *ofproto, struct facet *facet)
     if (actions_changed || facet->may_install != facet->installed) {
         if (facet->may_install) {
             uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
-            struct odp_flow_put put;
+            struct odp_flow_stats stats;
             struct ofpbuf key;
 
             ofpbuf_use_stack(&key, keybuf, sizeof keybuf);
             odp_flow_key_from_flow(&key, &facet->flow);
 
-            memset(&put.flow.stats, 0, sizeof put.flow.stats);
-            put.flow.key = key.data;
-            put.flow.key_len = key.size;
-            put.flow.actions = odp_actions->data;
-            put.flow.actions_len = odp_actions->size;
-            put.flow.flags = 0;
-            put.flags = ODPPF_CREATE | ODPPF_MODIFY | ODPPF_ZERO_STATS;
-            dpif_flow_put(ofproto->dpif, &put);
+            dpif_flow_put(ofproto->dpif,
+                          ODPPF_CREATE | ODPPF_MODIFY | ODPPF_ZERO_STATS,
+                          key.data, key.size,
+                          odp_actions->data, odp_actions->size, &stats);
 
-            facet_update_stats(ofproto, facet, &put.flow.stats);
+            facet_update_stats(ofproto, facet, &stats);
         } else {
             facet_uninstall(ofproto, facet);
         }
@@ -3495,23 +3476,14 @@ query_stats(struct ofproto *p, struct rule *rule,
      * to a rule. */
     ofpbuf_use_stack(&key, keybuf, sizeof keybuf);
     LIST_FOR_EACH (facet, list_node, &rule->facets) {
-        struct odp_flow odp_flow;
+        struct odp_flow_stats stats;
 
         ofpbuf_clear(&key);
         odp_flow_key_from_flow(&key, &facet->flow);
+        dpif_flow_get(p->dpif, 0, key.data, key.size, NULL, &stats);
 
-        odp_flow.key = key.data;
-        odp_flow.key_len = key.size;
-        odp_flow.actions = NULL;
-        odp_flow.actions_len = 0;
-        odp_flow.flags = 0;
-        if (!dpif_flow_get(p->dpif, &odp_flow)) {
-            packet_count += odp_flow.stats.n_packets;
-            byte_count += odp_flow.stats.n_bytes;
-        }
-
-        packet_count += facet->packet_count;
-        byte_count += facet->byte_count;
+        packet_count += stats.n_packets + facet->packet_count;
+        byte_count += stats.n_bytes + facet->byte_count;
     }
 
     /* Return the stats to the caller. */
@@ -4546,31 +4518,21 @@ ofproto_expire(struct ofproto *ofproto)
 static void
 ofproto_update_used(struct ofproto *p)
 {
+    const struct odp_flow_stats *stats;
     struct dpif_flow_dump dump;
+    const struct nlattr *key;
+    size_t key_len;
 
     dpif_flow_dump_start(&dump, p->dpif);
-    for (;;) {
-        uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
+    while (dpif_flow_dump_next(&dump, &key, &key_len, NULL, NULL, &stats)) {
         struct facet *facet;
-        struct odp_flow f;
         struct flow flow;
 
-        memset(&f, 0, sizeof f);
-        f.key = (struct nlattr *) keybuf;
-        f.key_len = sizeof keybuf;
-        if (!dpif_flow_dump_next(&dump, &f)) {
-            break;
-        }
-
-        if (f.key_len > sizeof keybuf) {
-            VLOG_WARN_RL(&rl, "ODP flow key overflowed buffer");
-            continue;
-        }
-        if (odp_flow_key_to_flow(f.key, f.key_len, &flow)) {
+        if (odp_flow_key_to_flow(key, key_len, &flow)) {
             struct ds s;
 
             ds_init(&s);
-            odp_flow_key_format(f.key, f.key_len, &s);
+            odp_flow_key_format(key, key_len, &s);
             VLOG_WARN_RL(&rl, "failed to convert ODP flow key to flow: %s",
                          ds_cstr(&s));
             ds_destroy(&s);
@@ -4580,13 +4542,13 @@ ofproto_update_used(struct ofproto *p)
         facet = facet_find(p, &flow);
 
         if (facet && facet->installed) {
-            facet_update_time(p, facet, &f.stats);
-            facet_account(p, facet, f.stats.n_bytes);
+            facet_update_time(p, facet, stats);
+            facet_account(p, facet, stats->n_bytes);
         } else {
             /* There's a flow in the datapath that we know nothing about.
              * Delete it. */
             COVERAGE_INC(ofproto_unexpected_rule);
-            dpif_flow_del(p->dpif, &f);
+            dpif_flow_del(p->dpif, key, key_len, NULL);
         }
     }
     dpif_flow_dump_done(&dump);
@@ -4688,39 +4650,37 @@ facet_active_timeout(struct ofproto *ofproto, struct facet *facet)
     if (ofproto->netflow && !facet_is_controller_flow(facet) &&
         netflow_active_timeout_expired(ofproto->netflow, &facet->nf_flow)) {
         struct ofexpired expired;
-        struct odp_flow odp_flow;
+
+        expired.flow = facet->flow;
+        expired.packet_count = facet->packet_count;
+        expired.byte_count = facet->byte_count;
+        expired.used = facet->used;
 
         /* Get updated flow stats.
          *
          * XXX We could avoid this call entirely if (1) ofproto_update_used()
          * updated TCP flags and (2) the dpif_flow_list_all() in
          * ofproto_update_used() zeroed TCP flags. */
-        memset(&odp_flow, 0, sizeof odp_flow);
         if (facet->installed) {
             uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
+            struct odp_flow_stats stats;
             struct ofpbuf key;
 
             ofpbuf_use_stack(&key, keybuf, sizeof keybuf);
             odp_flow_key_from_flow(&key, &facet->flow);
 
-            odp_flow.key = key.data;
-            odp_flow.key_len = key.size;
-            odp_flow.flags = ODPFF_ZERO_TCP_FLAGS;
-            dpif_flow_get(ofproto->dpif, &odp_flow);
-
-            if (odp_flow.stats.n_packets) {
-                facet_update_time(ofproto, facet, &odp_flow.stats);
-                netflow_flow_update_flags(&facet->nf_flow,
-                                          odp_flow.stats.tcp_flags);
+            if (!dpif_flow_get(ofproto->dpif, ODPFF_ZERO_TCP_FLAGS,
+                               key.data, key.size, NULL, &stats)) {
+                expired.packet_count += stats.n_packets;
+                expired.byte_count += stats.n_bytes;
+                if (stats.n_packets) {
+                    facet_update_time(ofproto, facet, &stats);
+                    netflow_flow_update_flags(&facet->nf_flow,
+                                              stats.tcp_flags);
+                }
             }
         }
 
-        expired.flow = facet->flow;
-        expired.packet_count = facet->packet_count +
-                               odp_flow.stats.n_packets;
-        expired.byte_count = facet->byte_count + odp_flow.stats.n_bytes;
-        expired.used = facet->used;
-
         netflow_expire(ofproto->netflow, &facet->nf_flow, &expired);
     }
 }
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 31b15e8..3167864 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -480,32 +480,27 @@ do_dump_dps(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 static void
 do_dump_flows(int argc OVS_UNUSED, char *argv[])
 {
+    const struct odp_flow_stats *stats;
+    const struct nlattr *actions;
     struct dpif_flow_dump dump;
+    const struct nlattr *key;
+    size_t actions_len;
     struct dpif *dpif;
+    size_t key_len;
     struct ds ds;
 
     run(parsed_dpif_open(argv[1], false, &dpif), "opening datapath");
 
     ds_init(&ds);
     dpif_flow_dump_start(&dump, dpif);
-    for (;;) {
-        enum { MAX_ACTIONS = 4096 }; /* An arbitrary but large number. */
-        uint32_t actions[MAX_ACTIONS * sizeof(struct nlattr) / 4];
-        uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
-        struct odp_flow f;
-
-        memset(&f, 0, sizeof f);
-        f.actions = (struct nlattr *) actions;
-        f.actions_len = sizeof actions;
-        f.key = (struct nlattr *) keybuf;
-        f.key_len = sizeof keybuf;
-
-        if (!dpif_flow_dump_next(&dump, &f)) {
-            break;
-        }
-
+    while (dpif_flow_dump_next(&dump, &key, &key_len,
+                               &actions, &actions_len, &stats)) {
         ds_clear(&ds);
-        format_odp_flow(&ds, &f);
+        odp_flow_key_format(key, key_len, &ds);
+        ds_put_cstr(&ds, ", ");
+        format_odp_flow_stats(&ds, stats);
+        ds_put_cstr(&ds, ", actions:");
+        format_odp_actions(&ds, actions, actions_len);
         printf("%s\n", ds_cstr(&ds));
     }
     dpif_flow_dump_done(&dump);
-- 
1.7.1





More information about the dev mailing list