From feebdea2e5550a71c7accda936b6a55962f60a04 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 26 Jan 2011 07:03:39 -0800 Subject: [PATCH] dpif: Eliminate "struct odp_flow" from client-visible interface. 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 e0619b58..fb682f28 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 8bb0ea4a..80b890f8 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 2218c112..bded2901 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 463d4e7f..487b60a3 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 3c376ec1..22e35801 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 013a7f18..60807ba0 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 9486661a..2062c064 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 d928f686..ab945b0f 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 31b15e8d..3167864a 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); -- 2.30.2