refactor: Convert platform hooks from weak symbols to C++ polymorphism

Replace the weak symbol-based platform hook system (init, config_endpoint,
sort_rails, device_set_guid) with a modern C++ polymorphic design using
abstract base classes, virtual methods, and singleton pattern with
compile-time platform selection. This architectural change eliminates
runtime null pointer checks, provides type safety, and improves code
organization while preserving existing functionality.

Signed-off-by: Hershel Shah <[email protected]>

Author: hershys-aws

include/nccl_ofi_platform.h

@@ -5,58 +5,175 @@
 #ifndef NCCL_OFI_PLATFORM_H_
 #define NCCL_OFI_PLATFORM_H_
 
+#include <memory>
+#include <map>
+
 #include <rdma/fabric.h>
 #include <rdma/fi_endpoint.h>
 
-/* Declare platform-specific hooks that can be provided by platform-specific
- * source files (such as the optionally compiled platform_aws.c).  The functions
- * here are declared as weak symbols so that linkage will not break if no
- * platform specific hook is provided; in that case the hook will be NULL at
- * runtime.
- */
-
-/* Platform-specific initialization hook.
- */
-int platform_init(const char **provider_filter) __attribute__((weak));
-
-/* Platform-specific endpoint configuration hook
- */
-int platform_config_endpoint(struct fi_info *info, struct fid_ep *ep) __attribute__((weak));
+#include "nccl_ofi_system.h"
 
-/* Platform-specific hook to sort in the multi-rail protocol of the
- * plugin
+/**
+ * @brief Abstract base class representing a platform implementation for NCCL OFI plugin
  *
- * Rail-oriented networks or traffic flows are a common performance
- * optimization for ML networks.  Generally, Libfabric providers sort
- * their provider list by BDFs, which are indicitive of physical
- * ordering and good enough.  However, on some platforms (especially
- * virtualized platforms), this might not actually be sufficient and
- * another sorting mechanism may be required to properly group NICs.
+ * The Platform class provides an interface for platform-specific operations and configurations
+ * in the NCCL OFI plugin. It defines virtual methods that must be implemented by concrete
+ * platform implementations to handle platform-specific initialization, endpoint configuration,
+ * and rail sorting operations.
  *
- * This interface is called in the topology initialization code to
- * order NICs that are behind the same PCIe root complex / switch.
- * The info_list will have num_rails providers listed, and will later
- * be split into num_groups groups (based on the number of
- * accelerators that are also behind the PCIe switch).
+ * Each platform implementation can specify its priority level for selection, with higher
+ * priority platforms being preferred over lower priority ones.
  *
- * Providers of this interface should sort the provided info_list such
- * that the Nth provider on this node will be assumed to talk to the
- * Nth provider on remote nodes (ie, identify the "rail id" and sort
- * by that).
+ * Future platform are to be implemented by inheriting this class and overriding the
+ * given functions. Look at PlatformAWS or Default as an example.
  *
- * @param info_list: pointer to list of `num_rails` info objects
- * @param num_rails: number of rails
+ * @see Default
+ * @see PlatformAWS
+ * @see PlatformManager
  */
-void platform_sort_rails(struct fi_info **info_list, size_t num_rails, size_t num_groups) __attribute__((weak));
+class Platform {
+public:
+	virtual ~Platform() = default;
+
+	/**
+	 * @brief	Get platform name
+	 *
+	 * @return	Platform name string
+	 */
+	virtual const char* get_name() const = 0;
+
+	/**
+	 * @brief	Get platform priority for selection.
+	 *
+	 * @return	Priority value (higher values have higher priority)
+	 */
+	virtual int get_priority() = 0;
+
+	/**
+	 * @brief	Platform-specific initialization hook
+	 *
+	 * @param	provider_filter	Pointer to provider filter string
+	 *
+	 * @return	0 on success, error code on failure
+	 */
+	virtual int init(const char **provider_filter) = 0;
+
+	/**
+	 * @brief	Platform-specific endpoint configuration hook
+	 *
+	 * @param	info	Fabric info structure
+	 * @param	ep	Fabric endpoint
+	 *
+	 * @return	0 on success, error code on failure
+	 */
+	virtual int config_endpoint(struct fi_info *info, struct fid_ep *ep) = 0;
+
+	/**
+	 * @brief	Platform-specific hook to sort in the multi-rail protocol of the plugin
+	 *
+	 * 		Rail-oriented networks or traffic flows are a common performance
+	 * 		optimization for ML networks. Generally, Libfabric providers sort
+	 * 		their provider list by BDFs, which are indicitive of physical
+	 * 		ordering and good enough. However, on some platforms (especially
+	 * 		virtualized platforms), this might not actually be sufficient and
+	 * 		another sorting mechanism may be required to properly group NICs.
+	 *
+	 * 		This interface is called in the topology initialization code to
+	 * 		order NICs that are behind the same PCIe root complex / switch.
+	 * 		The info_list will have num_rails providers listed, and will later
+	 * 		be split into num_groups groups (based on the number of
+	 * 		accelerators that are also behind the PCIe switch).
+	 *
+	 * 		Providers of this interface should sort the provided info_list such
+	 * 		that the Nth provider on this node will be assumed to talk to the
+	 * 		Nth provider on remote nodes (ie, identify the "rail id" and sort
+	 * 		by that).
+	 *
+	 * @param	info_list	Array of fabric info pointers to sort
+	 * @param	num_rails	Number of rails in the list
+	 * @param	num_groups	Number of groups to split rails into
+	 */
+	virtual void sort_rails(struct fi_info **info_list, size_t num_rails, size_t num_groups) = 0;
+
+	/**
+	 * @brief	Platform-specific device GUID setter
+	 *
+	 * 		Sets device GUID to uniquely identify the network device
+	 *
+	 * @param	info	Fabric info structure
+	 * @param	device	Network device to set GUID for
+	 */
+	virtual void device_set_guid(struct fi_info *info, nccl_net_ofi_device_t *device) = 0;
+};
+
+using PlatformPtr = std::unique_ptr<Platform>;
+
+class Default : public Platform {
+public:
+	const char* get_name() const override { return "Default"; }
+	int get_priority() override { return 0; }
+	int init(const char **provider_filter) override { return 0; }
+	int config_endpoint(struct fi_info *info, struct fid_ep *ep) override { return 0; }
+	void sort_rails(struct fi_info **info_list, size_t num_rails, size_t num_groups) override {}
+	void device_set_guid(struct fi_info *info, nccl_net_ofi_device_t *device) override {
+		uint32_t node_id = nccl_ofi_get_unique_node_id();
+		/*
+		 * Use device_index as lower 8 bits
+		 * Use node_id as next 32 bits (bits 8-39)
+		 * Upper 24 bits remain 0
+		 */
+		device->guid = (static_cast<uint64_t>(node_id) << 8) | device->dev_id;
+	}
+};
+
+class PlatformManager {
+public:
+	/**
+	 * @brief	Get global instance
+	 *
+	 * @return	Reference to global instance
+	 */
+	static PlatformManager& get_global();
+
+	/**
+	 * @brief	Register a platform with the manager
+	 *
+	 * 		Platforms are automatically sorted by priority in the internal map.
+	 * 		Higher priority values take precedence and duplicates are dropped.
+	 *
+	 * @param	platform	Platform instance to register (moved)
+	 */
+	void register_platform(PlatformPtr&& platform);
+
+	/**
+	 * @brief	Get the highest priority platform instance
+	 *
+	 * 		Returns the platform with the highest priority value.
+	 *
+	 * @return	Reference to highest priority platform
+	 */
+	inline Platform& get_platform() { return *platforms_.rbegin()->second; }
+
+	/**
+	 * @brief	Get number of registered platforms (for testing)
+	 *
+	 * @return	Number of platforms in the manager
+	 */
+	inline size_t get_platform_count() { return platforms_.size(); }
+protected:
+	/**
+	 * @brief	Default constructor
+	 *		Register the default Platform by default. A static global
+	 *		instance is meant to be used in the plugin and the unit
+	 *		tests leverage the protected scope.
+	 */
+	PlatformManager() {
+		register_platform(std::make_unique<Default>());
+	}
+
+private:
+	std::map<int, PlatformPtr> platforms_;
+};
 
-/*
- * Platform-specific guid property setter
- *
- * This overrides the default guid setter (nccl_net_ofi_device_set_guid()) which
- * is based on network device index and IP address. Platforms can set
- * device->guid to be any 64-bit value as they seem fit to uniquely identify the
- * network device.
- */
-void platform_device_set_guid(struct fi_info *info, nccl_net_ofi_device_t *device) __attribute__((weak));
 
 #endif // End NCCL_OFI_PLATFORM_H_

include/platform-aws.h

@@ -11,51 +11,80 @@
 #define PLATFORM_AWS_H_
 
 #include <map>
+#include <mutex>
 #include <string>
+#include <unordered_map>
 
 #include "nccl_ofi_param.h"
+#include "nccl_ofi_platform.h"
 
 #define PLATFORM_NAME_P6E_GB200 "p6e-gb200"
 
-struct ec2_platform_data {
-	const char* name;
-	const char* regex;
-	const char* topology;
-	int default_dup_conns;
-	float latency;
-	bool gdr_required;
-	PROTOCOL default_protocol;
-	bool domain_per_thread;
-	std::map<std::string, std::string> env;
-};
+class PlatformAWS : public Platform {
+public:
+	const char* get_name() const override { return "AWS"; }
+	int get_priority() override { return 100; }
+	int init(const char **provider_filter) override;
+	int config_endpoint(struct fi_info *info, struct fid_ep *ep) override;
+	void sort_rails(struct fi_info **info_list, size_t num_rails, size_t num_groups) override;
+	void device_set_guid(struct fi_info *info, nccl_net_ofi_device_t *device) override;
 
+protected:
+	struct ec2_platform_data {
+		const char* name;
+		const char* regex;
+		const char* topology;
+		int default_dup_conns;
+		float latency;
+		bool gdr_required;
+		PROTOCOL default_protocol;
+		bool domain_per_thread;
+		std::map<std::string, std::string> env;
+	};
 
-struct platform_aws_node_guid {
-	uint8_t func_idx;
-	uint8_t per_card_pci_bus;
-	uint16_t per_card_pci_domain;
-	uint32_t func_mac_low_bytes;
-};
+	struct platform_aws_node_guid {
+		uint8_t func_idx;
+		uint8_t per_card_pci_bus;
+		uint16_t per_card_pci_domain;
+		uint32_t func_mac_low_bytes;
+	};
 
-/*
- * @brief        Get the platform data map
- *
- * This function exists solely to test
- * platform_aws_get_platform_entry() against the production data map.
- */
-struct ec2_platform_data *platform_aws_get_platform_map(size_t *len);
+	static const ec2_platform_data platform_data_map[];
 
+	// Platform data functions
+	const ec2_platform_data *get_platform_data();
+	const ec2_platform_data *get_platform_map(size_t *len) const;
+	static const ec2_platform_data *get_platform_entry(const char *platform_type,
+					      const ec2_platform_data *platform_data_list,
+					      size_t platform_data_len);
 
-/*
- * @brief	Returns platform data for current platform type, if found
- *
- * @input	Platform type
- *
- * @return	NULL, if no topology found
- * 		platform data, if match found
- */
-struct ec2_platform_data *platform_aws_get_platform_entry(const char *platform_type,
-							  struct ec2_platform_data *platform_data_list,
-							  size_t platform_data_len);
+	// Endpoint configuration functions
+	int validate_rdma_write(struct fid_ep *ep);
+	int configure_ep_inorder(struct fid_ep *ep, int optname, const char* optname_name, bool *have_ordering);
+	int configure_ep_max_msg_size(struct fid_ep *ep);
+	int configure_nvls_option();
+	int configure_tuner();
+
+	// GUID and rail functions
+	const platform_aws_node_guid* get_node_guid_fields(struct fi_info *info);
+	inline int get_rail_vf_idx(struct fi_info *info) {
+		const auto* fields = get_node_guid_fields(info);
+		return fields ? fields->func_idx : -EIO;
+	}
+
+private:
+	std::mutex mutex_;
+
+	// Cache for GUID fields to avoid repeated sysfs reads
+	std::unordered_map<std::string, platform_aws_node_guid> guid_cache_;
+
+	// Platform data state
+	bool platform_data_init_ = false;
+	const ec2_platform_data *cached_platform_data_ = nullptr;
+
+	// Endpoint config state
+	bool nccl_proto_configured_ = false;
+	bool need_ordering_ = false;
+};
 
 #endif // End NCCL_OFI_H_

m4/ax_platform_aws.m4

@@ -21,6 +21,7 @@ AC_DEFUN([AX_CHECK_PLATFORM_AWS],[
   AM_CONDITIONAL([WANT_PLATFORM_AWS], [test "${want_platform_aws}" = "yes"])
   AS_IF([test "${want_platform_aws}" = "yes"],
         [NCCL_OFI_PLATFORM="AWS"
+         AC_DEFINE([WANT_AWS_PLATFORM], [1], [Define to 1 if AWS platform optimizations are enabled])
          AC_MSG_CHECKING([for Libfabric 1.22.0 or greater])
          AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
 [[#include <rdma/fabric.h>

src/Makefile.am

@@ -33,6 +33,7 @@ sources = \
 	nccl_ofi_dmabuf.cpp \
 	nccl_ofi_ep_addr_list.cpp \
 	nccl_ofi_param.cpp \
+	nccl_ofi_platform.cpp \
 	tracepoint.cpp
 
 if WANT_PLATFORM_AWS

src/nccl_ofi_net.cpp

@@ -30,6 +30,9 @@
 #include "nccl_ofi_idpool.h"
 #include "nccl_ofi_dmabuf.h"
 #include "nccl_ofi_platform.h"
+#ifdef WANT_AWS_PLATFORM
+#include "platform-aws.h"
+#endif
 #include "nccl_ofi_ofiutils.h"
 #include "nccl_ofi_system.h"
 
@@ -175,11 +178,13 @@ int nccl_net_ofi_create_plugin(nccl_net_ofi_plugin_t **plugin_p)
 	nic_dup_conns = ofi_nccl_nic_dup_conns();
 	cq_read_count = ofi_nccl_cq_read_count();
 
-	if (platform_init) {
-		ret = platform_init(&provider_filter);
-		if (ret != 0)
-			goto exit;
-	}
+#ifdef WANT_AWS_PLATFORM
+	PlatformManager::get_global().register_platform(std::make_unique<PlatformAWS>());
+#endif
+
+	ret = PlatformManager::get_global().get_platform().init(&provider_filter);
+	if (ret != 0)
+		goto exit;
 
 	if (ofi_nccl_progress_model.get_source() != ParamSource::DEFAULT) {
 		NCCL_OFI_INFO(NCCL_INIT | NCCL_NET, "Requesting progress model %s",
@@ -794,11 +799,7 @@ nccl_net_ofi_device_t::nccl_net_ofi_device_t(nccl_net_ofi_plugin_t *plugin_arg,
 		throw std::runtime_error("Base device constructor: device name alloc failed");
 	}
 
-	if (platform_device_set_guid) {
-		platform_device_set_guid(info, this);
-	} else {
-		nccl_net_ofi_device_set_guid(info, this);
-	}
+	PlatformManager::get_global().get_platform().device_set_guid(info, this);
 
 	/* Intiaialize mutex for endpoint access */
 	ret = nccl_net_ofi_mutex_init(&this->device_lock, nullptr);