diff mbox series

[ovs-dev,02/10] ofproto-dpif: Uninitialize 'xlate_cache' to free resources

Message ID 1568236716-18105-2-git-send-email-pkusunyifeng@gmail.com
State Accepted
Commit 584992d064d918eca540a9ba44abef0b7b569252
Headers show
Series [ovs-dev,01/10] raft: Free leaked json data | expand

Commit Message

Yifeng Sun Sept. 11, 2019, 9:18 p.m. UTC
Valgrind reported:

1210: ofproto-dpif - continuation after clone

==32205== 4,392 (1,440 direct, 2,952 indirect) bytes in 12 blocks are definitely lost in loss record 359 of 362
==32205==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32205==    by 0x532574: xmalloc (util.c:138)
==32205==    by 0x4F98CA: ofpbuf_init (ofpbuf.c:123)
==32205==    by 0x42C07B: nxt_resume (ofproto-dpif.c:5110)
==32205==    by 0x41796F: handle_nxt_resume (ofproto.c:3677)
==32205==    by 0x424583: handle_single_part_openflow (ofproto.c:8473)
==32205==    by 0x424583: handle_openflow (ofproto.c:8606)
==32205==    by 0x4579E2: ofconn_run (connmgr.c:1318)
==32205==    by 0x4579E2: connmgr_run (connmgr.c:355)
==32205==    by 0x41E0F5: ofproto_run (ofproto.c:1845)
==32205==    by 0x40BA63: bridge_run__ (bridge.c:2971)
==32205==    by 0x410CF3: bridge_run (bridge.c:3029)
==32205==    by 0x407614: main (ovs-vswitchd.c:127)

This is because 'xcache' was not destroyed properly. This patch fixes it.

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 ofproto/ofproto-dpif.c | 1 +
 1 file changed, 1 insertion(+)

Comments

William Tu Sept. 17, 2019, 4:16 p.m. UTC | #1
On Wed, Sep 11, 2019 at 02:18:28PM -0700, Yifeng Sun wrote:
> Valgrind reported:
> 
> 1210: ofproto-dpif - continuation after clone
> 
> ==32205== 4,392 (1,440 direct, 2,952 indirect) bytes in 12 blocks are definitely lost in loss record 359 of 362
> ==32205==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==32205==    by 0x532574: xmalloc (util.c:138)
> ==32205==    by 0x4F98CA: ofpbuf_init (ofpbuf.c:123)
> ==32205==    by 0x42C07B: nxt_resume (ofproto-dpif.c:5110)
> ==32205==    by 0x41796F: handle_nxt_resume (ofproto.c:3677)
> ==32205==    by 0x424583: handle_single_part_openflow (ofproto.c:8473)
> ==32205==    by 0x424583: handle_openflow (ofproto.c:8606)
> ==32205==    by 0x4579E2: ofconn_run (connmgr.c:1318)
> ==32205==    by 0x4579E2: connmgr_run (connmgr.c:355)
> ==32205==    by 0x41E0F5: ofproto_run (ofproto.c:1845)
> ==32205==    by 0x40BA63: bridge_run__ (bridge.c:2971)
> ==32205==    by 0x410CF3: bridge_run (bridge.c:3029)
> ==32205==    by 0x407614: main (ovs-vswitchd.c:127)
> 
> This is because 'xcache' was not destroyed properly. This patch fixes it.
> 
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>

LGTM
Acked-by: William Tu <u9012063@gmail.com>

> ---
>  ofproto/ofproto-dpif.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 751535249e21..46fa1357163b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5148,6 +5148,7 @@ nxt_resume(struct ofproto *ofproto_,
>      /* Clean up. */
>      ofpbuf_uninit(&odp_actions);
>      dp_packet_uninit(&packet);
> +    xlate_cache_uninit(&xcache);
>  
>      return error;
>  }
> -- 
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ilya Maximets Jan. 14, 2021, 4:28 p.m. UTC | #2
On 9/11/19 11:18 PM, Yifeng Sun wrote:
> Valgrind reported:
> 
> 1210: ofproto-dpif - continuation after clone
> 
> ==32205== 4,392 (1,440 direct, 2,952 indirect) bytes in 12 blocks are definitely lost in loss record 359 of 362
> ==32205==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==32205==    by 0x532574: xmalloc (util.c:138)
> ==32205==    by 0x4F98CA: ofpbuf_init (ofpbuf.c:123)
> ==32205==    by 0x42C07B: nxt_resume (ofproto-dpif.c:5110)
> ==32205==    by 0x41796F: handle_nxt_resume (ofproto.c:3677)
> ==32205==    by 0x424583: handle_single_part_openflow (ofproto.c:8473)
> ==32205==    by 0x424583: handle_openflow (ofproto.c:8606)
> ==32205==    by 0x4579E2: ofconn_run (connmgr.c:1318)
> ==32205==    by 0x4579E2: connmgr_run (connmgr.c:355)
> ==32205==    by 0x41E0F5: ofproto_run (ofproto.c:1845)
> ==32205==    by 0x40BA63: bridge_run__ (bridge.c:2971)
> ==32205==    by 0x410CF3: bridge_run (bridge.c:3029)
> ==32205==    by 0x407614: main (ovs-vswitchd.c:127)
> 
> This is because 'xcache' was not destroyed properly. This patch fixes it.
> 
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> Acked-by: William Tu <u9012063@gmail.com>
> ---
>  ofproto/ofproto-dpif.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 751535249e21..46fa1357163b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5148,6 +5148,7 @@ nxt_resume(struct ofproto *ofproto_,
>      /* Clean up. */
>      ofpbuf_uninit(&odp_actions);
>      dp_packet_uninit(&packet);
> +    xlate_cache_uninit(&xcache);
>  
>      return error;
>  }
> 

I backported this patch additionally to branches 2.11-2.8 since
this issue causes serious memory leaks there:
  https://github.com/openvswitch/ovs-issues/issues/168

Will also revist other patches from this series later.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 751535249e21..46fa1357163b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5148,6 +5148,7 @@  nxt_resume(struct ofproto *ofproto_,
     /* Clean up. */
     ofpbuf_uninit(&odp_actions);
     dp_packet_uninit(&packet);
+    xlate_cache_uninit(&xcache);
 
     return error;
 }