From: wenxu <we...@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 <we...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev