diff mbox series

[ovs-dev,3/3] binding: fixed ovn-installed not properly removed (recomputes)

Message ID 20230717150641.2069008-4-xsimonar@redhat.com
State Accepted
Headers show
Series ovn-installed | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Xavier Simonart July 17, 2023, 3:06 p.m. UTC
If, while a port was claimed, a recompute is triggered at the same time
as the iface-id is removed, this was resulting in ovn-installed not
being properly removed.

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
---
 controller/binding.c |  8 ++++++
 tests/ovn-macros.at  |  9 +++++++
 tests/ovn.at         | 64 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+)
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 75319c4e7..66799fa4a 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2037,6 +2037,14 @@  build_local_bindings(struct binding_ctx_in *b_ctx_in,
                 update_local_lports(iface_id, b_ctx_out);
                 smap_replace(b_ctx_out->local_iface_ids, iface_rec->name,
                              iface_id);
+            } else if (smap_get_bool(&iface_rec->external_ids,
+                       OVN_INSTALLED_EXT_ID, false)) {
+                /* Interface should not be claimed (ovn_installed).
+                 * This can happen if iface-id was removed as we recompute.
+                 */
+                if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
+                                                   iface_rec->name,
+                                                   &iface_rec->header_.uuid);
             }
         }
     }
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 4898b0bc8..13d5dc3d4 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -873,6 +873,15 @@  wake_up_ovs() {
   AT_CHECK([kill -CONT $(cat $hv/ovs-vswitchd.pid)])
 }
 
+sleep_ovsdb() {
+  echo OVSDB $1 going to sleep
+  AT_CHECK([kill -STOP $(cat $1/ovsdb-server.pid)])
+}
+wake_up_ovsdb() {
+  echo OVSDB $1 waking up
+  AT_CHECK([kill -CONT $(cat $1/ovsdb-server.pid)])
+}
+
 OVS_END_SHELL_HELPERS
 
 m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])])
diff --git a/tests/ovn.at b/tests/ovn.at
index 11da1bd78..882a548db 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -36594,6 +36594,70 @@  wake_up_controller hv-1
 # Make sure ovn-controller is still OK
 ovn-nbctl --wait=hv sync
 OVS_WAIT_UNTIL([test $(as hv-1 ovs-vsctl list qos | grep -c linux-htb) -eq 1])
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-install and recomputes])
+AT_KEYWORDS([ovn-install])
+
+ovn_start
 
+check ovn-nbctl ls-add ls1
+check ovn-nbctl --wait=sb add Logical-Switch ls1 other_config vlan-passthru=true
+net_add n
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach n br-phys 192.168.0.1
+
+check ovn-nbctl --wait=hv sync
+
+check ovn-nbctl lsp-add ls1 lsp1
+check ovn-nbctl lsp-add ls1 lsp2
+as hv1 ovs-vsctl --no-wait -- add-port br-int vif1 \
+                    -- set Interface vif1 external_ids:iface-id=lsp1 \
+                    -- set Interface vif1 type=internal
+as hv1 ovs-vsctl --no-wait -- add-port br-int vif2 \
+                    -- set Interface vif2 external_ids:iface-id=lsp2 \
+                    -- set Interface vif2 type=internal
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+sleep_ovsdb hv1
+
+# Do following operations through one nbctl command
+# Otherwise, they would probably result in multiple sb updates to ovn-controller
+# As ovsdb is sleeping those multiple updates would resulti in requests to ovsdb being
+# postponed as ovsdb becomes ro.
+# Hence, patch port creation would be delayed after ovsdb becomes rw.
+check ovn-nbctl --wait=hv lsp-add ls1 ln1                  \
+                -- lsp-set-addresses ln1 unknown \
+                -- lsp-set-type ln1 localnet     \
+                -- lsp-set-options ln1 network_name=phys
+
+# Controller should now have created the patch port, but is not yet notified of the ofport of those interfaces
+sleep_controller hv1
+wake_up_ovsdb hv1
+check as hv1 ovs-vsctl remove Interface vif1 external_ids iface-id
+
+# Now wake up controller. It should receive patch port ofport and iface vif1 removal notification
+wake_up_controller hv1
+check as hv1 ovs-vsctl --no-wait -- del-port vif2
+
+# Check ovn-install has been removed.
+OVS_WAIT_UNTIL([test x`as hv1 ovs-vsctl get Interface vif2 external_ids:ovn-installed` = x])
+OVS_WAIT_UNTIL([test x`as hv1 ovs-vsctl get Interface vif1 external_ids:ovn-installed` = x])
+
+# Check ports are down
+OVS_WAIT_UNTIL([test `ovn-sbctl get Port_Binding lsp1 up` = 'false'])
+OVS_WAIT_UNTIL([test `ovn-sbctl get Port_Binding lsp2 up` = 'false'])
+
+# Check ports are unbound
+wait_column "" Port_Binding chassis logical_port=lsp1
+wait_column "" Port_Binding chassis logical_port=lsp2
+OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])