[ovs-dev] [PATCH ovn] logical-fields: fix memory leak caused by initialize ovnfield_by_name twice

Damijan Skvarc damjan.skvarc at gmail.com
Thu Feb 27 13:17:37 UTC 2020


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
with the help of ovsthread_once_XXXX functionality.

Problem was reported by valgrind with flood of messages similar to this one:

==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 <damjan.skvarc at gmail.com>
---
 lib/logical-fields.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/logical-fields.c b/lib/logical-fields.c
index 25ace58..569e31e 100644
--- a/lib/logical-fields.c
+++ b/lib/logical-fields.c
@@ -254,12 +254,8 @@ 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);
-    }
+    ovn_init_ovnfields();
+
     expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU);
 }
 
@@ -295,3 +291,20 @@ ovn_destroy_ovnfields(void)
 {
     shash_destroy(&ovnfield_by_name);
 }
+
+void
+ovn_init_ovnfields(void)
+{
+    static struct ovsthread_once field_by_name_once =
+        OVSTHREAD_ONCE_INITIALIZER;
+
+    if (ovsthread_once_start(&field_by_name_once)) {
+       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);
+       }
+       ovsthread_once_done(&field_by_name_once);
+    }
+}
-- 
2.7.4



More information about the dev mailing list