UCP/CORE: Allow specifying mlx devices without a port in UCX_NET_DEVICES by guy-ealey-morag · Pull Request #11142 · openucx/ucx

guy-ealey-morag · 2026-01-23T16:35:23Z

What?

Allow specifying mlx devices in UCX_NET_DEVICES without a port, and as ranges.

Why?

Currently the only valid format is UCX_NET_DEVICES=mlx5_0:1,mlx5_1:1,mlx5_2:1

This PR now allows specifying mlx devices without a port mlx5_0,mlx5_1,mlx5_2

src/uct/api/uct.h

src/ucp/core/ucp_context.c

guy-ealey-morag · 2026-01-26T13:52:48Z

I added unit tests to ensure the correct behavior of the different device name formats

src/ucp/core/ucp_context.c

test/gtest/ucp/test_ucp_context.cc

brminich · 2026-01-28T11:43:22Z

test/gtest/ucp/test_ucp_context.cc

+    EXPECT_GT(count, 0) << "Expected at least one device with base name '"
+                        << test_base_name << "' to be selected";


can we expect exactly one device?

test/gtest/ucp/test_ucp_context.cc

brminich · 2026-01-28T11:48:55Z

src/ucp/core/ucp_context.c

+        ucs_assert(dev_name_suffix > resource->dev_name &&
+                   dev_name_suffix <= resource->dev_name + strlen(resource->dev_name));


i'd move this assert inside ucp_get_dev_name_suffix if really needed

Removed the assert

src/ucp/core/ucp_context.c

test/gtest/ucp/test_ucp_context.cc

src/ucp/core/ucp_context.c

evgeny-leksikov

pls reword "colon" -> "delimeter" in all places
Add a negative test with duplicates like UCX_NET_DEVICES=mlx5_1:1,mlx5_1,mlx5_2
the rest LGTM 👍

guy-ealey-morag · 2026-02-02T13:03:12Z

Due to some flaws I found in the current implementation of devices ranges (like mlx5_[0-2]),
I decided to remove that logic from this PR and leave only the base name support for now.

evgeny-leksikov

pls avoid force push during review

evgeny-leksikov · 2026-02-02T21:06:36Z

test/gtest/ucp/test_ucp_context.cc

+        EXPECT_EQ(m_warnings.size() - warn_count, 1)
+                << "Expected exactly one warning";
+
+        // print all warnings
+        for (const std::string &warn : m_warnings) {
+            std::cout << warn << std::endl;
+        }
+
+        /* Check that a warning about duplicate device was printed */
+        std::string expected_warn = "device '" + duplicate_dev_name +
+                                    "' was selected multiple times";
+        bool found_warning        = false;
+        for (size_t i = warn_count; i < m_warnings.size(); ++i) {
+            if (m_warnings[i].find(expected_warn) != std::string::npos) {
+                found_warning = true;
+                break;
+            }
+        }
+
+        EXPECT_TRUE(found_warning)
+                << "Expected warning about duplicate device '"
+                << duplicate_dev_name << "' with config '" << devices_config
+                << "'";
+    }


how about to add a wrapper for hide_warns_logger and handle all these checks just in time?

I want to to match the warning string with the device name duplicate_dev_name.
It's not trivial to pass it to a warpper and the solutions I found make it too complicated for what it is.

evgeny-leksikov · 2026-02-02T21:07:12Z

test/gtest/ucp/test_ucp_context.cc

+
+        // print all warnings
+        for (const std::string &warn : m_warnings) {
+            std::cout << warn << std::endl;


TEST_MESSAGE is preferable

evgeny-leksikov · 2026-02-02T21:17:32Z

src/ucp/core/ucp_context.c

+        memcpy(dev_basename_p, dev_name, basename_len);
+        dev_basename_p[basename_len] = '\0';


why memcpy? maybe ucs_strncpy_zero?

evgeny-leksikov · 2026-02-02T21:21:00Z

src/ucp/core/ucp_context.c

+    const char *delimiter = strchr(dev_name, ':');
+    size_t basename_len;
+
+    if (delimiter != NULL) {


minor: how about to use ucs_string_split? (probably overkill in this case)

It's a little overkill, and also ucs_string_split modifies the string in-place so I prefer avoiding it here

evgeny-leksikov · 2026-02-02T21:22:32Z

src/ucp/core/ucp_context.c

+    /* for network devices, also search for the base name (before the delimiter) */
+    if (dev_type == UCT_DEVICE_TYPE_NET) {
+        ucp_get_dev_basename(resource->dev_name, dev_basename);
+        if (*dev_basename != '\0') {


ucs_string_is_empty

evgeny-leksikov · 2026-02-02T21:25:12Z

src/ucp/core/ucp_context.c

                                      tl_cfg_mask));
 }

+static UCS_F_ALWAYS_INLINE void


why UCS_F_ALWAYS_INLINE?

evgeny-leksikov · 2026-02-02T21:29:05Z

src/ucp/core/ucp_context.c

 }

+static UCS_F_ALWAYS_INLINE void
+ucp_get_dev_basename(const char *dev_name, char *dev_basename_p)


I would suggest to move it to ucp_device.<h|c> files + rename to ucp_device_name_get_base

pls revert this change. ucp_device is about GPU initiated APIs and in that context device stands for GPU.

also it is not API

brminich · 2026-02-03T08:32:51Z

test/gtest/ucp/test_ucp_context.cc

+    }
+
+    /* Get all mlx5 network device names from the context */
+    static std::set<std::string> get_mlx5_device_names(const entity &e) {


maybe now we can search for any device name containing : delimeter?
it does not have to be just mlx anymore

I generalized the tests to run on any device with : delimiter.
I only kept the specific "mlx" references in the warning tests because it's simpler to create the different cases this way.

src/ucp/core/ucp_context.c

src/ucp/core/ucp_device.c

src/ucp/core/ucp_context.c

brminich

👍 besides comment from Thomas

guy-ealey-morag · 2026-02-03T14:16:00Z

👍 besides comment from Thomas

Fixed

guy-ealey-morag requested a review from brminich January 23, 2026 16:37

guy-ealey-morag force-pushed the mlx-default-port branch 5 times, most recently from 66145ae to b3000e6 Compare January 23, 2026 17:02

iyastreb reviewed Jan 26, 2026

View reviewed changes

src/uct/api/uct.h Outdated Show resolved Hide resolved

src/ucp/core/ucp_context.c Outdated Show resolved Hide resolved

guy-ealey-morag requested a review from iyastreb January 27, 2026 09:59

guy-ealey-morag added 5 commits January 27, 2026 12:37

UCP/CORE: Allow specifying mlx devices without a port in UCX_NET_DEVICES

3dd14bd

UCP/CORE: Add range detection for mlx devices

cb8a423

UCP/CORE: Do base name and suffix calculation on the fly

94d32e7

UCP/CORE: Fix closing bracket detection for ranges

db68bd1

UCP/CORE: Add unit tests for NET_DEVICES

77411b4

guy-ealey-morag force-pushed the mlx-default-port branch from 4110644 to 77411b4 Compare January 27, 2026 10:37

brminich reviewed Jan 28, 2026

View reviewed changes

UCP/CORE: Fix PR comments

e3be8e9

guy-ealey-morag force-pushed the mlx-default-port branch from 7dc8adb to e3be8e9 Compare January 28, 2026 13:35

guy-ealey-morag requested a review from brminich January 28, 2026 13:59

evgeny-leksikov reviewed Jan 28, 2026

View reviewed changes

src/ucp/core/ucp_context.c Outdated Show resolved Hide resolved

evgeny-leksikov reviewed Jan 28, 2026

View reviewed changes

test/gtest/ucp/test_ucp_context.cc Outdated Show resolved Hide resolved

evgeny-leksikov reviewed Jan 29, 2026

View reviewed changes

src/ucp/core/ucp_context.c Outdated Show resolved Hide resolved

UCP/CORE: Fix PR comments

838ec0f

guy-ealey-morag force-pushed the mlx-default-port branch from 1ace8ff to 838ec0f Compare January 29, 2026 09:24

UCP/CORE: Refactor to not reference mlx in UCP

ea17b0b

evgeny-leksikov reviewed Jan 30, 2026

View reviewed changes

UCP/CORE: PR comment fixes

795b01b

guy-ealey-morag force-pushed the mlx-default-port branch from ea7be81 to 0335732 Compare February 2, 2026 14:32

UCP/CORE: Remove device range logic

cd8fdcf

guy-ealey-morag force-pushed the mlx-default-port branch from 0335732 to cd8fdcf Compare February 2, 2026 15:07

guy-ealey-morag requested a review from evgeny-leksikov February 2, 2026 16:29

evgeny-leksikov reviewed Feb 2, 2026

View reviewed changes

brminich reviewed Feb 3, 2026

View reviewed changes

UCP/CORE: Fix PR comments

477f1c1

guy-ealey-morag requested review from brminich and evgeny-leksikov February 3, 2026 10:09

tvegas1 reviewed Feb 3, 2026

View reviewed changes

src/ucp/core/ucp_context.c Outdated Show resolved Hide resolved

tvegas1 reviewed Feb 3, 2026

View reviewed changes

src/ucp/core/ucp_device.c Outdated Show resolved Hide resolved

src/ucp/core/ucp_device.c Outdated Show resolved Hide resolved

UCP/CORE: Fix PR comments

7a09f2f

guy-ealey-morag requested a review from tvegas1 February 3, 2026 11:51

UCP/CORE: move ucp_get_dev_basename back to ucp_context.c

edeb647

tvegas1 reviewed Feb 3, 2026

View reviewed changes

src/ucp/core/ucp_context.c Outdated Show resolved Hide resolved

brminich reviewed Feb 3, 2026

View reviewed changes

UCP/CORE: Fix PR comment

799894a

tvegas1 approved these changes Feb 3, 2026

View reviewed changes

brminich approved these changes Feb 3, 2026

View reviewed changes

brminich merged commit f3a1ae1 into openucx:master Feb 4, 2026
147 checks passed

		EXPECT_GT(count, 0) << "Expected at least one device with base name '"
		<< test_base_name << "' to be selected";

		ucs_assert(dev_name_suffix > resource->dev_name &&
		dev_name_suffix <= resource->dev_name + strlen(resource->dev_name));

		memcpy(dev_basename_p, dev_name, basename_len);
		dev_basename_p[basename_len] = '\0';

Conversation

guy-ealey-morag commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

Uh oh!

Uh oh!

Uh oh!

guy-ealey-morag commented Jan 26, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evgeny-leksikov left a comment

Choose a reason for hiding this comment

Uh oh!

guy-ealey-morag commented Feb 2, 2026

Uh oh!

evgeny-leksikov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guy-ealey-morag Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brminich Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brminich left a comment

Choose a reason for hiding this comment

Uh oh!

guy-ealey-morag commented Feb 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

guy-ealey-morag commented Jan 23, 2026 •

edited

Loading

guy-ealey-morag Feb 3, 2026 •

edited

Loading

brminich Feb 3, 2026 •

edited

Loading