On Thu, 2017-05-25 at 21:05 -0700, Greg Rose wrote:
> On Thu, 2017-05-25 at 09:53 -0700, Greg Rose wrote:
> > On Mon, 2017-05-22 at 07:40 -0700, Greg Rose wrote:
> > > On Mon, 2017-05-22 at 12:55 +0800, Yunjian Wang wrote:
> > > > The memory leak was triggered each time on calling netflow_unref() with
> > > > containing netflow_flows. And flows need to be removed and destroyed.
> > > >
> > > > Signed-off-by: Yunjian Wang <[email protected]>
> > > > ---
> > > > ofproto/netflow.c | 7 +++++++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/ofproto/netflow.c b/ofproto/netflow.c
> > > > index 55f7814..29c5f3e 100644
> > > > --- a/ofproto/netflow.c
> > > > +++ b/ofproto/netflow.c
> > > > @@ -409,10 +409,17 @@ netflow_ref(const struct netflow *nf_)
> > > > void
> > > > netflow_unref(struct netflow *nf)
> > > > {
> > > > + struct netflow_flow *nf_flow, *next;
> > > > +
> > > > if (nf && ovs_refcount_unref_relaxed(&nf->ref_cnt) == 1) {
> > > > atomic_count_dec(&netflow_count);
> > > > collectors_destroy(nf->collectors);
> > > > ofpbuf_uninit(&nf->packet);
> > > > + HMAP_FOR_EACH_SAFE (nf_flow, next, hmap_node, &nf->flows) {
> > > > + hmap_remove(&nf->flows, &nf_flow->hmap_node);
> > > > + free(nf_flow);
> > > > + }
> > > > + hmap_destroy(&nf->flows);
> > > > free(nf);
> > > > }
> > > > }
> > >
> > > This looks right to me. The only other place I see the flows freed is
> > > when they're detected as idle. If the flow is never detected as idle
> > > then I don't see anywhere else that they are freed up after the xzalloc
> > > in netflow_flow_update().
> > >
> > > Reviewed-by: Greg Rose <[email protected]>
> > >
> >
> > I'm trying to test this but the condition never seems to be met and thus
> > the 4 lines of additional code free flows never executes. Is there some
> > particular flow or type of network traffic that will execute this code?
> >
> > I'd like to add a Tested-by for this but unless I can get the code to
> > execute I can't.
> >
> > - Greg
>
> OK, I've finally got my netflow configuration working right with ntopng
> collecting the flows. I can test the patch tomorrow, it's getting late
> for me now.
>
> Thanks,
>
> - Greg
Actually, ntopng doesn't work as the flow collector itself (I spoke too
soon) but I am successfully creating netflows which I can see with
tcpdump. Then we are hitting the condition net netflow_unref() when I
execute this command:
ovs-vsctl clear Bridge int-br1 netflow
However, the '*' marked lines of code are never executed when I place a
gdb breakpoint on it:
HMAP_FOR_EACH_SAFE (nf_flow, next, hmap_node, &nf->flows) {
* hmap_remove(&nf->flows, &nf_flow->hmap_node);
* free(nf_flow);
}
It seems the code is never reached. Unless we can see some example of
the code being reached and executed I worry about this patch.
Thanks,
- Greg
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
