dpif: Shift ufid support checking up to dpif_backer.

Previously, the dpif layer was responsible for determining datapath
support for UFIDs, which resulted in all ovs-dpctl utilities
inserting/deleting flows from the datapath each time they are run.
Shift this responsibility up to the dpif_backer.

There are two users of this functionality: Revalidators check for UFID
support to request a terser dump using UFIDs, and dpif-netlink uses this
to request flow_del operations to only return the UFID/stats. The latter
case was previously hidden from revalidators, but this change makes them
aware of it, and reuses the same "udpif->enable_ufid" flag for reducing
overhead of both flow dump and flow delete.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

3 files changed, 56 insertions, 6 deletions

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index f0cd4cc07..6feaa756f 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -430,6 +430,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
 {
     if (udpif && n_handlers && n_revalidators) {
         size_t i;
+        bool enable_ufid;
 
         udpif->n_handlers = n_handlers;
         udpif->n_revalidators = n_revalidators;
@@ -444,7 +445,8 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
                 "handler", udpif_upcall_handler, handler);
         }
 
-        atomic_init(&udpif->enable_ufid, dpif_get_enable_ufid(udpif->dpif));
+        enable_ufid = ofproto_dpif_get_enable_ufid(udpif->backer);
+        atomic_init(&udpif->enable_ufid, enable_ufid);
         dpif_enable_upcall(udpif->dpif);
 
         ovs_barrier_init(&udpif->reval_barrier, udpif->n_revalidators);
@@ -1673,7 +1675,8 @@ exit:
 }
 
 static void
-delete_op_init__(struct ukey_op *op, const struct dpif_flow *flow)
+delete_op_init__(struct udpif *udpif, struct ukey_op *op,
+                 const struct dpif_flow *flow)
 {
     op->ukey = NULL;
     op->dop.type = DPIF_OP_FLOW_DEL;
@@ -1681,10 +1684,11 @@ delete_op_init__(struct ukey_op *op, const struct dpif_flow *flow)
     op->dop.u.flow_del.key_len = flow->key_len;
     op->dop.u.flow_del.ufid = flow->ufid_present ? &flow->ufid : NULL;
     op->dop.u.flow_del.stats = &op->stats;
+    atomic_read_relaxed(&udpif->enable_ufid, &op->dop.u.flow_del.terse);
 }
 
 static void
-delete_op_init(struct ukey_op *op, struct udpif_key *ukey)
+delete_op_init(struct udpif *udpif, struct ukey_op *op, struct udpif_key *ukey)
 {
     op->ukey = ukey;
     op->dop.type = DPIF_OP_FLOW_DEL;
@@ -1692,6 +1696,7 @@ delete_op_init(struct ukey_op *op, struct udpif_key *ukey)
     op->dop.u.flow_del.key_len = ukey->key_len;
     op->dop.u.flow_del.ufid = ukey->ufid_present ? &ukey->ufid : NULL;
     op->dop.u.flow_del.stats = &op->stats;
+    atomic_read_relaxed(&udpif->enable_ufid, &op->dop.u.flow_del.terse);
 }
 
 static void
@@ -1858,7 +1863,7 @@ revalidate(struct revalidator *revalidator)
                 } else {
                     log_unexpected_flow(f, error);
                     if (error != ENOENT) {
-                        delete_op_init__(&ops[n_ops++], f);
+                        delete_op_init__(udpif, &ops[n_ops++], f);
                     }
                 }
                 continue;
@@ -1889,7 +1894,7 @@ revalidate(struct revalidator *revalidator)
             ukey->flow_exists = keep;
 
             if (!keep) {
-                delete_op_init(&ops[n_ops++], ukey);
+                delete_op_init(udpif, &ops[n_ops++], ukey);
             }
             ovs_mutex_unlock(&ukey->mutex);
         }
@@ -1958,7 +1963,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
                                                        ukey)))) {
                 struct ukey_op *op = &ops[n_ops++];
 
-                delete_op_init(op, ukey);
+                delete_op_init(udpif, op, ukey);
                 if (n_ops == REVALIDATE_MAX_BATCH) {
                     push_ukey_ops(udpif, umap, ops, n_ops);
                     n_ops = 0;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2165bdac9..81c2c6daa 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -271,6 +271,9 @@ struct dpif_backer {
     struct recirc_id_pool *rid_pool;       /* Recirculation ID pool. */
     bool enable_recirc;   /* True if the datapath supports recirculation */
 
+    /* True if the datapath supports unique flow identifiers */
+    bool enable_ufid;
+
     /* True if the datapath supports variable-length
      * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
      * False if the datapath supports only 8-byte (or shorter) userdata. */
@@ -373,6 +376,12 @@ ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *ofproto)
     return ofproto->backer->enable_recirc;
 }
 
+bool
+ofproto_dpif_get_enable_ufid(struct dpif_backer *backer)
+{
+    return backer->enable_ufid;
+}
+
 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 *,
@@ -866,6 +875,7 @@ struct odp_garbage {
 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_ufid(struct dpif_backer *backer);
 static bool check_masked_set_action(struct dpif_backer *backer);
 
 static int
@@ -963,6 +973,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     backer->enable_recirc = check_recirc(backer);
     backer->max_mpls_depth = check_max_mpls_depth(backer);
     backer->masked_set_action = check_masked_set_action(backer);
+    backer->enable_ufid = check_ufid(backer);
     backer->rid_pool = recirc_id_pool_create();
 
     backer->enable_tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif);
@@ -1031,6 +1042,39 @@ check_recirc(struct dpif_backer *backer)
     return enable_recirc;
 }
 
+/* Tests whether 'dpif' supports userspace flow ids. We can skip serializing
+ * some flow attributes for datapaths that support this feature.
+ *
+ * Returns true if 'dpif' supports UFID for flow operations.
+ * Returns false if  'dpif' does not support UFID. */
+static bool
+check_ufid(struct dpif_backer *backer)
+{
+    struct flow flow;
+    struct odputil_keybuf keybuf;
+    struct ofpbuf key;
+    ovs_u128 ufid;
+    bool enable_ufid;
+
+    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), &ufid);
+
+    enable_ufid = dpif_probe_feature(backer->dpif, "UFID", &key, &ufid);
+
+    if (enable_ufid) {
+        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_ufid;
+}
+
 /* 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
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index c03e60630..1170c3379 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -75,6 +75,7 @@ BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
 
 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_ufid(struct dpif_backer *backer);
 
 struct rule_dpif *rule_dpif_lookup(struct ofproto_dpif *, struct flow *,
                                    struct flow_wildcards *, bool take_ref,