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?
>
> 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.
> 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
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
