[ovs-dev] [PATCHv6 13/14] dpif: Minimize memory copy for revalidation.

Joe Stringer joestringer at nicira.com
Fri Sep 26 09:28:17 UTC 2014


One of the limiting factors on the number of flows that can be supported
in the datapath is the overhead of assembling flow dump messages in the
datapath. This patch modifies the flow_dump interface to allow
revalidators to skip dumping the key, mask and actions from the
datapath, by making use of the unique identifiers introduced in earlier
patches.

For each flow dump, the dpif user specifies whether to skip these
attributes, allowing the common case to only dump a pair of 128-bit ID
and flow stats. This increases the number of flows that a revalidator
can handle per second by 50% or more.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
v6: Split out from "dpif: Add Unique flow identifiers."
    Rebase.
v5: Always pass flow_key down to dpif, to improve error logging.
    Fix extraneous netflow_unref.
    Rebase.
v4: Fix bug where UID-based terse dump wasn't enabled by default.
    Fix race conditions with tests.
    Combine dpif,upcall,dpif-netdev,dpif-linux changes into one patch.
    Display whether terse dump is enabled in ovs-appctl upcall/show.
v3: Rebase.
---
 lib/dpctl.c                   |    4 ++-
 lib/dpif-netdev.c             |   64 ++++++++++++++++++---------------
 lib/dpif-netlink.c            |   16 +++++++--
 lib/dpif-provider.h           |    9 +++--
 lib/dpif.c                    |    7 ++--
 lib/dpif.h                    |    2 +-
 ofproto/ofproto-dpif-upcall.c |   76 +++++++++++++++++++++++++++++++++++----
 ofproto/ofproto-dpif.c        |   79 ++++++++++++++++++++++++++++++++++++++++-
 ofproto/ofproto-dpif.h        |    2 ++
 tests/dpif-netdev.at          |    4 +++
 tests/ofproto-dpif.at         |    4 +++
 11 files changed, 220 insertions(+), 47 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 80f6ab1..a389b6b 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -736,7 +736,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
     }
 
     ds_init(&ds);
-    flow_dump = dpif_flow_dump_create(dpif);
+    flow_dump = dpif_flow_dump_create(dpif, false);
     flow_dump_thread = dpif_flow_dump_thread_create(flow_dump);
     while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) {
         if (filter) {
@@ -760,6 +760,8 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
             minimatch_destroy(&minimatch);
         }
         ds_clear(&ds);
+        odp_format_uid(&f.uid, &ds);
+        ds_put_cstr(&ds, " ");
         odp_flow_format(f.key, f.key_len, f.mask, f.mask_len,
                         &portno_names, &ds, dpctl_p->verbosity);
         ds_put_cstr(&ds, ", ");
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a8a8f9e..f709f14 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1421,33 +1421,37 @@ get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow,
 static void
 dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
                             struct ofpbuf *key_buf, struct ofpbuf *mask_buf,
-                            struct dpif_flow *flow)
+                            struct dpif_flow *flow, bool terse)
 {
-    struct flow_wildcards wc;
-    struct dp_netdev_actions *actions;
-    size_t offset;
-
-    minimask_expand(&netdev_flow->cr.match.mask, &wc);
-
-    /* Key */
-    offset = ofpbuf_size(key_buf);
-    flow->key = ofpbuf_tail(key_buf);
-    odp_flow_key_from_flow(key_buf, &netdev_flow->flow, &wc.masks,
-                           netdev_flow->flow.in_port.odp_port, true);
-    flow->key_len = ofpbuf_size(key_buf) - offset;
-
-    /* Mask */
-    offset = ofpbuf_size(mask_buf);
-    flow->mask = ofpbuf_tail(mask_buf);
-    odp_flow_key_from_mask(mask_buf, &wc.masks, &netdev_flow->flow,
-                           odp_to_u32(wc.masks.in_port.odp_port),
-                           SIZE_MAX, true);
-    flow->mask_len = ofpbuf_size(mask_buf) - offset;
-
-    /* Actions */
-    actions = dp_netdev_flow_get_actions(netdev_flow);
-    flow->actions = actions->actions;
-    flow->actions_len = actions->size;
+    if (terse) {
+        memset(flow, 0, sizeof *flow);
+    } else {
+        struct flow_wildcards wc;
+        struct dp_netdev_actions *actions;
+        size_t offset;
+
+        minimask_expand(&netdev_flow->cr.match.mask, &wc);
+
+        /* Key */
+        offset = ofpbuf_size(key_buf);
+        flow->key = ofpbuf_tail(key_buf);
+        odp_flow_key_from_flow(key_buf, &netdev_flow->flow, &wc.masks,
+                               netdev_flow->flow.in_port.odp_port, true);
+        flow->key_len = ofpbuf_size(key_buf) - offset;
+
+        /* Mask */
+        offset = ofpbuf_size(mask_buf);
+        flow->mask = ofpbuf_tail(mask_buf);
+        odp_flow_key_from_mask(mask_buf, &wc.masks, &netdev_flow->flow,
+                               odp_to_u32(wc.masks.in_port.odp_port),
+                               SIZE_MAX, true);
+        flow->mask_len = ofpbuf_size(mask_buf) - offset;
+
+        /* Actions */
+        actions = dp_netdev_flow_get_actions(netdev_flow);
+        flow->actions = actions->actions;
+        flow->actions_len = actions->size;
+    }
 
     memcpy(&flow->uid, &netdev_flow->uid, sizeof flow->uid);
     get_dpif_flow_stats(netdev_flow, &flow->stats);
@@ -1559,7 +1563,7 @@ dpif_netdev_flow_get(const struct dpif *dpif, const struct dpif_flow_get *get)
     netdev_flow = dp_netdev_find_flow(dp, get->uid);
     if (netdev_flow) {
         dp_netdev_flow_to_dpif_flow(netdev_flow, get->buffer, get->buffer,
-                                    get->flow);
+                                    get->flow, false);
      } else {
         error = ENOENT;
     }
@@ -1741,7 +1745,7 @@ dpif_netdev_flow_dump_cast(struct dpif_flow_dump *dump)
 }
 
 static struct dpif_flow_dump *
-dpif_netdev_flow_dump_create(const struct dpif *dpif_)
+dpif_netdev_flow_dump_create(const struct dpif *dpif_, bool terse)
 {
     struct dpif_netdev_flow_dump *dump;
 
@@ -1749,6 +1753,7 @@ dpif_netdev_flow_dump_create(const struct dpif *dpif_)
     dpif_flow_dump_init(&dump->up, dpif_);
     memset(&dump->pos, 0, sizeof dump->pos);
     dump->status = 0;
+    dump->up.terse = terse;
     ovs_mutex_init(&dump->mutex);
 
     return &dump->up;
@@ -1837,7 +1842,8 @@ dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread *thread_,
 
         ofpbuf_use_stack(&key, keybuf, sizeof *keybuf);
         ofpbuf_use_stack(&mask, maskbuf, sizeof *maskbuf);
-        dp_netdev_flow_to_dpif_flow(netdev_flow, &key, &mask, f);
+        dp_netdev_flow_to_dpif_flow(netdev_flow, &key, &mask, f,
+                                    dump->up.terse);
     }
 
     return n_flows;
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index aa25673..2f67521 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1202,12 +1202,14 @@ dpif_netlink_flow_dump_cast(struct dpif_flow_dump *dump)
 }
 
 static struct dpif_flow_dump *
-dpif_netlink_flow_dump_create(const struct dpif *dpif_)
+dpif_netlink_flow_dump_create(const struct dpif *dpif_, bool terse)
 {
+    static const uint32_t dump_flags = OVS_UID_F_SKIP_KEY | OVS_UID_F_SKIP_MASK
+                                       | OVS_UID_F_SKIP_ACTIONS;
     const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
     struct dpif_netlink_flow_dump *dump;
     struct dpif_netlink_flow request;
-    struct ofpbuf *buf;
+    struct ofpbuf *buf, uid;
 
     dump = xmalloc(sizeof *dump);
     dpif_flow_dump_init(&dump->up, dpif_);
@@ -1216,11 +1218,19 @@ dpif_netlink_flow_dump_create(const struct dpif *dpif_)
     request.cmd = OVS_FLOW_CMD_GET;
     request.dp_ifindex = dpif->dp_ifindex;
 
+    ofpbuf_use_stack(&uid, &request.uid_buf, sizeof request.uid_buf);
+    if (terse) {
+        odp_uid_to_nlattrs(&uid, NULL, dump_flags);
+    }
+    request.uid = ofpbuf_data(&uid);
+    request.uid_len = ofpbuf_size(&uid);
+
     buf = ofpbuf_new(1024);
     dpif_netlink_flow_to_ofpbuf(&request, buf);
     nl_dump_start(&dump->nl_dump, NETLINK_GENERIC, buf);
     ofpbuf_delete(buf);
     atomic_init(&dump->status, 0);
+    dump->up.terse = terse;
 
     return &dump->up;
 }
@@ -1331,7 +1341,7 @@ dpif_netlink_flow_dump_next(struct dpif_flow_dump_thread *thread_,
             break;
         }
 
-        if (datapath_flow.actions) {
+        if (dump->up.terse || datapath_flow.actions) {
             /* Common case: the flow includes actions. */
             dpif_netlink_flow_to_dpif_flow(&dpif->dpif, &flows[n_flows++],
                                            &datapath_flow);
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 65cf505..60bec76 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -53,6 +53,7 @@ static inline void dpif_assert_class(const struct dpif *dpif,
 
 struct dpif_flow_dump {
     struct dpif *dpif;
+    bool terse;         /* If true, key/mask/actions may be omitted. */
 };
 
 static inline void
@@ -256,8 +257,12 @@ struct dpif_class {
      *
      * 'flow_dump_create' and 'flow_dump_thread_create' must initialize the
      * structures that they return with dpif_flow_dump_init() and
-     * dpif_flow_dump_thread_init(), respectively. */
-    struct dpif_flow_dump *(*flow_dump_create)(const struct dpif *dpif);
+     * dpif_flow_dump_thread_init(), respectively.
+     *
+     * If 'terse' is true, then only UID and statistics will
+     * be returned in the dump. Otherwise, all fields will be returned. */
+    struct dpif_flow_dump *(*flow_dump_create)(const struct dpif *dpif,
+                                               bool terse);
     int (*flow_dump_destroy)(struct dpif_flow_dump *dump);
 
     struct dpif_flow_dump_thread *(*flow_dump_thread_create)(
diff --git a/lib/dpif.c b/lib/dpif.c
index 54d1c18..3e233d8 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -926,14 +926,15 @@ dpif_flow_del(struct dpif *dpif,
 }
 
 /* Creates and returns a new 'struct dpif_flow_dump' for iterating through the
- * flows in 'dpif'.
+ * flows in 'dpif'. If 'terse' is true, then only UID and statistics will
+ * be returned in the dump. Otherwise, all fields will be returned.
  *
  * This function always successfully returns a dpif_flow_dump.  Error
  * reporting is deferred to dpif_flow_dump_destroy(). */
 struct dpif_flow_dump *
-dpif_flow_dump_create(const struct dpif *dpif)
+dpif_flow_dump_create(const struct dpif *dpif, bool terse)
 {
-    return dpif->dpif_class->flow_dump_create(dpif);
+    return dpif->dpif_class->flow_dump_create(dpif, terse);
 }
 
 /* Destroys 'dump', which must have been created with dpif_flow_dump_create().
diff --git a/lib/dpif.h b/lib/dpif.h
index 095fa01..a2dbc1f 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -561,7 +561,7 @@ int dpif_flow_get(struct dpif *,
  *
  * All error reporting is deferred to the call to dpif_flow_dump_destroy().
  */
-struct dpif_flow_dump *dpif_flow_dump_create(const struct dpif *);
+struct dpif_flow_dump *dpif_flow_dump_create(const struct dpif *, bool terse);
 int dpif_flow_dump_destroy(struct dpif_flow_dump *);
 
 struct dpif_flow_dump_thread *dpif_flow_dump_thread_create(
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 916f26f..079e0e4 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -114,6 +114,7 @@ struct udpif {
     struct dpif_flow_dump *dump;       /* DPIF flow dump state. */
     long long int dump_duration;       /* Duration of the last flow dump. */
     struct seq *dump_seq;              /* Increments each dump iteration. */
+    atomic_bool terse_dump;            /* If true, skip dumping flow attrs. */
 
     /* There are 'N_UMAPS' maps containing 'struct udpif_key' elements.
      *
@@ -254,6 +255,10 @@ static void upcall_unixctl_disable_megaflows(struct unixctl_conn *, int argc,
                                              const char *argv[], void *aux);
 static void upcall_unixctl_enable_megaflows(struct unixctl_conn *, int argc,
                                             const char *argv[], void *aux);
+static void upcall_unixctl_disable_terse_dump(struct unixctl_conn *, int argc,
+                                              const char *argv[], void *aux);
+static void upcall_unixctl_enable_terse_dump(struct unixctl_conn *, int argc,
+                                             const char *argv[], void *aux);
 static void upcall_unixctl_set_flow_limit(struct unixctl_conn *conn, int argc,
                                             const char *argv[], void *aux);
 static void upcall_unixctl_dump_wait(struct unixctl_conn *conn, int argc,
@@ -293,6 +298,10 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
                                  upcall_unixctl_disable_megaflows, NULL);
         unixctl_command_register("upcall/enable-megaflows", "", 0, 0,
                                  upcall_unixctl_enable_megaflows, NULL);
+        unixctl_command_register("upcall/disable-terse-dump", "", 0, 0,
+                                 upcall_unixctl_disable_terse_dump, NULL);
+        unixctl_command_register("upcall/enable-terse-dump", "", 0, 0,
+                                 upcall_unixctl_enable_terse_dump, NULL);
         unixctl_command_register("upcall/set-flow-limit", "", 1, 1,
                                  upcall_unixctl_set_flow_limit, NULL);
         unixctl_command_register("revalidator/wait", "", 0, 0,
@@ -307,6 +316,7 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
     udpif->dump_seq = seq_create();
     latch_init(&udpif->exit_latch);
     list_push_back(&all_udpifs, &udpif->list_node);
+    atomic_init(&udpif->terse_dump, false);
     atomic_init(&udpif->n_flows, 0);
     atomic_init(&udpif->n_flows_timestamp, LLONG_MIN);
     ovs_mutex_init(&udpif->n_flows_mutex);
@@ -422,6 +432,8 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
                 "handler", udpif_upcall_handler, handler);
         }
 
+        atomic_init(&udpif->terse_dump,
+                    dpif_backer_get_enable_uid(udpif->backer));
         dpif_enable_upcall(udpif->dpif);
 
         ovs_barrier_init(&udpif->reval_barrier, udpif->n_revalidators);
@@ -714,7 +726,10 @@ udpif_revalidator(void *arg)
 
             start_time = time_msec();
             if (!udpif->reval_exit) {
-                udpif->dump = dpif_flow_dump_create(udpif->dpif);
+                bool terse_dump;
+
+                atomic_read_relaxed(&udpif->terse_dump, &terse_dump);
+                udpif->dump = dpif_flow_dump_create(udpif->dpif, terse_dump);
             }
         }
 
@@ -1556,14 +1571,14 @@ exit:
 }
 
 static void
-delete_op_init(struct ukey_op *op, struct udpif_key *ukey)
+delete_op_init(struct ukey_op *op, struct udpif_key *ukey, bool terse_dump)
 {
     op->ukey = ukey;
     op->dop.type = DPIF_OP_FLOW_DEL;
     op->dop.u.flow_del.key = ukey->key;
     op->dop.u.flow_del.key_len = ukey->key_len;
     op->dop.u.flow_del.uid = &ukey->uid;
-    op->dop.u.flow_del.terse = false;
+    op->dop.u.flow_del.terse = terse_dump;
     op->dop.u.flow_del.stats = &op->stats;
 }
 
@@ -1609,8 +1624,7 @@ push_ukey_ops__(struct udpif *udpif, struct ukey_op *ops, size_t n_ops)
             }
             ovs_mutex_unlock(&op->ukey->mutex);
 
-            if (odp_flow_key_to_flow(op->dop.u.flow_del.key,
-                                     op->dop.u.flow_del.key_len, &flow)
+            if (odp_flow_key_to_flow(op->ukey->key, op->ukey->key_len, &flow)
                 == ODP_FIT_ERROR) {
                 continue;
             }
@@ -1670,10 +1684,12 @@ revalidate(struct revalidator *revalidator)
     struct dpif_flow_dump_thread *dump_thread;
     uint64_t dump_seq, reval_seq;
     unsigned int flow_limit;
+    bool terse_dump;
 
     dump_seq = seq_read(udpif->dump_seq);
     reval_seq = seq_read(udpif->reval_seq);
     atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
+    atomic_read_relaxed(&udpif->terse_dump, &terse_dump);
     dump_thread = dpif_flow_dump_thread_create(udpif->dump);
     for (;;) {
         struct ukey_op ops[REVALIDATE_MAX_BATCH];
@@ -1753,7 +1769,7 @@ revalidate(struct revalidator *revalidator)
             ukey->flow_exists = keep;
 
             if (!keep) {
-                delete_op_init(&ops[n_ops++], ukey);
+                delete_op_init(&ops[n_ops++], ukey, terse_dump);
             }
             ovs_mutex_unlock(&ukey->mutex);
         }
@@ -1789,10 +1805,12 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
     struct udpif *udpif;
     uint64_t dump_seq, reval_seq;
     int slice;
+    bool terse_dump;
 
     udpif = revalidator->udpif;
     dump_seq = seq_read(udpif->dump_seq);
     reval_seq = seq_read(udpif->reval_seq);
+    atomic_read_relaxed(&udpif->terse_dump, &terse_dump);
     slice = revalidator - udpif->revalidators;
     ovs_assert(slice < udpif->n_revalidators);
 
@@ -1822,7 +1840,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
                                                        ukey)))) {
                 struct ukey_op *op = &ops[n_ops++];
 
-                delete_op_init(op, ukey);
+                delete_op_init(op, ukey, terse_dump);
                 if (n_ops == REVALIDATE_MAX_BATCH) {
                     push_ukey_ops(udpif, umap, ops, n_ops);
                     n_ops = 0;
@@ -1862,15 +1880,23 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
 
     LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
         unsigned int flow_limit;
+        bool terse_dump;
         size_t i;
 
         atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
+        atomic_read_relaxed(&udpif->terse_dump, &terse_dump);
 
         ds_put_format(&ds, "%s:\n", dpif_name(udpif->dpif));
         ds_put_format(&ds, "\tflows         : (current %lu)"
             " (avg %u) (max %u) (limit %u)\n", udpif_get_n_flows(udpif),
             udpif->avg_n_flows, udpif->max_n_flows, flow_limit);
         ds_put_format(&ds, "\tdump duration : %lldms\n", udpif->dump_duration);
+        ds_put_format(&ds, "\tterse dump : ");
+        if (terse_dump) {
+            ds_put_format(&ds, "enabled\n");
+        } else {
+            ds_put_format(&ds, "disabled\n");
+        }
         ds_put_char(&ds, '\n');
 
         for (i = 0; i < n_revalidators; i++) {
@@ -1918,6 +1944,42 @@ upcall_unixctl_enable_megaflows(struct unixctl_conn *conn,
     unixctl_command_reply(conn, "megaflows enabled");
 }
 
+/* Disable skipping flow attributes during flow dump/delete.
+ *
+ * This command is only needed for advanced debugging, so it's not
+ * documented in the man page. */
+static void
+upcall_unixctl_disable_terse_dump(struct unixctl_conn *conn,
+                                  int argc OVS_UNUSED,
+                                  const char *argv[] OVS_UNUSED,
+                                  void *aux OVS_UNUSED)
+{
+    struct udpif *udpif;
+
+    LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
+        atomic_store(&udpif->terse_dump, false);
+    }
+    unixctl_command_reply(conn, "Datapath dumping tersely using UID disabled");
+}
+
+/* Re-enable skipping flow attributes during flow dump/delete.
+ *
+ * This command is only needed for advanced debugging, so it's not
+ * documented in the man page. */
+static void
+upcall_unixctl_enable_terse_dump(struct unixctl_conn *conn,
+                                 int argc OVS_UNUSED,
+                                 const char *argv[] OVS_UNUSED,
+                                 void *aux OVS_UNUSED)
+{
+    struct udpif *udpif;
+
+    LIST_FOR_EACH (udpif, list_node, &all_udpifs) {
+        atomic_store(&udpif->terse_dump, true);
+    }
+    unixctl_command_reply(conn, "Datapath dumping tersely using UID enabled");
+}
+
 /* Set the flow limit.
  *
  * This command is only needed for advanced debugging, so it's not
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b090721..6749d9d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -280,6 +280,9 @@ struct dpif_backer {
     /* Maximum number of MPLS label stack entries that the datapath supports
      * in a match */
     size_t max_mpls_depth;
+
+    /* Support for userspace-generated unique flow ids. */
+    bool enable_uid;
 };
 
 /* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -362,6 +365,18 @@ ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *ofproto)
     return ofproto->backer->enable_recirc;
 }
 
+bool
+ofproto_dpif_get_enable_uid(const struct ofproto_dpif *ofproto)
+{
+    return ofproto->backer->enable_uid;
+}
+
+bool
+dpif_backer_get_enable_uid(const struct dpif_backer *backer)
+{
+    return backer->enable_uid;
+}
+
 static struct ofport_dpif *get_ofp_port(const struct ofproto_dpif *ofproto,
                                         ofp_port_t ofp_port);
 static void ofproto_trace(struct ofproto_dpif *, struct flow *,
@@ -851,6 +866,7 @@ static bool check_variable_length_userdata(struct dpif_backer *backer);
 static size_t check_max_mpls_depth(struct dpif_backer *backer);
 static bool check_recirc(struct dpif_backer *backer);
 static bool check_masked_set_action(struct dpif_backer *backer);
+static bool check_uid(struct dpif_backer *backer);
 
 static int
 open_dpif_backer(const char *type, struct dpif_backer **backerp)
@@ -948,6 +964,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     backer->max_mpls_depth = check_max_mpls_depth(backer);
     backer->masked_set_action = check_masked_set_action(backer);
     backer->rid_pool = recirc_id_pool_create();
+    backer->enable_uid = check_uid(backer);
 
     error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
     if (error) {
@@ -1026,6 +1043,66 @@ done:
     return enable_recirc;
 }
 
+/* Tests whether 'backer''s datapath supports userspace flow ids. Only newer
+ * datapaths support OVS_FLOW_ATTR_UID in flow commands. We can skip
+ * serializing some flow attributes for datapaths that support this feature.
+ *
+ * Returns true if 'backer' supports UID for flow operations.
+ * Returns false if 'backer' does not support UID. */
+static bool
+check_uid(struct dpif_backer *backer)
+{
+    struct flow flow;
+    struct odputil_keybuf keybuf;
+    struct ofpbuf key, replybuf;
+    struct dpif_flow reply;
+    uint64_t stub[DPIF_FLOW_BUFSIZE / 8];
+    ovs_u128 uid;
+    int error;
+    bool enable_uid = false;
+
+    memset(&flow, 0, sizeof flow);
+    flow.dl_type = htons(0x1234);
+
+    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
+    odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
+    dpif_flow_hash(backer->dpif, ofpbuf_data(&key), ofpbuf_size(&key), &uid);
+    error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE,
+                          ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL,
+                          0, &uid, NULL);
+
+    if (error && error != EEXIST) {
+        VLOG_WARN("%s: UID feature probe failed (%s).",
+                  dpif_name(backer->dpif), ovs_strerror(error));
+        goto done;
+    }
+
+    ofpbuf_use_stack(&replybuf, &stub, sizeof stub);
+    error = dpif_flow_get(backer->dpif, NULL, 0, &uid, &replybuf, &reply,
+                          true);
+    if (!error && !memcmp(&uid, &reply.uid, sizeof uid)) {
+        enable_uid = true;
+    }
+
+    error = dpif_flow_del(backer->dpif, ofpbuf_data(&key), ofpbuf_size(&key),
+                          &uid, NULL, false);
+    if (error) {
+        VLOG_WARN("%s: failed to delete UID feature probe flow",
+                  dpif_name(backer->dpif));
+    }
+
+done:
+    if (enable_uid) {
+        VLOG_INFO("%s: Datapath supports userspace flow ids",
+                  dpif_name(backer->dpif));
+    } else {
+        VLOG_INFO("%s: Datapath does not support userspace flow ids",
+                  dpif_name(backer->dpif));
+    }
+
+    return enable_uid;
+}
+
 /* Tests whether 'backer''s datapath supports variable-length
  * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.  We need
  * to disable some features on older datapaths that don't support this
@@ -4932,7 +5009,7 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
     }
 
     ds_init(&ds);
-    flow_dump = dpif_flow_dump_create(ofproto->backer->dpif);
+    flow_dump = dpif_flow_dump_create(ofproto->backer->dpif, false);
     flow_dump_thread = dpif_flow_dump_thread_create(flow_dump);
     while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) {
         struct flow flow;
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index a8c5d48..7db18d4 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -87,6 +87,8 @@ enum rule_dpif_lookup_verdict {
 
 size_t ofproto_dpif_get_max_mpls_depth(const struct ofproto_dpif *);
 bool ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *);
+bool ofproto_dpif_get_enable_uid(const struct ofproto_dpif *);
+bool dpif_backer_get_enable_uid(const struct dpif_backer *);
 
 uint8_t rule_dpif_lookup(struct ofproto_dpif *, struct flow *,
                          struct flow_wildcards *, struct rule_dpif **rule,
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 5f17769..168f8d4 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -94,6 +94,8 @@ OVS_VSWITCHD_START(
   [add-port br0 p1 -- set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock
    set bridge br0 datapath-type=dummy other-config:datapath-id=1234 \
                   fail-mode=secure])
+AT_CHECK([ovs-appctl upcall/disable-terse-dump], [0], [Datapath dumping tersely using UID disabled
+], [])
 AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
 
 AT_CHECK([ovs-ofctl add-flow br0 action=normal])
@@ -110,6 +112,8 @@ skb_priority(0/0),skb_mark(0/0),recirc_id(0),dp_hash(0/0),in_port(1),eth(src=50:
 # Now, the same again without megaflows.
 AT_CHECK([ovs-appctl upcall/disable-megaflows], [0], [megaflows disabled
 ])
+AT_CHECK([ovs-appctl upcall/disable-terse-dump], [0], [Datapath dumping tersely using UID disabled
+], [])
 AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
 sleep 1
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 2f83d4b..95f6778 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -4732,6 +4732,8 @@ OVS_VSWITCHD_START([add-br br1 \
 ADD_OF_PORTS([br0], [2])
 ADD_OF_PORTS([br1], [3])
 
+AT_CHECK([ovs-appctl upcall/disable-terse-dump], [0], [Datapath dumping tersely using UID disabled
+], [])
 AT_CHECK([ovs-appctl time/stop])
 AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
 
@@ -5307,6 +5309,8 @@ table=0 in_port=1,ip,nw_dst=10.0.0.3 actions=drop
 ])
 AT_CHECK([ovs-appctl upcall/disable-megaflows], [0], [megaflows disabled
 ], [])
+AT_CHECK([ovs-appctl upcall/disable-terse-dump], [0], [Datapath dumping tersely using UID disabled
+], [])
 AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg], [0], [], [])
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 for i in 1 2 3 4; do
-- 
1.7.10.4




More information about the dev mailing list