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>

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a0e508c..890870c 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1584,12 +1584,6 @@ get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow,
     }
 }
 
-static bool
-dpif_netdev_check_ufid(struct dpif *dpif OVS_UNUSED)
-{
-    return true;
-}
-
 /* Converts to the dpif_flow format, using 'key_buf' and 'mask_buf' for
  * storing the netlink-formatted key/mask. 'key_buf' may be the same as
  * 'mask_buf'. Actions will be returned without copying, by relying on RCU to
@@ -3254,7 +3248,6 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_enable_upcall,
     dpif_netdev_disable_upcall,
     dpif_netdev_get_datapath_version,
-    dpif_netdev_check_ufid,
 };
 
 static void

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 63bbddc..bf06cc9 100644 (file)
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -138,8 +138,6 @@ static void dpif_netlink_flow_get_stats(const struct dpif_netlink_flow *,
                                         struct dpif_flow_stats *);
 static void dpif_netlink_flow_to_dpif_flow(struct dpif *, struct dpif_flow *,
                                            const struct dpif_netlink_flow *);
-static bool dpif_netlink_check_ufid__(struct dpif *dpif);
-static bool dpif_netlink_check_ufid(struct dpif *dpif);
 
 /* One of the dpif channels between the kernel and userspace. */
 struct dpif_channel {
@@ -190,11 +188,6 @@ struct dpif_netlink {
     /* Change notification. */
     struct nl_sock *port_notifier; /* vport multicast group subscriber. */
     bool refresh_channels;
-
-    /* If the datapath supports indexing flows using unique identifiers, then
-     * we can reduce the size of netlink messages by omitting fields like the
-     * flow key during flow operations. */
-    bool ufid_supported;
 };
 
 static void report_loss(struct dpif_netlink *, struct dpif_channel *,
@@ -311,7 +304,6 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif **dpifp)
               dp->dp_ifindex, dp->dp_ifindex);
 
     dpif->dp_ifindex = dp->dp_ifindex;
-    dpif->ufid_supported = dpif_netlink_check_ufid__(&dpif->dpif);
     *dpifp = &dpif->dpif;
 
     return 0;
@@ -1329,8 +1321,7 @@ dpif_netlink_init_flow_del(struct dpif_netlink *dpif,
                            struct dpif_netlink_flow *request)
 {
     return dpif_netlink_init_flow_del__(dpif, del->key, del->key_len,
-                                        del->ufid, dpif->ufid_supported,
-                                        request);
+                                        del->ufid, del->terse, request);
 }
 
 struct dpif_netlink_flow_dump {
@@ -1729,40 +1720,6 @@ dpif_netlink_handler_uninit(struct dpif_handler *handler)
 }
 #endif
 
-/* Checks support for unique flow identifiers. */
-static bool
-dpif_netlink_check_ufid__(struct dpif *dpif_)
-{
-    struct flow flow;
-    struct odputil_keybuf keybuf;
-    struct ofpbuf key;
-    ovs_u128 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(dpif_, ofpbuf_data(&key), ofpbuf_size(&key), &ufid);
-
-    return dpif_probe_feature(dpif_, "UFID", &key, &ufid);
-}
-
-static bool
-dpif_netlink_check_ufid(struct dpif *dpif_)
-{
-    const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
-
-    if (dpif->ufid_supported) {
-        VLOG_INFO("%s: Datapath supports userspace flow ids",
-                  dpif_name(dpif_));
-    } else {
-        VLOG_INFO("%s: Datapath does not support userspace flow ids",
-                  dpif_name(dpif_));
-    }
-    return dpif->ufid_supported;
-}
-
 /* Synchronizes 'channels' in 'dpif->handlers'  with the set of vports
  * currently in 'dpif' in the kernel, by adding a new set of channels for
  * any kernel vport that lacks one and deleting any channels that have no
@@ -2319,7 +2276,6 @@ const struct dpif_class dpif_netlink_class = {
     NULL,                       /* enable_upcall */
     NULL,                       /* disable_upcall */
     dpif_netlink_get_datapath_version, /* get_datapath_version */
-    dpif_netlink_check_ufid,
 };
 
 static int

diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 568c71f..7b4878e 100644 (file)
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -373,10 +373,6 @@ struct dpif_class {
     /* Get datapath version. Caller is responsible for freeing the string
      * returned.  */
     char *(*get_datapath_version)(void);
-
-    /* Returns whether 'dpif' supports unique flow identifiers for flow
-     * operations. */
-    bool (*get_ufid_support)(struct dpif *);
 };
 
 extern const struct dpif_class dpif_netlink_class;

diff --git a/lib/dpif.c b/lib/dpif.c
index a2696c6..649ce09 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -905,20 +905,6 @@ dpif_probe_feature(struct dpif *dpif, const char *name,
     return enable_feature;
 }
 
-/* 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. */
-bool
-dpif_get_enable_ufid(struct dpif *dpif)
-{
-    if (dpif->dpif_class->get_ufid_support) {
-        return dpif->dpif_class->get_ufid_support(dpif);
-    }
-    return false;
-}
-
 /* A dpif_operate() wrapper for performing a single DPIF_OP_FLOW_GET. */
 int
 dpif_flow_get(struct dpif *dpif,
@@ -987,6 +973,7 @@ dpif_flow_del(struct dpif *dpif,
     op.u.flow_del.key_len = key_len;
     op.u.flow_del.ufid = ufid;
     op.u.flow_del.stats = stats;
+    op.u.flow_del.terse = false;
 
     opp = &op;
     dpif_operate(dpif, &opp, 1);

diff --git a/lib/dpif.h b/lib/dpif.h
index 834b75f..c5bdf91 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -517,7 +517,6 @@ enum dpif_flow_put_flags {
 
 bool dpif_probe_feature(struct dpif *, const char *name,
                         const struct ofpbuf *key, const ovs_u128 *ufid);
-bool dpif_get_enable_ufid(struct dpif *);
 void dpif_flow_hash(const struct dpif *, const void *key, size_t key_len,
                     ovs_u128 *hash);
 int dpif_flow_flush(struct dpif *);
@@ -656,6 +655,8 @@ struct dpif_flow_del {
     const struct nlattr *key;       /* Flow to delete. */
     size_t key_len;                 /* Length of 'key' in bytes. */
     const ovs_u128 *ufid;           /* Unique identifier of flow to delete. */
+    bool terse;                     /* OK to skip sending/receiving full flow
+                                     * info? */
 
     /* Output. */
     struct dpif_flow_stats *stats;  /* Optional flow statistics. */

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index f0cd4cc..6feaa75 100644 (file)
--- 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 2165bda..81c2c6d 100644 (file)
--- 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 c03e606..1170c33 100644 (file)
--- 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,