On 9/8/26 1:47 PM, Dumitru Ceara wrote:
> On 9/7/26 10:06 AM, Felix Moebius via dev wrote:
>> ovn-controller currently delays the acknowledgement of pending flow
>> updates if it encounters further flow changes that are part of the same
>> req_cfg while waiting on the barrier response from vswitchd.
>> However, req_cfg will only be bumped when either nb_cfg changes or when
>> there are changes to ports on this chassis such as claiming or releasing
>> a port.
>>
>> This can delay the port installation indefinitely (or until the CMS
>> signals a timeout) if flows keep changing faster than vswitchd can ack
>> the flow installation barrier. This can happen when the CMS keeps making
>> changes to the logical topology that affect this chassis without bumping
>> nb_cfg or when there are constant changes to relevant BFD or MAC_Binding
>> entries. Neutron for example does not bump nb_cfg except for when
>> performing its agent health check.
>>
>> It does not make sense to delay the flow installation in terms of
>> correctness of the flows being installed initially for a particular port
>> as that behavior heavily depends on the timing of the original flow
>> installation and unrelated events such as plugging an additional port on
>> the chassis which will cause req_cfg to be bumped. Even without this,
>> flow updates are scoped to ovsdb transaction boundaries which is the
>> relevant property in terms of consistency.
>>
>> Signed-off-by: Felix Moebius <[email protected]>
>> Co-authored-by: Martin Morgenstern <[email protected]>
>> Signed-off-by: Martin Morgenstern <[email protected]>
>> ---
>
> Hi Felix, Martin,
>
Hi again,
> Thanks for the fix! Looks good to me.
>
> At least, I _think_ this is a bug fix, I re-read 84748d013155 ("ovn:
> Make it possible for CMS to detect when the OVN system is up-to-date.")
> multiple times and it seems wrong to me to potentially indefinitely
> delay the ack, there's a lot of scenarios in which ofctrl_put() gets
> called and "may put".
>
> I think it's quite hard to find a reliable test for this so I didn't ask
> for it.
>
> I applied it to main, 26.09, 26.03 and 25.09.
>
I just realized that we should've fixed the same issue in br-controller
too. I posted a patch for that now, it would be great if you could
review it:
https://mail.openvswitch.org/pipermail/ovs-dev/2026-September/435731.html
Thanks,
Dumitru
> Martin, I also added you to the AUTHORS.rst list, I think we might have
> forgotten to do that when we accepted previous patches of yours, sorry.
>
> Regards,
> Dumitru
>
>> controller/ofctrl.c | 11 -----------
>> 1 file changed, 11 deletions(-)
>>
>> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
>> index f3ca6613d..c90b20023 100644
>> --- a/controller/ofctrl.c
>> +++ b/controller/ofctrl.c
>> @@ -3061,16 +3061,6 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
>> mem_stats.oflow_update_usage -=
>> ofctrl_flow_update_size(fup);
>> ovs_list_remove(&fup->list_node);
>> free(fup);
>> - } else if (req_cfg == fup->req_cfg) {
>> - /* This ofctrl_flow_update is for the same configuration as
>> - * 'req_cfg'. Probably, some change to the physical
>> topology
>> - * means that we had to revise the OpenFlow flow table even
>> - * though the logical topology did not change. Update
>> fp->xid,
>> - * so that we don't send a notification that we're
>> up-to-date
>> - * until we're really caught up. */
>> - VLOG_DBG("advanced xid target for req_cfg=%"PRId64,
>> req_cfg);
>> - fup->xid = xid_;
>> - goto done;
>> } else {
>> break;
>> }
>> @@ -3082,7 +3072,6 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
>> fup->xid = xid_;
>> fup->req_cfg = req_cfg;
>> mem_stats.oflow_update_usage += ofctrl_flow_update_size(fup);
>> - done:;
>> } else if (!ovs_list_is_empty(&flow_updates)) {
>> /* Getting up-to-date with 'req_cfg' didn't require any extra flow
>> * table changes, so whenever we get up-to-date with the most recent
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
