[ovs-dev] [PATCH ovn v2] ovn-controller: Fix nb_cfg update with monitor_cond_change in flight.

Dumitru Ceara dceara at redhat.com
Thu Nov 19 10:50:57 UTC 2020


On 11/19/20 5:15 AM, Ben Pfaff wrote:
> On Tue, Nov 17, 2020 at 05:50:20PM +0100, Dumitru Ceara wrote:
>> It is not correct for ovn-controller to pass the current SB_Global.nb_cfg
>> value to ofctrl_put() if there are pending changes to conditional
>> monitoring clauses (local or in flight).  It might be that after the
>> monitor condition is acked by the SB, records that were added to the SB
>> before SB_Global.nb_cfg was set are now sent as updates to
>> ovn-controller.  These should be first installed in OVS before
>> ovn-controller reports that it caught up with the current
>> SB_Global.nb_cfg value.
>>
>> Also, ofctrl_put should not advance cur_cfg if there are flow updates in
>> flight.
>>
>> Fixes: ca278d98a4f5 ("ovn-controller: Initial use of incremental engine - quiet mode.")
>> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
>>
>> ---
>> v2:
>> - use new IDL *set_condition() return value.
>> - fix ofctrl_put to not advance cur_cfg if there are flow updates in
>>   flight.
> 
> ovn-controller has changed (advanced?) to the point that I have trouble

I would say "became more complex"..

> understanding the code now.  I'm going to assume that you understand it
> pretty well, but please allow me to ask a question here.  Will this

I'm doing my best but there are so many dependencies and side effects in
the code..

> always manage to come up to date with some version of the sb, or is
> there a chance that it never will report a consistent value if the sb
> keeps changing quickly?  I wasn't able to figure that out with a quick
> look.
> 

Now, back to your question:

TLDR: If ovn-controller has to continuously change its monitor
conditions I *think* there is a chance that it will never report that is
caught up with a specific SB seqno.  However, I tried various scenarios
(with quick SB updates or claiming/releasing OVS interfaces in quick
succession) and I didn't manage to make "ovn-nbctl --wait=hv sync" block
indefinitely.

Moreover, I think the question is if it's correct for ovn-controller to
report it has caught up if there are unseen SB flows that correspond to
its currently requested but not acked monitor condition?

Long version:

The main processing loop in ovn-controller looks something like this:

a. In the beginning of the loop monitor_cond_change requests (due to the
previous iteration) are sent when ovsdb_idl_loop_run(&ovnsb_idl_loop) is
executed.  monitor_cond_change replies for older requests are also
handled here.

b. Local datapaths (along with a lot of other stuff) are updated inside
the incremental processing engine in engine_run().

c. ovn-controller requests OVS to update flows using as barrier seqno
the last nb_cfg value read when no monitor_cond_change was pending.

d. Then monitor conditions are changed (if local datapaths have changed)
in update_sb_monitors().

e. Get the last seqno confirmed by OVS (ofctrl_get_cur_cfg()) and store
it in chassis_private.nb_cfg.

If in every iteration at step "b" above local datapaths change (due to
SB changes or OVS interfaces being claimed/released) then in the next
iteration we'll be requesting a cond change at step "a".

If at some point at step "a" SB_Global.nb_cfg gets updated, this won't
get propagated to ofctrl in step "c" until monitor conditions stop
changing for at least one iteration.

However, in practice:
- If SB changes cause monitor_cond_change that means port_bindings are
being added/deleted/updated but that can happen, I *think*, only with
ovn-northd intervention but ovn-northd should be already waiting for
ovn-controller to catch up "ovn-nbctl --wait=hv sync".
- If OVS ports are continuously claimed/released I think it's safe to
assume that ovn-controller is never really up to date with the contents
of the SB.

Another issue i see now is that steps "c" and "d" should probably be
swapped.  I can do that in a v3 if you're OK with the new semantics of
nb_cfg.

P.S.: Sorry for the long email.

Regards,
Dumitru



More information about the dev mailing list