diff mbox series

[ovs-dev,1/2] ovn-controller-vtep: Fix leak of multicast macs.

Message ID 20201125150030.22532.14780.stgit@dceara.remote.csb
State Accepted
Headers show
Series Enable LeakSanitizer when running tests with AddressSanitizer. | expand

Commit Message

Dumitru Ceara Nov. 25, 2020, 3 p.m. UTC
Also, enable valgrind runs for ovn-controller-vtep when running "make
check-valgrind".

Fixes: dd6218709a00 ("ovn-controller-vtep: Support BUM traffic for the VTEP Schema.")
Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
Note: This is a brute force approach of just tracking all pointers in
a hmapx and unifying cleanup.  There might be a better approach but I
couldn't figure it out.
---
 controller-vtep/vtep.c |   10 +++++++---
 tests/automake.mk      |    1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Numan Siddique Nov. 30, 2020, 2:56 a.m. UTC | #1
On Wed, Nov 25, 2020 at 8:30 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> Also, enable valgrind runs for ovn-controller-vtep when running "make
> check-valgrind".
>
> Fixes: dd6218709a00 ("ovn-controller-vtep: Support BUM traffic for the VTEP Schema.")
> Suggested-by: Ilya Maximets <i.maximets@ovn.org>
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Thanks. I applied this patch to master and backported upto 20.03.

Numan

> ---
> Note: This is a brute force approach of just tracking all pointers in
> a hmapx and unifying cleanup.  There might be a better approach but I
> couldn't figure it out.
> ---
>  controller-vtep/vtep.c |   10 +++++++---
>  tests/automake.mk      |    1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/controller-vtep/vtep.c b/controller-vtep/vtep.c
> index 5538c7d..f3b02f6 100644
> --- a/controller-vtep/vtep.c
> +++ b/controller-vtep/vtep.c
> @@ -18,6 +18,7 @@
>  #include "vtep.h"
>
>  #include "lib/hash.h"
> +#include "lib/hmapx.h"
>  #include "openvswitch/hmap.h"
>  #include "openvswitch/shash.h"
>  #include "lib/ovn-util.h"
> @@ -496,12 +497,12 @@ vtep_run(struct controller_vtep_ctx *ctx)
>      struct shash physical_locators = SHASH_INITIALIZER(&physical_locators);
>      struct shash vtep_pbs = SHASH_INITIALIZER(&vtep_pbs);
>      struct shash non_vtep_pbs = SHASH_INITIALIZER(&non_vtep_pbs);
> +    struct hmapx mcast_macs_ptrs = HMAPX_INITIALIZER(&mcast_macs_ptrs);
>      const struct vteprec_physical_switch *vtep_ps;
>      const struct vteprec_logical_switch *vtep_ls;
>      const struct vteprec_ucast_macs_remote *umr;
>      const struct sbrec_port_binding *port_binding_rec;
>      const struct vteprec_mcast_macs_remote *mmr;
> -    struct shash_node *node;
>
>      /* Collects 'Physical_Switch's. */
>      VTEPREC_PHYSICAL_SWITCH_FOR_EACH (vtep_ps, ctx->vtep_idl) {
> @@ -527,7 +528,8 @@ vtep_run(struct controller_vtep_ctx *ctx)
>
>      /* Collects 'Mcast_Macs_Remote's. */
>      VTEPREC_MCAST_MACS_REMOTE_FOR_EACH (mmr, ctx->vtep_idl) {
> -        struct mmr_hash_node_data *mmr_ext = xmalloc(sizeof *mmr_ext);;
> +        struct mmr_hash_node_data *mmr_ext = xmalloc(sizeof *mmr_ext);
> +        hmapx_add(&mcast_macs_ptrs, mmr_ext);
>          char *mac_tnlkey =
>              xasprintf("%s_%"PRId64, mmr->MAC,
>                        mmr->logical_switch && mmr->logical_switch->n_tunnel_key
> @@ -575,11 +577,13 @@ vtep_run(struct controller_vtep_ctx *ctx)
>      sset_destroy(&vtep_pswitches);
>      shash_destroy(&vtep_lswitches);
>      shash_destroy(&ucast_macs_rmts);
> -    SHASH_FOR_EACH (node, &mcast_macs_rmts) {
> +    struct hmapx_node *node;
> +    HMAPX_FOR_EACH (node, &mcast_macs_ptrs) {
>          struct mmr_hash_node_data *mmr_ext = node->data;
>          shash_destroy(&mmr_ext->physical_locators);
>          free(mmr_ext);
>      }
> +    hmapx_destroy(&mcast_macs_ptrs);
>      shash_destroy(&mcast_macs_rmts);
>      shash_destroy(&physical_locators);
>      shash_destroy(&vtep_pbs);
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 44be209..0ee58af 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -101,6 +101,7 @@ check-lcov: all $(check_DATA) clean-lcov
>
>  valgrind_wrappers = \
>         tests/valgrind/ovn-controller \
> +       tests/valgrind/ovn-controller-vtep \
>         tests/valgrind/ovn-nbctl \
>         tests/valgrind/ovn-northd \
>         tests/valgrind/ovn-sbctl \
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller-vtep/vtep.c b/controller-vtep/vtep.c
index 5538c7d..f3b02f6 100644
--- a/controller-vtep/vtep.c
+++ b/controller-vtep/vtep.c
@@ -18,6 +18,7 @@ 
 #include "vtep.h"
 
 #include "lib/hash.h"
+#include "lib/hmapx.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/shash.h"
 #include "lib/ovn-util.h"
@@ -496,12 +497,12 @@  vtep_run(struct controller_vtep_ctx *ctx)
     struct shash physical_locators = SHASH_INITIALIZER(&physical_locators);
     struct shash vtep_pbs = SHASH_INITIALIZER(&vtep_pbs);
     struct shash non_vtep_pbs = SHASH_INITIALIZER(&non_vtep_pbs);
+    struct hmapx mcast_macs_ptrs = HMAPX_INITIALIZER(&mcast_macs_ptrs);
     const struct vteprec_physical_switch *vtep_ps;
     const struct vteprec_logical_switch *vtep_ls;
     const struct vteprec_ucast_macs_remote *umr;
     const struct sbrec_port_binding *port_binding_rec;
     const struct vteprec_mcast_macs_remote *mmr;
-    struct shash_node *node;
 
     /* Collects 'Physical_Switch's. */
     VTEPREC_PHYSICAL_SWITCH_FOR_EACH (vtep_ps, ctx->vtep_idl) {
@@ -527,7 +528,8 @@  vtep_run(struct controller_vtep_ctx *ctx)
 
     /* Collects 'Mcast_Macs_Remote's. */
     VTEPREC_MCAST_MACS_REMOTE_FOR_EACH (mmr, ctx->vtep_idl) {
-        struct mmr_hash_node_data *mmr_ext = xmalloc(sizeof *mmr_ext);;
+        struct mmr_hash_node_data *mmr_ext = xmalloc(sizeof *mmr_ext);
+        hmapx_add(&mcast_macs_ptrs, mmr_ext);
         char *mac_tnlkey =
             xasprintf("%s_%"PRId64, mmr->MAC,
                       mmr->logical_switch && mmr->logical_switch->n_tunnel_key
@@ -575,11 +577,13 @@  vtep_run(struct controller_vtep_ctx *ctx)
     sset_destroy(&vtep_pswitches);
     shash_destroy(&vtep_lswitches);
     shash_destroy(&ucast_macs_rmts);
-    SHASH_FOR_EACH (node, &mcast_macs_rmts) {
+    struct hmapx_node *node;
+    HMAPX_FOR_EACH (node, &mcast_macs_ptrs) {
         struct mmr_hash_node_data *mmr_ext = node->data;
         shash_destroy(&mmr_ext->physical_locators);
         free(mmr_ext);
     }
+    hmapx_destroy(&mcast_macs_ptrs);
     shash_destroy(&mcast_macs_rmts);
     shash_destroy(&physical_locators);
     shash_destroy(&vtep_pbs);
diff --git a/tests/automake.mk b/tests/automake.mk
index 44be209..0ee58af 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -101,6 +101,7 @@  check-lcov: all $(check_DATA) clean-lcov
 
 valgrind_wrappers = \
 	tests/valgrind/ovn-controller \
+	tests/valgrind/ovn-controller-vtep \
 	tests/valgrind/ovn-nbctl \
 	tests/valgrind/ovn-northd \
 	tests/valgrind/ovn-sbctl \