On Mon, Aug 25, 2025 at 12:02 PM Xavier Simonart via dev <
ovs-dev@openvswitch.org> wrote:
> next_cfg updates are used as a way to know if transaction with sb got
> poperly committed.
> Before this patch, next_cfg was only updated if
> ovsdb_idl_loop_commit_and_wait returned
> TXN_SUCCESS, i.e. if there was not new transaction in fly.
>
> We might have had the following case:
> - Loop 1: - A transaction is sent; ovsdb_idl_loop_commit_and_wait return
> -1 (in_progress).
> - Loop 2: - The transaction completes (in ovsdb_idl_loop_run), and a new
> transaction is sent.
> - ovsdb_idl_loop_commit_and_wait return -1 (new txn is
> in_progress).
> - next_cfg was not updated despite the fact that the first
> transaction succeeded.
>
> Note that next_cfg is (still) updated when there is no commit in fly, and
> no transaction occurs
> in the loop.
>
> Fixes: a6b0007918f4 ("controller: Remove only commited virtual port
> binding requests.")
> Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
> Signed-off-by: Ales Musil <amu...@redhat.com>
> ---
> controller/ovn-controller.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index f345e5b57..9f50fda08 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -4910,6 +4910,16 @@ br_int_remote_update(struct br_int_remote *remote,
> remote->probe_interval = MIN(probe_interval / 1000, INT_MAX);
> }
>
> +static void
> +ovsdb_idl_loop_next_cfg_inc(struct ovsdb_idl_loop *idl_loop)
> +{
> + if (idl_loop->next_cfg == INT64_MAX) {
> + idl_loop->next_cfg = 0;
> + } else {
> + idl_loop->next_cfg++;
> + }
> +}
> +
> int
> main(int argc, char *argv[])
> {
> @@ -5471,6 +5481,7 @@ main(int argc, char *argv[])
> VLOG_INFO("OVN internal version is : [%s]", ovn_version);
>
> /* Main loop. */
> + int ovnsb_txn_status = 1;
> bool sb_monitor_all = false;
> while (!exit_args.exiting) {
> ovsrcu_quiesce_end();
> @@ -5520,6 +5531,9 @@ main(int argc, char *argv[])
> = ovsdb_idl_loop_run(&ovnsb_idl_loop);
> unsigned int new_ovnsb_cond_seqno
> = ovsdb_idl_get_condition_seqno(ovnsb_idl_loop.idl);
> + if (ovnsb_idl_txn && ovnsb_txn_status == -1) {
> + ovsdb_idl_loop_next_cfg_inc(&ovnsb_idl_loop);
> + }
> if (new_ovnsb_cond_seqno != ovnsb_cond_seqno) {
> if (!new_ovnsb_cond_seqno) {
> VLOG_INFO("OVNSB IDL reconnected, force recompute.");
> @@ -5925,16 +5939,12 @@ main(int argc, char *argv[])
> poll_immediate_wake();
> }
>
> - int ovnsb_txn_status =
> ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
> + ovnsb_txn_status =
> ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
> if (!ovnsb_txn_status) {
> VLOG_INFO("OVNSB commit failed, force recompute next time.");
> engine_set_force_recompute_immediate();
> } else if (ovnsb_txn_status == 1) {
> - if (ovnsb_idl_loop.next_cfg == INT64_MAX) {
> - ovnsb_idl_loop.next_cfg = 0;
> - } else {
> - ovnsb_idl_loop.next_cfg++;
> - }
> + ovsdb_idl_loop_next_cfg_inc(&ovnsb_idl_loop);
> } else if (ovnsb_txn_status == -1) {
> /* The commit is still in progress */
> } else {
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thank you Xavier and Dumitru,
I went ahead and merged both patches into 24.09.
Regards,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
