fix(libkrun): carry the packet-count vsock backpressure patch - #512
Merged
Conversation
Bring the Linux libkrun package in line with the fix merged on the macOS source build (gominimal/minimal#926): replace the fill-loop + signal-queue patches with the single packet-count backpressure patch. The bug is the guest kernel's per-skb receive-queue ceiling: since Linux 6.12.92 it resets the connection (RST + ENOBUFS) once (skb_queue_len + 1) * SKB_TRUESIZE(0) > buf_alloc -- a second, packet-count receive ceiling the vsock credit protocol does not express. A bulk host->guest upload that fragments into sub-SKB_TRUESIZE(0) packets exhausts it with the byte window mostly unused. The patch meters packet count the same way byte credit is metered: recv_to_pkt() tracks the guest's skb_queue_len exactly (the tail rx_cnt of each emitted packet, retired as fwd_cnt passes it) and waits on the existing WaitForCredit path before emitting one packet too many. Unlike the fill-loop approach it does not change delivery granularity -- send() stays 1:1 with recv(); it is pure backpressure, invisible until the reader falls behind. It bundles the credit-request wakeup fix (push_credit_req must set signal_queue), which the count ceiling makes reachable during bulk transfer, so the two former patches collapse into one. Applies clean to the pinned v1.19.4 tarball via `patch -Np1`. The macOS build of the same patch passed 100/100 uploads with the guest kernel logging zero receive-queue resets. Refs: gominimal/minimal#869, gominimal/minimal#926
Contributor
📝 WalkthroughWalkthroughThe libkrun vsock patch now tracks outstanding guest packets, limits transmission using skb queue capacity, explicitly signals credit requests, and consolidates build integration onto one patch. ChangesVirtio-vsock buffering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UnixSocket
participant UnixProxy
participant Guest
UnixSocket->>UnixProxy: receive payload
UnixProxy->>Guest: emit RW packet and record rx_cnt tail
Guest->>UnixProxy: report peer_fwd_cnt
UnixProxy->>UnixProxy: drain forwarded packets
sequenceDiagram
participant UnixProxy
participant process_proxy_update
participant Guest
UnixProxy->>process_proxy_update: emit credit request with signal_queue
process_proxy_update->>Guest: signal used queue
Guest->>UnixProxy: provide credit update
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/libkrun/0001-vsock-bound-outstanding-packets.patch (1)
105-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for packet-accounting boundaries.
Please cover exact-tail draining, wrapping counters, the first packet at the ceiling boundary, and changes to
peer_buf_alloc. These cases determine whether the new backpressure path stalls or overruns the guest queue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/libkrun/0001-vsock-bound-outstanding-packets.patch` around lines 105 - 144, Add regression tests for drain_forwarded and skb_ceiling_reached covering exact-tail removal, wrapping rx/fwd counters, the first packet at the calculated ceiling boundary, and updates to peer_buf_alloc. Verify each boundary produces the expected outstanding queue state and backpressure decision, including recalculation after the allocation changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/libkrun/0001-vsock-bound-outstanding-packets.patch`:
- Around line 167-170: Update the recv_to_pkt method receiver from &self to &mut
self so its rx_cnt and outstanding mutations compile normally, and adjust any
callers to provide a mutable receiver as needed.
In `@packages/libkrun/build.sh`:
- Around line 10-14: Update the patch application command in build.sh for
0001-vsock-bound-outstanding-packets.patch to enforce zero fuzz with --fuzz=0
and reject any application that reports line offsets or fuzz before proceeding
with the build.
---
Nitpick comments:
In `@packages/libkrun/0001-vsock-bound-outstanding-packets.patch`:
- Around line 105-144: Add regression tests for drain_forwarded and
skb_ceiling_reached covering exact-tail removal, wrapping rx/fwd counters, the
first packet at the calculated ceiling boundary, and updates to peer_buf_alloc.
Verify each boundary produces the expected outstanding queue state and
backpressure decision, including recalculation after the allocation changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce2bbddc-264f-4d8d-8d21-6796e607fb95
📒 Files selected for processing (5)
packages/libkrun/0001-vsock-bound-outstanding-packets.patchpackages/libkrun/0001-vsock-signal-the-used-queue-when-requesting-credit.patchpackages/libkrun/0002-vsock-fill-the-rx-descriptor-instead-of-one-recv-per-packet.patchpackages/libkrun/build.nclpackages/libkrun/build.sh
💤 Files with no reviewable changes (2)
- packages/libkrun/0001-vsock-signal-the-used-queue-when-requesting-credit.patch
- packages/libkrun/0002-vsock-fill-the-rx-descriptor-instead-of-one-recv-per-packet.patch
Comment on lines
+167
to
+170
| + // One skb will be queued in the guest for this packet; | ||
| + // record its tail so drain_forwarded() can retire it | ||
| + // when the guest forwards past it. | ||
| + self.outstanding.push_back(self.rx_cnt); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
patch="packages/libkrun/0001-vsock-bound-outstanding-packets.patch"
rg -n -C2 'fn recv_to_pkt|rx_cnt \+=|outstanding\.push_back' "$patch"
if rg -q 'fn recv_to_pkt\(&self' "$patch" &&
rg -q '^\+.*outstanding\.push_back' "$patch"; then
echo "recv_to_pkt has an immutable receiver but mutates outstanding"
exit 1
fiRepository: gominimal/pkgs
Length of output: 968
🏁 Script executed:
#!/bin/bash
set -euo pipefail
patch="packages/libkrun/0001-vsock-bound-outstanding-packets.patch"
# Show the surrounding hunk with line numbers.
sed -n '120,190p' "$patch" | cat -n
# Inspect the file for the type of `self`-owned fields used in recv_to_pkt.
rg -n -C3 'struct .*|rx_cnt|outstanding|peer_avail_credit|init_data_pkt|recv_to_pkt' "$patch"Repository: gominimal/pkgs
Length of output: 8105
Make recv_to_pkt take &mut self. The body mutates rx_cnt and outstanding, so the current &self receiver won’t compile unless these fields use interior mutability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/libkrun/0001-vsock-bound-outstanding-packets.patch` around lines 167
- 170, Update the recv_to_pkt method receiver from &self to &mut self so its
rx_cnt and outstanding mutations compile normally, and adjust any callers to
provide a mutable receiver as needed.
Comment on lines
+10
to
+14
| # vsock fix (see build.ncl). Applied by explicit name, not by glob. `set -e` | ||
| # plus patch's non-zero exit on a rejected hunk makes a stale patch abort the | ||
| # build; a silently-skipped patch would publish a libkrun that looks fixed and | ||
| # is not. | ||
| patch -Np1 -i "0001-vsock-bound-outstanding-packets.patch" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- packages/libkrun/build.sh ---'
cat -n packages/libkrun/build.sh | sed -n '1,120p'
echo
echo '--- package files mentioning 0001-vsock-bound-outstanding-packets.patch ---'
rg -n "0001-vsock-bound-outstanding-packets\.patch|vsock fix|build\.ncl" packages/libkrun -S
echo
echo '--- patch file status/metadata ---'
ls -l packages/libkrun/*vsock* 2>/dev/null || trueRepository: gominimal/pkgs
Length of output: 2519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'build.sh exists: '; test -f packages/libkrun/build.sh && echo yes || echo no
printf 'patch exists: '; test -f packages/libkrun/0001-vsock-bound-outstanding-packets.patch && echo yes || echo no
if test -f packages/libkrun/build.sh; then
echo '--- build.sh ---'
sed -n '1,80p' packages/libkrun/build.sh
fi
if test -f packages/libkrun/0001-vsock-bound-outstanding-packets.patch; then
echo '--- patch head ---'
sed -n '1,80p' packages/libkrun/0001-vsock-bound-outstanding-packets.patch
fiRepository: gominimal/pkgs
Length of output: 5427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
lsRepository: gominimal/pkgs
Length of output: 304
Reject fuzzy or offset patch application. patch -Np1 -i ... can still succeed when GNU patch applies the hunk with fuzz or line offsets, so a drifted patch may build cleanly without guaranteeing this fix landed exactly. Use --fuzz=0 and fail on any offset/fuzz output before applying it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/libkrun/build.sh` around lines 10 - 14, Update the patch application
command in build.sh for 0001-vsock-bound-outstanding-packets.patch to enforce
zero fuzz with --fuzz=0 and reject any application that reports line offsets or
fuzz before proceeding with the build.
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the Linux libkrun package in line with the vsock upload fix that shipped on the macOS source build (gominimal/minimal#926, merged). Until now Linux carried the older fill-loop + signal-queue patches; this replaces both with the single packet-count backpressure patch.
The bug (host-OS-independent)
min activateuploads over the guest vsock connection fail mid-transfer withENOBUFS(gominimal/minimal#869). Since Linux 6.12.92,virtio_transport_inc_rx_pkt()resets the connection once(skb_queue_len + 1) * SKB_TRUESIZE(0) > buf_alloc— a second, packet-count receive ceiling that the vsock credit protocol (which meters bytes only) does not express. libkrun'sUnixProxyfragments a bulk stream into sub-SKB_TRUESIZE(0)packets (576 B on arm64), exhausting the count ceiling with the byte window mostly unused. The fragmenting code and the guest kernel are identical across host OSes, so Linux VM-backed sessions hit this exactly as macOS did.The fix
Meter packet count the same way byte credit is metered.
recv_to_pkt()tracks the guest'sskb_queue_lenexactly — the tailrx_cntof each emitted packet, retired asfwd_cntpasses it — and takes the existingWaitForCreditpath before emitting one packet too many, bounded by a conservativebuf_alloc / SKB_OVERHEAD.Unlike the previous fill-loop patch it does not change delivery granularity:
send()stays 1:1 withrecv(), nothing is delayed for batching. It is pure backpressure, invisible until the reader falls behind — the more defensible shape, and why #926 replaced the coalescing approach on the macOS side. It bundles the credit-request wakeup fix (push_credit_reqmust setsignal_queue), which the count ceiling makes reachable during bulk transfer, so the two former patches collapse into one.Changes
0001-vsock-signal-…and0002-vsock-fill-….0001-vsock-bound-outstanding-packets.patch(identical to the one ingominimal/minimalvendor/libkrun/patches/).build.sh: apply the single patch.build.ncl: oneLocalpatch dep; comment rewritten.Verification
v1.19.4tarball viapatch -Np1(the exact mechanismbuild.shuses) — dry-run touches onlysrc/devices/src/virtio/vsock/{unix.rs,tsi_stream.rs}.Note: I could not run a Linux
libkrun.sobuild locally (macOS host). The patch application and source are verified; the Linux package build itself will be exercised by CI here.Refs: gominimal/minimal#869, gominimal/minimal#926
Summary by CodeRabbit