On 02/06/2017 23:08, Flavio Leitner wrote:
On Sun, May 28, 2017 at 02:59:56PM +0300, Roi Dayan wrote:
From: Paul Blakey <pa...@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.


right. maybe not an important change? in a later commit we remove
this function and refactor log_flow_put_message() from dpif.c and
use that. "dpif-netlink: Use dpif logging functions"



+}
+
+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.


ok
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to