From d17a9c36dfa71b04ebea82e4720279b35d42677f Mon Sep 17 00:00:00 2001
From: Dumitru Ceara <dceara@redhat.com>
Date: Thu, 18 Feb 2021 19:49:57 +0100
Subject: [PATCH 2/2] ofctrl: Do not link a desired flow twice.

Depending on the logical flow matches, multiple SB flows can point to
the same desired flow.  If it happens that the desired flow conflicts
with a more restrictive (already installed) flow, then we shouldn't try
to add the desired flow multiple times to the list maintained for the
installed flow.

Fixes: 6f0b1e02d9ab ("ofctrl: Incremental processing for flow installation by tracking.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Mark Gray <mark.d.gray@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>

(cherry-picked from upstream master commit 6975c649f932633042ca54df2d8f8f0eb866c344)

Change-Id: I0292d5116bad193fbf14487b82aa58b0f40fb142
---
 controller/ofctrl.c |  2 +-
 tests/ovn.at        | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 1c9694a63..415d9b7e1 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1989,7 +1989,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table,
                  * tracked, so it must have been modified. */
                 installed_flow_mod(&i->flow, &f->flow, msgs);
                 ovn_flow_log(&i->flow, "updating installed (tracked)");
-            } else {
+            } else if (!f->installed_flow) {
                 /* Adding a new flow that conflicts with an existing installed
                  * flow, so add it to the link.  If this flow becomes active,
                  * e.g., it is less restrictive than the previous active flow
diff --git a/tests/ovn.at b/tests/ovn.at
index 1f077cf88..e403fd0f4 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -13973,6 +13973,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \
  table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction()
 ])
 
+# Add another ACL that overlaps with the existing less restrictive ones.
+check ovn-nbctl acl-add ls1 to-lport 3 'udp || ((ip4.src==10.0.0.1 || ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4))' allow
+check ovn-nbctl --wait=hv sync
+
+# Check OVS flows, the same conjunctive flows as above should still be there,
+# with an additional conjunction action.
+#
+# New non-conjunctive flows should be added to match on 'udp'.
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \
+   grep "priority=1003" | \
+   sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl
+ table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
+ table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
+ table=45, priority=1003,conj_id=4,ip,metadata=0x1 actions=resubmit(,46)
+ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction(),conjunction()
+ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction(),conjunction()
+ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46)
+ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(),conjunction()
+ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction()
+ table=45, priority=1003,udp,metadata=0x1 actions=resubmit(,46)
+ table=45, priority=1003,udp6,metadata=0x1 actions=resubmit(,46)
+])
+
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 
-- 
2.29.2