From patchwork Wed Mar 4 17:18:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damijan Skvarc X-Patchwork-Id: 1249131 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.136; helo=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=UU0QJh7I; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48XgZV3NMWz9sRY for ; Thu, 5 Mar 2020 04:18:26 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 78C4F204A3; Wed, 4 Mar 2020 17:18:22 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ajLk6zbeu8P2; Wed, 4 Mar 2020 17:18:19 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 6D04220488; Wed, 4 Mar 2020 17:18:19 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4D3B6C1AE2; Wed, 4 Mar 2020 17:18:19 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id A3D3DC013E for ; Wed, 4 Mar 2020 17:18:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 8CE6C849B8 for ; Wed, 4 Mar 2020 17:18:17 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kSakL3h-BUfY for ; Wed, 4 Mar 2020 17:18:16 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 45A64844EA for ; Wed, 4 Mar 2020 17:18:16 +0000 (UTC) Received: by mail-wr1-f65.google.com with SMTP id r17so3385951wrj.7 for ; Wed, 04 Mar 2020 09:18:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=BYSk7l++bTBfgjSdZP0GfOyMWk/IbU4Pfx2JE1ENVoM=; b=UU0QJh7Ia0++/0JlLFnOLbcnV2cAFzmHX28H+iREZrOPh+3EW4gaT8+UA1QZSSc3OW xECQTRT484NzURLFd1L4kqD2PyNVat5almFp1kavbG5USiUBEU0Dxwvvvvbqup4bGWwL rp58UPklSEfJffgcF/gFEtzHtLkiibnMSzXjQMBv0j7E8qqYDklieyfIwDOZeWIgIdso n3rxFBaTlqq3Qb/M8KE+i8/JyiiaBVdxFRU8tucSMQrcZ/K4E0ExGxOuEvLqTXBsS1os QQJrm7v2UlGzchm4pBqwbIqphgdZ5N0Oy+Sx48WuZAaZ/OgARy5r8UYBLGx3wVrUlMXz iSJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=BYSk7l++bTBfgjSdZP0GfOyMWk/IbU4Pfx2JE1ENVoM=; b=p65Tcrd6CAq1c8TcW10ePpe/n62bA4aNjGRy16WqpDXt0nZo7qCehGInasuLSlRczg hUy0HD2tYHPc9mffrFeHYF+jCAGlh9lphRNR//op0egYme16ZSydfukOVu+Cs1wy3gz/ 5zepq/0u7iMkmSbyjG0nkpzrL8p5NLQ+1JhVmygJNs8N7Apr9I+5HXcXO4pwvOSbyXo/ sfnllYtYmJZbtBbDzkekAwaLEbP48/Gx3uFyIlU9t+1r7ajuVEhEdMuLfLcMb3OwWrgB dA7Rrw8Cr/8GYDVhZ3MxwVDTHKoaHX8rDjeUGIQPcLLgh4jqNKArEOBPnM1IVH/lbB7J g9+g== X-Gm-Message-State: ANhLgQ2R6yE85wgvNADa3ekaoy24nhWF0yXnMEIz5Ekj0xyn6QZEjBjk IP1xc84UMLUi+y07paAUpihfaxuWbYE= X-Google-Smtp-Source: ADFU+vstUHYNYoKgd1AMWAZHryDt0B5fOidgWrFdEpxSpViSHNpS7JW75Pj+ztvZ6eKOuVJD1keWHA== X-Received: by 2002:a5d:4b50:: with SMTP id w16mr5023833wrs.230.1583342294336; Wed, 04 Mar 2020 09:18:14 -0800 (PST) Received: from damijan-PC.lan.i-tech.si (mail.i-tech.si. [89.212.78.105]) by smtp.gmail.com with ESMTPSA id y1sm3997112wrh.65.2020.03.04.09.18.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 04 Mar 2020 09:18:13 -0800 (PST) From: Damijan Skvarc To: dev@openvswitch.org Date: Wed, 4 Mar 2020 18:18:09 +0100 Message-Id: <1583342289-10691-1-git-send-email-damjan.skvarc@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: References: Subject: [ovs-dev] [PATCH v2] logical-fields: fix memory leak caused by initialize ovnfield_by_name twice 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" ovnfield_by_name is hash of strings which is used to quickly find field by name. This hash is initialized from ovn_init_symtab(). In case the latter function is called multiple times then also ovnfield_by_name is initialized multiple times but without freeing previously allocated memory resources what cause memory leaks. This actually happens in ovn-controller which calls ovn_init_symtab() function twice, once from ofctrl.c and the other time from lflow.c files. Problem was solved by initializing ovnfield_by_name entity only once and using design pattern from stopwatch.c or meta_flow.c files (ovs). Problem was reported by valgrind with flood of messages (190) while executing ovn test suite: ==5999== 47 (32 direct, 15 indirect) bytes in 1 blocks are definitely lost in loss record 86 of 102 ==5999== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==5999== by 0x50635D: xmalloc (util.c:138) ==5999== by 0x4F6513: shash_add_nocopy__ (shash.c:109) ==5999== by 0x4F6585: shash_add_nocopy (shash.c:121) ==5999== by 0x4F65BD: shash_add (shash.c:129) ==5999== by 0x4F6602: shash_add_once (shash.c:136) ==5999== by 0x4395B7: ovn_init_symtab (logical-fields.c:261) ==5999== by 0x406C91: main (ovn-controller.c:1750) Signed-off-by: Damijan Skvarc --- controller/lflow.c | 3 +-- include/ovn/logical-fields.h | 1 - lib/logical-fields.c | 39 +++++++++++++++++++++++++++------------ 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index ee11fc6..143ea3c 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -827,7 +827,7 @@ lflow_handle_changed_neighbors( } } - + /* Translates logical flows in the Logical_Flow table in the OVN_SB database * into OpenFlow flows. See ovn-architecture(7) for more information. */ void @@ -846,5 +846,4 @@ lflow_destroy(void) { expr_symtab_destroy(&symtab); shash_destroy(&symtab); - ovn_destroy_ovnfields(); } diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index 9b7c34f..c7bd2db 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -130,5 +130,4 @@ ovn_field_from_id(enum ovn_field_id id) const char *event_to_string(enum ovn_controller_event event); int string_to_event(const char *s); const struct ovn_field *ovn_field_from_name(const char *name); -void ovn_destroy_ovnfields(void); #endif /* ovn/lib/logical-fields.h */ diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 25ace58..a007085 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -254,12 +254,6 @@ ovn_init_symtab(struct shash *symtab) expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false); expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false); - shash_init(&ovnfield_by_name); - for (int i = 0; i < OVN_FIELD_N_IDS; i++) { - const struct ovn_field *of = &ovn_fields[i]; - ovs_assert(of->id == i); /* Fields must be in the enum order. */ - shash_add_once(&ovnfield_by_name, of->name, of); - } expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU); } @@ -284,14 +278,35 @@ string_to_event(const char *s) return -1; } -const struct ovn_field * -ovn_field_from_name(const char *name) +static void +ovn_destroy_ovnfields(void) { - return shash_find_data(&ovnfield_by_name, name); + shash_destroy(&ovnfield_by_name); } -void -ovn_destroy_ovnfields(void) +static void +ovn_do_init_ovnfields(void) { - shash_destroy(&ovnfield_by_name); + shash_init(&ovnfield_by_name); + for (int i = 0; i < OVN_FIELD_N_IDS; i++) { + const struct ovn_field *of = &ovn_fields[i]; + ovs_assert(of->id == i); /* Fields must be in the enum order. */ + shash_add_once(&ovnfield_by_name, of->name, of); + } + atexit(ovn_destroy_ovnfields); +} + +static void +ovn_init_ovnfields(void) +{ + static pthread_once_t once = PTHREAD_ONCE_INIT; + pthread_once(&once, ovn_do_init_ovnfields); +} + +const struct ovn_field * +ovn_field_from_name(const char *name) +{ + ovn_init_ovnfields(); + + return shash_find_data(&ovnfield_by_name, name); }