UCP/CORE: Fix PR comments

Author: guy-ealey-morag

src/ucp/core/ucp_context.c

@@ -75,10 +75,6 @@
 #define UCP_MLX_PREFIX       "mlx"
 #define UCP_MLX_DEFAULT_PORT "1"
 
-#define UCP_IS_RESOURCE_MLX_DEFAULT_PORT(_dev_name, _dev_name_suffix) \
-    ((!strncmp(_dev_name, UCP_MLX_PREFIX, strlen(UCP_MLX_PREFIX))) && \
-     (!strcmp(_dev_name_suffix, UCP_MLX_DEFAULT_PORT)))
-
 typedef enum ucp_transports_list_search_result {
     UCP_TRANSPORTS_LIST_SEARCH_RESULT_PRIMARY             = UCS_BIT(0),
     UCP_TRANSPORTS_LIST_SEARCH_RESULT_AUX_IN_MAIN         = UCS_BIT(1),
@@ -1029,8 +1025,8 @@ static uint64_t ucp_str_array_search_in_ranges(const char **array,
         }
 
         n = 0;
-        if (sscanf(p, "[%lu-%lu]%n", &range_start, &range_end, &n) != 2 ||
-            n == 0 || p[n] != '\0' || range_start > range_end) {
+        if ((sscanf(p, "[%lu-%lu]%n", &range_start, &range_end, &n) != 2) ||
+            (n == 0) || (p[n] != '\0') || (range_start > range_end)) {
             continue; /* Invalid range */
         }
 
@@ -1095,6 +1091,13 @@ static const char *ucp_get_dev_name_suffix(const char *dev_name)
     return colon + 1;
 }
 
+static inline int
+ucp_is_dev_mlx_default_port(const char *dev_name, const char *dev_name_suffix)
+{
+    return (!strncmp(dev_name, UCP_MLX_PREFIX, strlen(UCP_MLX_PREFIX))) &&
+           (!strcmp(dev_name_suffix, UCP_MLX_DEFAULT_PORT));
+}
+
 static int ucp_is_resource_in_device_list(const uct_tl_resource_desc_t *resource,
                                           const ucs_config_names_array_t *devices,
                                           uint64_t *dev_cfg_mask,
@@ -1113,10 +1116,8 @@ static int ucp_is_resource_in_device_list(const uct_tl_resource_desc_t *resource
 
     if (dev_type == UCT_DEVICE_TYPE_NET) {
         dev_name_suffix = ucp_get_dev_name_suffix(resource->dev_name);
-        ucs_assert(dev_name_suffix > resource->dev_name &&
-                   dev_name_suffix <= resource->dev_name + strlen(resource->dev_name));
 
-        if (UCP_IS_RESOURCE_MLX_DEFAULT_PORT(resource->dev_name, dev_name_suffix)) {
+        if (ucp_is_dev_mlx_default_port(resource->dev_name, dev_name_suffix)) {
             ucs_strncpy_zero(dev_name_base, resource->dev_name,
                              (size_t)(dev_name_suffix - resource->dev_name));
 
@@ -1132,8 +1133,8 @@ static int ucp_is_resource_in_device_list(const uct_tl_resource_desc_t *resource
         }
     }
 
-    /* if the user's list is 'all', use all the available resources */
     if (!mask) {
+        /* if the user's list is 'all', use all the available resources */
         mask = ucp_str_array_search((const char**)devices[dev_type].names,
                                     devices[dev_type].count, UCP_RSC_CONFIG_ALL,
                                     NULL);

test/gtest/ucp/test_ucp_context.cc

@@ -206,9 +206,7 @@ UCS_TEST_P(test_ucp_net_devices_config, base_name_selects_default_port)
     }
 
     std::set<std::string> base_names = get_mlx5_base_names(mlx5_devices);
-    if (base_names.empty()) {
-        UCS_TEST_SKIP_R("No mlx5 devices with port suffix found");
-    }
+    ASSERT_FALSE(base_names.empty());
 
     /* Pick the first base name for testing */
     std::string test_base_name = *base_names.begin();
@@ -222,8 +220,9 @@ UCS_TEST_P(test_ucp_net_devices_config, base_name_selects_default_port)
     /* Verify that devices matching the base name were selected */
     std::set<std::string> selected_devices = get_mlx5_device_names(*e);
     size_t count = count_mlx5_resources_with_prefix(selected_devices, test_base_name);
-    EXPECT_GT(count, 0) << "Expected at least one device with base name '"
-                        << test_base_name << "' to be selected";
+    EXPECT_EQ(count, 1) << "Expected exactly one device with base name '"
+                        << test_base_name << "' to be selected, found: "
+                        << testing::PrintToString(selected_devices);
 
     std::string expected_dev = test_base_name + ":1";
     EXPECT_TRUE(has_device(selected_devices, expected_dev))
@@ -341,6 +340,42 @@ UCS_TEST_P(test_ucp_net_devices_config, duplicate_device_warning)
                                << test_base_name << "'";
 }
 
+/*
+ * Test that a range not covering all devices only selects matching devices.
+ * E.g., "mlx5_[1-2]" should match mlx5_1 and mlx5_2, but not mlx5_0.
+ */
+UCS_TEST_P(test_ucp_net_devices_config, partial_range_selection)
+{
+    entity *e = create_entity();
+
+    std::set<std::string> mlx5_devices = get_mlx5_device_names(*e);
+    if (mlx5_devices.empty()) {
+        UCS_TEST_SKIP_R("No mlx5 network device available");
+    }
+
+    std::set<std::string> base_names = get_mlx5_base_names(mlx5_devices);
+
+    if (!has_device(base_names, "mlx5_0") ||
+        !has_device(base_names, "mlx5_1") ||
+        !has_device(base_names, "mlx5_2")) {
+        UCS_TEST_SKIP_R(
+                "Need mlx5_0, mlx5_1, and mlx5_2 devices for this test");
+    }
+
+    m_entities.clear();
+
+    modify_config("NET_DEVICES", "mlx5_[1-2]");
+    e = create_entity();
+
+    std::set<std::string> selected_devices = get_mlx5_device_names(*e);
+
+    std::set<std::string> selected_base_names = get_mlx5_base_names(
+            selected_devices);
+
+    std::set<std::string> expected_base_names = {"mlx5_1", "mlx5_2"};
+    EXPECT_EQ(selected_base_names, expected_base_names);
+}
+
 /*
  * Test that non-mlx devices are not affected by the mlx default port logic.
  */