binding: fixed ovn-installed not properly removed (recomputes)

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 <[email protected]> Signed-off-by: Mark Michelson <[email protected]> Acked-by: Ales Musil <[email protected]> Acked-by: Mark Michelson <[email protected]>
ovn-org · Jul 19, 2023 · 30952c2 · 30952c2
1 parent ca54c2f
commit 30952c2
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
 ])