Thanks for this, Tim!

Acked-by: Mark Michelson <[email protected]>

I pushed this to main and branch-26.03.

On Thu, May 28, 2026 at 11:17 AM Tim Rozet via dev
<[email protected]> wrote:
>
> Datapath sync can delete one logical datapath and create another in the
> same engine run.  In that case the synced datapath layer may reuse the
> freed sparse-array index for the new datapath before northd has removed
> the old datapath from its local datapath array.
>
> The northd incremental LS/LR handlers process new datapaths before
> deleted datapaths.  If the reused index is still occupied locally,
> ods_assign_array_index() asserts when adding the new datapath.
>
> Detect this case before creating the new northd datapath and fall back
> to a full recompute instead.  The recompute path rebuilds the datapath
> arrays from the synced datapath state and avoids the ordering conflict.
>
> Add a regression test that deletes one logical switch/router and creates
> another in the same transaction.
>
> Fixes: 6b1b16469185 ("northd: Base ovn_datapath index on synced_datapath 
> index.")
> Assisted-by: OpenAI Codex
> Signed-off-by: Tim Rozet <[email protected]>
> ---
>  northd/northd.c     |  6 ++++++
>  tests/ovn-northd.at | 20 ++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 0ea7c1b95..edec652d4 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -5126,6 +5126,9 @@ northd_handle_ls_changes(struct ovsdb_idl_txn 
> *ovnsb_idl_txn,
>              goto fail;
>          }
>
> +        if (sparse_array_get(&nd->ls_datapaths.dps, synced->sdp->index)) {
> +            goto fail;
> +        }
>          struct ovn_datapath *od = ovn_datapath_create(
>              &nd->ls_datapaths.datapaths, &new_ls->header_.uuid, new_ls,
>              NULL, synced->sdp);
> @@ -5458,6 +5461,9 @@ northd_handle_lr_changes(const struct northd_input *ni,
>          if (new_lr->copp || (new_lr->n_ports > 0)) {
>              goto fail;
>          }
> +        if (sparse_array_get(&nd->lr_datapaths.dps, synced->sdp->index)) {
> +            goto fail;
> +        }
>          struct ovn_datapath *od = ovn_datapath_create(
>              &nd->lr_datapaths.datapaths, &new_lr->header_.uuid,
>              NULL, new_lr, synced->sdp);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 26a19bd96..a6e90fbaa 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -18770,6 +18770,26 @@ OVN_CLEANUP_NORTHD
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Datapath incremental processing index reuse])
> +ovn_start
> +
> +check ovn-nbctl --wait=sb ls-add sw-old
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb ls-del sw-old -- ls-add sw-new
> +check_engine_compute northd recompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +check ovn-nbctl --wait=sb lr-add lr-old
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lr-del lr-old -- lr-add lr-new
> +check_engine_compute northd recompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +OVN_CLEANUP_NORTHD
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD_NO_HV([
>  AT_SETUP([Synced logical switch and router incremental procesesing])
>  ovn_start
> --
> 2.53.0
>
> _______________________________________________
> 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

Reply via email to