[ovs-dev] [dpif-logging 2/3] dpif: Change provider interface to consistently use operation structs.

Ethan Jackson ethan at nicira.com
Mon Jan 16 21:30:22 UTC 2012


Looks good.

Ethan

On Tue, Dec 27, 2011 at 13:23, Ben Pfaff <blp at nicira.com> wrote:
> Until now, a "flow put" has represented its parameters in two different
> ways, depending on whether it was coming from dpif_flow_put() or from
> dpif_operate(), and similarly for an "execute" operation.  This commit
> adopts the operation struct consistently within the dpif provider
> interface, which seems cleaner.
>
> This commit also factors out logging for flow puts and executes, which
> is useful in the following commit.
>
> This doesn't change the dpif client interface, since the two forms are
> more convenient for clients than always filling out an operation struct.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/dpif-linux.c    |   88 +++++++++++---------------
>  lib/dpif-netdev.c   |   42 ++++++-------
>  lib/dpif-provider.h |   44 ++++++-------
>  lib/dpif.c          |  170 +++++++++++++++++++++++++++++++-------------------
>  4 files changed, 182 insertions(+), 162 deletions(-)
>
> diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> index e232278..14a5e8f 100644
> --- a/lib/dpif-linux.c
> +++ b/lib/dpif-linux.c
> @@ -668,9 +668,7 @@ dpif_linux_flow_get(const struct dpif *dpif_,
>  }
>
>  static void
> -dpif_linux_init_flow_put(struct dpif *dpif_, enum dpif_flow_put_flags flags,
> -                         const struct nlattr *key, size_t key_len,
> -                         const struct nlattr *actions, size_t actions_len,
> +dpif_linux_init_flow_put(struct dpif *dpif_, const struct dpif_flow_put *put,
>                          struct dpif_linux_flow *request)
>  {
>     static struct nlattr dummy_action;
> @@ -678,37 +676,33 @@ dpif_linux_init_flow_put(struct dpif *dpif_, enum dpif_flow_put_flags flags,
>     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
>
>     dpif_linux_flow_init(request);
> -    request->cmd = (flags & DPIF_FP_CREATE
> +    request->cmd = (put->flags & DPIF_FP_CREATE
>                     ? OVS_FLOW_CMD_NEW : OVS_FLOW_CMD_SET);
>     request->dp_ifindex = dpif->dp_ifindex;
> -    request->key = key;
> -    request->key_len = key_len;
> +    request->key = put->key;
> +    request->key_len = put->key_len;
>     /* Ensure that OVS_FLOW_ATTR_ACTIONS will always be included. */
> -    request->actions = actions ? actions : &dummy_action;
> -    request->actions_len = actions_len;
> -    if (flags & DPIF_FP_ZERO_STATS) {
> +    request->actions = put->actions ? put->actions : &dummy_action;
> +    request->actions_len = put->actions_len;
> +    if (put->flags & DPIF_FP_ZERO_STATS) {
>         request->clear = true;
>     }
> -    request->nlmsg_flags = flags & DPIF_FP_MODIFY ? 0 : NLM_F_CREATE;
> +    request->nlmsg_flags = put->flags & DPIF_FP_MODIFY ? 0 : NLM_F_CREATE;
>  }
>
>  static int
> -dpif_linux_flow_put(struct dpif *dpif_, enum dpif_flow_put_flags flags,
> -                    const struct nlattr *key, size_t key_len,
> -                    const struct nlattr *actions, size_t actions_len,
> -                    struct dpif_flow_stats *stats)
> +dpif_linux_flow_put(struct dpif *dpif_, const struct dpif_flow_put *put)
>  {
>     struct dpif_linux_flow request, reply;
>     struct ofpbuf *buf;
>     int error;
>
> -    dpif_linux_init_flow_put(dpif_, flags, key, key_len, actions, actions_len,
> -                             &request);
> +    dpif_linux_init_flow_put(dpif_, put, &request);
>     error = dpif_linux_flow_transact(&request,
> -                                     stats ? &reply : NULL,
> -                                     stats ? &buf : NULL);
> -    if (!error && stats) {
> -        dpif_linux_flow_get_stats(&reply, stats);
> +                                     put->stats ? &reply : NULL,
> +                                     put->stats ? &buf : NULL);
> +    if (!error && put->stats) {
> +        dpif_linux_flow_get_stats(&reply, put->stats);
>         ofpbuf_delete(buf);
>     }
>     return error;
> @@ -832,39 +826,35 @@ dpif_linux_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *state_)
>
>  static struct ofpbuf *
>  dpif_linux_encode_execute(int dp_ifindex,
> -                          const struct nlattr *key, size_t key_len,
> -                          const struct nlattr *actions, size_t actions_len,
> -                          const struct ofpbuf *packet)
> +                          const struct dpif_execute *d_exec)
>  {
> -    struct ovs_header *execute;
> +    struct ovs_header *k_exec;
>     struct ofpbuf *buf;
>
> -    buf = ofpbuf_new(128 + actions_len + packet->size);
> +    buf = ofpbuf_new(128 + d_exec->actions_len + d_exec->packet->size);
>
>     nl_msg_put_genlmsghdr(buf, 0, ovs_packet_family, NLM_F_REQUEST,
>                           OVS_PACKET_CMD_EXECUTE, OVS_PACKET_VERSION);
>
> -    execute = ofpbuf_put_uninit(buf, sizeof *execute);
> -    execute->dp_ifindex = dp_ifindex;
> +    k_exec = ofpbuf_put_uninit(buf, sizeof *k_exec);
> +    k_exec->dp_ifindex = dp_ifindex;
>
> -    nl_msg_put_unspec(buf, OVS_PACKET_ATTR_PACKET, packet->data, packet->size);
> -    nl_msg_put_unspec(buf, OVS_PACKET_ATTR_KEY, key, key_len);
> -    nl_msg_put_unspec(buf, OVS_PACKET_ATTR_ACTIONS, actions, actions_len);
> +    nl_msg_put_unspec(buf, OVS_PACKET_ATTR_PACKET,
> +                      d_exec->packet->data, d_exec->packet->size);
> +    nl_msg_put_unspec(buf, OVS_PACKET_ATTR_KEY, d_exec->key, d_exec->key_len);
> +    nl_msg_put_unspec(buf, OVS_PACKET_ATTR_ACTIONS,
> +                      d_exec->actions, d_exec->actions_len);
>
>     return buf;
>  }
>
>  static int
> -dpif_linux_execute__(int dp_ifindex, const struct nlattr *key, size_t key_len,
> -                     const struct nlattr *actions, size_t actions_len,
> -                     const struct ofpbuf *packet)
> +dpif_linux_execute__(int dp_ifindex, const struct dpif_execute *execute)
>  {
>     struct ofpbuf *request;
>     int error;
>
> -    request = dpif_linux_encode_execute(dp_ifindex,
> -                                        key, key_len, actions, actions_len,
> -                                        packet);
> +    request = dpif_linux_encode_execute(dp_ifindex, execute);
>     error = nl_sock_transact(genl_sock, request, NULL);
>     ofpbuf_delete(request);
>
> @@ -872,15 +862,11 @@ dpif_linux_execute__(int dp_ifindex, const struct nlattr *key, size_t key_len,
>  }
>
>  static int
> -dpif_linux_execute(struct dpif *dpif_,
> -                   const struct nlattr *key, size_t key_len,
> -                   const struct nlattr *actions, size_t actions_len,
> -                   const struct ofpbuf *packet)
> +dpif_linux_execute(struct dpif *dpif_, const struct dpif_execute *execute)
>  {
>     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
>
> -    return dpif_linux_execute__(dpif->dp_ifindex, key, key_len,
> -                                actions, actions_len, packet);
> +    return dpif_linux_execute__(dpif->dp_ifindex, execute);
>  }
>
>  static void
> @@ -900,9 +886,7 @@ dpif_linux_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
>             struct dpif_flow_put *put = &op->u.flow_put;
>             struct dpif_linux_flow request;
>
> -            dpif_linux_init_flow_put(dpif_, put->flags, put->key, put->key_len,
> -                                     put->actions, put->actions_len,
> -                                     &request);
> +            dpif_linux_init_flow_put(dpif_, put, &request);
>             if (put->stats) {
>                 request.nlmsg_flags |= NLM_F_ECHO;
>             }
> @@ -911,9 +895,8 @@ dpif_linux_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
>         } else if (op->type == DPIF_OP_EXECUTE) {
>             struct dpif_execute *execute = &op->u.execute;
>
> -            txn->request = dpif_linux_encode_execute(
> -                dpif->dp_ifindex, execute->key, execute->key_len,
> -                execute->actions, execute->actions_len, execute->packet);
> +            txn->request = dpif_linux_encode_execute(dpif->dp_ifindex,
> +                                                     execute);
>         } else {
>             NOT_REACHED();
>         }
> @@ -1309,6 +1292,7 @@ dpif_linux_vport_send(int dp_ifindex, uint32_t port_no,
>  {
>     struct ofpbuf actions, key, packet;
>     struct odputil_keybuf keybuf;
> +    struct dpif_execute execute;
>     struct flow flow;
>     uint64_t action;
>
> @@ -1321,8 +1305,12 @@ dpif_linux_vport_send(int dp_ifindex, uint32_t port_no,
>     ofpbuf_use_stack(&actions, &action, sizeof action);
>     nl_msg_put_u32(&actions, OVS_ACTION_ATTR_OUTPUT, port_no);
>
> -    return dpif_linux_execute__(dp_ifindex, key.data, key.size,
> -                                actions.data, actions.size, &packet);
> +    execute.key = key.data;
> +    execute.key_len = key.size;
> +    execute.actions = actions.data;
> +    execute.actions_len = actions.size;
> +    execute.packet = &packet;
> +    return dpif_linux_execute__(dp_ifindex, &execute);
>  }
>
>  static bool
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 742d00c..ebaa7b8 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -715,29 +715,26 @@ clear_stats(struct dp_netdev_flow *flow)
>  }
>
>  static int
> -dpif_netdev_flow_put(struct dpif *dpif, enum dpif_flow_put_flags flags,
> -                    const struct nlattr *nl_key, size_t nl_key_len,
> -                    const struct nlattr *actions, size_t actions_len,
> -                    struct dpif_flow_stats *stats)
> +dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put)
>  {
>     struct dp_netdev *dp = get_dp_netdev(dpif);
>     struct dp_netdev_flow *flow;
>     struct flow key;
>     int error;
>
> -    error = dpif_netdev_flow_from_nlattrs(nl_key, nl_key_len, &key);
> +    error = dpif_netdev_flow_from_nlattrs(put->key, put->key_len, &key);
>     if (error) {
>         return error;
>     }
>
>     flow = dp_netdev_lookup_flow(dp, &key);
>     if (!flow) {
> -        if (flags & DPIF_FP_CREATE) {
> +        if (put->flags & DPIF_FP_CREATE) {
>             if (hmap_count(&dp->flow_table) < MAX_FLOWS) {
> -                if (stats) {
> -                    memset(stats, 0, sizeof *stats);
> +                if (put->stats) {
> +                    memset(put->stats, 0, sizeof *put->stats);
>                 }
> -                return add_flow(dpif, &key, actions, actions_len);
> +                return add_flow(dpif, &key, put->actions, put->actions_len);
>             } else {
>                 return EFBIG;
>             }
> @@ -745,13 +742,13 @@ dpif_netdev_flow_put(struct dpif *dpif, enum dpif_flow_put_flags flags,
>             return ENOENT;
>         }
>     } else {
> -        if (flags & DPIF_FP_MODIFY) {
> -            int error = set_flow_actions(flow, actions, actions_len);
> +        if (put->flags & DPIF_FP_MODIFY) {
> +            int error = set_flow_actions(flow, put->actions, put->actions_len);
>             if (!error) {
> -                if (stats) {
> -                    get_dpif_flow_stats(flow, stats);
> +                if (put->stats) {
> +                    get_dpif_flow_stats(flow, put->stats);
>                 }
> -                if (flags & DPIF_FP_ZERO_STATS) {
> +                if (put->flags & DPIF_FP_ZERO_STATS) {
>                     clear_stats(flow);
>                 }
>             }
> @@ -864,30 +861,29 @@ dpif_netdev_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *state_)
>  }
>
>  static int
> -dpif_netdev_execute(struct dpif *dpif,
> -                    const struct nlattr *key_attrs, size_t key_len,
> -                    const struct nlattr *actions, size_t actions_len,
> -                    const struct ofpbuf *packet)
> +dpif_netdev_execute(struct dpif *dpif, const struct dpif_execute *execute)
>  {
>     struct dp_netdev *dp = get_dp_netdev(dpif);
>     struct ofpbuf copy;
>     struct flow key;
>     int error;
>
> -    if (packet->size < ETH_HEADER_LEN || packet->size > UINT16_MAX) {
> +    if (execute->packet->size < ETH_HEADER_LEN ||
> +        execute->packet->size > UINT16_MAX) {
>         return EINVAL;
>     }
>
>     /* Make a deep copy of 'packet', because we might modify its data. */
> -    ofpbuf_init(&copy, DP_NETDEV_HEADROOM + packet->size);
> +    ofpbuf_init(&copy, DP_NETDEV_HEADROOM + execute->packet->size);
>     ofpbuf_reserve(&copy, DP_NETDEV_HEADROOM);
> -    ofpbuf_put(&copy, packet->data, packet->size);
> +    ofpbuf_put(&copy, execute->packet->data, execute->packet->size);
>
>     flow_extract(&copy, 0, 0, -1, &key);
> -    error = dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key);
> +    error = dpif_netdev_flow_from_nlattrs(execute->key, execute->key_len,
> +                                          &key);
>     if (!error) {
>         dp_netdev_execute_actions(dp, &copy, &key,
> -                                  actions, actions_len);
> +                                  execute->actions, execute->actions_len);
>     }
>
>     ofpbuf_uninit(&copy);
> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index 154b8a6..7b77c9e 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -210,30 +210,28 @@ struct dpif_class {
>                     struct ofpbuf **actionsp, struct dpif_flow_stats *stats);
>
>     /* Adds or modifies a flow in 'dpif'.  The flow is specified by the Netlink
> -     * attributes with types OVS_KEY_ATTR_* in the 'key_len' bytes starting at
> -     * 'key'.  The associated actions are specified by the Netlink attributes
> -     * with types OVS_ACTION_ATTR_* in the 'actions_len' bytes starting at
> -     * 'actions'.
> +     * attributes with types OVS_KEY_ATTR_* in the 'put->key_len' bytes
> +     * starting at 'put->key'.  The associated actions are specified by the
> +     * Netlink attributes with types OVS_ACTION_ATTR_* in the
> +     * 'put->actions_len' bytes starting at 'put->actions'.
>      *
>      * - If the flow's key does not exist in 'dpif', then the flow will be
> -     *   added if 'flags' includes DPIF_FP_CREATE.  Otherwise the operation
> -     *   will fail with ENOENT.
> +     *   added if 'put->flags' includes DPIF_FP_CREATE.  Otherwise the
> +     *   operation will fail with ENOENT.
>      *
> -     *   If the operation succeeds, then 'stats', if nonnull, must be zeroed.
> +     *   If the operation succeeds, then 'put->stats', if nonnull, must be
> +     *   zeroed.
>      *
>      * - If the flow's key does exist in 'dpif', then the flow's actions will
> -     *   be updated if 'flags' includes DPIF_FP_MODIFY.  Otherwise the
> +     *   be updated if 'put->flags' includes DPIF_FP_MODIFY.  Otherwise the
>      *   operation will fail with EEXIST.  If the flow's actions are updated,
> -     *   then its statistics will be zeroed if 'flags' includes
> +     *   then its statistics will be zeroed if 'put->flags' includes
>      *   DPIF_FP_ZERO_STATS, and left as-is otherwise.
>      *
> -     *   If the operation succeeds, then 'stats', if nonnull, must be set to
> -     *   the flow's statistics before the update.
> +     *   If the operation succeeds, then 'put->stats', if nonnull, must be set
> +     *   to the flow's statistics before the update.
>      */
> -    int (*flow_put)(struct dpif *dpif, enum dpif_flow_put_flags flags,
> -                    const struct nlattr *key, size_t key_len,
> -                    const struct nlattr *actions, size_t actions_len,
> -                    struct dpif_flow_stats *stats);
> +    int (*flow_put)(struct dpif *dpif, const struct dpif_flow_put *put);
>
>     /* 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
> @@ -283,15 +281,13 @@ struct dpif_class {
>      * successful call to the 'flow_dump_start' function for 'dpif'.  */
>     int (*flow_dump_done)(const struct dpif *dpif, void *state);
>
> -    /* Performs the 'actions_len' bytes of actions in 'actions' on the Ethernet
> -     * frame specified in 'packet' taken from the flow specified in the
> -     * 'key_len' bytes of 'key'.  ('key' is mostly redundant with 'packet', but
> -     * it contains some metadata that cannot be recovered from 'packet', such
> -     * as tun_id and in_port.) */
> -    int (*execute)(struct dpif *dpif,
> -                   const struct nlattr *key, size_t key_len,
> -                   const struct nlattr *actions, size_t actions_len,
> -                   const struct ofpbuf *packet);
> +    /* Performs the 'execute->actions_len' bytes of actions in
> +     * 'execute->actions' on the Ethernet frame specified in 'execute->packet'
> +     * taken from the flow specified in the 'execute->key_len' bytes of
> +     * 'execute->key'.  ('execute->key' is mostly redundant with
> +     * 'execute->packet', but it contains some metadata that cannot be
> +     * recovered from 'execute->packet', such as tun_id and in_port.) */
> +    int (*execute)(struct dpif *dpif, const struct dpif_execute *execute);
>
>     /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order
>      * in which they are specified, placing each operation's results in the
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 43189e2..3ec83a2 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -86,6 +86,10 @@ static void log_flow_message(const struct dpif *dpif, int error,
>  static void log_operation(const struct dpif *, const char *operation,
>                           int error);
>  static bool should_log_flow_message(int error);
> +static void log_flow_put_message(struct dpif *, const struct dpif_flow_put *,
> +                                 int error);
> +static void log_execute_message(struct dpif *, const struct dpif_execute *,
> +                                int error);
>
>  static void
>  dp_initialize(void)
> @@ -763,6 +767,23 @@ dpif_flow_get(const struct dpif *dpif,
>     return error;
>  }
>
> +static int
> +dpif_flow_put__(struct dpif *dpif, const struct dpif_flow_put *put)
> +{
> +    int error;
> +
> +    COVERAGE_INC(dpif_flow_put);
> +    assert(!(put->flags & ~(DPIF_FP_CREATE | DPIF_FP_MODIFY
> +                            | DPIF_FP_ZERO_STATS)));
> +
> +    error = dpif->dpif_class->flow_put(dpif, put);
> +    if (error && put->stats) {
> +        memset(put->stats, 0, sizeof *put->stats);
> +    }
> +    log_flow_put_message(dpif, put, error);
> +    return error;
> +}
> +
>  /* Adds or modifies a flow in 'dpif'.  The flow is specified by the Netlink
>  * attributes with types OVS_KEY_ATTR_* in the 'key_len' bytes starting at
>  * 'key'.  The associated actions are specified by the Netlink attributes with
> @@ -789,35 +810,15 @@ dpif_flow_put(struct dpif *dpif, enum dpif_flow_put_flags flags,
>               const struct nlattr *actions, size_t actions_len,
>               struct dpif_flow_stats *stats)
>  {
> -    int error;
> -
> -    COVERAGE_INC(dpif_flow_put);
> -    assert(!(flags & ~(DPIF_FP_CREATE | DPIF_FP_MODIFY | DPIF_FP_ZERO_STATS)));
> -
> -    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)) {
> -        struct ds s;
> -
> -        ds_init(&s);
> -        ds_put_cstr(&s, "put");
> -        if (flags & DPIF_FP_CREATE) {
> -            ds_put_cstr(&s, "[create]");
> -        }
> -        if (flags & DPIF_FP_MODIFY) {
> -            ds_put_cstr(&s, "[modify]");
> -        }
> -        if (flags & DPIF_FP_ZERO_STATS) {
> -            ds_put_cstr(&s, "[zero]");
> -        }
> -        log_flow_message(dpif, error, ds_cstr(&s), key, key_len, stats,
> -                         actions, actions_len);
> -        ds_destroy(&s);
> -    }
> -    return error;
> +    struct dpif_flow_put put;
> +
> +    put.flags = flags;
> +    put.key = key;
> +    put.key_len = key_len;
> +    put.actions = actions;
> +    put.actions_len = actions_len;
> +    put.stats = stats;
> +    return dpif_flow_put__(dpif, &put);
>  }
>
>  /* Deletes a flow from 'dpif' and returns 0, or returns ENOENT if 'dpif' does
> @@ -937,6 +938,23 @@ dpif_flow_dump_done(struct dpif_flow_dump *dump)
>     return dump->error == EOF ? 0 : dump->error;
>  }
>
> +static int
> +dpif_execute__(struct dpif *dpif, const struct dpif_execute *execute)
> +{
> +    int error;
> +
> +    COVERAGE_INC(dpif_execute);
> +    if (execute->actions_len > 0) {
> +        error = dpif->dpif_class->execute(dpif, execute);
> +    } else {
> +        error = 0;
> +    }
> +
> +    log_execute_message(dpif, execute, error);
> +
> +    return error;
> +}
> +
>  /* Causes 'dpif' to perform the 'actions_len' bytes of actions in 'actions' on
>  * the Ethernet frame specified in 'packet' taken from the flow specified in
>  * the 'key_len' bytes of 'key'.  ('key' is mostly redundant with 'packet', but
> @@ -950,30 +968,14 @@ dpif_execute(struct dpif *dpif,
>              const struct nlattr *actions, size_t actions_len,
>              const struct ofpbuf *buf)
>  {
> -    int error;
> -
> -    COVERAGE_INC(dpif_execute);
> -    if (actions_len > 0) {
> -        error = dpif->dpif_class->execute(dpif, key, key_len,
> -                                          actions, actions_len, buf);
> -    } else {
> -        error = 0;
> -    }
> -
> -    if (!(error ? VLOG_DROP_WARN(&error_rl) : VLOG_DROP_DBG(&dpmsg_rl))) {
> -        struct ds ds = DS_EMPTY_INITIALIZER;
> -        char *packet = ofp_packet_to_string(buf->data, buf->size, buf->size);
> -        ds_put_format(&ds, "%s: execute ", dpif_name(dpif));
> -        format_odp_actions(&ds, actions, actions_len);
> -        if (error) {
> -            ds_put_format(&ds, " failed (%s)", strerror(error));
> -        }
> -        ds_put_format(&ds, " on packet %s", packet);
> -        vlog(THIS_MODULE, error ? VLL_WARN : VLL_DBG, "%s", ds_cstr(&ds));
> -        ds_destroy(&ds);
> -        free(packet);
> -    }
> -    return error;
> +    struct dpif_execute execute;
> +
> +    execute.key = key;
> +    execute.key_len = key_len;
> +    execute.actions = actions;
> +    execute.actions_len = actions_len;
> +    execute.packet = buf;
> +    return dpif_execute__(dpif, &execute);
>  }
>
>  /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order in
> @@ -994,24 +996,14 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops)
>
>     for (i = 0; i < n_ops; i++) {
>         struct dpif_op *op = ops[i];
> -        struct dpif_flow_put *put;
> -        struct dpif_execute *execute;
>
>         switch (op->type) {
>         case DPIF_OP_FLOW_PUT:
> -            put = &op->u.flow_put;
> -            op->error = dpif_flow_put(dpif, put->flags,
> -                                      put->key, put->key_len,
> -                                      put->actions, put->actions_len,
> -                                      put->stats);
> +            op->error = dpif_flow_put__(dpif, &op->u.flow_put);
>             break;
>
>         case DPIF_OP_EXECUTE:
> -            execute = &op->u.execute;
> -            op->error = dpif_execute(dpif, execute->key, execute->key_len,
> -                                     execute->actions,
> -                                     execute->actions_len,
> -                                     execute->packet);
> +            op->error = dpif_execute__(dpif, &op->u.execute);
>             break;
>
>         default:
> @@ -1251,3 +1243,51 @@ 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_put_message(struct dpif *dpif, const struct dpif_flow_put *put,
> +                     int error)
> +{
> +    if (should_log_flow_message(error)) {
> +        struct ds s;
> +
> +        ds_init(&s);
> +        ds_put_cstr(&s, "put");
> +        if (put->flags & DPIF_FP_CREATE) {
> +            ds_put_cstr(&s, "[create]");
> +        }
> +        if (put->flags & DPIF_FP_MODIFY) {
> +            ds_put_cstr(&s, "[modify]");
> +        }
> +        if (put->flags & DPIF_FP_ZERO_STATS) {
> +            ds_put_cstr(&s, "[zero]");
> +        }
> +        log_flow_message(dpif, error, ds_cstr(&s),
> +                         put->key, put->key_len, put->stats,
> +                         put->actions, put->actions_len);
> +        ds_destroy(&s);
> +    }
> +}
> +
> +static void
> +log_execute_message(struct dpif *dpif, const struct dpif_execute *execute,
> +                    int error)
> +{
> +    if (!(error ? VLOG_DROP_WARN(&error_rl) : VLOG_DROP_DBG(&dpmsg_rl))) {
> +        struct ds ds = DS_EMPTY_INITIALIZER;
> +        char *packet;
> +
> +        packet = ofp_packet_to_string(execute->packet->data,
> +                                      execute->packet->size,
> +                                      execute->packet->size);
> +        ds_put_format(&ds, "%s: execute ", dpif_name(dpif));
> +        format_odp_actions(&ds, execute->actions, execute->actions_len);
> +        if (error) {
> +            ds_put_format(&ds, " failed (%s)", strerror(error));
> +        }
> +        ds_put_format(&ds, " on packet %s", packet);
> +        vlog(THIS_MODULE, error ? VLL_WARN : VLL_DBG, "%s", ds_cstr(&ds));
> +        ds_destroy(&ds);
> +        free(packet);
> +    }
> +}
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list