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 > --- > 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
