Skip to content

Firefly: intra-node LoopReq drops all but the first send segment, causing copyIoVec copied == len abort on multi-rank-per-node collectives #2686

Description

@deanchester

Summary

ProcessQueuesState::copyIoVec() aborts on the assertion copied == len
(ctrlMsgProcessQueuesState.cc) when an Allgather-class collective is run with multiple
ranks bound to a single endpoint NIC.

The root cause is a specific defect in the intra-node loopback delivery path: the
LoopReq message wrapper keeps only the first data segment of a multi-segment send, while
still reporting the full multi-segment byte count in its header. When the sender's I/O
vector has more than one segment, copyIoVec runs out of source bytes before copied reaches
len, and the assertion fires. The collective send/recv windows themselves were verified to
be byte-symmetric; the truncation happens on the receive side of the loopback path.

Root cause

Intra-node sends are delivered through the loopback link. processSendLoop packs the match
header plus every data segment of the sender's I/O vector into a single vector:

// ctrlMsgProcessQueuesState.cc:272-275  (processSendLoop)
std::vector<IoVec> vec;
vec.insert( vec.begin(), hdrVec );                  // vec[0] = MatchHdr
vec.insert( vec.begin() + 1, req->ioVec().begin(),  // vec[1..N] = ALL data segments
                                    req->ioVec().end() );

On the receive side, the LoopReq constructor keeps only the first data segment
(vec[1]) and discards vec[2..N]:

// ctrlMsgProcessQueuesState.h:223-228  (LoopReq)
LoopReq(int _srcCore, std::vector<IoVec>& _vec, void* _key ) :
    Msg( (MatchHdr*)_vec[0].addr.getBacking() ),
    srcCore( _srcCore ), vec(_vec), key( _key)
{
    m_ioVec.push_back( vec[1] );   // <-- only vec[1]; vec[2..N] are dropped
}

However, the MatchHdr carried by the LoopReq (vec[0]) still describes the total
payload across all segments. At delivery:

// ctrlMsgProcessQueuesState.cc:674-681  (processShortList_3)
size_t length = ctx->hdr().count * ctx->hdr().dtypeSize;   // = TOTAL bytes (all segments)
...
copyIoVec( req->ioVec(), ctx->ioVec(), length );           // src = LoopReq.m_ioVec = [vec[1]] only

copyIoVec walks src, which now contains only the first segment. copied stalls at
vec[1].len, but len is the full multi-segment total, so copied < len and the assertion
aborts.

Why a sender produces multiple segments

Allgather::initIoVec() (funcSM/allgather.cc:199-243) coalesces only physically
contiguous
chunks into a single IoVec. In the recursive-doubling Allgather, a stage's send
window spans numChunks = 2^stage chunks, and the window wraps around the modular buffer
(recvStart = mod((rank+1) + offset*(size-2), size)). A wrapped window is split into two (or
more) non-contiguous runs, so initIoVec emits two or more IoVec segments.

As soon as a stage's send window is multi-segment and the peer is intra-node, the LoopReq
truncation bites.

Why the observed trigger conditions match exactly

  • Multi-rank-per-node / high PPN (≥56): the loopback path is taken only for intra-node
    peers (enterSendprocessSendLoop when m_nic->isLocal(...)). At high PPN almost every
    recursive-doubling neighbour is on-node, so multi-segment loop sends are exercised
    constantly and the failure is reliable.
  • 131072-element / 2-ranks-per-node: with 2 ranks/node the single on-node neighbour is hit
    every iteration, and a large payload makes the wrapped send window large and almost
    certainly multi-segment — so it reproduces even at PPN = 2.
  • NIC-independent: the defect is entirely within libfirefly.so's loopback control plane;
    the NIC model is never involved.
  • PingPong / small collectives do not trip it: single-segment sends place the entire
    payload in vec[1], so copied == len holds and the bug cannot manifest.

Contrast with the network short path

ShortRecvBuffer (ctrlMsgProcessQueuesState.h:208) also does
m_ioVec.push_back( ioVec[1] ), but there ioVec[1] is a single contiguous DMA buffer sized
to the whole message — one segment is correct. The loopback path reuses the same
single-segment assumption but feeds it a genuinely multi-segment sender I/O vector, which is
where the assumption breaks.

Reproduction

sst <allgather_sdl>.py -- --shape=4,1:4 --ppn=56 --count=1 --iterations=10

Any configuration placing a large number of MPI ranks on a single NIC and running an
Allgather-class collective reproduces it. Point-to-point and small-rank-count collectives do
not.

Symptoms

  • Sub-second abort: Assertion failed: (copied == len), function copyIoVec, file ctrlMsgProcessQueuesState.cc.
  • Stack trace through libfirefly.so: ... -> processShortList_3 -> copyIoVec.
  • Simulation produces no result line and exits non-zero.
  • Under NDEBUG (release builds) the assert is compiled out, so instead of aborting the
    copy loop's assert( rV < dst.size() ) is also gone and the code silently produces a
    short/incorrect receive buffer — a latent data-correctness hazard, not just a crash.

Fix

Primary fix — preserve all segments in LoopReq. The constructor must copy every data
segment, not just vec[1]:

// ctrlMsgProcessQueuesState.h  (LoopReq)
LoopReq(int _srcCore, std::vector<IoVec>& _vec, void* _key ) :
    Msg( (MatchHdr*)_vec[0].addr.getBacking() ),
    srcCore( _srcCore ), vec(_vec), key( _key)
{
    // vec[0] is the MatchHdr; vec[1..N] are the sender's data segments.
    // A wrapped / non-contiguous send window (e.g. a recursive-doubling
    // Allgather stage) produces multiple segments; keeping only vec[1]
    // truncates the payload so copyIoVec under-copies (copied < len).
    for ( size_t i = 1; i < vec.size(); i++ ) {
        m_ioVec.push_back( vec[i] );
    }
}

Defence-in-depth — make copyIoVec memory-safe and diagnosable. Independently of the
LoopReq fix, harden copyIoVec so a future under-sizing cannot corrupt memory under
NDEBUG:

  • Bound the copy loop on rV < dst.size() in the loop condition (not via assert, which is
    compiled out under NDEBUG).
  • Replace the bare assert( copied == len ) with a dbg().fatal(...) that reports copied,
    len, dst.size(), src.size(), total destination capacity, and total source bytes, so
    any residual mismatch is a loud, greppable diagnostic rather than an opaque abort or silent
    corruption.

A silent clamp is not appropriate here: sender and receiver are the same collective
algorithm, so a mismatch is an internal defect, and quietly truncating would hide incorrect
collective results.

Impact

Blocks simulation of high-PPN Allgather (and related multi-segment intra-node) collectives.
Any campaign sweeping ranks-per-node must mark those cells as unavailable. Under release
builds the same defect risks silent data corruption rather than a clean abort.

Environment

  • sstsimulator/sst-elements, firefly element, master.
  • Loopback delivery path in ProcessQueuesState; defect in LoopReq
    (ctrlMsgProcessQueuesState.h:223-228), surfacing in copyIoVec
    (ctrlMsgProcessQueuesState.cc:1045).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions