| Message ID | 1471655210-2514-1-git-send-email-ramu.ramamurthy@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
"dev" <dev-bounces@openvswitch.org> wrote on 08/19/2016 08:06:50 PM: > From: Ramu Ramamurthy <ramu.ramamurthy@gmail.com> > To: dev@openvswitch.org > Cc: Suryanarayan Ramamurthy/San Jose/IBM@IBMUS > Date: 08/19/2016 08:07 PM > Subject: [ovs-dev] [PATCH] ovn: fix ovn-northd leaks in build_acl > Sent by: "dev" <dev-bounces@openvswitch.org> > > From: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> > > The following leaks are due to missing ds_destroy in a few > places in build_acl. > > 5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93 > at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x449507: xrealloc (util.c:123) > by 0x42CC73: ds_reserve (dynamic-string.c:63) > by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) > by 0x42D176: ds_put_format (dynamic-string.c:142) > by 0x40D380: build_acls (ovn-northd.c:2320) > by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) > by 0x4072D9: build_lflows (ovn-northd.c:3845) > by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) > by 0x4072D9: main (ovn-northd.c:4375) > > 9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93 > at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x449507: xrealloc (util.c:123) > by 0x42CC73: ds_reserve (dynamic-string.c:63) > by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) > by 0x42D176: ds_put_format (dynamic-string.c:142) > by 0x40D505: build_acls (ovn-northd.c:2346) > by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) > by 0x4072D9: build_lflows (ovn-northd.c:3845) > by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) > by 0x4072D9: main (ovn-northd.c:4375) > --- > ovn/northd/ovn-northd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 625937d..756d188 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -2324,6 +2324,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows) > ovn_lflow_add( > lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr (&match), > actions); > + ds_destroy(&match); > } > } > > @@ -2350,6 +2351,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows) > ovn_lflow_add( > lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr (&match), > actions); > + ds_destroy(&match); > } > } > } > -- Looks good to me on inspection, but I've not tried it out... Acked-by: Ryan Moats <rmoats@us.ibm.com>
"dev" <dev-bounces@openvswitch.org> wrote on 08/19/2016 08:06:50 PM: > From: Ramu Ramamurthy <ramu.ramamurthy@gmail.com> > To: dev@openvswitch.org > Cc: Suryanarayan Ramamurthy/San Jose/IBM@IBMUS > Date: 08/19/2016 08:07 PM > Subject: [ovs-dev] [PATCH] ovn: fix ovn-northd leaks in build_acl > Sent by: "dev" <dev-bounces@openvswitch.org> > > From: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> > > The following leaks are due to missing ds_destroy in a few > places in build_acl. > > 5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93 > at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x449507: xrealloc (util.c:123) > by 0x42CC73: ds_reserve (dynamic-string.c:63) > by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) > by 0x42D176: ds_put_format (dynamic-string.c:142) > by 0x40D380: build_acls (ovn-northd.c:2320) > by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) > by 0x4072D9: build_lflows (ovn-northd.c:3845) > by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) > by 0x4072D9: main (ovn-northd.c:4375) > > 9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93 > at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck- > amd64-linux.so) > by 0x449507: xrealloc (util.c:123) > by 0x42CC73: ds_reserve (dynamic-string.c:63) > by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) > by 0x42D176: ds_put_format (dynamic-string.c:142) > by 0x40D505: build_acls (ovn-northd.c:2346) > by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) > by 0x4072D9: build_lflows (ovn-northd.c:3845) > by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) > by 0x4072D9: main (ovn-northd.c:4375) > --- > ovn/northd/ovn-northd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 625937d..756d188 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -2324,6 +2324,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows) > ovn_lflow_add( > lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr (&match), > actions); > + ds_destroy(&match); > } > } > > @@ -2350,6 +2351,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows) > ovn_lflow_add( > lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr (&match), > actions); > + ds_destroy(&match); > } > } > } > -- I had given a base ack for this previously, and I can say that as of this message that this is the last *definitive* loss of memory in ovn-northd, ovn-controller, and ovsdb-server. Because of that, I'd like to see this merge into master and 2.6 before the release is cut... Ryan Ryan
On Fri, Aug 19, 2016 at 9:06 PM, Ramu Ramamurthy <ramu.ramamurthy@gmail.com> wrote: > From: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> > > The following leaks are due to missing ds_destroy in a few > places in build_acl. > > 5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93 > at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_ > memcheck-amd64-linux.so) > by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_ > memcheck-amd64-linux.so) > by 0x449507: xrealloc (util.c:123) > by 0x42CC73: ds_reserve (dynamic-string.c:63) > by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) > by 0x42D176: ds_put_format (dynamic-string.c:142) > by 0x40D380: build_acls (ovn-northd.c:2320) > by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) > by 0x4072D9: build_lflows (ovn-northd.c:3845) > by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) > by 0x4072D9: main (ovn-northd.c:4375) > > 9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93 > at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_ > memcheck-amd64-linux.so) > by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_ > memcheck-amd64-linux.so) > by 0x449507: xrealloc (util.c:123) > by 0x42CC73: ds_reserve (dynamic-string.c:63) > by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) > by 0x42D176: ds_put_format (dynamic-string.c:142) > by 0x40D505: build_acls (ovn-northd.c:2346) > by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) > by 0x4072D9: build_lflows (ovn-northd.c:3845) > by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) > by 0x4072D9: main (ovn-northd.c:4375) > --- > ovn/northd/ovn-northd.c | 2 ++ > 1 file changed, 2 insertions(+) > I was about to apply this, but it's missing your Signed-off-by. Can you please re-submit with Signed-off-by included? Thanks,
Hi Russell, I updated the patch with a Signed-off-by, but it landed here as new on patchworks, https://patchwork.ozlabs.org/patch/665079/ instead of updating the original here: https://patchwork.ozlabs.org/patch/661079/ On Thu, Sep 1, 2016 at 4:50 PM, Russell Bryant <russell@ovn.org> wrote: > > On Fri, Aug 19, 2016 at 9:06 PM, Ramu Ramamurthy <ramu.ramamurthy@gmail.com> > wrote: >> >> From: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> >> >> The following leaks are due to missing ds_destroy in a few >> places in build_acl. >> >> 5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93 >> at 0x4C29BFD: malloc (in >> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) >> by 0x4C2BACB: realloc (in >> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) >> by 0x449507: xrealloc (util.c:123) >> by 0x42CC73: ds_reserve (dynamic-string.c:63) >> by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) >> by 0x42D176: ds_put_format (dynamic-string.c:142) >> by 0x40D380: build_acls (ovn-northd.c:2320) >> by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) >> by 0x4072D9: build_lflows (ovn-northd.c:3845) >> by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) >> by 0x4072D9: main (ovn-northd.c:4375) >> >> 9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93 >> at 0x4C29BFD: malloc (in >> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) >> by 0x4C2BACB: realloc (in >> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) >> by 0x449507: xrealloc (util.c:123) >> by 0x42CC73: ds_reserve (dynamic-string.c:63) >> by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) >> by 0x42D176: ds_put_format (dynamic-string.c:142) >> by 0x40D505: build_acls (ovn-northd.c:2346) >> by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) >> by 0x4072D9: build_lflows (ovn-northd.c:3845) >> by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) >> by 0x4072D9: main (ovn-northd.c:4375) >> --- >> ovn/northd/ovn-northd.c | 2 ++ >> 1 file changed, 2 insertions(+) > > > I was about to apply this, but it's missing your Signed-off-by. Can you > please re-submit with Signed-off-by included? > > Thanks, > > -- > Russell Bryant
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 625937d..756d188 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -2324,6 +2324,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows) ovn_lflow_add( lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr(&match), actions); + ds_destroy(&match); } } @@ -2350,6 +2351,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows) ovn_lflow_add( lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr(&match), actions); + ds_destroy(&match); } } }
From: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> The following leaks are due to missing ds_destroy in a few places in build_acl. 5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93 at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x449507: xrealloc (util.c:123) by 0x42CC73: ds_reserve (dynamic-string.c:63) by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) by 0x42D176: ds_put_format (dynamic-string.c:142) by 0x40D380: build_acls (ovn-northd.c:2320) by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) by 0x4072D9: build_lflows (ovn-northd.c:3845) by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) by 0x4072D9: main (ovn-northd.c:4375) 9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93 at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x449507: xrealloc (util.c:123) by 0x42CC73: ds_reserve (dynamic-string.c:63) by 0x42D08F: ds_put_format_valist (dynamic-string.c:161) by 0x42D176: ds_put_format (dynamic-string.c:142) by 0x40D505: build_acls (ovn-northd.c:2346) by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472) by 0x4072D9: build_lflows (ovn-northd.c:3845) by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971) by 0x4072D9: main (ovn-northd.c:4375) --- ovn/northd/ovn-northd.c | 2 ++ 1 file changed, 2 insertions(+)