It's highly unlikely that two flows will hash to the same UID. However,
we could handle multiple upcalls from the same flow. If the flow keys
are the same as a previous flow, then the current upcalls belongs to a
flow that already has a datapath flow installed. If the keys are
different, then we have generated the same UID hash for two different
flows, and we should log a warning.
Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
v5: Compare flow keys rather than hashes of flow keys.
v4: Initial post.
---
ofproto/ofproto-dpif-upcall.c | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index dea3304..45c22a5 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -46,7 +46,7 @@
VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
-COVERAGE_DEFINE(handler_install_conflict);
+COVERAGE_DEFINE(handler_duplicate_upcall);
COVERAGE_DEFINE(upcall_ukey_contention);
COVERAGE_DEFINE(dumped_duplicate_flow);
COVERAGE_DEFINE(dumped_new_flow);
@@ -1286,23 +1286,41 @@ ukey_new(struct upcall *upcall)
* On success, returns true, installs the ukey and returns it in a locked
* state. Otherwise, returns false. */
static bool
-ukey_install_start(struct udpif *udpif, struct udpif_key *ukey)
- OVS_TRY_LOCK(true, ukey->mutex)
+ukey_install_start(struct udpif *udpif, struct udpif_key *new_ukey)
+ OVS_TRY_LOCK(true, new_ukey->mutex)
{
struct umap *umap;
+ struct udpif_key *old_ukey;
uint32_t idx;
bool locked = false;
- idx = ukey->hash % N_UMAPS;
+ idx = new_ukey->hash % N_UMAPS;
umap = &udpif->ukeys[idx];
ovs_mutex_lock(&umap->mutex);
- if (!ukey_lookup(udpif, &ukey->uid)) {
- ovs_mutex_lock(&ukey->mutex);
- cmap_insert(&umap->cmap, &ukey->cmap_node, ukey->hash);
- locked = true;
+ old_ukey = ukey_lookup(udpif, &new_ukey->uid);
+ if (old_ukey) {
+ /* Uncommon case: A ukey is already installed with the same UID. */
+ if (old_ukey->key_len == new_ukey->key_len
+ && !memcmp(old_ukey->key, new_ukey->key, new_ukey->key_len)) {
+ COVERAGE_INC(handler_duplicate_upcall);
+ } else {
+ struct ds ds = DS_EMPTY_INITIALIZER;
+
+ odp_format_uid(&old_ukey->uid, &ds);
+ ds_put_cstr(&ds, " ");
+ odp_flow_key_format(old_ukey->key, old_ukey->key_len, &ds);
+ ds_put_cstr(&ds, "\n");
+ odp_format_uid(&new_ukey->uid, &ds);
+ ds_put_cstr(&ds, " ");
+ odp_flow_key_format(new_ukey->key, new_ukey->key_len, &ds);
+
+ VLOG_WARN("Conflicting ukey for flows:\n%s", ds_cstr(&ds));
+ ds_destroy(&ds);
+ }
} else {
- COVERAGE_INC(handler_install_conflict);
- VLOG_DBG("Failed to ukey_install_start(): 0x%"PRIx32, ukey->hash);
+ ovs_mutex_lock(&new_ukey->mutex);
+ cmap_insert(&umap->cmap, &new_ukey->cmap_node, new_ukey->hash);
+ locked = true;
}
ovs_mutex_unlock(&umap->mutex);
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
