Fix incorrect TCP connections.

bosilca · bosilca · commit f146bf944e61 · 2024-01-21T14:40:26.000-05:00
If nodes have the same IP addresses (for containers or other purposes) and
these addresses get published as part of the modex, a remote peer might try to
use one of the addresses to connect. As both nodes have the same IP, there are
several cases:
- the "remote" port is not used by an OMPI process locally, the connection is
  refused or it timeouts. This is the "nicest" outcome, as a new IP will be
  used resulting in a successful connection and the continuation of the
  application.
- the "remote" port is used by another OMPI process on the local node. A
  connection will be established but the incorrect guid will be exchanged
  leading to complaints, connection dropped and/or deadlocks.
- the "remote" port is used by this process, basically resulting in a
  connection-to-self. Bad things happen, as we don't support TCP connections to
  self. Some output messages are generated, but the outcome is most likely a
  deadlock.

Up to now, users were expected to exclude such interfaces from the accepted
interfaces, but this patch removes this need. If we discover a local IP as part
of the IP list of a remote peer, we drop it and never try to use it. This does
not apply to local processes, so we can still use these interfaces for node
level communications (which will work as we will connect to the correct port
according to the destination process).

Signed-off-by: George Bosilca &lt;bosilca@icl.utk.edu&gt;
diff --git a/opal/mca/btl/tcp/btl_tcp_component.c b/opal/mca/btl/tcp/btl_tcp_component.c
@@ -15,7 +15,7 @@
  * Copyright (c) 2009      Oak Ridge National Laboratory
  * Copyright (c) 2012-2015 Los Alamos National Security, LLC.  All rights
  *                         reserved.
- * Copyright (c) 2013-2015 NVIDIA Corporation.  All rights reserved.
+ * Copyright (c) 2013-2024 NVIDIA Corporation.  All rights reserved.
  * Copyright (c) 2014-2019 Intel, Inc.  All rights reserved.
  * Copyright (c) 2014-2017 Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
@@ -1502,9 +1502,32 @@ static void mca_btl_tcp_component_recv_handler(int sd, short flags, void *user)
     /* lookup the corresponding process */
     btl_proc = mca_btl_tcp_proc_lookup(&guid);
     if (NULL == btl_proc) {
-        opal_show_help("help-mpi-btl-tcp.txt", "server accept cannot find guid", true,
-                       opal_process_info.nodename, getpid());
+        const char *peer = opal_fd_get_peer_name(sd);
+        if( 0 == opal_compare_proc(opal_process_info.my_name, guid) ) {
+            opal_show_help("help-mpi-btl-tcp.txt", "server cannot accept connection from self", true,
+                           peer, OPAL_NAME_PRINT(guid),
+                           opal_process_info.nodename, getpid());
+            /**
+             * Special case: we used an interface to send data to a remote peer
+             * but that interface is only local, and we received our own message.
+             * If we just close the socket this will confuse the connection, as
+             * it will not be able to know that the interface should not be
+             * used. Instead, we cabn identify ourselves by sending our guid
+             * back to ourselves, marking the interface as improper for future
+             * communications.
+             */
+            if (sizeof(hs_msg) != mca_btl_tcp_send_blocking(sd, &hs_msg, sizeof(hs_msg))) {
+                opal_show_help("help-mpi-btl-tcp.txt", "client handshake fail", true,
+                               opal_process_info.nodename, sizeof(hs_msg),
+                               "connect ACK failed to send magic-id and guid");
+            }
+        } else {
+            opal_show_help("help-mpi-btl-tcp.txt", "server accept cannot find guid", true,
+                           OPAL_NAME_PRINT(opal_process_info.my_name), opal_process_info.nodename,
+                           getpid(), OPAL_NAME_PRINT(guid), peer);
+        }
         CLOSE_THE_SOCKET(sd);
+        free((char*)peer);
         return;
     }
 
diff --git a/opal/mca/btl/tcp/btl_tcp_endpoint.c b/opal/mca/btl/tcp/btl_tcp_endpoint.c
@@ -791,6 +791,14 @@ static int mca_btl_tcp_endpoint_start_connect(mca_btl_base_endpoint_t *btl_endpo
             CLOSE_THE_SOCKET(btl_endpoint->endpoint_sd);
             return OPAL_ERROR;
         }
+        char tmp[2][16];
+        inet_ntop(AF_INET, &((struct sockaddr_in *)&btl_endpoint->endpoint_btl->tcp_ifaddr)->sin_addr, tmp[0], 16);
+        inet_ntop(AF_INET, &((struct sockaddr_in *)&endpoint_addr)->sin_addr, tmp[1], 16);
+        opal_output(0, "proc %s bind socket to %s:%d before connecting to peer %s at %s:%d\n",
+            OPAL_NAME_PRINT(OPAL_PROC_MY_NAME),
+            tmp[0], htons(((struct sockaddr_in *) &btl_endpoint->endpoint_btl->tcp_ifaddr)->sin_port),
+            OPAL_NAME_PRINT(btl_endpoint->endpoint_proc->proc_opal->proc_name),
+            tmp[1], ntohs(((struct sockaddr_in *) &endpoint_addr)->sin_port));
     }
 #if OPAL_ENABLE_IPV6
     if (endpoint_addr.ss_family == AF_INET6) {
diff --git a/opal/mca/btl/tcp/btl_tcp_proc.c b/opal/mca/btl/tcp/btl_tcp_proc.c
@@ -409,7 +409,68 @@ mca_btl_tcp_proc_t *mca_btl_tcp_proc_create(opal_proc_t *proc)
         goto cleanup;
     }
 
-    btl_proc->proc_addr_count = size / sizeof(mca_btl_tcp_modex_addr_t);
+    /**
+     * If the peer is physically located on another node, remove allinterfaces
+     * that have the an IP address identical to any local interface (this will
+     * remove all the local and virtual interfaces).
+     */
+    size_t count = size / sizeof(mca_btl_tcp_modex_addr_t);
+    if (!OPAL_PROC_ON_LOCAL_NODE(proc->proc_flags)) {
+        opal_list_t *local_ifs = &mca_btl_tcp_component.local_ifs;
+        opal_if_t *local_iter;
+        char tmp[2][16];
+        for (uint32_t i = 0; i < count; /* no automatic progress */) {
+            OPAL_LIST_FOREACH (local_iter, local_ifs, opal_if_t) {
+                if (MCA_BTL_TCP_AF_INET == remote_addrs[i].addr_family &&
+                    AF_INET == local_iter->af_family) {
+                    if (!memcmp(&((struct sockaddr_in *)&local_iter->if_addr)->sin_addr,
+                                remote_addrs[i].addr,
+                                sizeof(struct in_addr))) {
+                        /* we found a match */
+                        inet_ntop(AF_INET, remote_addrs[i].addr, tmp[0], 16);
+                        inet_ntop(AF_INET, &((struct sockaddr_in *) &local_iter->if_addr)->sin_addr,
+                                  tmp[1], 16);
+                        goto match_found;
+                    }
+                }
+#if OPAL_ENABLE_IPV6
+                else if (MCA_BTL_TCP_AF_INET6 == remote_addrs[i].addr_family &&
+                         AF_INET6 == local_iter->af_family) {
+                    if (!memcmp(&((struct sockaddr_in6 *) &local_iter->if_addr)->sin6_addr,
+                                remote_addrs[i].addr,
+                                sizeof(struct in6_addr))) {
+                        /* we found a match */
+                        inet_ntop(AF_INET6, remote_addrs[i].addr, tmp[0], 16);
+                        inet_ntop(AF_INET6, &((struct sockaddr_in6 *) &local_iter->if_addr)->sin6_addr,
+                                  tmp[1], 16);
+                        goto match_found;
+                    }
+                }
+#endif /* OPAL_ENABLE_IPV6 */
+            }
+            if (MCA_BTL_TCP_AF_INET == remote_addrs[i].addr_family) {
+                inet_ntop(AF_INET, remote_addrs[i].addr, tmp[0], 16);
+            } else {
+                inet_ntop(AF_INET6, remote_addrs[i].addr, tmp[0], 16);
+            }
+            opal_output_verbose(20, opal_btl_base_framework.framework_output,
+                                "btl: tcp: Accept IP %s from %s\n", tmp[0],
+                                OPAL_NAME_PRINT(proc->proc_name));
+            i++;  /* go to the next remote interface */
+            continue;
+        match_found:
+            opal_output_verbose(20, opal_btl_base_framework.framework_output,
+                                "btl: tcp: Drop IP %s from %s because it matches "
+                                "the local IP %s (%s))!\n",
+                                tmp[0], OPAL_NAME_PRINT(proc->proc_name), tmp[1],
+                                local_iter->if_name);
+            count--;
+            memmove(&remote_addrs[i], &remote_addrs[i + 1],
+                    (count - i) * sizeof(mca_btl_tcp_modex_addr_t));
+            break;
+        }
+    }
+    btl_proc->proc_addr_count = count;
     btl_proc->proc_addrs = malloc(btl_proc->proc_addr_count * sizeof(mca_btl_tcp_addr_t));
     if (NULL == btl_proc->proc_addrs) {
         rc = OPAL_ERR_OUT_OF_RESOURCE;
diff --git a/opal/mca/btl/tcp/help-mpi-btl-tcp.txt b/opal/mca/btl/tcp/help-mpi-btl-tcp.txt
@@ -113,6 +113,19 @@ aborting your job.
   Flag:       %s
   Error:      %s (%d)
 #
+[server cannot accept connection from self]
+WARNING: Open MPI accepted a TCP connection from what appears to be
+the same Open MPI process. This is not supported, please exclude
+(using btl_tcp_if_exclude) the interface corresponding to the
+following IP address %s
+
+This attempted connection will be ignored; your MPI job may or may not
+continue properly.
+
+  Process guid:        %s
+  on host:             %s
+  PID:                 %d
+#
 [server accept cannot find guid]
 WARNING: Open MPI accepted a TCP connection from what appears to be a
 another Open MPI process but cannot find a corresponding process
@@ -121,8 +134,11 @@ entry for that peer.
 This attempted connection will be ignored; your MPI job may or may not
 continue properly.
 
-  Local host: %s
-  PID:        %d
+  Process guid:        %s
+  on host:             %s
+  PID:                 %d
+  guid:                %s
+  Source IP of socket: %s
 #
 [server getpeername failed]
 WARNING: Open MPI failed to look up the peer IP address information of
