On Wed, Jun 17, 2026 at 11:00 AM Dumitru Ceara <[email protected]> wrote:
> Hi Ales,
>
> I was chatting with Xavier this morning about this.
>
Hi Dumitru.
>
> On 6/16/26 7:42 PM, Ales Musil wrote:
> >>> /* Table of ipv6_ra_state structures, keyed on logical port name.
> >>> @@ -4724,14 +4722,15 @@ pinctrl_wait(struct ovsdb_idl_txn
> *ovnsb_idl_txn)
> >>> {
> >>> ovs_mutex_lock(&pinctrl_mutex);
> >>> if (ovnsb_idl_txn) {
> >> Not related to this pach, but why did we ever need to do the check only
> >> if ovnsb_idl_txn?
> >>
> >> I think https://github.com/ovn-org/ovn/commit/d305d9f shouldn't have
> >> needed to add it at all, right?
> >>
> > Yeah I think that was a mistake.
> >
>
> Looking closer at the code, it wasn't a mistake.
>
> It actually prevents busy loops in ovn-controller if the SB is slow(er)
> and we have longer periods in ovn-controller with a transaction in
> flight _and_ also pending controller events and vport bindings.
>
> If we remove the txn check, we'd busy loop in ovn-controller until the
> SB commits the currently in-flight transaction.
>
> >
> >>> - wait_put_mac_bindings();
> >>> wait_controller_event();
> >>> wait_put_vport_bindings();
> >>> - wait_put_fdbs();
> >>> seq_wait(pinctrl_main_seq, main_seq);
> >>> }
> >>> wait_activated_ports();
>
> This one, wait_activated_ports(), is probably OK as we don't seem to
> need SB access for updating the state of these ports.
>
> >>> ovs_mutex_unlock(&pinctrl_mutex);
> >>> +
> >>> + wait_put_mac_bindings();
> >>> + wait_put_fdbs();
>
> But this means we should also guard these two with an if (ovnsb_idl_txn).
>
> What do you think?
>
Yeah that makes sense, I'll send a patch in a bit.
> >>> }
>
> Regards,
> Dumitru
>
>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
