dpif: Refactor datapath feature detection.

Various functions in ofproto-dpif and dpif-netlink detect support for
features in very similar ways. Refactor their common code to a single
function.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

4 files changed, 50 insertions, 82 deletions

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 354529003..2492a6bf7 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1333,19 +1333,6 @@ dpif_netlink_init_flow_del(struct dpif_netlink *dpif,
                                         request);
 }
 
-static int
-dpif_netlink_flow_del(struct dpif_netlink *dpif,
-                      const struct nlattr *key, size_t key_len,
-                      const ovs_u128 *ufid, bool terse)
-{
-    struct dpif_netlink_flow request;
-
-    dpif_netlink_init_flow_del__(dpif, key, key_len, ufid, terse, &request);
-
-    /* Ignore stats */
-    return dpif_netlink_flow_transact(&request, NULL, NULL);
-}
-
 struct dpif_netlink_flow_dump {
     struct dpif_flow_dump up;
     struct nl_dump nl_dump;
@@ -1746,14 +1733,10 @@ dpif_netlink_handler_uninit(struct dpif_handler *handler)
 static bool
 dpif_netlink_check_ufid__(struct dpif *dpif_)
 {
-    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
     struct flow flow;
     struct odputil_keybuf keybuf;
-    struct ofpbuf key, *replybuf;
-    struct dpif_netlink_flow reply;
+    struct ofpbuf key;
     ovs_u128 ufid;
-    int error;
-    bool enable_ufid = false;
 
     memset(&flow, 0, sizeof flow);
     flow.dl_type = htons(0x1234);
@@ -1761,31 +1744,8 @@ dpif_netlink_check_ufid__(struct dpif *dpif_)
     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);
-    error = dpif_flow_put(dpif_, DPIF_FP_CREATE | DPIF_FP_PROBE,
-                          ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL,
-                          0, &ufid, NULL);
-
-    if (error && error != EEXIST) {
-        VLOG_WARN("%s: UFID feature probe failed (%s).",
-                  dpif_name(dpif_), ovs_strerror(error));
-        return false;
-    }
-
-    error = dpif_netlink_flow_get__(dpif, NULL, 0, &ufid, true, &reply,
-                                    &replybuf);
-    if (!error && reply.ufid_present && ovs_u128_equal(&ufid, &reply.ufid)) {
-        enable_ufid = true;
-    }
-    ofpbuf_delete(replybuf);
-
-    error = dpif_netlink_flow_del(dpif, ofpbuf_data(&key), ofpbuf_size(&key),
-                                  &ufid, false);
-    if (error) {
-        VLOG_WARN("%s: failed to delete UFID feature probe flow",
-                  dpif_name(dpif_));
-    }
 
-    return enable_ufid;
+    return dpif_probe_feature(dpif_, "UFID", &key, &ufid);
 }
 
 static bool
diff --git a/lib/dpif.c b/lib/dpif.c
index 87954ed3b..a39050cbf 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -860,6 +860,48 @@ dpif_flow_flush(struct dpif *dpif)
     return error;
 }
 
+/* Attempts to install 'key' into the datapath, fetches it, then deletes it.
+ * Returns true if the datapath supported installing 'flow', false otherwise.
+ */
+bool
+dpif_probe_feature(struct dpif *dpif, const char *name,
+                   const struct ofpbuf *key, const ovs_u128 *ufid)
+{
+    struct dpif_flow flow;
+    struct ofpbuf reply;
+    uint64_t stub[DPIF_FLOW_BUFSIZE / 8];
+    bool enable_feature = false;
+    int error;
+
+    error = dpif_flow_put(dpif, DPIF_FP_CREATE | DPIF_FP_PROBE,
+                          ofpbuf_data(key), ofpbuf_size(key), NULL, 0, NULL, 0,
+                          ufid, NULL);
+    if (error && error != EEXIST) {
+        if (error != EINVAL) {
+            VLOG_WARN("%s: %s flow probe failed (%s)",
+                      dpif_name(dpif), name, ovs_strerror(error));
+        }
+        return false;
+    }
+
+    ofpbuf_use_stack(&reply, &stub, sizeof stub);
+    error = dpif_flow_get(dpif, ofpbuf_data(key), ofpbuf_size(key), ufid,
+                          &reply, &flow);
+    if (!error
+        && (!ufid || (flow.ufid_present && ovs_u128_equal(ufid, &flow.ufid)))) {
+        enable_feature = true;
+    }
+
+    error = dpif_flow_del(dpif, ofpbuf_data(key), ofpbuf_size(key), ufid,
+                          NULL);
+    if (error) {
+        VLOG_WARN("%s: failed to delete %s feature probe flow",
+                  dpif_name(dpif), name);
+    }
+
+    return enable_feature;
+}
+
 /* Tests whether 'dpif' supports userspace flow ids. We can skip serializing
  * some flow attributes for datapaths that support this feature.
  *
diff --git a/lib/dpif.h b/lib/dpif.h
index 527cedc6b..834b75f0a 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -515,6 +515,8 @@ enum dpif_flow_put_flags {
     DPIF_FP_PROBE = 1 << 3      /* Suppress error messages, if any. */
 };
 
+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);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5b3e64c1b..ffaffe429 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1009,8 +1009,7 @@ check_recirc(struct dpif_backer *backer)
     struct flow flow;
     struct odputil_keybuf keybuf;
     struct ofpbuf key;
-    int error;
-    bool enable_recirc = false;
+    bool enable_recirc;
 
     memset(&flow, 0, sizeof flow);
     flow.recirc_id = 1;
@@ -1018,28 +1017,9 @@ check_recirc(struct dpif_backer *backer)
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
+    enable_recirc = dpif_probe_feature(backer->dpif, "recirculation", &key,
+                                       NULL);
 
-    error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_PROBE,
-                          ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL,
-                          0, NULL, NULL);
-    if (error && error != EEXIST) {
-        if (error != EINVAL) {
-            VLOG_WARN("%s: Reciculation flow probe failed (%s)",
-                      dpif_name(backer->dpif), ovs_strerror(error));
-        }
-        goto done;
-    }
-
-    error = dpif_flow_del(backer->dpif, ofpbuf_data(&key), ofpbuf_size(&key),
-                          NULL, NULL);
-    if (error) {
-        VLOG_WARN("%s: failed to delete recirculation feature probe flow",
-                  dpif_name(backer->dpif));
-    }
-
-    enable_recirc = true;
-
-done:
     if (enable_recirc) {
         VLOG_INFO("%s: Datapath supports recirculation",
                   dpif_name(backer->dpif));
@@ -1139,7 +1119,6 @@ check_max_mpls_depth(struct dpif_backer *backer)
     for (n = 0; n < FLOW_MAX_MPLS_LABELS; n++) {
         struct odputil_keybuf keybuf;
         struct ofpbuf key;
-        int error;
 
         memset(&flow, 0, sizeof flow);
         flow.dl_type = htons(ETH_TYPE_MPLS);
@@ -1147,24 +1126,9 @@ check_max_mpls_depth(struct dpif_backer *backer)
 
         ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
         odp_flow_key_from_flow(&key, &flow, NULL, 0, false);
-
-        error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_PROBE,
-                              ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0,
-                              NULL, 0, NULL, NULL);
-        if (error && error != EEXIST) {
-            if (error != EINVAL) {
-                VLOG_WARN("%s: MPLS stack length feature probe failed (%s)",
-                          dpif_name(backer->dpif), ovs_strerror(error));
-            }
+        if (!dpif_probe_feature(backer->dpif, "MPLS", &key, NULL)) {
             break;
         }
-
-        error = dpif_flow_del(backer->dpif, ofpbuf_data(&key),
-                              ofpbuf_size(&key), NULL, NULL);
-        if (error) {
-            VLOG_WARN("%s: failed to delete MPLS feature probe flow",
-                      dpif_name(backer->dpif));
-        }
     }
 
     VLOG_INFO("%s: MPLS label stack length probed as %d",