[ovs-dev] [PATCH ovn] ovn-controller: Fix wrong conj_id match flows when caching is enabled.

numans at ovn.org numans at ovn.org
Fri Jan 22 08:33:51 UTC 2021


From: Numan Siddique <numans at ovn.org>

When the below ACL is added -
ovn-nbctl acl-add ls1 to-lport 3
  '(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

ovn-controller installs the below OF flows

table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2)
table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2)
table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(2,2/2)
table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)

When a full recompute is triggered, ovn-controller deletes the last
OF flow with the match conj_id=2 and adds the below OF flow

table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)

For subsequent recomputes, the conj_id keeps increasing by 1.

This disrupts the traffic which matches on conjuction action flows.

This patch fixes this issue.

Fixes: 1213bc8270("ovn-controller: Cache logical flow expr matches.")
Signed-off-by: Numan Siddique <numans at ovn.org>
---
 controller/lflow.c |  7 +++++++
 tests/ovn.at       | 28 ++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/controller/lflow.c b/controller/lflow.c
index c02585b1e..a9420a7c4 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -754,6 +754,13 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
                                       &m->match, &conj, &lflow->header_.uuid);
             ofpbuf_uninit(&conj);
         }
+
+        if (m->match.wc.masks.conj_id) {
+            /* Reset the conj_id back to relative conj id. If caching is
+             * enabled, then processing of the expr match next time (due to
+             * full recompute) will result in the wrong conj_id match flow. */
+            m->match.flow.conj_id -= conj_id_ofs;
+        }
     }
 
     ofpbuf_uninit(&ofpacts);
diff --git a/tests/ovn.at b/tests/ovn.at
index 8f884241d..0e297d2f2 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -13884,6 +13884,34 @@ reset_pcap_file hv1-vif2 hv1/vif2
 rm -f 2.packets
 > 2.expected
 
+# Trigger recompute and make sure that the traffic still works as expected.
+as hv1 ovn-appctl -t ovn-controller recompute
+
+# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
+for src in `seq 1 2`; do
+    for dst in `seq 3 4`; do
+        sip=`ip_to_hex 10 0 0 $src`
+        dip=`ip_to_hex 10 0 0 $dst`
+
+        test_ip 1 f00000000001 f00000000002 $sip $dip 2
+    done
+done
+
+# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped.
+dip=`ip_to_hex 10 0 0 5`
+for src in `seq 1 2`; do
+    sip=`ip_to_hex 10 0 0 $src`
+
+    test_ip 1 f00000000001 f00000000002 $sip $dip
+done
+
+cat 2.expected > expout
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
+AT_CHECK([cat 2.packets], [0], [expout])
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 2.packets
+> 2.expected
+
 # Add two less restrictive allow ACLs for src IP 10.0.0.1.
 ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' allow
 ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow
-- 
2.29.2



More information about the dev mailing list