[ovs-dev] [PATCH V9 14/31] dpif-netlink: Use netdev flow put api to insert a flow

Flavio Leitner fbl at sysclose.org
Fri Jun 2 20:08:23 UTC 2017


On Sun, May 28, 2017 at 02:59:56PM +0300, Roi Dayan wrote:
> From: Paul Blakey <paulb at mellanox.com>
> 
> Using the new netdev flow api operate will now try and
> offload flows to the relevant netdev of the input port.
> Other operate methods flows will come in later patches.
> 
[...]

> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 6f36b5e..8438e71 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
[...]

>  static void
> -dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
> +dbg_print_flow(const struct nlattr *key, size_t key_len,
> +               const struct nlattr *mask, size_t mask_len,
> +               const struct nlattr *actions, size_t actions_len,
> +               const ovs_u128 *ufid,
> +               const char *op)
> +{


perhaps:
      if (VLOG_IS_DBG_ENABLED()) {

> +        struct ds s;
> +
> +        ds_init(&s);
> +        ds_put_cstr(&s, op);
> +        ds_put_cstr(&s, " (");
> +        odp_format_ufid(ufid, &s);
> +        ds_put_cstr(&s, ")");
> +        if (key_len) {
> +            ds_put_cstr(&s, "\nflow (verbose): ");
> +            odp_flow_format(key, key_len, mask, mask_len, NULL, &s, true);
> +            ds_put_cstr(&s, "\nflow: ");
> +            odp_flow_format(key, key_len, mask, mask_len, NULL, &s, false);
> +        }
> +        ds_put_cstr(&s, "\nactions: ");
> +        format_odp_actions(&s, actions, actions_len);
> +        VLOG_DBG("\n%s", ds_cstr(&s));
> +        ds_destroy(&s);
     }

to avoid those operations on every flow put?
or maybe on the caller to avoid the function call.


> +}
> +
> +static int
> +try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)
>  {
> -    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> +    int err = EOPNOTSUPP;
>  
> +    switch (op->type) {
> +    case DPIF_OP_FLOW_PUT: {
> +        struct dpif_flow_put *put = &op->u.flow_put;
> +
> +        if (!put->ufid) {
> +            break;
> +        }
> +        dbg_print_flow(put->key, put->key_len, put->mask, put->mask_len,
> +                       put->actions, put->actions_len, put->ufid,
> +                       (put->flags & DPIF_FP_MODIFY ? "PUT(MODIFY)" : "PUT"));
> +        err = parse_flow_put(dpif, put);
> +        break;
> +    }
> +    case DPIF_OP_FLOW_DEL:
> +    case DPIF_OP_FLOW_GET:
> +    case DPIF_OP_EXECUTE:
> +    default:
> +        break;
> +    }
> +
> +    return err;
> +}
> +
> +static void
> +dpif_netlink_operate_chunks(struct dpif_netlink *dpif, struct dpif_op **ops,
> +                            size_t n_ops)
> +{
>      while (n_ops > 0) {
>          size_t chunk = dpif_netlink_operate__(dpif, ops, n_ops);
> +
>          ops += chunk;
>          n_ops -= chunk;
>      }
>  }
>  
> +static void
> +dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
> +{
> +    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> +    struct dpif_op *new_ops[OPERATE_MAX_OPS];
> +    int count = 0;
> +    int i = 0;
> +    int err = 0;
> +
> +    if (netdev_is_flow_api_enabled()) {
> +        while (n_ops > 0) {
> +            count = 0;
> +
> +            while (n_ops > 0 && count < OPERATE_MAX_OPS) {
> +                struct dpif_op *op = ops[i++];
> +
> +                err = try_send_to_netdev(dpif, op);
> +                if (err && err != EEXIST) {
> +                    new_ops[count++] = op;
> +                } else {
> +                    op->error = err;
> +                }
> +
> +                n_ops--;
> +            }
> +
> +            dpif_netlink_operate_chunks(dpif, new_ops, count);
> +        }
> +
> +        return;
> +    }
> +
> +    dpif_netlink_operate_chunks(dpif, ops, n_ops);
> +}
> +

The above could be:

    if (netdev_is_flow_api_enabled()) {
        ...
    } else {
       dpif_netlink_operate_chunks(dpif, ops, n_ops);
    }
   
   return;
}


Otherwise the patch looks good to me.

-- 
Flavio



More information about the dev mailing list