| Message ID | 1495276383-6012-1-git-send-email-wangyunjian@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, May 20, 2017 at 06:33:03PM +0800, Yunjian Wang wrote: > The memory leak was triggered each time on calling > netflow_unref with containing netflow_flows. > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > ofproto/netflow.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ofproto/netflow.c b/ofproto/netflow.c > index 55f7814..6bcbfe5 100644 > --- a/ofproto/netflow.c > +++ b/ofproto/netflow.c > @@ -413,6 +413,7 @@ netflow_unref(struct netflow *nf) > atomic_count_dec(&netflow_count); > collectors_destroy(nf->collectors); > ofpbuf_uninit(&nf->packet); > + hmap_destroy(&nf->flows); > free(nf); > } > } Thank you for the bug fix, which appears correct. However, going a bit further, I don't see a guarantee that 'flows' is empty when the object is destroyed. Should there also be a loop that removes and destroys any remaining flows? Thanks, Ben.
> -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Sunday, May 21, 2017 2:33 AM > To: wangyunjian <wangyunjian@huawei.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] netflow: Fix memory leak in netflow_unref. > > On Sat, May 20, 2017 at 06:33:03PM +0800, Yunjian Wang wrote: > > The memory leak was triggered each time on calling > > netflow_unref with containing netflow_flows. > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > ofproto/netflow.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/ofproto/netflow.c b/ofproto/netflow.c > > index 55f7814..6bcbfe5 100644 > > --- a/ofproto/netflow.c > > +++ b/ofproto/netflow.c > > @@ -413,6 +413,7 @@ netflow_unref(struct netflow *nf) > > atomic_count_dec(&netflow_count); > > collectors_destroy(nf->collectors); > > ofpbuf_uninit(&nf->packet); > > + hmap_destroy(&nf->flows); > > free(nf); > > } > > } > > Thank you for the bug fix, which appears correct. > > However, going a bit further, I don't see a guarantee that 'flows' is > empty when the object is destroyed. Should there also be a loop that > removes and destroys any remaining flows? Yes, I agree that flows need to be removed and destroyed. I will make a new patch fix it. Thanks, Yunjian > > Thanks, > > Ben.
diff --git a/ofproto/netflow.c b/ofproto/netflow.c index 55f7814..6bcbfe5 100644 --- a/ofproto/netflow.c +++ b/ofproto/netflow.c @@ -413,6 +413,7 @@ netflow_unref(struct netflow *nf) atomic_count_dec(&netflow_count); collectors_destroy(nf->collectors); ofpbuf_uninit(&nf->packet); + hmap_destroy(&nf->flows); free(nf); } }
The memory leak was triggered each time on calling netflow_unref with containing netflow_flows. Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- ofproto/netflow.c | 1 + 1 file changed, 1 insertion(+)