[ovs-dev] [PATCH ovn] ovn-controller: Fix incremental processing of Port_Binding deletes.

[Dumitru Ceara]

On 8/31/20 1:24 PM, Numan Siddique wrote:
> 
> 
> On Mon, Aug 31, 2020 at 3:55 PM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
> 
>     If a Port_Binding is deleted from the Southbound DB and the
>     corresponding OVS interface is also deleted from the OVS DB, and
>     if both updates are received and processed by ovn-controller in
>     the same iteration, ovn-controller should not be allowed to write
>     to the (to be deleted) Port_Binding record stored in memory.
> 
>     This commit also adds three new unixctl debug commands for
>     ovn-controller:
>     - debug/pause: pause ovn-controller processing, except unixctl commands.
>     - debug/resume: resume ovn-controller processing.
>     - debug/status: return the status of the ovn-controller processing.
> 
>     These new commands are needed by the test for this scenario as without
>     them we have no way of ensuring predictable results. Users should not
>     use these commands in production. This is also why the commands are not
>     documented.
> 
>     CC: Numan Siddique <numans at ovn.org <mailto:numans at ovn.org>>
>     Fixes: 6b0f01116bab ("ovn-controller: Handle runtime data changes in
>     flow output engine")
>     Reported-by: Tim Rozet <trozet at redhat.com <mailto:trozet at redhat.com>>
>     Reported-at: https://bugzilla.redhat.com/1871961
>     Signed-off-by: Dumitru Ceara <dceara at redhat.com
>     <mailto:dceara at redhat.com>>
>     ---
> 
> 
> Thanks Dumitru for the fix. 
> 
> The issue is seen because we handle OVS interface changes first and then
> the port binding changes.
> I think we can address this issue with lesser changes to the code by
> handling port binding changes
> first in the runtime data engine.
> 
> I think the I-P engine respects the order of inputs defined on an engine
> node right ? If the engine guarantees
> this, then I think we can take this approach.
> 
> What do you think ?
> 
> Thanks
> Numan
>  

Yes, that works and is indeed a way smaller change. The new test case
will fail if the order of the incremental engine nodes is changed back.

I'll also add a comment in ovn-controller.c to mention why order is
relevant.

I'll send a v2 soon.

Regards,
Dumitru