dpif: Turn dpif_flow_hash function into generic odp_flow_key_hash.

Current implementation of dpif_flow_hash() doesn't depend on datapath
interface and only complicates the callers by forcing them to figure
out what is their current 'dpif'.  If we'll need different hashing
for different 'dpif's we'll implement an API for dpif-providers
and each dpif implementation will be able to use their local function
directly without calling it via dpif API.

This change will allow us to not store 'dpif' pointer in the userspace
datapath implementation which is broken and will be removed in next
commits.

This patch moves dpif_flow_hash() to odp-util module and replaces
unused odp_flow_key_hash() by it, along with removing of unused 'dpif'
argument.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 24218210d4a8863124a3814031b2a6aa88a0056e..add27be02e1f233e522660f63769fc3c4a67b95d 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3014,7 +3014,7 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread *pmd,
     /* If a UFID is not provided, determine one based on the key. */
     if (!ufidp && key && key_len
         && !dpif_netdev_flow_from_nlattrs(key, key_len, &flow, false)) {
-        dpif_flow_hash(pmd->dp->dpif, &flow, sizeof flow, &ufid);
+        odp_flow_key_hash(&flow, sizeof flow, &ufid);
         ufidp = &ufid;
     }
 
@@ -3233,7 +3233,7 @@ dp_netdev_get_mega_ufid(const struct match *match, ovs_u128 *mega_ufid)
         ((uint8_t *)&masked_flow)[i] = ((uint8_t *)&match->flow)[i] &
                                        ((uint8_t *)&match->wc)[i];
     }
-    dpif_flow_hash(NULL, &masked_flow, sizeof(struct flow), mega_ufid);
+    odp_flow_key_hash(&masked_flow, sizeof masked_flow, mega_ufid);
 }
 
 static struct dp_netdev_flow *
@@ -3437,7 +3437,7 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put)
     if (put->ufid) {
         ufid = *put->ufid;
     } else {
-        dpif_flow_hash(dpif, &match.flow, sizeof match.flow, &ufid);
+        odp_flow_key_hash(&match.flow, sizeof match.flow, &ufid);
     }
 
     /* The Netlink encoding of datapath flow keys cannot express
@@ -6646,7 +6646,7 @@ handle_packet_upcall(struct dp_netdev_pmd_thread *pmd,
     ofpbuf_clear(actions);
     ofpbuf_clear(put_actions);
 
-    dpif_flow_hash(pmd->dp->dpif, &match.flow, sizeof match.flow, &ufid);
+    odp_flow_key_hash(&match.flow, sizeof match.flow, &ufid);
     error = dp_netdev_upcall(pmd, packet, &match.flow, &match.wc,
                              &ufid, DPIF_UC_MISS, NULL, actions,
                              put_actions);
@@ -7206,7 +7206,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
             struct dp_packet *packet;
             DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
                 flow_extract(packet, &flow);
-                dpif_flow_hash(dp->dpif, &flow, sizeof flow, &ufid);
+                odp_flow_key_hash(&flow, sizeof flow, &ufid);
                 dp_execute_userspace_action(pmd, packet, should_steal, &flow,
                                             &ufid, &actions, userdata);
             }

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 3d97c786e6e02b559f59e7b8b5a6e4db57369d1b..d865c0bdb939ffcb0c505af6e68de51c413e4ec1 100644 (file)
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -155,7 +155,7 @@ static int dpif_netlink_flow_transact(struct dpif_netlink_flow *request,
                                       struct ofpbuf **bufp);
 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 *,
+static void dpif_netlink_flow_to_dpif_flow(struct dpif_flow *,
                                            const struct dpif_netlink_flow *);
 
 /* One of the dpif channels between the kernel and userspace. */
@@ -1569,7 +1569,7 @@ dpif_netlink_flow_dump_thread_destroy(struct dpif_flow_dump_thread *thread_)
 }
 
 static void
-dpif_netlink_flow_to_dpif_flow(struct dpif *dpif, struct dpif_flow *dpif_flow,
+dpif_netlink_flow_to_dpif_flow(struct dpif_flow *dpif_flow,
                                const struct dpif_netlink_flow *datapath_flow)
 {
     dpif_flow->key = datapath_flow->key;
@@ -1584,8 +1584,8 @@ dpif_netlink_flow_to_dpif_flow(struct dpif *dpif, struct dpif_flow *dpif_flow,
         dpif_flow->ufid = datapath_flow->ufid;
     } else {
         ovs_assert(datapath_flow->key && datapath_flow->key_len);
-        dpif_flow_hash(dpif, datapath_flow->key, datapath_flow->key_len,
-                       &dpif_flow->ufid);
+        odp_flow_key_hash(datapath_flow->key, datapath_flow->key_len,
+                          &dpif_flow->ufid);
     }
     dpif_netlink_flow_get_stats(datapath_flow, &dpif_flow->stats);
     dpif_flow->attrs.offloaded = false;
@@ -1763,8 +1763,7 @@ dpif_netlink_flow_dump_next(struct dpif_flow_dump_thread *thread_,
         if (dump->up.terse || datapath_flow.actions) {
             /* Common case: we don't want actions, or the flow includes
              * actions. */
-            dpif_netlink_flow_to_dpif_flow(&dpif->dpif, &flows[n_flows++],
-                                           &datapath_flow);
+            dpif_netlink_flow_to_dpif_flow(&flows[n_flows++], &datapath_flow);
         } else {
             /* Rare case: the flow does not include actions.  Retrieve this
              * individual flow again to get the actions. */
@@ -1782,8 +1781,7 @@ dpif_netlink_flow_dump_next(struct dpif_flow_dump_thread *thread_,
 
             /* Save this flow.  Then exit, because we only have one buffer to
              * handle this case. */
-            dpif_netlink_flow_to_dpif_flow(&dpif->dpif, &flows[n_flows++],
-                                           &datapath_flow);
+            dpif_netlink_flow_to_dpif_flow(&flows[n_flows++], &datapath_flow);
             break;
         }
     }
@@ -1976,8 +1974,7 @@ dpif_netlink_operate__(struct dpif_netlink *dpif,
 
                 op->error = dpif_netlink_flow_from_ofpbuf(&reply, txn->reply);
                 if (!op->error) {
-                    dpif_netlink_flow_to_dpif_flow(&dpif->dpif, get->flow,
-                                                   &reply);
+                    dpif_netlink_flow_to_dpif_flow(get->flow, &reply);
                 }
             }
             break;
@@ -2501,8 +2498,8 @@ dpif_netlink_queue_to_priority(const struct dpif *dpif OVS_UNUSED,
 }
 
 static int
-parse_odp_packet(const struct dpif_netlink *dpif, struct ofpbuf *buf,
-                 struct dpif_upcall *upcall, int *dp_ifindex)
+parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall *upcall,
+                 int *dp_ifindex)
 {
     static const struct nl_policy ovs_packet_policy[] = {
         /* Always present. */
@@ -2543,7 +2540,7 @@ parse_odp_packet(const struct dpif_netlink *dpif, struct ofpbuf *buf,
     upcall->key = CONST_CAST(struct nlattr *,
                              nl_attr_get(a[OVS_PACKET_ATTR_KEY]));
     upcall->key_len = nl_attr_get_size(a[OVS_PACKET_ATTR_KEY]);
-    dpif_flow_hash(&dpif->dpif, upcall->key, upcall->key_len, &upcall->ufid);
+    odp_flow_key_hash(upcall->key, upcall->key_len, &upcall->ufid);
     upcall->userdata = a[OVS_PACKET_ATTR_USERDATA];
     upcall->out_tun_key = a[OVS_PACKET_ATTR_EGRESS_TUN_KEY];
     upcall->actions = a[OVS_PACKET_ATTR_ACTIONS];
@@ -2637,7 +2634,7 @@ dpif_netlink_recv_windows(struct dpif_netlink *dpif, uint32_t handler_id,
                 return error;
             }
 
-            error = parse_odp_packet(dpif, buf, upcall, &dp_ifindex);
+            error = parse_odp_packet(buf, upcall, &dp_ifindex);
             if (!error && dp_ifindex == dpif->dp_ifindex) {
                 return 0;
             } else if (error) {
@@ -2712,7 +2709,7 @@ dpif_netlink_recv__(struct dpif_netlink *dpif, uint32_t handler_id,
                 return error;
             }
 
-            error = parse_odp_packet(dpif, buf, upcall, &dp_ifindex);
+            error = parse_odp_packet(buf, upcall, &dp_ifindex);
             if (!error && dp_ifindex == dpif->dp_ifindex) {
                 return 0;
             } else if (error) {

diff --git a/lib/dpif.c b/lib/dpif.c
index 955eaf6d027240952e7d473741611c8dc4d148e6..9d9c716c13aa783c5644bc12215aa0aa31adbbf8 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -913,22 +913,6 @@ dpif_flow_stats_format(const struct dpif_flow_stats *stats, struct ds *s)
     }
 }
 
-/* Places the hash of the 'key_len' bytes starting at 'key' into '*hash'. */
-void
-dpif_flow_hash(const struct dpif *dpif OVS_UNUSED,
-               const void *key, size_t key_len, ovs_u128 *hash)
-{
-    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
-    static uint32_t secret;
-
-    if (ovsthread_once_start(&once)) {
-        secret = random_uint32();
-        ovsthread_once_done(&once);
-    }
-    hash_bytes128(key, key_len, secret, hash);
-    uuid_set_bits_v4((struct uuid *)hash);
-}
-
 /* Deletes all flows from 'dpif'.  Returns 0 if successful, otherwise a
  * positive errno value.  */
 int

diff --git a/lib/dpif.h b/lib/dpif.h
index a88851eba0c0eb3193c4e5605d2cf968f070d6b6..c21e897aaea2e040d94d5ec5f417528afe727f42 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -534,8 +534,6 @@ enum dpif_flow_put_flags {
 bool dpif_probe_feature(struct dpif *, const char *name,
                         const struct ofpbuf *key, const struct ofpbuf *actions,
                         const ovs_u128 *ufid);
-void dpif_flow_hash(const struct dpif *, const void *key, size_t key_len,
-                    ovs_u128 *hash);
 int dpif_flow_flush(struct dpif *);
 int dpif_flow_put(struct dpif *, enum dpif_flow_put_flags,
                   const struct nlattr *key, size_t key_len,
@@ -842,7 +840,7 @@ void dpif_register_dp_purge_cb(struct dpif *, dp_purge_callback *, void *aux);
 /* A callback to process an upcall, currently implemented only by dpif-netdev.
  *
  * The caller provides the 'packet' and 'flow' to process, the corresponding
- * 'ufid' as generated by dpif_flow_hash(), the polling thread id 'pmd_id',
+ * 'ufid' as generated by odp_flow_key_hash(), the polling thread id 'pmd_id',
  * the 'type' of the upcall, and if 'type' is DPIF_UC_ACTION then the
  * 'userdata' attached to the action.
  *

diff --git a/lib/odp-util.c b/lib/odp-util.c
index d6cb9e62ad292e40269e5d1cbfc505d66fe2d7ea..1225724154accc642e46b2f116cfd14f7447be20 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6435,11 +6435,20 @@ odp_key_to_dp_packet(const struct nlattr *key, size_t key_len,
     }
 }
 
-uint32_t
-odp_flow_key_hash(const struct nlattr *key, size_t key_len)
+/* Places the hash of the 'key_len' bytes starting at 'key' into '*hash'.
+ * Generated value has format of random UUID. */
+void
+odp_flow_key_hash(const void *key, size_t key_len, ovs_u128 *hash)
 {
-    BUILD_ASSERT_DECL(!(NLA_ALIGNTO % sizeof(uint32_t)));
-    return hash_bytes32(ALIGNED_CAST(const uint32_t *, key), key_len, 0);
+    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+    static uint32_t secret;
+
+    if (ovsthread_once_start(&once)) {
+        secret = random_uint32();
+        ovsthread_once_done(&once);
+    }
+    hash_bytes128(key, key_len, secret, hash);
+    uuid_set_bits_v4((struct uuid *)hash);
 }
 
 static void

diff --git a/lib/odp-util.h b/lib/odp-util.h
index f15e258e6b4a9bb627d83c12f1663d25fdf11bee..4ecce1aac5d6774bfe409b03208a4b7626d2b908 100644 (file)
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -243,7 +243,7 @@ struct odp_flow_key_parms {
 void odp_flow_key_from_flow(const struct odp_flow_key_parms *, struct ofpbuf *);
 void odp_flow_key_from_mask(const struct odp_flow_key_parms *, struct ofpbuf *);
 
-uint32_t odp_flow_key_hash(const struct nlattr *, size_t);
+void odp_flow_key_hash(const void *key, size_t key_len, ovs_u128 *hash);
 
 /* Estimated space needed for metadata. */
 enum { ODP_KEY_METADATA_SIZE = 9 * 8 };

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f782f865f60eea1c61e1f8fc03a688a78f0b3a5e..d3cb392077df1dbf2a22fd6d6298d7981577b3e2 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -929,7 +929,7 @@ check_ufid(struct dpif_backer *backer)
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&odp_parms, &key);
-    dpif_flow_hash(backer->dpif, key.data, key.size, &ufid);
+    odp_flow_key_hash(key.data, key.size, &ufid);
 
     enable_ufid = dpif_probe_feature(backer->dpif, "UFID", &key, NULL, &ufid);