ukey_install() returns boolean signaling if the ukey was installed
or not. Installation may fail for a few reasons:
1. Conflicting ukey.
2. Mutex contention while trying to replace existing ukey.
3. The same ukey already exists and active.
Only the first case here signals an actual problem. Third one is
a little odd for userspace datapath, but harmless. Second is the
most common one that can easily happen during normal operation
since other threads like revalidators may be currently working on
this ukey preventing an immediate access.
Since only the first case is actually worth logging and it already
has its own log message, removing the 'upcall installation fails'
warning from the upcall_cb(). This should fix most of the random
failures of userspace system tests in CI.
While at it, also fixing coverage counters. Mutex contention was
mistakenly counted as a duplicate upcall. ukey contention for
revalidators was counted only in one of two places.
New counter added for the ukey contention on replace. We should
not re-use existing upcall_ukey_contention counter for this, since
it may lead to double counting.
Fixes: 67f08985d769 ("upcall: Replace ukeys for deleted flows.")
Fixes: 9cec8274ed9a ("ofproto-dpif-upcall: Add VLOG_WARN_RL logs for
upcall_cb() error.")
Signed-off-by: Ilya Maximets <[email protected]>
---
ofproto/ofproto-dpif-upcall.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index d88195636..73901b651 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -59,6 +59,7 @@ COVERAGE_DEFINE(handler_duplicate_upcall);
COVERAGE_DEFINE(revalidate_missed_dp_flow);
COVERAGE_DEFINE(ukey_dp_change);
COVERAGE_DEFINE(ukey_invalid_stat_reset);
+COVERAGE_DEFINE(ukey_replace_contention);
COVERAGE_DEFINE(upcall_flow_limit_grew);
COVERAGE_DEFINE(upcall_flow_limit_hit);
COVERAGE_DEFINE(upcall_flow_limit_kill);
@@ -1449,8 +1450,6 @@ upcall_cb(const struct dp_packet *packet, const struct
flow *flow, ovs_u128 *ufi
}
if (upcall.ukey && !ukey_install(udpif, upcall.ukey)) {
- static struct vlog_rate_limit rll = VLOG_RATE_LIMIT_INIT(1, 1);
- VLOG_WARN_RL(&rll, "upcall_cb failure: ukey installation fails");
error = ENOSPC;
}
out:
@@ -1948,15 +1947,15 @@ try_ukey_replace(struct umap *umap, struct udpif_key
*old_ukey,
transition_ukey(old_ukey, UKEY_DELETED);
transition_ukey(new_ukey, UKEY_VISIBLE);
replaced = true;
+ COVERAGE_INC(upcall_ukey_replace);
+ } else {
+ COVERAGE_INC(handler_duplicate_upcall);
}
ovs_mutex_unlock(&old_ukey->mutex);
- }
-
- if (replaced) {
- COVERAGE_INC(upcall_ukey_replace);
} else {
- COVERAGE_INC(handler_duplicate_upcall);
+ COVERAGE_INC(ukey_replace_contention);
}
+
return replaced;
}
@@ -3009,6 +3008,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool
purge)
/* Handler threads could be holding a ukey lock while it installs a
* new flow, so don't hang around waiting for access to it. */
if (ovs_mutex_trylock(&ukey->mutex)) {
+ COVERAGE_INC(upcall_ukey_contention);
continue;
}
ukey_state = ukey->state;
--
2.44.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
