[ovs-dev] [PATCH ovn v2] ofctrl.c: Avoid repeatedly linking an installed flow and a desired flow.

[Dumitru Ceara]

On 10/8/20 9:34 PM, Han Zhou wrote:
> In update_installed_flows_by_compare() there are two loops. The first
> loop iterates the installed flows and find its peer in desired flows to:
> 
> 1. uninstall flows that are not needed anymore
> 
> 2. update flows if needed
> 
> At the same time, it links the desired flow found for the installed flow
> which also set the desired flow as the current active installed flow.
> 
> The second loop iterates the desired flows and find its peer in installed
> flows to install missing flows. At the same time it will detect if there
> are conflict desired flows matching same installed flow then just link
> them.
> 
> However, currently in the second loop, it blindly link the desired flows to the
> installed flows, without checking if it is already linked in the first loop.
> Lucky enough, this won't cause any real problem so far, because when there are
> conflict flows, the one found in the first loop will be set as active in the
> installed_flow, and in the function link_installed_to_desired() checks if it is
> already the active desired flow it just does nothing but return.  However, the
> check in the link_installed_to_desired() is confusing because a desired_flow
> may be linked to the installed_flow already but not the active flow, and the
> check is insufficient. It should be rather an assertion and let the caller
> ensure that a pair of desired_flow and installed_flow is never linked twice.
> 
> For the above reason, this patch does the following changes:
> 
> 1. Removes the check in link_installed_to_desired() and convert it to an assert.
> 
> 2. Before calling link_installed_to_desired() in the above mentioned loop,
>    check if the desired flow is already installed.
> 
> Signed-off-by: Han Zhou <hzhou at ovn.org>

Acked-by: Dumitru Ceara <dceara at redhat.com>

Thanks,
Dumitru