Hi Ales,
I was chatting with Xavier this morning about this.
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?
>>> }
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
