Skip to content

UCT/TCP: Fix inverted priority of the default network interface #12241

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.

Sign up for GitHub

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

Open
dmitrygx wants to merge 5 commits into
base: master
Choose a base branch
from
+38 −7
Open

UCT/TCP: Fix inverted priority of the default network interface #12241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1bad9a5
UCT/TCP: Fix inverted priority of the default network interface
dmitrygx Nov 4, 2026
fc5c6e2
UCT/TCP: Drop redundant prefer_default check in iface query
dmitrygx Nov 4, 2026
a83e1fb
GTEST/UCT: Skip prefer_default_disabled on non-default netdevs
dmitrygx Nov 4, 2026
0e432b1
UCT: Document priority order and use explicit TCP priority values
dmitrygx Nov 4, 2026
817b90d
UCT/TCP: Store the iface priority in netif_caps
dmitrygx Nov 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/uct/api/uct.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,8 @@ struct uct_iface_attr {
uct_ppn_bandwidth_t bandwidth; /**< Bandwidth model */
ucs_linear_func_t latency; /**< Latency as function of number of
active endpoints */
uint8_t priority; /**< Priority of device */
uint8_t priority; /**< Priority of device, a higher
value is preferred */
size_t max_num_eps; /**< Maximum number of endpoints */
unsigned dev_num_paths;/**< How many network paths can be
utilized on the device used by
Expand Down
2 changes: 1 addition & 1 deletion src/uct/tcp/tcp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ typedef struct uct_tcp_iface {
struct {
double latency; /* Netdev latency */
double bandwidth; /* Min of netdev and PCI BW */
int is_default; /* Netdev has the default route */
uint8_t priority; /* Iface priority */
} netif_caps;

struct {
Expand Down
15 changes: 10 additions & 5 deletions src/uct/tcp/tcp_iface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,7 @@ static ucs_status_t uct_tcp_iface_query(uct_iface_h tl_iface,
attr->bandwidth.dedicated = 0;
attr->latency.m = 0;
attr->overhead = 50e-6; /* 50 usec */
attr->priority = (iface->config.prefer_default &&
!iface->netif_caps.is_default) ? 1 : 0;
attr->priority = iface->netif_caps.priority;

return UCS_OK;
}
Expand Down Expand Up @@ -672,6 +671,7 @@ static ucs_status_t uct_tcp_iface_netif_caps_init(uct_tcp_iface_t *iface)
const char *sysfs_path;
char *path_buffer;
ucs_status_t status;
int is_default;

status = uct_tcp_netif_caps(iface->if_name, &iface->netif_caps.latency,
&network_bw);
Expand All @@ -691,12 +691,17 @@ static ucs_status_t uct_tcp_iface_netif_caps_init(uct_tcp_iface_t *iface)
iface->netif_caps.bandwidth = ucs_min(pci_bw, network_bw);

if (!iface->config.prefer_default) {
iface->netif_caps.is_default = 0;
iface->netif_caps.priority = 0;
return UCS_OK;
}

return uct_tcp_netif_is_default(iface->if_name,
&iface->netif_caps.is_default);
status = uct_tcp_netif_is_default(iface->if_name, &is_default);
if (status != UCS_OK) {
return status;
}

iface->netif_caps.priority = is_default ? 1 : 0;
return UCS_OK;
}

static UCS_CLASS_INIT_FUNC(uct_tcp_iface_t, uct_md_h md, uct_worker_h worker,
Expand Down
25 changes: 25 additions & 0 deletions test/gtest/uct/tcp/test_tcp.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,16 @@ class test_uct_tcp : public uct_test {
}
}

int netif_is_default()
{
const char *if_name = m_tcp_iface->if_name;
int is_default;

ASSERT_UCS_OK(uct_tcp_netif_is_default(if_name, &is_default));
UCS_TEST_MESSAGE << if_name << ": is_default " << is_default;
return is_default;
}

private:
void init_data(void *buf, size_t msg_size) {
uct_tcp_am_hdr_t *tcp_am_hdr;
Expand Down Expand Up @@ -310,5 +320,20 @@ UCS_TEST_P(test_uct_tcp, check_addr_len)
EXPECT_EQ(sizeof(uct_tcp_ep_addr_t), iface_attr.ep_addr_len);
}

UCS_TEST_P(test_uct_tcp, prefer_default)
{
/* UCP prefers the higher priority when transport scores are equal */
EXPECT_EQ(netif_is_default() ? 1 : 0, m_ent->iface_attr().priority);
}

UCS_TEST_P(test_uct_tcp, prefer_default_disabled, "TCP_PREFER_DEFAULT=n")
{
if (!netif_is_default()) {
UCS_TEST_SKIP_R("not the default network interface");
}

EXPECT_EQ(0, m_ent->iface_attr().priority);

@svc-nvidia-pr-review svc-nvidia-pr-review Nov 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this check is vacuous for non-default netdevs (e.g. tcp/lo), where priority is 0 either way. maybe query uct_tcp_netif_is_default() here too and skip if it is not the default netdev, so the test really covers the disabled path?

}


_UCT_INSTANTIATE_TEST_CASE(test_uct_tcp, tcp)
Loading