northd: fix missing port up when deleting and adding back an lsp

When a logical switch port was deleted and added back quickly, it could
happen that the lsp was never reported up

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 0608e70)

northd/northd.c

@@ -5181,23 +5181,21 @@ lsp_can_be_inc_processed(const struct nbrec_logical_switch_port *nbsp)
 }
 
 static bool
-ls_port_has_changed(const struct nbrec_logical_switch_port *old,
-                    const struct nbrec_logical_switch_port *new)
+ls_port_has_changed(const struct nbrec_logical_switch_port *new)
 {
-    if (old != new) {
-        return true;
-    }
     /* XXX: Need a better OVSDB IDL interface for this check. */
     return (nbrec_logical_switch_port_row_get_seqno(new,
                                 OVSDB_IDL_CHANGE_MODIFY) > 0);
 }
 
 static struct ovn_port *
-ovn_port_find_in_datapath(struct ovn_datapath *od, const char *name)
+ovn_port_find_in_datapath(struct ovn_datapath *od,
+                          const struct nbrec_logical_switch_port *nbsp)
 {
     struct ovn_port *op;
-    HMAP_FOR_EACH_WITH_HASH (op, dp_node, hash_string(name, 0), &od->ports) {
-        if (!strcmp(op->key, name)) {
+    HMAP_FOR_EACH_WITH_HASH (op, dp_node, hash_string(nbsp->name, 0),
+                             &od->ports) {
+        if (!strcmp(op->key, nbsp->name) && op->nbsp == nbsp) {
             return op;
         }
     }
@@ -5382,7 +5380,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
     /* Compare the individual ports in the old and new Logical Switches */
     for (size_t j = 0; j < changed_ls->n_ports; ++j) {
         struct nbrec_logical_switch_port *new_nbsp = changed_ls->ports[j];
-        op = ovn_port_find_in_datapath(od, new_nbsp->name);
+        op = ovn_port_find_in_datapath(od, new_nbsp);
 
         if (!op) {
             if (!lsp_can_be_inc_processed(new_nbsp)) {
@@ -5399,7 +5397,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
             }
             ovs_list_push_back(&ls_change->added_ports,
                                 &op->list);
-        } else if (ls_port_has_changed(op->nbsp, new_nbsp)) {
+        } else if (ls_port_has_changed(new_nbsp)) {
             /* Existing port updated */
             bool temp = false;
             if (lsp_is_type_changed(op->sb, new_nbsp, &temp) ||
@@ -5672,9 +5670,9 @@ northd_handle_sb_port_binding_changes(
              * notification of that transaction, and we can ignore in this
              * case. Fallback to recompute otherwise, to avoid dangling
              * sb idl pointers and other unexpected behavior. */
-            if (op) {
-                VLOG_WARN_RL(&rl, "A port-binding for %s is deleted but the "
-                            "LSP still exists.", pb->logical_port);
+            if (op && op->sb == pb) {
+                VLOG_WARN_RL(&rl, "A port-binding for %s is deleted but "
+                            "the LSP still exists.", pb->logical_port);
                 return false;
             }
         } else {

tests/ovn.at

@@ -37232,3 +37232,48 @@ check_flow_count hv2 12
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([port up with slow northd])
+ovn_start
+
+sleep_northd() {
+  echo northd going to sleep
+  AT_CHECK([kill -STOP $(cat northd/ovn-northd.pid)])
+}
+
+wake_up_northd() {
+  echo northd going to sleep
+  AT_CHECK([kill -CONT $(cat northd/ovn-northd.pid)])
+}
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.11
+
+check ovn-nbctl --wait=hv ls-add ls0
+# Create a pilot port and wait it up to make sure we are ready for the real
+# tests, so that the counters measured are accurate.
+check ovn-nbctl --wait=hv lsp-add ls0 lsp-pilot -- lsp-set-addresses lsp-pilot "unknown"
+ovs-vsctl add-port br-int lsp-pilot -- set interface lsp-pilot external_ids:iface-id=lsp-pilot
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+check ovn-nbctl --wait=hv lsp-add ls0 lsp0-2 -- lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:02 192.168.0.12"
+ovs-vsctl add-port br-int lsp0-2 -- set interface lsp0-2 external_ids:iface-id=lsp0-2
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+sleep_northd
+check ovn-nbctl lsp-del lsp0-2
+check ovn-nbctl lsp-add ls0 lsp0-2 -- lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:02 192.168.0.12"
+wake_up_northd
+
+check ovn-nbctl --wait=sb sync
+wait_for_ports_up
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])