[ovs-dev] [PATCH v2 ovn] logical-fields: fix memory leak caused by initialize ovnfield_by_name twice
Damijan Skvarc
damjan.skvarc at gmail.com
Thu Mar 5 06:21:41 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
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 <damjan.skvarc at gmail.com>
---
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);
}
--
2.7.4
More information about the dev
mailing list