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,
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.
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
