From patchwork Sat May 25 10:18:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "wushaohua@chinatelecom.cn" X-Patchwork-Id: 1939220 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4VmdMc3sBJz1ydW for ; Sat, 25 May 2024 20:25:14 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 580A140503; Sat, 25 May 2024 10:25:12 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id dd4wBfCv4byP; Sat, 25 May 2024 10:25:11 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 3049A404B8 Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 3049A404B8; Sat, 25 May 2024 10:25:11 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BADC2C0072; Sat, 25 May 2024 10:25:10 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 268ADC0037 for ; Sat, 25 May 2024 10:25:09 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 1B1F9404D1 for ; Sat, 25 May 2024 10:25:09 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id xHnoYHxtCNDx for ; Sat, 25 May 2024 10:25:08 +0000 (UTC) X-Greylist: delayed 394 seconds by postgrey-1.37 at util1.osuosl.org; Sat, 25 May 2024 10:25:06 UTC DMARC-Filter: OpenDMARC Filter v1.4.2 smtp2.osuosl.org 8BACD404B8 Authentication-Results: smtp2.osuosl.org; dmarc=none (p=none dis=none) header.from=chinatelecom.cn DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 8BACD404B8 Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=182.42.159.233; helo=chinatelecom.cn; envelope-from=wushaohua@chinatelecom.cn; receiver= Received: from chinatelecom.cn (smtpnm6-01.21cn.com [182.42.159.233]) by smtp2.osuosl.org (Postfix) with ESMTP id 8BACD404B8 for ; Sat, 25 May 2024 10:25:06 +0000 (UTC) HMM_SOURCE_IP: 192.168.138.117:64329.2114947555 HMM_ATTACHE_NUM: 0000 HMM_SOURCE_TYPE: SMTP Received: from clientip-101.43.140.16 (unknown [192.168.138.117]) by chinatelecom.cn (HERMES) with SMTP id B671C100085DE; Sat, 25 May 2024 18:18:15 +0800 (CST) X-189-SAVE-TO-SEND: +wushaohua@chinatelecom.cn Received: from ([101.43.140.16]) by gateway-ssl-dep-84c6c6b769-jsnsj with ESMTP id 1ba7a24fabda41e5acb8fac546847d01 for dev@openvswitch.org; Sat, 25 May 2024 18:18:17 CST X-Transaction-ID: 1ba7a24fabda41e5acb8fac546847d01 X-Real-From: wushaohua@chinatelecom.cn X-Receive-IP: 101.43.140.16 X-MEDUSA-Status: 0 From: wushaohua@chinatelecom.cn To: dev@openvswitch.org Date: Sat, 25 May 2024 18:18:14 +0800 Message-Id: <20240525101814.1952081-1-wushaohua@chinatelecom.cn> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20240430093609.1739186-1-wushaohua@chinatelecom.cn> References: <20240430093609.1739186-1-wushaohua@chinatelecom.cn> MIME-Version: 1.0 Cc: wushaohua@chinatelecom.cn Subject: [ovs-dev] [PATCH v3] ofproto-dpif-rid: Fix duplicate entries. 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" From: Shaohua Wu In scenarios with multiple PMDs, there may be simultaneous requests for recirc_id from multiple PMD threads.In recirc_alloc_id_ctx, we first check if there is a duplicate entry in the metadata_map for the same frozen_state field. If successful, we directly retrieve the recirc_id. If unsuccessful, we create a new recirc_node and insert it into id_map and metadata_map. There is no locking mechanism to prevent the possibility of two threads with the same state simultaneously inserting, meaning their IDs are different, but their frozen_states are the same. trace log: static struct recirc_id_node * recirc_alloc_id__(const struct frozen_state *state, uint32_t hash) { ovs_assert(state->action_set_len <= state->ofpacts_len); struct recirc_id_node *node = xzalloc(sizeof *node); node->hash = hash; ovs_refcount_init(&node->refcount); node->is_hotupgrade = false; frozen_state_clone(CONST_CAST(struct frozen_state *, &node->state), state); struct recirc_id_node *hash_recirc_node = recirc_find_equal(&node->state, state); if (hash_recirc_node) { VLOG_INFO("wsh:hash equal:hash_recirc_node %p id:%u, hash_recirc_node hash:%u,node %p hash:%u\n", hash_recirc_nodeļ¼Œ hash_recirc_node->id, hash_recirc_node->hash, node, node->hash); } ovs_mutex_lock(&mutex); ... } Log recording: 2024-04-27T12:28:47.973Z|00006|ofproto_dpif_rid(pmd-c08/id:13)|INFO|wsh:hash equal: hash_recirc_node 0x7fb29c0276a0 id:27,hash_recirc_node hash:3224122528,node 0x7fb2900194d0 hash:3224122528 2024-04-27T12:28:47.973Z|00009|ofproto_dpif_rid(pmd-c02/id:15)|INFO|wsh:hash equal: hash_recirc_node 0x7fb29c0276a0 id:27,hash_recirc_node hash:3224122528,node 0x7fb288025270 hash:3224122528 2024-04-27T12:28:47.973Z|00006|ofproto_dpif_rid(pmd-c03/id:14)|INFO|wsh:hash equal: hash_recirc_node 0x7fb29c0276a0 id:27,hash_recirc_node hash:3224122528,node 0x7fb29401d4e0 hash:3224122528 2024-04-27T12:28:47.973Z|00004|ofproto_dpif_rid(pmd-c09/id:12)|INFO|node->id:28,hash:4019648042,table_id:75 2024-04-27T12:28:47.973Z|00007|ofproto_dpif_rid(pmd-c08/id:13)|INFO|wsh:hash equal: hash_recirc_node 0x7fb29c028d40 id:28,hash_recirc_node hash:4019648042,node 0x7fb29001ac30 hash:4019648042 2024-04-27T12:28:48.065Z|00005|ofproto_dpif_rid(pmd-c09/id:12)|INFO|node->id:29,hash:3800776147,table_id:30 2024-04-27T12:28:48.101Z|00007|ofproto_dpif_rid(pmd-c03/id:14)|INFO|node->id:30,hash:1580334976,table_id:75 Signed-off-by: Shaohua Wu --- v1->v2:modify log recording , add trace code. v2->v3:Optimize recirc_alloc_id_ctx. The recirc_alloc_id interface is used for bond configuration and must obtain a unique recirc_id. Signed-off-by: wushaohua --- ofproto/ofproto-dpif-rid.c | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index f01468025..eeee81f05 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -277,11 +277,42 @@ recirc_find_id(const struct frozen_state *target) uint32_t recirc_alloc_id_ctx(const struct frozen_state *state) { + ovs_assert(state->action_set_len <= state->ofpacts_len); + struct recirc_id_node *node = NULL; + struct recirc_id_node *find_node = NULL; uint32_t hash = frozen_state_hash(state); - struct recirc_id_node *node = recirc_ref_equal(state, hash); - if (!node) { - node = recirc_alloc_id__(state, hash); + + node = xzalloc(sizeof *node); + node->hash = hash; + ovs_refcount_init(&node->refcount); + frozen_state_clone(CONST_CAST(struct frozen_state *, &node->state), state); + + ovs_mutex_lock(&mutex); + find_node = recirc_ref_equal(state, hash); + if (find_node) { + ovs_mutex_unlock(&mutex); + recirc_id_node_free(node); + return find_node->id; } + + for (;;) { + /* Claim the next ID. The ID space should be sparse enough for the + allocation to succeed at the first try. We do skip the first + RECIRC_POOL_STATIC_IDS IDs on the later rounds, though, as some of + the initial allocations may be for long term uses (like bonds). */ + node->id = next_id++; + if (OVS_UNLIKELY(!node->id)) { + next_id = RECIRC_POOL_STATIC_IDS + 1; + node->id = next_id++; + } + /* Find if the id is free. */ + if (OVS_LIKELY(!recirc_find__(node->id))) { + break; + } + } + cmap_insert(&id_map, &node->id_node, node->id); + cmap_insert(&metadata_map, &node->metadata_node, node->hash); + ovs_mutex_unlock(&mutex); return node->id; }