On Thu, Nov 20, 2025 at 9:04 AM Xavier Simonart via dev <
[email protected]> wrote:

> seq_read/seq_wait should be used before processing any changes.
> Before this patch, if pinctrl thread tried to notify main thread
> while ovn-controller already run  pinctrl_run but not yet pinctrl_wait,
> the wake up was lost.
>
> Signed-off-by: Xavier Simonart <[email protected]>
> ---
>

Hi Xavier,

thank you for the patch. I have one question/comment down below.


>  controller/pinctrl.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 84a1375b2..7fc111b24 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -4073,6 +4073,13 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>              int64_t cur_cfg)
>  {
>      ovs_mutex_lock(&pinctrl_mutex);
> +
> +    /* Do not wake up if no txn available. We will wake up on sb update.*/
> +    if (ovnsb_idl_txn) {
> +        int64_t new_seq = seq_read(pinctrl_main_seq);
>

If I understand that correctly we only need to read here before
processing any updates, the wait can still be in the pinctrl_wait.
It would make more sense logically if we would do the seq_wait()
in pinctrl_wait() WDYT?

Also it seems that statctrl has the same problem. And for whatever
reason we are using int64_t, but seq_read is returning uint64_t.
So we should probably fix that too while at it.

+        seq_wait(pinctrl_main_seq, new_seq);
> +    }
> +
>      run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
>                           sbrec_port_binding_by_key,
>                           sbrec_mac_binding_by_lport_ip);
> @@ -4593,8 +4600,6 @@ pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn)
>      wait_put_mac_bindings(ovnsb_idl_txn);
>      wait_controller_event(ovnsb_idl_txn);
>      wait_put_vport_bindings(ovnsb_idl_txn);
> -    int64_t new_seq = seq_read(pinctrl_main_seq);
> -    seq_wait(pinctrl_main_seq, new_seq);
>      wait_put_fdbs(ovnsb_idl_txn);
>      wait_activated_ports();
>      ovs_mutex_unlock(&pinctrl_mutex);
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to