northd: Fix duplicate logical port detection.

When reconciling SB Port_Bindings and NB Logical_Switch_Ports or
Logical_Router_Ports make sure we properly detect ports that are
incorrectly attached to multiple logical datapaths.

Reported-at: https://bugzilla.redhat.com/1918582
Reported-by: Jianlin Shi <jishi@redhat.com>
Fixes: 8bf9075 ("ovn-northd: Fix tunnel_key allocation for SB Port_Bindings.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>

(cherry-picked from master commit 7b404e6)

northd/ovn-northd.c

@@ -1231,17 +1231,38 @@ ovn_port_destroy(struct hmap *ports, struct ovn_port *port)
     }
 }
 
+/* Returns the ovn_port that matches 'name'.  If 'prefer_bound' is true and
+ * multiple ports share the same name, gives precendence to ports bound to
+ * an ovn_datapath.
+ */
 static struct ovn_port *
-ovn_port_find(const struct hmap *ports, const char *name)
+ovn_port_find__(const struct hmap *ports, const char *name,
+                bool prefer_bound)
 {
+    struct ovn_port *matched_op = NULL;
     struct ovn_port *op;
 
     HMAP_FOR_EACH_WITH_HASH (op, key_node, hash_string(name, 0), ports) {
         if (!strcmp(op->key, name)) {
-            return op;
+            matched_op = op;
+            if (!prefer_bound || op->od) {
+                return op;
+            }
         }
     }
-    return NULL;
+    return matched_op;
+}
+
+static struct ovn_port *
+ovn_port_find(const struct hmap *ports, const char *name)
+{
+    return ovn_port_find__(ports, name, false);
+}
+
+static struct ovn_port *
+ovn_port_find_bound(const struct hmap *ports, const char *name)
+{
+    return ovn_port_find__(ports, name, true);
 }
 
 static uint32_t
@@ -2024,15 +2045,13 @@ join_logical_ports(struct northd_context *ctx,
             for (size_t i = 0; i < od->nbs->n_ports; i++) {
                 const struct nbrec_logical_switch_port *nbsp
                     = od->nbs->ports[i];
-                struct ovn_port *op = ovn_port_find(ports, nbsp->name);
-                if (op && op->sb->datapath == od->sb) {
-                    if (op->nbsp || op->nbrp) {
-                        static struct vlog_rate_limit rl
-                            = VLOG_RATE_LIMIT_INIT(5, 1);
-                        VLOG_WARN_RL(&rl, "duplicate logical port %s",
-                                     nbsp->name);
-                        continue;
-                    }
+                struct ovn_port *op = ovn_port_find_bound(ports, nbsp->name);
+                if (op && (op->od || op->nbsp || op->nbrp)) {
+                    static struct vlog_rate_limit rl
+                        = VLOG_RATE_LIMIT_INIT(5, 1);
+                    VLOG_WARN_RL(&rl, "duplicate logical port %s", nbsp->name);
+                    continue;
+                } else if (op && (!op->sb || op->sb->datapath == od->sb)) {
                     ovn_port_set_nb(op, nbsp, NULL);
                     ovs_list_remove(&op->list);
 
@@ -2115,16 +2134,15 @@ join_logical_ports(struct northd_context *ctx,
                     continue;
                 }
 
-                struct ovn_port *op = ovn_port_find(ports, nbrp->name);
-                if (op && op->sb->datapath == od->sb) {
-                    if (op->nbsp || op->nbrp) {
-                        static struct vlog_rate_limit rl
-                            = VLOG_RATE_LIMIT_INIT(5, 1);
-                        VLOG_WARN_RL(&rl, "duplicate logical router port %s",
-                                     nbrp->name);
-                        destroy_lport_addresses(&lrp_networks);
-                        continue;
-                    }
+                struct ovn_port *op = ovn_port_find_bound(ports, nbrp->name);
+                if (op && (op->od || op->nbsp || op->nbrp)) {
+                    static struct vlog_rate_limit rl
+                        = VLOG_RATE_LIMIT_INIT(5, 1);
+                    VLOG_WARN_RL(&rl, "duplicate logical router port %s",
+                                 nbrp->name);
+                    destroy_lport_addresses(&lrp_networks);
+                    continue;
+                } else if (op && (!op->sb || op->sb->datapath == od->sb)) {
                     ovn_port_set_nb(op, NULL, nbrp);
                     ovs_list_remove(&op->list);
                     ovs_list_push_back(both, &op->list);
@@ -2169,7 +2187,7 @@ join_logical_ports(struct northd_context *ctx,
                     char *redirect_name =
                         ovn_chassis_redirect_name(nbrp->name);
                     struct ovn_port *crp = ovn_port_find(ports, redirect_name);
-                    if (crp && crp->sb->datapath == od->sb) {
+                    if (crp && crp->sb && crp->sb->datapath == od->sb) {
                         crp->derived = true;
                         ovn_port_set_nb(crp, NULL, nbrp);
                         ovs_list_remove(&crp->list);

tests/ovn-northd.at

@@ -1219,3 +1219,48 @@ AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0
 ])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- check LSP attached to multiple LS])
+ovn_start
+
+check ovn-nbctl ls-add ls1 \
+    -- ls-add ls2 \
+    -- lsp-add ls1 p1
+check ovn-nbctl --wait=sb sync
+
+uuid=$(fetch_column nb:Logical_Switch_Port _uuid name=p1)
+check ovn-nbctl set Logical_Switch ls2 ports=$uuid
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([grep -qE 'duplicate logical port p1' northd/ovn-northd.log], [0])
+
+AT_CLEANUP
+
+AT_SETUP([ovn -- check LRP attached to multiple LR])
+ovn_start
+
+check ovn-nbctl lr-add lr1 \
+    -- lr-add lr2 \
+    -- lrp-add lr1 p1 00:00:00:00:00:01 10.0.0.1/24
+check ovn-nbctl --wait=sb sync
+
+uuid=$(fetch_column nb:Logical_Router_Port _uuid name=p1)
+check ovn-nbctl set Logical_Router lr2 ports=$uuid
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([grep -qE 'duplicate logical router port p1' northd/ovn-northd.log], [0])
+
+AT_CLEANUP
+
+AT_SETUP([ovn -- check duplicate LSP/LRP])
+ovn_start
+
+check ovn-nbctl ls-add ls \
+    -- lsp-add ls p1 \
+    -- lr-add lr \
+    -- lrp-add lr p1 00:00:00:00:00:01 10.0.0.1/24
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([grep -qE 'duplicate logical.*port p1' northd/ovn-northd.log], [0])
+
+AT_CLEANUP