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

Han Zhou hzhou at ovn.org
Fri Oct 2 20:05:04 UTC 2020

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

However, currently in the second loop, it blindly link the desired flows
to the installed flows, without checking if it is already linked. 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 also be the one
traversed first in the second loop. After the first loop, each installed
flow will be linked to one and only one desired flow, and in the second
loop the same ones will be readded to the list - readding the only element
in the list becomes a no-op.

However, this is still a bug and this patch fixes it to avoid potential
problems and confusion in the code.

Signed-off-by: Han Zhou <hzhou at ovn.org>
 controller/ofctrl.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 81a00c844..3276b45b0 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1687,8 +1687,12 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table,
             /* Copy 'd' from 'flow_table' to installed_flows. */
             i = installed_flow_dup(d);
             hmap_insert(&installed_flows, &i->match_hmap_node, i->flow.hash);
+            link_installed_to_desired(i, d);
+        } else if (i->desired_flow != d) {
+            /* This is a desired_flow that conflicts with one installed
+             * previously. */
+            link_installed_to_desired(i, d);
-        link_installed_to_desired(i, d);

More information about the dev mailing list