From patchwork Wed Feb 17 16:34:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gaetan Rivet X-Patchwork-Id: 1441287 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=u256.net header.i=@u256.net header.a=rsa-sha256 header.s=fm1 header.b=KNG4zGUx; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm2 header.b=L+DWeMOb; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DgkBP2ZMKz9sSC for ; Thu, 18 Feb 2021 03:41:33 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 8D44E86EA1; Wed, 17 Feb 2021 16:41:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UZoZ5wojYfqv; Wed, 17 Feb 2021 16:41:25 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 528CA86D72; Wed, 17 Feb 2021 16:41:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 32D96C1E6F; Wed, 17 Feb 2021 16:41:17 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 16D8DC0893 for ; Wed, 17 Feb 2021 16:41:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id EF5CB6F47D for ; Wed, 17 Feb 2021 16:41:12 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LKtsQ9T2QXA8 for ; Wed, 17 Feb 2021 16:41:11 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id E1ECA6F5E0; Wed, 17 Feb 2021 16:41:11 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from new4-smtp.messagingengine.com (new4-smtp.messagingengine.com [66.111.4.230]) by smtp3.osuosl.org (Postfix) with ESMTPS id 990BA6F52B for ; Wed, 17 Feb 2021 16:41:09 +0000 (UTC) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.nyi.internal (Postfix) with ESMTP id B8B5458036C; Wed, 17 Feb 2021 11:34:54 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 17 Feb 2021 11:34:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.net; h=from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=fm1; bh=yu1xVdPdKGzja hDmvT+2FmUN1U2iZMxoC5ExU4er38s=; b=KNG4zGUxocsIzcuJe/Ciedlyu46YK UOVkTMA2i7XY1FDF+da7doB0w1W6wqoiZkwlhAo9WHmnbvdMwKuPOY6nnfKLzbN1 s2Q9Ea1FM7pz9j3yWsrjlqDrWk/0dV5UJJuSGOpoj6Abweh2sZ9RGSA5IK1zcM7S oWsQeKt3FQFZzTyS6U5gs0SrYjvA6AflLFwD6P0qbz7aLVz0gmV8kl5j2iRAHeai PHmn2gNAnBsFOfjhrIcMcqGyUffPX84FdzJrPgCaHl1K2m30a3pm1d/NtoMI4YoG BQabAY7c6c/4PfoDORCdOlph+8hTPO+CD0pH5Kkb3XQFASwqIBCcVD2uQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; bh=yu1xVdPdKGzjahDmvT+2FmUN1U2iZMxoC5ExU4er38s=; b=L+DWeMOb pyenbGZ6UotAeCRAR9kYr3WP2kA4Ohvnj7Ard43yT8xFRNkqszdQSN9l+2YScgx0 aXRNGsCwiC735i7+L3k67M4DUJfCkVN7NhmhwPm5xHEa3/3gzL5tDe+/gek0zmYz nksqwgKjIPUdcfoGYGeYmiyLb4z7l18yHxBP4WebdiB8saK1mlajnaVlTgwstU1F vTvr2/lTw+8NQP7FoIvhDqbKRosBxp1lBaCAX3jgktRtU4DJRkFrjMjpUfmQj2KE WNSkHgP+7YNL09cnPP3o7VdqF4yIuWxdm4gKLaHKCuWeRy0bWfS9ORQPGLn5TK+K lMoL+TlacPZU2Q== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrjedvgdeihecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkofgjfhgggfestdekredtredttdenucfhrhhomhepifgrvghtrghn ucftihhvvghtuceoghhrihhvvgesuhdvheeirdhnvghtqeenucggtffrrghtthgvrhhnpe ehgfevffekteehteefieefvdehleefjeefheevudetjefhkeeutdekieeuvdetheenucfk phepkeeirddvheegrddvgeefrddugeeinecuvehluhhsthgvrhfuihiivgepfeenucfrrg hrrghmpehmrghilhhfrhhomhepghhrihhvvgesuhdvheeirdhnvght X-ME-Proxy: Received: from inocybe.home (lfbn-poi-1-930-146.w86-254.abo.wanadoo.fr [86.254.243.146]) by mail.messagingengine.com (Postfix) with ESMTPA id BCBBE240065; Wed, 17 Feb 2021 11:34:53 -0500 (EST) From: Gaetan Rivet To: dev@openvswitch.org Date: Wed, 17 Feb 2021 17:34:43 +0100 Message-Id: <97e250478cde86a1bb27cfc3131fd45b5d6de2ca.1613557616.git.grive@u256.net> X-Mailer: git-send-email 2.30.0 In-Reply-To: References: MIME-Version: 1.0 Cc: Eli Britstein , ameerm@nvidia.com, majd@nvidia.com Subject: [ovs-dev] [PATCH v1 9/9] conntrack: Use an atomic conn expiration value X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" A lock is taken during conn_lookup() to check whether a connection is expired before returning it. This lock can have some contention. Even though this lock ensures a consistent sequence of writes, it does not imply a specific order. A ct_clean thread taking the lock first could read a value that would be updated immediately after by a PMD waiting on the same lock, just as well as the opposite order. As such, the expiration time can be stale anytime it is read. In this context, using an atomic will ensure the same write consistency while keeping the same (lack of) guarantee for reads. Reading the atomic will however be less costly than taking and releasing the lock. Signed-off-by: Gaetan Rivet Reviewed-by: Eli Britstein Acked-by: William Tu --- lib/conntrack-private.h | 2 +- lib/conntrack-tp.c | 2 +- lib/conntrack.c | 25 +++++++++++++++---------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index e8ffb5bf9..7f3f8714a 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -148,7 +148,7 @@ struct conn { struct ovs_mutex lock; /* Guards all mutable fields. */ struct conn_expire exp; ovs_u128 label; - long long expiration; + atomic_llong expiration; uint32_t mark; int seq_skew; diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c index 01ffa30df..a4ea19c1b 100644 --- a/lib/conntrack-tp.c +++ b/lib/conntrack-tp.c @@ -276,7 +276,7 @@ conn_schedule_expiration(struct conntrack *ct, struct conn *conn, OVS_REQUIRES(conn->lock) { conn_expire_init(conn, ct); - conn->expiration = now + tp_value * 1000; + atomic_store_relaxed(&conn->expiration, now + tp_value * 1000); conn->exp.tm = tm; if (!atomic_flag_test_and_set(&conn->exp.insert_once)) { ovsrcu_postpone(conn_expire_insert, conn); diff --git a/lib/conntrack.c b/lib/conntrack.c index e042683aa..193d550cb 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -99,6 +99,7 @@ static enum ct_update_res conn_update(struct conntrack *ct, struct conn *conn, struct dp_packet *pkt, struct conn_lookup_ctx *ctx, long long now); +static long long int conn_expiration(const struct conn *); static bool conn_expired(struct conn *, long long now); static void set_mark(struct dp_packet *, struct conn *, uint32_t val, uint32_t mask); @@ -1555,10 +1556,8 @@ ct_sweep(struct conntrack *ct, long long now, size_t limit) for (unsigned i = 0; i < N_CT_TM; i++) { RCULIST_FOR_EACH (conn, exp.node, &ct->exp_lists[i]) { - ovs_mutex_lock(&conn->lock); - if (now < conn->expiration || count >= limit) { - min_expiration = MIN(min_expiration, conn->expiration); - ovs_mutex_unlock(&conn->lock); + if (!conn_expired(conn, now) || count >= limit) { + min_expiration = MIN(min_expiration, conn_expiration(conn)); if (count >= limit) { /* Do not check other lists. */ COVERAGE_INC(conntrack_long_cleanup); @@ -1566,7 +1565,6 @@ ct_sweep(struct conntrack *ct, long long now, size_t limit) } break; } else { - ovs_mutex_unlock(&conn->lock); conn_clean(ct, conn); } count++; @@ -2395,14 +2393,21 @@ conn_update(struct conntrack *ct, struct conn *conn, struct dp_packet *pkt, return update_res; } +static long long int +conn_expiration(const struct conn *conn) +{ + long long int expiration; + + atomic_read_relaxed(&CONST_CAST(struct conn *, conn)->expiration, + &expiration); + return expiration; +} + static bool conn_expired(struct conn *conn, long long now) { if (conn->conn_type == CT_CONN_TYPE_DEFAULT) { - ovs_mutex_lock(&conn->lock); - bool expired = now >= conn->expiration ? true : false; - ovs_mutex_unlock(&conn->lock); - return expired; + return now >= conn_expiration(conn); } return false; } @@ -2545,7 +2550,7 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry, entry->mark = conn->mark; memcpy(&entry->labels, &conn->label, sizeof entry->labels); - long long expiration = conn->expiration - now; + long long expiration = conn_expiration(conn) - now; struct ct_l4_proto *class = l4_protos[conn->key.nw_proto]; if (class->conn_get_protoinfo) {