On Thu, Sep 17, 2020 at 5:45 AM Numan Siddique <[email protected]> wrote: > > > > On Thu, Sep 17, 2020 at 1:15 PM Dumitru Ceara <[email protected]> wrote: >> >> On 9/16/20 8:01 PM, Han Zhou wrote: >> > If a resource doesn't have any lflows referencing it any more, the >> > node ref_lflow_node in lflow_resource_ref.ref_lflow_table should >> > be removed and released. Otherwise, the table could keep growing >> > in some scenarios, until a recompute is triggered. Now that the >> > chance of triggering recompute is lower and there are more resources >> > references maintained (for type port-binding), this problem is >> > more likely to happen than before. This patch fixes the problem >> > by releasing the node as soon as it is not needed. >> > >> > Fixes: d2aa2c7cafe ("ovn-controller: Maintain resource references for logical flows.") >> > Signed-off-by: Han Zhou <[email protected]> >> > --- >> > controller/lflow.c | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/controller/lflow.c b/controller/lflow.c >> > index db078d2..b549067 100644 >> > --- a/controller/lflow.c >> > +++ b/controller/lflow.c >> > @@ -292,6 +292,10 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, >> > LIST_FOR_EACH_SAFE (lrln, next, list_node, &lfrn->lflow_ref_head) { >> > ovs_list_remove(&lrln->list_node); >> > hmap_remove(&lrln->rlfn->lflow_uuids, &lrln->hmap_node); >> >> I still think a short comment would be useful here, e.g.: >> >> /* Resources that are not referred by any logical flows would be >> * cleaned up in any case during a recompute so it's better to >> * remove them early. >> */ >> >> > + if (hmap_is_empty(&lrln->rlfn->lflow_uuids)) { >> > + hmap_remove(&lfrr->ref_lflow_table, &lrln->rlfn->node); >> > + ref_lflow_node_destroy(lrln->rlfn); >> > + } >> > free(lrln); >> > } >> > free(lfrn); >> > >> >> In any case, >> >> Acked-by: Dumitru Ceara <[email protected]> > > > Acked-by: Numan Siddique <[email protected]> >
Thank you both. I applied to master, branch 20.09 and 20.06, with a comment at the place suggested by Dumitru: + /* Clean up the node in ref_lflow_table if the resource is not + * referred by any logical flows. */ Just to explain a little bit here: I didn't add the exact statement from Dumitru because the reason why it is cleaned here is not because recompute will clean it anyway. Instead, it is cleaned because it is natural to clean it when it is not needed (the node was created on-demand, so should be released when not needed). A consequence of not doing it is that there is a chance the recompute does not happen before this table grows unreasonably large, which is bad. Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
