Repository navigation
fix(rccl): net-ib-cast: enable by_order for no cts-offload path (drop 2026 08) #11172
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
Changes from all commits
7d6e624
5a2c302
e8510c1
a8c91bc
b5c8b49
c6177c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -69,9 +69,10 @@ static inline bool IbCastIsCtsOffloadEnabled(int isP2p) { | |||||||||||||||
|
|
||||||||||||||||
| static int IbCastResolveRecvMatchingScheme(bool useCtsOffload) { | ||||||||||||||||
| // Order matters here: | ||||||||||||||||
| // BY_ORDER -> ctsoffload | ||||||||||||||||
| // BY_ID -> failover | ||||||||||||||||
| // BY_INDEX -> default or user requested | ||||||||||||||||
| // BY_ORDER -> CTS offload (forced), else AINIC/user request after remaps | ||||||||||||||||
| // BY_ID -> failover / OOO RQ (beats explicit or default BY_ORDER) | ||||||||||||||||
| // BY_INDEX -> default (non-AINIC) or user requested | ||||||||||||||||
| // Explicit SCHEME=BY_ORDER is not a hard force: same as AINIC default. | ||||||||||||||||
|
|
||||||||||||||||
| if (useCtsOffload) { | ||||||||||||||||
| return BY_ORDER; | ||||||||||||||||
|
|
@@ -82,12 +83,29 @@ static int IbCastResolveRecvMatchingScheme(bool useCtsOffload) { | |||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| int64_t requested = ncclParamIbCastReceiverSideMatchingScheme(); | ||||||||||||||||
| if (requested == -2 || requested == BY_ORDER) { | ||||||||||||||||
| return BY_INDEX; | ||||||||||||||||
| if (requested == -2) { | ||||||||||||||||
| return IbCastAinicRoce ? BY_ORDER : BY_INDEX; | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Defaulting to BY_ORDER here makes |
||||||||||||||||
| } | ||||||||||||||||
| return requested; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| extern int64_t ncclParamIbCastReceiverSideMatchingScheme(); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
| // Resolve live: IbCastOffloadEnabled is still mutated after the first call | ||||||||||||||||
| // (PORT_FAILOVER clears it in IbCastInitDevices). Caching BY_ORDER from the | ||||||||||||||||
| // pre-failover value would disable resiliency on comms that actually run BY_ID. | ||||||||||||||||
| bool IbCastByOrderRequested() { | ||||||||||||||||
| return IbCastResolveRecvMatchingScheme(IbCastOffloadEnabled) == BY_ORDER; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| void IbCastReportMatchingScheme() | ||||||||||||||||
| { | ||||||||||||||||
| INFO(NCCL_NET, "NET/IB-CAST: collectives communicators: CTS Offload: %s RecvMatchingScheme: %d", | ||||||||||||||||
| IbCastIsCtsOffloadEnabled(0)? "ON":"OFF" , IbCastResolveRecvMatchingScheme(IbCastIsCtsOffloadEnabled(0))); | ||||||||||||||||
| INFO(NCCL_NET, "NET/IB-CAST: P2P communicators: CTS Offload: %s RecvMatchingScheme: %d", | ||||||||||||||||
| IbCastIsCtsOffloadEnabled(1)? "ON":"OFF" , IbCastResolveRecvMatchingScheme(IbCastIsCtsOffloadEnabled(1))); | ||||||||||||||||
| } | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| ncclResult_t IbCastInitCommDevBase(int ibDevN, struct ncclIbNetCommDevBase* base, void* cq_context, int cqSize) { | ||||||||||||||||
| base->ibDevN = ibDevN; | ||||||||||||||||
| ncclIbDev* ibDev = IbCastDevs + ibDevN; | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,15 +23,32 @@ FILE* IbCastQpSchedLogStream; | |
| // 1. RCCL_IB_QP_SCHED_ENABLE / NCCL_IB_QP_SCHED_ENABLE explicitly set (!=−1) → honour it. | ||
| // 2. NCCL_NET=ib-cast explicitly set → force enable. | ||
| // 3. Otherwise → disable on AINIC (default path), enable everywhere else. | ||
| // BY_ORDER comms no-op WRR/split-data (IbCastQpSchedDoWrr returns immediately). | ||
| // An explicit ENABLE=1 still wins, but that combination is unsupported: warn so it | ||
| // is visible instead of silently running a scheduler that never remaps. | ||
| bool rcclUseIbCastQpSched() { | ||
| int64_t schedParam = rcclParamIbCastQpSchedEnable(); | ||
| const bool byOrder = IbCastByOrderRequested(); | ||
| if (schedParam != -1) { | ||
| bool enabled = (schedParam != 0); | ||
| INFO(NCCL_NET | NCCL_ENV, "(IB-CAST) NCCL_IB_QP_SCHED_ENABLE explicitly set to %s", | ||
| enabled ? "enabled" : "disabled"); | ||
| if (enabled && byOrder) { | ||
| WARN("NET/IB: BY_ORDER matching requested (NCCL_IB_RECEIVER_SIDE_MATCHING_SCHEME=%d) but the QP " | ||
| "scheduler is force-enabled (NCCL_IB_QP_SCHED_ENABLE=%ld): unsupported combination; " | ||
| "WRR/split-data are no-ops on BY_ORDER comms", | ||
| BY_ORDER, schedParam); | ||
| } | ||
| return enabled; | ||
| } | ||
|
|
||
| if (byOrder) { | ||
| WARN("NET/IB: BY_ORDER matching requested (NCCL_IB_RECEIVER_SIDE_MATCHING_SCHEME=%d): disabling the QP " | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This warning fires on the plain AINIC default path where the user set nothing, and it fires twice per init: once through init.cc:612 and once through |
||
| "scheduler, split-data and WRR", | ||
| BY_ORDER); | ||
| return false; | ||
| } | ||
|
|
||
| const char* netEnv = ncclGetEnv("NCCL_NET"); | ||
| if (netEnv && strcasecmp(netEnv, "ib-cast") == 0) { | ||
| INFO(NCCL_NET | NCCL_ENV, "(IB-CAST) NCCL_NET=ib-cast: forcing QP scheduler enabled"); | ||
|
|
@@ -180,6 +197,7 @@ void IbCastLogSched(struct ncclIbSendComm* comm) { | |
| } | ||
|
|
||
| void IbCastUpdateSchedParmsTry(struct ncclIbNetCommBase* base, int nreqs, int size) { | ||
| if (base->recvMatchingScheme == BY_ORDER) return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This early return is what currently keeps BY_ORDER working, which is the opposite of what the warning at scheduler.cc:36 claims it does. The receiver arms one WQE per QP across |
||
| if (!base->schedParmsInit) { | ||
| base->schedParms = castGlobalQpSchedParms; | ||
| base->schedParmsInit = true; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IbCastReportMatchingSchemeleaves the resolved scheme observable only through INFO, so nothing can assert which scheme a comm actually picked.ncclIbCastSchedState(net_ib_cast_inspect.h:27-39) is already filled byncclIbCastGetSchedState(scheduler.cc:435-459) from the samencclIbNetCommBasethat carriesrecvMatchingScheme(common_cast.h:505), so one int field there would make the default testable. That header is outside this PR, so this is a follow-up rather than a change to make here.