fix(minvmd): carry libkrun vsock patches, fixing bulk host->guest uploads - #884

Takes the 12,784,144-byte project upload from 0/6 to 12/12. The credit-floor
patch this PR originally carried was refuted by measurement and has been
dropped; what replaces it is a different fix, aimed at the constraint that
actually binds.

Counts, per build

Every batch ran on the same dev stack (just up), the same minvmd binary, and
libkrun built from the same pinned commit with the same script — only the dylib
differed, swapped under a stopped VM and confirmed by lsof on the live VMM
process before and after each batch.

The two debug rows are a confound, not a result: enabling libkrun's logger
perturbs timing enough to mask the bug on both builds. It is recorded because
it nearly produced a false positive, and because it disqualifies per-packet
logging as a diagnostic here.

Note the third row: the IRQ fix alone does not fix the upload. The fill-loop
does.

What the measurement showed

Per-packet logging dissolves the bug, so the discriminator was counters only —
no locks, allocation, or formatting on the data path, one summary per connection
at teardown, with FIONREAD sampled immediately before each recv() so
available bytes are directly comparable with max_len.

Instrument validated before being believed: it preserved the failure at 6/6.

Aggregate over the six failing 12.8 MB uploads — 40,749 packets:

Representative connection:

pkts=6828 bytes=12944350 min=16 max=3732 mean=1895
recv_calls=6828 under1k=2453 wait_credit=2
socket_limited=5069 credit_limited=1755 neither=4
avail_mean=2189 credit_mean=236528 inflight_at_close=262144 buf_alloc=262144
  payload_hist: 2^5:10 2^6:401 2^7:171 2^8:139 2^9:902 2^10:830 2^11:1492 2^12:2883
  avail_hist:   2^0:6  2^5:10  2^6:400 2^7:169 2^8:141 2^9:901 2^10:826 2^11:1492 2^12:1231 2^13:1652

The two histograms track each other almost exactly up to 2^11 (401/400,
171/169, 902/901, 830/826, 1492/1492): we read precisely what the socket
happened to hold. That is the signature of socket-buffer-limited. They diverge
only at the top, where buf.len() (~4 KiB) caps a read that could have taken
4–8 KiB — the descriptor, still not credit.

Credit had ~109× more headroom than the socket had data, and the zero-credit
path fired 10 times in 40,749 reads. The peer's window was never the binding
constraint, which is exactly why raising a credit floor did nothing.

One honest caveat on the counter names: credit_limited is computed against
max_len = min(buf.len(), peer_credit) and so conflates the two. Given
credit_mean ≈ 240 KB against a ~4 KiB buf.len(), that bucket is
descriptor-full, not credit-limited. Read it that way.

The fix (0002)

recv_to_pkt() issued one recv() per RX descriptor and shipped whatever that
call happened to find. Against a bulk sender delivering the stream in chunks,
the muxer outruns the writer and fragments it. Each fragment then costs the peer
a whole skb of queue accounting — a Linux receiver charges every queued packet
SKB_TRUESIZE(0) (576 B on arm64) against buf_alloc regardless of payload —
so sub-KiB packets burn queue budget far faster than they deliver bytes.

The patch loops until the descriptor is full or the socket returns EAGAIN.

On the level-triggered trap: polling registers EV_ADD without EV_CLEAR
(src/utils/src/macos/epoll.rs), so anything that declines to read available
data spins. A fill-loop is safe precisely because it never leaves readable data
behind on purpose: every exit either drained the socket or filled a descriptor,
so the next notification corresponds to genuinely new data or to a descriptor we
can immediately make progress on. A threshold-and-defer design would have spun —
which is what the dropped credit-floor patch did, and why it needed 0001 to stop
hanging.

EOF and errors encountered after some bytes are read deliver the data first and
re-report on the next call.

The IRQ fix (0001)

Independent of the upload bug and worth upstreaming on its own. A proxy that
runs out of credit sets WaitingCreditUpdate, emits VSOCK_OP_CREDIT_REQUEST,
and disarms polling on its host fd. But push_packet() only places the request
in the used ring — the IRQ comes solely from process_proxy_update() when
ProxyUpdate::signal_queue is set, and that is taken from recv_pkt()'s
have_used. When the first packet of a batch is the one that has to wait,
have_used is false: the peer is never woken, never sends the credit update,
and polling is already disarmed on our side.

Reachable on unpatched upstream, just rare — a batch normally pushes data
packets first, and those set signal_queue themselves. It was not rare under
the credit-floor patch, which turned it into a multi-minute hang; that hang is
how it was found. tsi_stream.rs has the identical construct and the identical
gap. Both fixed.

Build wiring

scripts/build-libkrun-macos.sh had no patch step. It now applies every
*.patch in vendor/libkrun/patches/, sorted, with git apply, after the
pinned commit is fetched and its SHA verified. Verified both directions: the
real patches apply cleanly and build; a deliberately broken patch aborts with
exit 1 and ::error::9999-bogus.patch does not apply to libkrun 728df812….
A silently-skipped patch is impossible.

Instrumentation: not in this PR, deliberately

The counter patch is a debugging aid, not shippable code. It writes to a file
path from an env var, bypasses log!/debug! on purpose (the macros are inert
unless the embedder installs a logger, and installing one masks the bug), and
adds a FIONREAD syscall per read. My recommendation is that it does not
belong in the repo — it has no value except while this specific question is
open, and a checked-in debug patch in vendor/libkrun/patches/ would be applied
to every macOS build by the very mechanism this PR adds. It is preserved in the
investigation record; ask if you want it carried somewhere.

Trustworthiness of the A/B

• Dev minvmd is ad-hoc signed (flags=0x2(adhoc), TeamIdentifier=not set) with
  no hardened runtime, so library validation does not apply — that is what makes
  swapping a locally built dylib possible at all.
• LIBKRUN_PREFIX pointed at a private scratch dir. /opt/homebrew was never
  touched and never appeared in minvmd's rpath, and no libkrun* sat next to the
  binary, so neither a Homebrew copy nor @loader_path could mask a swap.
• Every batch prints the live minvmd path and lsofs the dylib actually mapped
  into the __krun-vmm process, before and after; mapped size tracked every swap.
• The stock control was rebuilt from the same pinned commit with the same script
  rather than reusing the shipped dylib, so arms differ only by the patches.
• Dev stack provably distinct from installed: Server: minimald 0.5.0-rc1 vs
  0.5.0-rc2.dev.10.g4bfdcb09.
• A concurrent process respawned the installed minvmd mid-run once, and the smoke
  test happily talked to it. Caught by checking the live binary; every batch now
  verifies before and after.

Verified vs assumed

Verified

• The bridge port is served by UnixAcceptorProxy → UnixProxy::new_reverse in
  unix.rs, not muxer.rs's UnixProxy::new path.
• push_packet() does not raise the IRQ; only process_proxy_update does. The
  resulting stall was observed and the fix removed it.
• Reads are socket-occupancy-limited, not credit-limited (numbers above).
• Fill-loop: 12/12 pass across two independent batches, against 6/6 fail on
  stock and 6/6 on stock+IRQ-fix+instrumentation.
• Instrument does not mask the bug (6/6 fail with it).
• Both patches apply to 728df812… and build; a stale patch aborts the build.
• Machine restored: installed dylib sha e2e8ebca… unchanged, installed stack
  live, Server: minimald 0.5.0-rc2.dev.10.g4bfdcb09, no MINVMD_* overrides,
  all validation sessions destroyed.

Assumed / not verified

• SKB_TRUESIZE(0) == 576 on arm64 and the 455-skb ceiling — carried from the
  earlier investigation, not re-derived. The fix does not depend on the exact
  number, only on fragmentation being costly to the receiver.
• No after-histogram. I did not rebuild instrumentation on top of the
  fill-loop to show packet sizes rising. The mechanism is inferred from the
  before-numbers plus the fix working, not directly observed post-fix.
• Throughput impact unmeasured. The fill-loop should reduce syscalls and packets
  per byte, but no benchmark was run.
• Only exercised on the dev guest (minimald 0.5.0-rc1, .scratch
  kernel/rootfs/initramfs). Not run against the installed guest, which cannot
  load an unsigned dylib.
• 12/12 is a strong signal but this is a timing-sensitive failure; a soak would
  be more convincing than two batches of six.
• tsi_dgram.rs shares the one-recv-per-descriptor shape; untouched, untested.
• The fill-loop is applied only to UnixProxy. tsi_stream.rs has the same
  pattern and likely the same fragmentation, but it does not serve this port and
  was left alone to keep the change reviewable.

Summary by CodeRabbit

• Bug Fixes

  • Improved macOS virtual socket reliability by ensuring queued updates trigger promptly.
  • Improved data transfer efficiency by filling receive buffers more effectively, reducing unnecessarily small packets.
  • Preserved correct handling of interruptions, drained sockets, connection closures, and receive errors.

• Build

  • macOS builds now consistently apply required compatibility patches and fail with clear diagnostics if a patch cannot be applied.