Hi Jacob, Thanks for your review. Em ter., 11 de ago. de 2026 às 17:33, Jacob Tanenbaum <[email protected]> escreveu:
> > > On Fri, Jul 24, 2026 at 12:41 PM Lucas Vargas Dias > <[email protected]> wrote: > >> The datapath-group bitmap stored in a lflow_ref_node records the set of >> datapaths (and the total datapath count, hence the bitmap length) that >> the ref contributed to a shared logical flow, so that unlinking the ref >> later releases exactly the right per-datapath refcounts. >> >> lflow_table_add_lflow__() asserted that, when re-linking such a ref node, >> the datapath-group bitmap length matched the current one. This held as >> long as the number of datapaths of a given type stayed constant between >> unlink and re-link. With incremental processing of datapath >> creation/deletion this is no longer true: adding (or removing) a >> datapath changes sparse_array_len() and therefore the bitmap length used >> by dp-group flows (e.g. shared load balancer flows), which made northd >> abort with: >> >> lflow-mgr.c: assertion lrn->dpgrp_bitmap_len == dp_bitmap_len failed >> > > Could you add a test case that exercises this? > Yes, I can. > >> Re-clone the recorded bitmap at the current length when it changed, so a >> subsequent unlink releases exactly the datapaths this ref acquired. >> >> Fixes: 3166f87b3991 ("northd: Fix lflow ref node's reference counting.") >> Assisted-By: Claude Opus 4.8, Claude Code >> Signed-off-by: Lucas Vargas Dias <[email protected]> >> --- >> northd/lflow-mgr.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c >> index ce9c4f854..4680538df 100644 >> --- a/northd/lflow-mgr.c >> +++ b/northd/lflow-mgr.c >> @@ -785,7 +785,17 @@ lflow_table_add_lflow__(struct lflow_table >> *lflow_table, >> >> if (!lrn->linked) { >> if (lrn->dpgrp_lflow) { >> - ovs_assert(lrn->dpgrp_bitmap_len == dp_bitmap_len); >> + /* The total number of datapaths of this type may have >> changed >> + * (e.g. a datapath was added or removed via incremental >> + * processing) since this ref node was created, changing >> the >> + * datapath-group bitmap length. Re-clone the recorded >> bitmap >> + * at the current length so that a subsequent unlink >> releases >> + * exactly the datapath refcounts this ref acquired >> here. */ >> + if (lrn->dpgrp_bitmap_len != dp_bitmap_len) { >> + bitmap_free(lrn->dpgrp_bitmap); >> + lrn->dpgrp_bitmap = bitmap_clone(dp_bitmap, >> dp_bitmap_len); >> + lrn->dpgrp_bitmap_len = dp_bitmap_len; >> + } >> > > I think lrn->dpgrp_bitmap should be recloned unconditionally. If the > non-highest-indexed datapath is removed sparse_array() returns the last bit > set + 1 so the data would be changed without the changing the length. > You're right. I'll adjust in a new version. Regards, Lucas > >> size_t index; >> BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) { >> /* Allocate a reference counter only if already >> used. */ >> -- >> 2.43.0 >> >> >> -- >> >> >> >> >> _'Esta mensagem é direcionada apenas para os endereços constantes no >> cabeçalho inicial. Se você não está listado nos endereços constantes no >> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa >> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas >> estão >> imediatamente anuladas e proibidas'._ >> >> >> * **'Apesar do Magazine Luiza tomar >> todas as precauções razoáveis para assegurar que nenhum vírus esteja >> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade >> por >> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* >> >> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
