Message ID | 20240208214932.12783-1-numans@ovn.org |
---|---|
State | Accepted |
Headers | show |
Series | northd memory and CPU increase fix due to lflow-mgr. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On Thu, Feb 8, 2024 at 1:50 PM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > When the lflows in an lflow_ref are unlinked by calling > lflow_ref_unlink_lflows(lflow_ref), the dp_ref counter > for each lflow in the lflow_ref is decremented (by calling > dp_refcnt_release()), but it is not incremented later > when the same lflow is linked back to the lflow_ref. > > This patch fixes it. > > Fixes: a623606052ea ("northd: Refactor lflow management into a separate module.") > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > northd/lflow-mgr.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c > index 61729e9039..df62cd6ab4 100644 > --- a/northd/lflow-mgr.c > +++ b/northd/lflow-mgr.c > @@ -690,19 +690,24 @@ lflow_table_add_lflow(struct lflow_table *lflow_table, > if (lrn->dpgrp_lflow) { > lrn->dpgrp_bitmap = bitmap_clone(dp_bitmap, dp_bitmap_len); > lrn->dpgrp_bitmap_len = dp_bitmap_len; > + } else { > + lrn->dp_index = od->index; > + } > + ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); > + hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, hash); > + } > > + if (!lrn->linked) { > + if (lrn->dpgrp_lflow) { > + ovs_assert(lrn->dpgrp_bitmap_len == dp_bitmap_len); > size_t index; > BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) { > dp_refcnt_use(&lflow->dp_refcnts_map, index); > } > } else { > - lrn->dp_index = od->index; > dp_refcnt_use(&lflow->dp_refcnts_map, lrn->dp_index); > } > - ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); > - hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, hash); > } > - > lrn->linked = true; > } > > -- > 2.43.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks Numan for the fix. Acked-by: Han Zhou <hzhou@ovn.org>
On 2/13/24 08:14, Han Zhou wrote: > On Thu, Feb 8, 2024 at 1:50 PM <numans@ovn.org> wrote: >> >> From: Numan Siddique <numans@ovn.org> >> >> When the lflows in an lflow_ref are unlinked by calling >> lflow_ref_unlink_lflows(lflow_ref), the dp_ref counter >> for each lflow in the lflow_ref is decremented (by calling >> dp_refcnt_release()), but it is not incremented later >> when the same lflow is linked back to the lflow_ref. >> >> This patch fixes it. >> >> Fixes: a623606052ea ("northd: Refactor lflow management into a separate > module.") >> Signed-off-by: Numan Siddique <numans@ovn.org> >> --- >> northd/lflow-mgr.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c >> index 61729e9039..df62cd6ab4 100644 >> --- a/northd/lflow-mgr.c >> +++ b/northd/lflow-mgr.c >> @@ -690,19 +690,24 @@ lflow_table_add_lflow(struct lflow_table > *lflow_table, >> if (lrn->dpgrp_lflow) { >> lrn->dpgrp_bitmap = bitmap_clone(dp_bitmap, > dp_bitmap_len); >> lrn->dpgrp_bitmap_len = dp_bitmap_len; >> + } else { >> + lrn->dp_index = od->index; >> + } >> + ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); >> + hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, > hash); >> + } >> >> + if (!lrn->linked) { >> + if (lrn->dpgrp_lflow) { >> + ovs_assert(lrn->dpgrp_bitmap_len == dp_bitmap_len); >> size_t index; >> BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) { >> dp_refcnt_use(&lflow->dp_refcnts_map, index); >> } >> } else { >> - lrn->dp_index = od->index; >> dp_refcnt_use(&lflow->dp_refcnts_map, lrn->dp_index); >> } >> - ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); >> - hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, > hash); >> } >> - >> lrn->linked = true; >> } >> >> -- >> 2.43.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks Numan for the fix. > Acked-by: Han Zhou <hzhou@ovn.org> Looks good to me too: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c index 61729e9039..df62cd6ab4 100644 --- a/northd/lflow-mgr.c +++ b/northd/lflow-mgr.c @@ -690,19 +690,24 @@ lflow_table_add_lflow(struct lflow_table *lflow_table, if (lrn->dpgrp_lflow) { lrn->dpgrp_bitmap = bitmap_clone(dp_bitmap, dp_bitmap_len); lrn->dpgrp_bitmap_len = dp_bitmap_len; + } else { + lrn->dp_index = od->index; + } + ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); + hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, hash); + } + if (!lrn->linked) { + if (lrn->dpgrp_lflow) { + ovs_assert(lrn->dpgrp_bitmap_len == dp_bitmap_len); size_t index; BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) { dp_refcnt_use(&lflow->dp_refcnts_map, index); } } else { - lrn->dp_index = od->index; dp_refcnt_use(&lflow->dp_refcnts_map, lrn->dp_index); } - ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); - hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, hash); } - lrn->linked = true; }