On Wed, Feb 12, 2020 at 5:44 PM Dumitru Ceara <[email protected]> wrote:
>
> On 2/12/20 12:11 PM, [email protected] wrote:
> > From: Numan Siddique <[email protected]>
> >
> > The patch [1], which added caching of lflow expr introduced a memory
> > leak. The patch [1] also didn't take care of deleting the expr from the
> > cache
> > for the deleted lflows. This results in those lflow exprs in cache hanging
> > in
> > forever. This patch also addresses these 2 issues.
> >
> > [1] - 8795bec737b9("ovn-controller: Cache logical flow expr tree for each
> > lflow.")
> >
> > Fixes: 8795bec737b9("ovn-controller: Cache logical flow expr tree for each
> > lflow.")
> > Signed-off-by: Numan Siddique <[email protected]>
>
> Hi Numan,
>
> Looks good to me. I also gave it a run on our scale testing setup and I
> don't see the memleak anymore.
>
> Acked-by: Dumitru Ceara <[email protected]>
Thanks Dumitru for the review and testing it out.
I applied this to master and branch-20.03.
Thanks
Numan
>
> Thanks,
> Dumitru
>
> > ---
> > controller/lflow.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index 780aa9331..79d89131a 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -402,6 +402,11 @@ lflow_handle_changed_flows(struct lflow_ctx_in
> > *l_ctx_in,
> > /* Delete entries from lflow resource reference. */
> > lflow_resource_destroy_lflow(l_ctx_out->lfrr,
> > &lflow->header_.uuid);
> > + struct lflow_expr *le =
> > + lflow_expr_get(l_ctx_out->lflow_expr_cache, lflow);
> > + if (le) {
> > + lflow_expr_delete(l_ctx_out->lflow_expr_cache, le);
> > + }
> > }
> > }
> >
> > @@ -660,6 +665,8 @@ consider_logical_flow(const struct sbrec_logical_flow
> > *lflow,
> > expr = expr_normalize(expr);
> >
> > lflow_expr_add(l_ctx_out->lflow_expr_cache, lflow, expr);
> > + } else {
> > + expr_destroy(prereqs);
> > }
> >
> > struct condition_aux cond_aux = {
> > @@ -910,6 +917,21 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct
> > lflow_ctx_out *l_ctx_out)
> > {
> > COVERAGE_INC(lflow_run);
> >
> > + /* when lflow_run is called, it's possible that some of the logical
> > flows
> > + * are deleted. We need to delete the lflow expr cache for these
> > lflows,
> > + * otherwise, they will not be deleted at all. */
> > + const struct sbrec_logical_flow *lflow;
> > + SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow,
> > +
> > l_ctx_in->logical_flow_table) {
> > + if (sbrec_logical_flow_is_deleted(lflow)) {
> > + struct lflow_expr *le =
> > + lflow_expr_get(l_ctx_out->lflow_expr_cache, lflow);
> > + if (le) {
> > + lflow_expr_delete(l_ctx_out->lflow_expr_cache, le);
> > + }
> > + }
> > + }
> > +
> > add_logical_flows(l_ctx_in, l_ctx_out);
> > add_neighbor_flows(l_ctx_in->sbrec_port_binding_by_name,
> > l_ctx_in->mac_binding_table, l_ctx_out->flow_table);
> >
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
