init: Set NCCL_PROTO to simple when GDR is unsupported

Force NCCL_PROTO to simple when GDR is not supported. While NCCL
disables the LL128 protocol when using host buffers, it leaves the LL
protocol enabled and polls on host memory directly. Many host-only
providers, like tcp and sockets, do not guarantee data delivering
ordering and can cause corruption when used with the LL protocol. Since
polling on host memory is expensive anyways, this has no real
performance implications and avoids dealing with the lack of a data
ordering hint in the Libfabric API.

Note: Warning for pre-existing NCCL_PROTO setting was removed as modern
NCCL supports complex protocol formats (e.g. "^LL", "allreduce:simple").
We can consider adding protocol format validation to put an INFO later.

Signed-off-by: Mozar Huang <[email protected]>

Author: mozarhua

include/nccl_ofi.h

@@ -791,4 +791,17 @@ int get_inject_rma_size_opt(struct fid_ep *ofi_ep,
  */
 long nccl_net_ofi_gettid(void);
 
+ /*
+ * @brief   Configures NCCL_PROTO environment variable to "simple".
+ *
+ * @details If NCCL_PROTO is not set, configures it to "simple" protocol.
+ *          If NCCL_PROTO is already set, skip the configuration.
+ *
+ * @input   log reason string
+ *
+ * @return  0 on success or when warning is issued
+ *          -errno in case of any failure
+ */
+int nccl_net_ofi_configure_nccl_proto_simple(const char *log_reason);
+
 #endif // End NCCL_OFI_H_

src/nccl_ofi_net.cpp

@@ -328,6 +328,24 @@ int nccl_net_ofi_create_plugin(nccl_net_ofi_plugin_t **plugin_p)
 		ret = -ENOTSUP;
 		goto exit;
 	}
+	/* Force SIMPLE protocol when using a provider that does not support
+	 * GDR. NCCL disables the LL128 protocol in this case, but leaves the
+	 * LL protocol enabled. Without GDR, the LL protocol polls on host
+	 * memory for completion flags. In addition to being slow, this assumes
+	 * that host memory is updated in 8 byte segments. However, most
+	 * providers that do not support HMEM (like the tcp or sockets
+	 * providers) do not make any guarantees about data delivery ordering.
+	 * There is not a good way to ask Libfabric providers about their data
+	 * delivery support in the general case, so take a conservative
+	 * approach and force the simple protocol whenever using a provider
+	 * that does not support HMEM.
+	 */
+	if (support_gdr != GDR_SUPPORTED) {
+		ret = nccl_net_ofi_configure_nccl_proto_simple("GDR");
+		if (ret != 0) {
+			goto exit;
+		}
+	}
 
 	*plugin_p = plugin;
 
@@ -1186,3 +1204,22 @@ int get_inject_rma_size_opt(struct fid_ep *ofi_ep,
 	return -FI_ENOPROTOOPT;
 #endif
 }
+
+
+int nccl_net_ofi_configure_nccl_proto_simple(const char *log_reason)
+{
+	int ret;
+
+	if (getenv("NCCL_PROTO") == NULL) {
+		NCCL_OFI_INFO(NCCL_INIT, "Setting NCCL_PROTO='simple' to prevent data corruption (reason: %s not supported)",
+			      log_reason);
+		ret = setenv("NCCL_PROTO", "simple", 1);
+		if (ret != 0) {
+			NCCL_OFI_WARN("Error setting NCCL_PROTO environment variable: %s",
+				      strerror(errno));
+			return -errno;
+		}
+	}
+
+	return 0;
+}

src/platform-aws.cpp

@@ -275,25 +275,6 @@ static int validate_rdma_write(struct fid_ep *ep)
 
 
 #if HAVE_CUDA
-static int configure_nccl_proto(void)
-{
-	int ret;
-
-	if (!getenv("NCCL_PROTO")) {
-		NCCL_OFI_INFO(NCCL_INIT, "Setting NCCL_PROTO to \"simple\"");
-		ret = setenv("NCCL_PROTO", "simple", 0);
-		if (ret != 0) {
-			NCCL_OFI_WARN("Error setting NCCL_PROTO environment variable: %s",
-				      strerror(errno));
-			return -errno;
-		}
-	} else if (strcasecmp(getenv("NCCL_PROTO"), "simple") != 0) {
-		NCCL_OFI_WARN("NCCL_PROTO was set to \"LL/LL128\", but the Libfabric endpoint does not support 128 byte in-order aligned stores. This endpoint may corrupt data during communication");
-	}
-
-	return 0;
-}
-
 /*
  * Try to set one of the in-order flags for either send/recv or rdma
  * on the current endpoint to true.  have_ordering will be the
@@ -742,7 +723,11 @@ int platform_config_endpoint(struct fi_info *info, struct fid_ep* endpoint) {
 			nccl_proto_configured = true;
 
 			if (!have_ordering) {
-				ret = configure_nccl_proto();
+				/* When byte delivery ordering is not guaranteed, force
+				 * the simple protocol as the LL/LL128 protocols can lead
+				 * to data corruption without data delivery ordering.
+				 */
+				ret = nccl_net_ofi_configure_nccl_proto_simple("byte delivery ordering");
 				if (ret != 0) {
 					NCCL_OFI_WARN("Failed to set NCCL_PROTO: %d", ret);
 					ret = -ENOTSUP;