tun, conn, device: allocate buffers in the edge devices#49
Draft
tun, conn, device: allocate buffers in the edge devices#49
Conversation
496dc1a to
61b6c72
Compare
ea6f82a to
e981b07
Compare
raggi
reviewed
Mar 12, 2026
nickkhyl
reviewed
Mar 12, 2026
nickkhyl
reviewed
Mar 12, 2026
jwhited
reviewed
Mar 18, 2026
jwhited
reviewed
Mar 18, 2026
jwhited
reviewed
Mar 18, 2026
jwhited
reviewed
Mar 18, 2026
Member
|
I ran some local, single TCP stream benchmarks w/iperf3 on Ubuntu 24.04 w/Intel i5-12400 CPUs. 4184faf (what's currently used by tailscale/tailscale): RSS is much improved w/lots of streams (-P 32): 1GB+ vs ~200MB |
jwhited
reviewed
Mar 18, 2026
jwhited
reviewed
Mar 18, 2026
jwhited
reviewed
Mar 18, 2026
jwhited
reviewed
Mar 18, 2026
472f369 to
74691ad
Compare
d7467f5 to
f357c5a
Compare
f357c5a to
3e6d836
Compare
e016f9e to
5c07b88
Compare
Also fixes Put-before-Get underflow bug. Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com> Change-Id: Icde836d1bef2e95850d3ae51b3c069056a6a6964
5c07b88 to
1fca00f
Compare
On Linux NativeTun will write a vector of coalesced buffers via writev(2) instead of copying fragments into a single buffer. Updates tailscale/corp#36989 Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com> Change-Id: Id1e9cd3cc9435c240b7a28ae6528cd7e6a6a6964
eb07970 to
48e7cd2
Compare
c4597b0 to
ffd019f
Compare
ffd019f to
55464f0
Compare
There was a problem hiding this comment.
Pull request overview
This PR inverts packet-buffer ownership by introducing a device/buffer.Buffer wrapper (with optional recycling) and updating the TUN + UDP receive paths to allocate/initialize buffers at the I/O edges rather than in device.
Changes:
- Introduce
device/buffer(Buffer wrapper + default pooled backing arrays) and replace*[MaxMessageSize]byteownership in device queues withbuffer.Buffer. - Extract
WaitPoolinto a standalonewaitpoolpackage and update device pool wiring to use it. - Update
tun.Device.Readandconn.ReceiveFuncAPIs (and all platform implementations/tests) to use[]buffer.Bufferand return packet lengths viaBuffer.Dataslicing.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| waitpool/waitpool.go | New capped sync.Pool wrapper package used outside device. |
| waitpool/waitpool_test.go | Updated tests for new package name + API (New) and atomic type. |
| waitpool/race_enabled_test.go | Package rename to waitpool. |
| waitpool/race_disabled_test.go | Package rename to waitpool. |
| tun/tun.go | Updates tun.Device.Read signature/documentation to []buffer.Buffer. |
| tun/tun_linux.go | Updates Linux TUN read path to allocate/resize buffer.Buffer and adapt virtio/GSO flow. |
| tun/tun_windows.go | Updates Windows TUN read path to use buffer.Buffer. |
| tun/tun_darwin.go | Updates Darwin TUN read path to use buffer.Buffer. |
| tun/tun_freebsd.go | Updates FreeBSD TUN read path to use buffer.Buffer. |
| tun/tun_openbsd.go | Updates OpenBSD TUN read path to use buffer.Buffer. |
| tun/tun_plan9.go | Updates Plan 9 TUN read path to use buffer.Buffer. |
| tun/tuntest/tuntest.go | Updates tuntest device read helper to the new buffer API. |
| tun/netstack/tun.go | Updates netstack TUN wrapper read implementation to use buffer.Buffer. |
| tun/offload.go | Updates GSOSplit signature to operate on []buffer.Buffer and set lengths via Data slicing. |
| tun/offload_test.go | Updates fuzz test to allocate []buffer.Buffer outputs and new GSOSplit signature. |
| tun/offload_linux_test.go | Updates virtio read tests to use []buffer.Buffer and infer sizes from len(Data). |
| device/buffer/buffer.go | New Buffer type with Claim/Release + recycler hook for pooled backings. |
| device/buffer/pool.go | New default buffer pool + EnsureAllocated helper. |
| device/buffer/pool_test.go | Benchmark for capped vs uncapped pool usage. |
| device/buffer/constants.go | Defines max backing size used by the buffer pool. |
| device/buffer/constants_default.go | Default platform values for max read size + pool cap. |
| device/buffer/constants_android.go | Android-specific max read size + pool cap. |
| device/buffer/constants_ios.go | iOS-specific max read size + pool cap (var). |
| device/buffer/constants_windows.go | Windows-specific max read size + pool cap. |
| device/constants.go | Replaces MaxSegmentSize-based sizing with buffer.MaxReadSize. |
| device/pools.go | Switches device pools to waitpool and removes message-buffer pool in favor of buffer. |
| device/device.go | Updates pool field types to *waitpool.WaitPool. |
| device/send.go | Refactors outbound pipeline to claim/release buffer.Buffer ownership per element. |
| device/receive.go | Refactors inbound pipeline to claim/release buffer.Buffer ownership per element. |
| device/channels.go | Updates queue draining to release buffer.Buffer instead of returning message buffers. |
| device/device_test.go | Updates fake TUN device signature to match new read API. |
| device/queueconstants_default.go | Removes preallocation-related constants now owned by device/buffer. |
| device/queueconstants_android.go | Removes preallocation-related constants now owned by device/buffer. |
| device/queueconstants_ios.go | Removes preallocation-related constants now owned by device/buffer. |
| device/queueconstants_windows.go | Removes preallocation-related constants now owned by device/buffer. |
| conn/conn.go | Updates ReceiveFunc type to accept []buffer.Buffer. |
| conn/conn_test.go | Updates test type declaration for new ReceiveFunc signature. |
| conn/bind_std.go | Updates std net bind receive path to write into buffer.Buffer. |
| conn/bind_std_test.go | Updates std net bind test harness to use buffer.Buffer. |
| conn/bind_windows.go | Updates Windows ring bind receive path to use buffer.Buffer. |
| conn/bindtest/bindtest.go | Updates bind test receive function to allocate/write into buffer.Buffer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f6d8589 to
7c2ff1b
Compare
Previously crypto device maintained a batch of preallocated to the MaxMessageSize buffers that the I/O only needs to read into. This change inverts the buffer ownership. A (wrapped) nil pointer is passed into I/O. Device expects a backing array to be allocated, and a slice of read data cut to offset+size returned from the read within the same wrapper. The wrapper is defined in buffer.Buffer, and carries the backing reference and an optional Recycler implementation to return the backing for reuse. I/O is encouraged to implement a buffer management solution that works best for its host OS. A shared sync.Pool is provided as a default option. Updates tailscale/corp#36989 Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com> Change-Id: I58908d9d3fd09441e9378a74b0ee19136a6a6964
7c2ff1b to
996c482
Compare
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.
Stacked on top, and blocked by #54.
This is a part of series of changes. Future PRs will implement right-sized buffer allocations, this one is concerned only with the allocation inversion and buffer recycling logic.
Previously device maintained a batch of preallocated to
the MaxMessageSize buffers that the I/O only needed to
read into.
This change inverts the buffer ownership. A (wrapped) nil
pointer is passed into I/O. Device expects a backing
array to be allocated, and a slice of read data cut to offset+size,
returned from read wi thin the same wrapper.
The wrapper is defined in buffer.Buffer, and carries
the backing reference and an optional Recycler implementation
to return the backing for reuse.
I/O is encouraged to implement a buffer management solution
that works best for it. A shared sync.Pool is provided as
a default option.
Updates tailscale/corp#36989