[ovs-dev] [PATCH v2 2/2] conntrack: replace conntrack lock form mutex to spinlock
wenxu at ucloud.cn
wenxu at ucloud.cn
Fri Jul 9 08:38:50 UTC 2021
From: wenxu <wenxu at ucloud.cn>
The ct_lock of conntrack is a global lock to protect the
conntrack table insert. Mutex lock will add more latency
for lock conflict and the poor performance for CPS of new
connections creating.
With our benchmark four pmd thread with simple conntrack
actions, The CPS is 300k. With this patch replace the
ct_lock to spinlock, the CPS improves to 500K.
Signed-off-by: wenxu <wenxu at ucloud.cn>
---
lib/conntrack-private.h | 2 +-
lib/conntrack-tp.c | 22 +++++++++++-----------
lib/conntrack.c | 48 ++++++++++++++++++++++++------------------------
3 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index e8332bd..e72615f 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -156,7 +156,7 @@ enum ct_timeout {
};
struct conntrack {
- struct ovs_mutex ct_lock; /* Protects 2 following fields. */
+ struct ovs_spin ct_lock; /* Protects 2 following fields. */
struct cmap conns OVS_GUARDED;
struct ovs_list exp_lists[N_CT_TM] OVS_GUARDED;
struct hmap zone_limits OVS_GUARDED;
diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
index a586d3a..3f15064 100644
--- a/lib/conntrack-tp.c
+++ b/lib/conntrack-tp.c
@@ -67,14 +67,14 @@ timeout_policy_get(struct conntrack *ct, int32_t tp_id)
{
struct timeout_policy *tp;
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
tp = timeout_policy_lookup(ct, tp_id);
if (!tp) {
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return NULL;
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return tp;
}
@@ -158,9 +158,9 @@ timeout_policy_delete(struct conntrack *ct, uint32_t tp_id)
{
int err;
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
err = timeout_policy_delete__(ct, tp_id);
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return err;
}
@@ -185,13 +185,13 @@ timeout_policy_update(struct conntrack *ct,
int err = 0;
uint32_t tp_id = new_tp->policy.id;
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
struct timeout_policy *tp = timeout_policy_lookup(ct, tp_id);
if (tp) {
err = timeout_policy_delete__(ct, tp_id);
}
timeout_policy_create(ct, new_tp);
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return err;
}
@@ -238,7 +238,7 @@ conn_update_expiration__(struct conntrack *ct, struct conn *conn,
{
ovs_mutex_unlock(&conn->lock);
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
ovs_mutex_lock(&conn->lock);
if (!conn->cleaned) {
conn->expiration = now + tp_value * 1000;
@@ -246,7 +246,7 @@ conn_update_expiration__(struct conntrack *ct, struct conn *conn,
ovs_list_push_back(&ct->exp_lists[tm], &conn->exp_node);
}
ovs_mutex_unlock(&conn->lock);
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
ovs_mutex_lock(&conn->lock);
}
@@ -262,7 +262,7 @@ conn_update_expiration(struct conntrack *ct, struct conn *conn,
ovs_mutex_unlock(&conn->lock);
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
ovs_mutex_lock(&conn->lock);
tp = timeout_policy_lookup(ct, conn->tp_id);
if (tp) {
@@ -271,7 +271,7 @@ conn_update_expiration(struct conntrack *ct, struct conn *conn,
val = ct_dpif_netdev_tp_def[tm_to_ct_dpif_tp(tm)];
}
ovs_mutex_unlock(&conn->lock);
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
ovs_mutex_lock(&conn->lock);
VLOG_DBG_RL(&rl, "Update timeout %s zone=%u with policy id=%d "
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 2e803ca..639ddf4 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -298,8 +298,8 @@ conntrack_init(void)
hindex_init(&ct->alg_expectation_refs);
ovs_rwlock_unlock(&ct->resources_lock);
- ovs_mutex_init_adaptive(&ct->ct_lock);
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_init(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
cmap_init(&ct->conns);
for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) {
ovs_list_init(&ct->exp_lists[i]);
@@ -307,7 +307,7 @@ conntrack_init(void)
hmap_init(&ct->zone_limits);
ct->zone_limit_seq = 0;
timeout_policy_init(ct);
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
ct->hash_basis = random_uint32();
atomic_count_init(&ct->n_conn, 0);
@@ -364,13 +364,13 @@ zone_limit_lookup_or_default(struct conntrack *ct, int32_t zone)
struct conntrack_zone_limit
zone_limit_get(struct conntrack *ct, int32_t zone)
{
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
struct conntrack_zone_limit czl = {DEFAULT_ZONE, 0, 0, 0};
struct zone_limit *zl = zone_limit_lookup_or_default(ct, zone);
if (zl) {
czl = zl->czl;
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return czl;
}
@@ -395,7 +395,7 @@ int
zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit)
{
int err = 0;
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
struct zone_limit *zl = zone_limit_lookup(ct, zone);
if (zl) {
zl->czl.limit = limit;
@@ -409,7 +409,7 @@ zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit)
zone);
}
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return err;
}
@@ -424,7 +424,7 @@ zone_limit_clean(struct conntrack *ct, struct zone_limit *zl)
int
zone_limit_delete(struct conntrack *ct, uint16_t zone)
{
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
struct zone_limit *zl = zone_limit_lookup(ct, zone);
if (zl) {
zone_limit_clean(ct, zl);
@@ -433,7 +433,7 @@ zone_limit_delete(struct conntrack *ct, uint16_t zone)
VLOG_INFO("Attempted delete of non-existent zone limit: zone %d",
zone);
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return 0;
}
@@ -497,7 +497,7 @@ conntrack_destroy(struct conntrack *ct)
pthread_join(ct->clean_thread, NULL);
latch_destroy(&ct->clean_thread_exit);
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
CMAP_FOR_EACH (conn, cm_node, &ct->conns) {
conn_clean_one(ct, conn);
}
@@ -515,8 +515,8 @@ conntrack_destroy(struct conntrack *ct)
}
hmap_destroy(&ct->timeout_policies);
- ovs_mutex_unlock(&ct->ct_lock);
- ovs_mutex_destroy(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
+ ovs_spin_destroy(&ct->ct_lock);
ovs_rwlock_wrlock(&ct->resources_lock);
struct alg_exp_node *alg_exp_node;
@@ -1116,11 +1116,11 @@ conn_update_state(struct conntrack *ct, struct dp_packet *pkt,
pkt->md.ct_state = CS_INVALID;
break;
case CT_UPDATE_NEW:
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
if (conn_lookup(ct, &conn->key, now, NULL, NULL)) {
conn_clean(ct, conn);
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
create_new_conn = true;
break;
case CT_UPDATE_VALID_NEW:
@@ -1302,11 +1302,11 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
/* Delete found entry if in wrong direction. 'force' implies commit. */
if (OVS_UNLIKELY(force && ctx->reply && conn)) {
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
if (conn_lookup(ct, &conn->key, now, NULL, NULL)) {
conn_clean(ct, conn);
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
conn = NULL;
}
@@ -1370,12 +1370,12 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
}
ovs_rwlock_unlock(&ct->resources_lock);
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
if (!conn_lookup(ct, &ctx->key, now, NULL, NULL)) {
conn = conn_not_found(ct, pkt, ctx, commit, now, nat_action_info,
helper, alg_exp, ct_alg_ctl, tp_id);
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
}
write_ct_md(pkt, zone, conn, &ctx->key, alg_exp);
@@ -1498,7 +1498,7 @@ ct_sweep(struct conntrack *ct, long long now, size_t limit)
long long min_expiration = LLONG_MAX;
size_t count = 0;
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
for (unsigned i = 0; i < N_CT_TM; i++) {
LIST_FOR_EACH_SAFE (conn, next, exp_node, &ct->exp_lists[i]) {
@@ -1523,7 +1523,7 @@ ct_sweep(struct conntrack *ct, long long now, size_t limit)
out:
VLOG_DBG("conntrack cleanup %"PRIuSIZE" entries in %lld msec", count,
time_msec() - now);
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return min_expiration;
}
@@ -2583,13 +2583,13 @@ conntrack_flush(struct conntrack *ct, const uint16_t *zone)
{
struct conn *conn;
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
CMAP_FOR_EACH (conn, cm_node, &ct->conns) {
if (!zone || *zone == conn->key.zone) {
conn_clean_one(ct, conn);
}
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return 0;
}
@@ -2604,7 +2604,7 @@ conntrack_flush_tuple(struct conntrack *ct, const struct ct_dpif_tuple *tuple,
memset(&key, 0, sizeof(key));
tuple_to_conn_key(tuple, zone, &key);
- ovs_mutex_lock(&ct->ct_lock);
+ ovs_spin_lock(&ct->ct_lock);
conn_lookup(ct, &key, time_msec(), &conn, NULL);
if (conn && conn->conn_type == CT_CONN_TYPE_DEFAULT) {
@@ -2614,7 +2614,7 @@ conntrack_flush_tuple(struct conntrack *ct, const struct ct_dpif_tuple *tuple,
error = ENOENT;
}
- ovs_mutex_unlock(&ct->ct_lock);
+ ovs_spin_unlock(&ct->ct_lock);
return error;
}
--
1.8.3.1
More information about the dev
mailing list