Skip to content

UCP/CORE: disable everything (aux as well) when ^ib is configured #10579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

amastbaum
Copy link
Contributor

@amastbaum amastbaum commented Mar 24, 2025

What?

Setting UCX_TLS=^ib now disables all ib transports, including auxiliary ones.
For other aliases, auxiliary disabling must be explicitly specified.

Why?

Based on #8778, when disabling ud based transports, they should still be available as auxiliary. Thus, UCX_TLS=^ud does not disable their auxiliary usage.

However, we still want UCX_TLS=^ib to be treated as a global exclusion of all ib transports, including auxiliary ones.
To support this, a new distinction now differentiates global aliases (e.g. ib) from specific aliases (e.g. ud, rc, etc.).

@amastbaum amastbaum added the WIP-DNM Work in progress / Do not review label Mar 24, 2025
@amastbaum amastbaum force-pushed the disable_aux_implicitly_when_not-ib_configured branch from bbf15ac to 3e602d9 Compare March 25, 2025 08:42
@amastbaum amastbaum requested review from gleon99 and yosefe March 25, 2025 09:08
@amastbaum amastbaum added Ready for Review and removed WIP-DNM Work in progress / Do not review labels Mar 25, 2025
@amastbaum amastbaum requested review from rakhmets and gleon99 March 25, 2025 16:01
@amastbaum amastbaum force-pushed the disable_aux_implicitly_when_not-ib_configured branch from f0412bd to 8c57b9a Compare March 26, 2025 10:06
rakhmets
rakhmets previously approved these changes Mar 26, 2025
@yosefe
Copy link
Contributor

yosefe commented Mar 31, 2025

can we add some gtest for it?

@yosefe
Copy link
Contributor

yosefe commented Mar 31, 2025

can we add some gtest for it?

or extend

run_ucx_tl_check() {
to test it?

@pentschev
Copy link
Contributor

I can confirm this resolves the original issue, now UCX_TLS=^ib disables ka(ud_verbs/...) without requiring the additional auxiliary to be disabled explicitly with UCX_TLS=^ib,ud:aux.

Thanks everyone for working on this!

@amastbaum amastbaum force-pushed the disable_aux_implicitly_when_not-ib_configured branch from 8c57b9a to ea48471 Compare April 7, 2025 12:54
rakhmets
rakhmets previously approved these changes Apr 7, 2025
if (override):
os.putenv("UCX_NUM_EPS", "2")

with _override_env("UCX_TLS", tls):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we extract common code with find_am_transport?

@amastbaum amastbaum requested review from gleon99 and yosefe April 14, 2025 07:15
@amastbaum amastbaum requested a review from yosefe April 14, 2025 09:58
@amastbaum amastbaum force-pushed the disable_aux_implicitly_when_not-ib_configured branch from 3320a51 to 19ca9ff Compare April 14, 2025 14:33
@yosefe yosefe enabled auto-merge April 14, 2025 14:50
@yosefe yosefe added this to the v1.19.0 milestone Apr 14, 2025
@yosefe yosefe merged commit aa7d109 into openucx:master Apr 15, 2025
151 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants