From 7ee5e2975804703d317cdd47e0bf481d153c6cd6 Mon Sep 17 00:00:00 2001 From: JackDoan Date: Wed, 29 Jul 2026 17:48:47 -0500 Subject: [PATCH] docs: align comments with the code they describe - multi_coalesce/batch: the ordering contract now states what Flush actually guarantees -- per-flow DATA order -- and names the two shapes later data may legally overtake (pure ACKs by design, and unparseable in-flow shapes as an accepted tradeoff). - validVnetHdr claimed DATA_VALID makes the stack skip L4 checksum verification; the tun write path ignores that bit entirely. What the header buys is the absence of NEEDS_CSUM. - tun_darwin Write said "only valid for single threaded use"; it is concurrency-safe and concurrent callers exist. - udp_coalesce eviction comment said "Seal it" but never sets sealed. - recordCapability: note the gauges are process-global while the state is per-socket (last writer wins). - drop a stale tunReadBufSize reference. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014ugV2edVqoz3tBvq9J6yWp --- overlay/batch/batch.go | 7 ++++++- overlay/batch/multi_coalesce.go | 19 +++++++++++++++---- overlay/batch/udp_coalesce.go | 3 ++- overlay/tio/tio_gso_linux.go | 11 +++++++---- overlay/tun_darwin.go | 4 +++- udp/udp_linux.go | 5 +++++ 6 files changed, 38 insertions(+), 11 deletions(-) diff --git a/overlay/batch/batch.go b/overlay/batch/batch.go index 217c62f1..d34f7751 100644 --- a/overlay/batch/batch.go +++ b/overlay/batch/batch.go @@ -5,7 +5,12 @@ import "net/netip" type RxBatcher interface { // Commit commits pkt to be flushed by the batch. The caller must keep pkt valid until the next Flush, and not re-use it. Commit(pkt []byte) error - // Flush emits every queued packet in arrival order. + // Flush emits every queued packet. The guarantee is per-flow DATA order: + // a flow's payload-bearing packets are never reordered relative to each + // other. Cross-flow and cross-lane order is not preserved, and two shapes + // may legally be overtaken by later same-flow data: pure ACKs (by design, + // stale ACKs are ignored) and unparseable shapes such as fragments (an + // accepted tradeoff; see MultiCoalescer). // Returns the first error observed; keeps draining so one bad packet doesn't hold up the rest. // After Flush returns, committed payload slices may be recycled. Flush() error diff --git a/overlay/batch/multi_coalesce.go b/overlay/batch/multi_coalesce.go index 603589d4..aff08449 100644 --- a/overlay/batch/multi_coalesce.go +++ b/overlay/batch/multi_coalesce.go @@ -13,10 +13,21 @@ import ( // // Lanes are processed independently: the TCP coalescer only sees TCP, the // UDP coalescer only sees UDP, and the passthrough lane handles everything else. -// Per-flow delivery order is preserved because a single 5-tuple only -// ever lands in one lane and each lane preserves its own slot order. -// Routing follows the flow, not the coalesceability: IPv4 fragments keep -// their L4 proto visible and IPv6 extension chains are walked to the +// The ordering contract is per-flow DATA order: a flow's payload-bearing +// packets are never reordered relative to each other, because a single +// 5-tuple only ever lands in one lane and each lane emits its slots in +// creation order. Two shapes are deliberately allowed to be overtaken by +// later same-flow data: +// - pure ACKs, which pass through without sealing the flow's open slot +// (a late ACK is just a stale ACK; see TCPCoalescer.commitParsed); +// - unparseable in-flow shapes (fragments, IP options), whose lane-level +// addPassthrough does not close the flow's open slot either. Closing it +// would need a full open-slot barrier (the flow key is unknown when the +// parse fails) — an accepted tradeoff: mid-flow fragments are rare and +// receivers reassemble regardless of arrival order. +// +// Routing still follows the flow, not the coalesceability: IPv4 fragments +// keep their L4 proto visible and IPv6 extension chains are walked to the // terminal proto, so a flow's non-coalesceable shapes ride its lane as // in-lane passthroughs rather than falling to the later-flushed pt lane. // diff --git a/overlay/batch/udp_coalesce.go b/overlay/batch/udp_coalesce.go index d6727e79..0657bbba 100644 --- a/overlay/batch/udp_coalesce.go +++ b/overlay/batch/udp_coalesce.go @@ -160,7 +160,8 @@ func (c *UDPCoalescer) commitParsed(pkt []byte, info parsedUDP) error { } return nil } - // Can't extend. Seal it and fall through to seed a fresh slot. + // Can't extend: evict it from openSlots and fall through to seed a + // fresh slot. (Eviction only; sealed is never set here.) delete(c.openSlots, info.fk) if c.lastSlot == open { c.lastSlot = nil diff --git a/overlay/tio/tio_gso_linux.go b/overlay/tio/tio_gso_linux.go index 1f0e9707..eed43429 100644 --- a/overlay/tio/tio_gso_linux.go +++ b/overlay/tio/tio_gso_linux.go @@ -20,7 +20,7 @@ import ( const maxSuperpacketLen = 65535 -// tunRxBufSize is the per-Read worst-case footprint inside rxBuf: one kernel-supplied packet body, which is at most ~64 KiB (tunReadBufSize). +// tunRxBufSize is the per-Read worst-case footprint inside rxBuf: one kernel-supplied packet body, which is at most ~64 KiB. // Segmentation happens at encrypt time on a per-routine MTU-sized scratch // (see SegmentSuperpacket), so rxBuf only holds raw kernel-supplied bytes. // We round up to give margin for the drain headroom check below. @@ -46,9 +46,12 @@ const tunDrainCap = 64 const gsoMaxIovs = 256 // validVnetHdr is the 10-byte virtio_net_hdr we prepend to every non-GSO TUN write. -// Only flag set is VIRTIO_NET_HDR_F_DATA_VALID, which marks the skb CHECKSUM_UNNECESSARY -// so the receiving network stack skips L4 checksum verification. -// All packets that reach the plain Write paths already carry a valid L4 checksum, so trusting them is safe. +// Only flag set is VIRTIO_NET_HDR_F_DATA_VALID. Note the tun write path +// (__virtio_net_hdr_to_skb) ignores this bit — only the virtio-net driver's RX +// helper honors it — so packets land CHECKSUM_NONE and the stack verifies the +// L4 checksum anyway. What matters here is what the header does NOT say: +// no NEEDS_CSUM, so the kernel is never asked to finish a checksum. +// All packets that reach the plain Write paths already carry a valid L4 checksum. var validVnetHdr = [virtio.Size]byte{unix.VIRTIO_NET_HDR_F_DATA_VALID} // Offload wraps a TUN file descriptor with poll-based reads. The FD provided will be changed to non-blocking. diff --git a/overlay/tun_darwin.go b/overlay/tun_darwin.go index 368182a9..1076b687 100644 --- a/overlay/tun_darwin.go +++ b/overlay/tun_darwin.go @@ -550,7 +550,9 @@ func (t *tun) Read(to []byte) (int, error) { return n - 4, nil } -// Write pushes one IP packet onto the utun device. Only valid for single threaded use. +// Write pushes one IP packet onto the utun device. Safe for concurrent use: +// the AF prefix and iovecs are per-call stack state, and the fd write itself +// serializes on the runtime's fd mutex (see the Queue contract in tio.go). func (t *tun) Write(from []byte) (int, error) { if len(from) == 0 { return 0, syscall.EIO diff --git a/udp/udp_linux.go b/udp/udp_linux.go index 854aa03c..ea7bc070 100644 --- a/udp/udp_linux.go +++ b/udp/udp_linux.go @@ -162,6 +162,11 @@ func (u *StdConn) prepareECNRecv() { // it is not — dashboards can show degraded state on partially-supported // kernels at a glance. Calling repeatedly with the same name updates the // existing gauge rather than registering a duplicate. +// +// Caveat: the gauge is process-global while the capability state it reports +// is per-socket. With multiple listen routines the last probe wins, and a +// runtime downgrade on one socket (e.g. the GSO EIO disable) flips the gauge +// for all of them. Treat it as "at least one socket looks like this." func recordCapability(name string, enabled bool) { g := metrics.GetOrRegisterGauge(name, nil) if enabled {