Skip to content

docs(rsync_io): audit SSH socketpair vs anonymous pipes for stdio transport (#1938)#3525

Merged
oferchen merged 1 commit into
masterfrom
docs/ssh-socketpair-1938
May 1, 2026
Merged

docs(rsync_io): audit SSH socketpair vs anonymous pipes for stdio transport (#1938)#3525
oferchen merged 1 commit into
masterfrom
docs/ssh-socketpair-1938

Conversation

@oferchen
Copy link
Copy Markdown
Owner

@oferchen oferchen commented May 1, 2026

Closes #1938.

Summary

Formalizes the SSH stdio transport audit at docs/audits/ssh-socketpair-vs-pipes.md under tracker #1938. Consolidates prior #1686 working notes (PR #3438), the io_uring boundary documented for #1858 (PR #3418), and the stderr socketpair shipped under #1689 (PR #3383). Restructures the document into the formal seven-section layout (summary, upstream reference, current implementation, trade-offs, decision matrix, recommendation, implementation notes) plus findings and references.

Recommends keeping anonymous pipes for the SSH wire and not pursuing the socketpair migration. The decisive argument is that splice(2) / vmsplice(2) require one of the two FDs to be a pipe; oc-rsync's zero-copy roadmap in #1860 depends on the wire being a pipe so the file<->wire splice path remains direct. A socketpair-backed wire would force a double-splice through an intermediate user-space pipe(2), eliminating the zero-copy benefit. The "unified poll FD" advantage of socketpair only matters inside an event loop, and the async-transport refactor (#2068, #1655) consolidates readiness tracking regardless of primitive.

The audit closes the two pending follow-ups in this tracker family:

  • Fix missing imports in daemon tests #1687 (prototype SSH subprocess using socketpair for bidirectional I/O) - do not implement, justified by the splice eligibility argument and absence of measured backpressure stalls on pipes.
  • Allow cross-compile workflow entries to be disabled #1902 (verify SSH socketpair vs anonymous-pipe wire claim against rsync_io source) - verified: crates/rsync_io/src/ssh/builder.rs:300-301 uses Stdio::piped() for stdin and stdout (anonymous pipes); upstream uses socketpair(AF_UNIX, SOCK_STREAM, 0) via util1.c::fd_pair called from pipe.c::piped_child. The divergence is intentional.

What stays on the roadmap (untouched by this audit):

Test plan

  • No code changes; documentation-only PR.
  • All upstream citations re-verified against target/interop/upstream-src/rsync-3.4.1/ source files.
  • All crates/rsync_io/src/ssh/ citations verified against worktree line numbers.
  • No em-dashes; conventional docs: prefix; branch name docs/ssh-socketpair-1938.
  • CI: rustdoc renders without broken intra-doc links.
  • CI: fmt+clippy (no code changes, expected to pass).
  • CI: nextest (stable) (no code changes, expected to pass).
  • CI: Windows / macOS / Linux musl (no code changes, expected to pass).

…nsport (#1938)

Formalizes the SSH stdio transport audit at docs/audits/ssh-socketpair-vs-pipes.md
under tracker #1938, consolidating prior #1686 working notes (PR #3438), the
io_uring boundary documented for #1858 (PR #3418), and the stderr socketpair
shipped under #1689 (PR #3383). Restructures the document into the formal
seven-section layout (summary, upstream reference, current implementation,
trade-offs, decision matrix, recommendation, implementation notes) plus
findings and references.

Recommends keeping anonymous pipes for the SSH wire and not pursuing the
socketpair migration: splice(2) and vmsplice(2) require a pipe end, the
zero-copy plan in #1860 depends on that, and the unified-FD argument is
subsumed by the async-transport refactor (#2068, #1655). Closes #1687
(prototype socketpair wire) as do-not-implement and #1902 (verify wire
claim against rsync_io source) as verified.

Citations were re-checked against worktree source: builder.rs:300-301
(Stdio::piped on the wire), aux_channel.rs:263-285 (UnixStream::pair for
stderr, Unix only), connection.rs:30-39 (SshConnection struct),
mod.rs:57-75 (io_uring boundary). Upstream evidence verified against
target/interop/upstream-src/rsync-3.4.1/: pipe.c:48-97 (piped_child),
util1.c:74-96 (fd_pair), main.c:504-663 (do_cmd dispatch),
clientserver.c:116-148 (daemon TCP wire), socket.c:736-846 (sock_exec
test escape).
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 1, 2026
@oferchen oferchen merged commit b3ebe79 into master May 1, 2026
12 checks passed
@oferchen oferchen deleted the docs/ssh-socketpair-1938 branch May 1, 2026 21:07
oferchen added a commit that referenced this pull request May 5, 2026
…nsport (#1938) (#3525)

Formalizes the SSH stdio transport audit at docs/audits/ssh-socketpair-vs-pipes.md
under tracker #1938, consolidating prior #1686 working notes (PR #3438), the
io_uring boundary documented for #1858 (PR #3418), and the stderr socketpair
shipped under #1689 (PR #3383). Restructures the document into the formal
seven-section layout (summary, upstream reference, current implementation,
trade-offs, decision matrix, recommendation, implementation notes) plus
findings and references.

Recommends keeping anonymous pipes for the SSH wire and not pursuing the
socketpair migration: splice(2) and vmsplice(2) require a pipe end, the
zero-copy plan in #1860 depends on that, and the unified-FD argument is
subsumed by the async-transport refactor (#2068, #1655). Closes #1687
(prototype socketpair wire) as do-not-implement and #1902 (verify wire
claim against rsync_io source) as verified.

Citations were re-checked against worktree source: builder.rs:300-301
(Stdio::piped on the wire), aux_channel.rs:263-285 (UnixStream::pair for
stderr, Unix only), connection.rs:30-39 (SshConnection struct),
mod.rs:57-75 (io_uring boundary). Upstream evidence verified against
target/interop/upstream-src/rsync-3.4.1/: pipe.c:48-97 (piped_child),
util1.c:74-96 (fd_pair), main.c:504-663 (do_cmd dispatch),
clientserver.c:116-148 (daemon TCP wire), socket.c:736-846 (sock_exec
test escape).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant