From: Numan Siddique <num...@ovn.org>

When the config ovn-monitor-all is set to True, then ovn-controller is not
programming the flows completely when it binds the logical port of
a logical switch until a full recompute is triggered.

This issue was introduced by the commit in Fixes - which added incremental 
procesing
for runtime data changes.

This patch fixes this issue.

Fixes: 3d096f833c4a("ovn-controller: Handle runtime data changes in flow output 
engine")

Reported-by: Dumitru Ceara <dce...@redhat.com>
Signed-off-by: Numan Siddique <num...@ovn.org>
---
 controller/binding.c |  17 ++++++--
 tests/ovn.at         | 101 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index b2c388146d..8f6d68006e 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -103,9 +103,20 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
     ld->localnet_port = NULL;
     ld->has_local_l3gateway = has_local_l3gateway;
 
-    if (tracked_datapaths &&
-        !tracked_binding_datapath_find(tracked_datapaths, datapath)) {
-        tracked_binding_datapath_create(datapath, true, tracked_datapaths);
+    if (tracked_datapaths) {
+        struct tracked_binding_datapath *tdp =
+            tracked_binding_datapath_find(tracked_datapaths, datapath);
+        if (!tdp) {
+            tracked_binding_datapath_create(datapath, true, tracked_datapaths);
+        } else {
+            /* Its possible that there is already an entry in tracked datapaths
+             * for this 'datapath'. tracked_binding_datapath_lport_add() may
+             * have created it. Since the 'datapath' is added to the
+             * local datapaths, set the tdp->is_new to true so that the flows
+             * for this datapath are programmed properly.
+             * */
+            tdp->is_new = true;
+        }
     }
 
     if (depth >= 100) {
diff --git a/tests/ovn.at b/tests/ovn.at
index 24d93bc245..ba1a534e92 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -20601,3 +20601,104 @@ OVS_WAIT_UNTIL([
 
 OVN_CLEANUP([hv1], [hv2])
 AT_CLEANUP
+
+AT_SETUP([ovn -- controller I-P handling with monitoring disabled])
+AT_KEYWORDS([lb])
+ovn_start
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+
+ovn-nbctl ls-add sw0
+
+ovn-nbctl lsp-add sw0 sw0-p1
+ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
+ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3"
+
+ovn-nbctl lsp-add sw0 sw0-p2
+ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4"
+ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4"
+
+as hv1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+    set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
+    ofport-request=1
+
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1) = xup])
+
+# Get the number of OF flows in hv1 and hv2
+hv1_offlows=$(as hv1 ovs-ofctl dump-flows br-int | wc -l)
+echo "hv1 flows : $hv1_offlows"
+AT_CHECK([test $hv1_offlows -gt 0])
+
+as hv2
+ovs-vsctl -- add-port br-int hv2-vif1 -- \
+    set interface hv2-vif1 external-ids:iface-id=sw0-p2 \
+    options:tx_pcap=hv2/vif1-tx.pcap \
+    options:rxq_pcap=hv2/vif1-rx.pcap \
+    ofport-request=1
+
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2) = xup])
+
+hv2_offlows=$(as hv2 ovs-ofctl dump-flows br-int | wc -l)
+echo "hv2 flows : $hv2_offlows"
+AT_CHECK([test $hv2_offlows -gt 0])
+
+ovn-nbctl ls-del sw0
+as hv1 ovs-vsctl del-port hv1-vif1
+as hv2 ovs-vsctl del-port hv2-vif1
+
+as hv1 ovs-vsctl set open . external_ids:ovn-monitor-all=true
+as hv2 ovs-vsctl set open . external_ids:ovn-monitor-all=true
+
+ovn-nbctl ls-add sw0
+
+ovn-nbctl lsp-add sw0 sw0-p1
+ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
+ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3"
+
+ovn-nbctl lsp-add sw0 sw0-p2
+ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4"
+ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4"
+
+as hv1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+    set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
+    ofport-request=1
+
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1) = xup])
+
+# Get the number of OF flows in hv1 and hv2
+hv1_offlows_mon=$(as hv1 ovs-ofctl dump-flows br-int | wc -l)
+echo "hv1 flows after monitor-all=true : $hv1_offlows"
+AT_CHECK([test "$hv1_offlows" = "$hv1_offlows_mon"])
+
+as hv2
+ovs-vsctl -- add-port br-int hv2-vif1 -- \
+    set interface hv2-vif1 external-ids:iface-id=sw0-p2 \
+    options:tx_pcap=hv2/vif1-tx.pcap \
+    options:rxq_pcap=hv2/vif1-rx.pcap \
+    ofport-request=1
+
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2) = xup])
+
+hv2_offlows_mon=$(as hv2 ovs-ofctl dump-flows br-int | wc -l)
+echo "hv2 flows after monitor-all=true : $hv2_offlows"
+AT_CHECK([test "$hv2_offlows" = "$hv2_offlows_mon"])
+
+OVN_CLEANUP([hv1], [hv2])
+AT_CLEANUP
-- 
2.26.2

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to