[ovs-dev] [PATCH ovn] ovn-controller: Always run the I-P OVS Interface change handler.

Dumitru Ceara dceara at redhat.com
Thu Dec 17 13:56:48 UTC 2020


On 12/17/20 2:30 PM, Numan Siddique wrote:
> On Thu, Dec 17, 2020 at 3:47 PM Dumitru Ceara <dceara at redhat.com> wrote:
>>
>> On 12/17/20 10:23 AM, Dumitru Ceara wrote:
>>> The incremental processing engine implementation is such that if an
>>> input node gets updated but the output node doesn't have a change
>>> handler for it then the output node is immediately recomputed.  That is,
>>> no other input change handlers are executed.
>>>
>>> In case of the en_physical_flow_changes node, if a ct-zone changes we
>>> were also skipping the OVS interface change handler.  That is incorrect
>>> as there is an implicit requirement that flows for OVS interfaces that
>>> got deleted should be cleared before physical_run() is called.
>>>
>>> Instead, we now add an explicit change handler for ct-zone changes.
>>> This change handler never processes ct-zone updates incrementally but
>>> ensures that the OVS interface change handler is also called.
>>>
>>> Moreover, OVS interface changes (including deletes) are now processed
>>> before physical_run() is called in the flow_output
>>> physical_flow_changes_handler.  This is a requirement because otherwise
>>> physical_run() will fail to add flows for new OVS interfaces that
>>> correspond to unchanged Port_Bindings.
>>>
>>> Reported-by: Daniel Alvarez <dalvarez at redhat.com>
>>
>> Reported-by: Krzysztof Klimonda <kklimonda at syntaxhighlighted.com>
>>
>> Sorry, I forgot to credit Chris for the report too.  I can add the above
>> in a v2 if needed but I'll wait for some reviews before that.
> 
> Thanks for the fix.
> 
> Acked-by: Numan Siddique <numans at ovn.org>
> 

Thanks for the review!

> I think if we were maintaining a separate flow table for physical
> flows, we could
> have cleared that flow table before calling physical_run.
> 

This might work too indeed.  Also, I think the incremental processing
engine could also be improved to avoid the ambiguity between NULL change
handler and a change handler that always return false.

In either case, I think it's better to make such intrusive changes only
after we release 20.12.

> Thanks
> Numan
> 

Regards,
Dumitru



More information about the dev mailing list