Repository navigation
Port-forward relay sets TCP_NODELAY on both of its sockets - #1350
Merged
Merged
Conversation
The relay behind a routed VM's published ports (and --forward-localhost and the proxy relay) copied bytes with Nagle on. A write that follows one the peer has not acknowledged then waits for the peer's delayed ACK: 40 ms on Linux, longer on macOS. A renderer frame is TLS records written back to back, so through the relay every frame's tail waited for that ACK. Measured on the staging wall from a Linux client, frame arrival minus the frame's scene time, renderer clock offset removed: the fcvm tiles over the host's IPv4 slot ports (this relay) arrived 30 to 35 ms later than over their routed IPv6 addresses; the EC2 tiles arrived the same over both. Test: a_message_written_in_two_parts_crosses_the_relay_without_waiting_for_an_ack writes 16 KB then 12 KB through the relay to a client with quick ACK off. Without TCP_NODELAY every one of 30 messages took 40.7 to 44.9 ms (the client's delayed-ACK timer); with it the test passes under 15 ms. make test-root with tcp_proxy, port_forward and publish tests: 93 passed. make lint clean.
Owner
Author
|
@codex review |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 22 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
ejc3
added a commit
to ejc3/aws
that referenced
this pull request
Oct 26, 2026
ami-0084954a5dfe26bf6 (47ec5fcf) -> ami-00d8ecc3edfb8059e (2c965afe), which adds ejc3/fcvm#1350: the relay behind a GPU host's IPv4 slot ports sets TCP_NODELAY, so a frame's second TLS record no longer waits for the browser's delayed ACK. The renderer source is unchanged from 31f42d03, so the tile AMI stays. Checked on a g4dn.metal launched from the new AMI: store restored from S3, a routed GPU restore within 4 to 8 ms of the host for 20 s, chronyd with an IPv6 NTP source.
ejc3
added a commit
to ejc3/aws
that referenced
this pull request
Oct 27, 2026
…P_NODELAY) (#699) * fcvm-fleet: host and tile AMIs from ejc3/fcvm main 47ec5fcf On the wall's staging page every fcvm tile was drawn dimmed as behind the common time: each fcvm VM's clock was 0.2 to 1.0 s behind the host. The host AMI (bdaa0003) predates ejc3/fcvm#1301, so its fc-agent stepped a restored guest to the whole second, and a routed guest reached no NTP server to correct it (ejc3/fcvm#1343). - us-east-1 host AMI ami-0913aacc80394e1bd -> ami-0084954a5dfe26bf6, built from ejc3/fcvm main 47ec5fcf (fleet/ami/host, revised from the previous image; store under store/fcvm-fleet-host-x86_64-47ec5fcf-20261026/). - us-east-1 tile AMI ami-08f3cc41395353a2f -> ami-07bf015c4588aebc2, revised from the previous image with the host AMI's renderer image (same image id), so both halves draw with one renderer. Checked on a g4dn.metal launched from the new host AMI: store restored from S3 (16 parts, 9.5 s), a routed GPU restore landed within 5 ms of the host and chronyd selected an IPv6 NTP server; a clock set 700 ms back was within 7 ms again 10 s after chronyd's next poll. The tile builder's renderer reached its first frame (image phase). Tested: python3 -S -B scripts/test-fcvm-fleet.py OK, terraform fmt -check. * fcvm-fleet: host AMI from ejc3/fcvm main 2c965afe ami-0084954a5dfe26bf6 (47ec5fcf) -> ami-00d8ecc3edfb8059e (2c965afe), which adds ejc3/fcvm#1350: the relay behind a GPU host's IPv4 slot ports sets TCP_NODELAY, so a frame's second TLS record no longer waits for the browser's delayed ACK. The renderer source is unchanged from 31f42d03, so the tile AMI stays. Checked on a g4dn.metal launched from the new AMI: store restored from S3, a routed GPU restore within 4 to 8 ms of the host for 20 s, chronyd with an IPv6 NTP source. --------- Co-authored-by: ejc3 <ejc3@users.noreply.github.com>
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
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.
The port-forward relay sets TCP_NODELAY on its client and upstream sockets.
Why
The staging wall's fcvm tiles reach a browser without IPv6 through the host's IPv4 slot ports, which this relay serves (routed mode). The relay copied with Nagle on, so a write following one the peer had not acknowledged waited for the peer's delayed ACK (40 ms on Linux, longer on macOS). A renderer frame is TLS records written back to back, so every frame's tail waited.
Measured on staging from a Linux client: frame arrival minus the frame's scene time, minus each renderer's clock offset (its
/healtht):On the owner's Mac (IPv4 only) the fcvm frames arrived 142 to 191 ms after their scene time against 36 to 48 ms for EC2; about 100 ms of that is the fcvm guests' clocks, which #1343 addresses.
Change
spawn_relay_loopsets TCP_NODELAY on both streams beforecopy_bidirectional. A failure is logged and the relay continues as before. The endpoints keep their own settings.Evidence
a_message_written_in_two_parts_crosses_the_relay_without_waiting_for_an_ack: 16 KB then 12 KB through the relay to a client with quick ACK turned off. On main all 30 messages took 40.7 to 44.9 ms (observed failing twice); with the change the median is under 15 ms.make test-root FILTER="-E 'test(/tcp_proxy|port_forward|publish/)'": 93 passed (rootless, bridged and routed port forwards, clone port forwards, the stress test).make lintclean.Contract and impact
Contract: the relay adds no coalescing delay. Host-side; staging gets it with the next fleet host AMI.