A p2p dial's stream survives the peer session under it being rebuilt,
so the link can go silently dead: the far side re-creates its session
while this side's stream becomes a zombie — writes succeed into the
void and nothing reports an error. firstInboundTimeout was a one-shot,
cleared by the first inbound byte and never re-armed, so such a dial
stayed "running" forever: on 2026-10-17 an Android spoke sat in a
two-way blackhole for 17 minutes after the hub killed and rebuilt its
relay session alone (derp peer-gone kill), while both ends reported
the path up; only a manual entrypoint restart healed it.
The registration refresh is echoed by the hub while the route to this
device is real, so silence is measurable. A p2p dial now re-arms the
read deadline to p2pInboundTimeout on every inbound byte; when it
expires the dial ends and the caller redials, which re-registers and
heals a hub-side rebuild — the same recovery a manual restart performs
(observed as "peer relay session up … client=true"). Socket dials
keep the one-shot semantics: their transport reports the far side gone
by itself.
The no-route report named the destination of the first unrouted packet
the process ever saw: the field was set while the window was empty and
never rewritten. A hub's own MLD noise (ff02::16) arrives early and
usually wins that race, so every later window blamed it -- including the
windows made of a peer's downlink being discarded, which is how a lost
registration stayed invisible in the log.
Set the window's destination when the window opens, and clear it with the
window, so each report names the miss it describes.
A spoke registers the addresses it holds by writing a handshake frame to
the hub, and the hub routes return traffic by that mapping. The handshake
ran once per dial, which was safe while a stream ending was what told a
spoke its peer was gone: the dial failed, the loop dialed again, and the
registration was re-sent.
A p2p dial no longer works that way. Its stream is kept across a peer
session rebuild, so the hub can drop the route while the dial still reads
as alive -- and then every return packet is discarded for want of a
route, silently at both ends. The link still reports itself "direct" and
healthy, the uplink keeps working, and only a manual restart of the
entrypoint brings the downlink back.
Re-send the handshake on a fixed interval when the dial target is a p2p
peer (network "ip"). A socket dial keeps the one-shot: its transport
reports the far side gone by itself.
waterutil.IsIPv4 and IsIPv6 index packet[0] with no length check, and a
tun device can legally return zero bytes with no error — an Android
VpnService fd does exactly that. routing such a read into a classifier
panics with "index out of range [0] with length 0", and in the c-shared
library the phone loads the runtime cannot unwind past the cgo boundary,
so the panic took the app: the spoke died on 2026-10-12 with three such
lines in crash.log, no stack, and a 17-minute gap before Android
restarted it.
The knowledge was already in the tree — sourceOf and destinationOf in
handler/tun/p2p.go carry the guard, with a comment naming this exact
panic — but the eight reads in the client and server loops and the
association classifier in handler/router did not. The asymmetry was the
bug.
The guard now lives once, in internal/util/tun, as IPv4Packet and
IPv6Packet, and every classifier goes through it. Tested at the real
entry point: a fake device that returns (0, nil) before handing over a
real IPv4 packet, asserted to be survived and forwarded.
The hub drops non-IP frames instead of writing them to the tun device,
where the kernel rejects them with EINVAL and takes the service down.
That guard was correct but silent about what it was dropping: a 4-byte
frame was seen in the field and the bytes are the only way to tell a
tagged-stream leak from anything else.
channelTag "P2PU" is exactly 4 bytes, so a tag leak is the obvious
suspect, but the sequence that preceded the sighting -- an untagged
inbound delivery after a 2s peek timeout -- was later reproduced 23
times, including forced mobile-data switches on the phone, without
producing another one. The source stays unconfirmed either way, so the
guard keeps logging up to 8 head bytes to make the next occurrence
identifiable from the log alone.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A frame that is not a parseable IPv4 or IPv6 packet is exactly what the
kernel refuses. Measured on a real up tun device: the version nibble is
the only thing it inspects, and anything but 4 or 6 comes back EINVAL at
any length -- 1421, 1500 and 2000-byte frames with a valid header all go
through untouched. That error left fromSpoke, unwound Handle, and took a
54-minute peer stream down with it.
The spoke has always refused these frames before its device write, which
is why a phone logs "unknown packet, discarded" all day and never dies
while the hub had no equivalent. destinationOf returning !ok is exactly
the kernel's reject condition, so that branch now drops and counts the
frame instead of handing it to the device.
This stops the malformed frame from killing the service; it does not fix
what put one on the wire. The candidate cause is concurrent serveEdge
writers racing on one local conn with no send lock on streamConn.Write,
which is still unproven. Silently accepted sub-MTU spliced frames also
remain -- the kernel does not validate TotalLen either.
A hub that decides which peer LANs exist is handed this handler as a
handler.Handler, so the table has to be reachable by shape: the route type
is exported and the p2p handler takes the same call the socket one does,
installing into the peerTable its packet path already reads.
The p2p hub wrote every spoke packet to its tun device, relying on the
host kernel's same-interface forwarding to hairpin it back to the
destination peer. Hosts without that forwarding silently dropped
peer-to-peer traffic. fromSpoke now resolves the destination through
the route table first and delivers to the owning peer's stream;
unroutable, self-addressed, and hub-local packets keep the device path.
Tests cover direct delivery (v4/v6/large), self-send drops, unrouted
fallback, and hub-own-address fallback. The suite passes under -race
except TestRunDeviceProbeReportsSent, which races on a clean tree.
A hub that refuses a spoke registration (bad passphrase, allowlist,
self-loop) used to stay silent and keep the stream: the spoke sat
'running' with zero traffic and nothing to look at. Return
ErrRegistrationRefused instead so the stream is torn down — the spoke
redials and re-registers, which also recovers on its own once the hub
is fixed. Malformed frames and bare echoes still return nil and leave
the stream up, so transport noise cannot turn into a reconnect storm.
Without a keepalive period the client reader had no deadline at all and
waited forever on a silent link. Bound it with a 30s first-inbound
one-shot (firstInboundTimeout, cleared by any inbound byte); the
keepalive>0 path is untouched.
A running tun entrypoint cannot tell a dead device fd from an idle link: both look like frozen stats. The probe writes a self-addressed IPv4/UDP packet (spokeIP->spokeIP, magic+seq) into the device every 30s and reads it back on its own bound socket; sent/acked deltas go to the caller through the metadata probeReport callback. Enabled by metadata probe (default off, gost path unchanged); new probeSent/probeAcked counters (101/102) in x stats.
A 20546-byte spoke datagram hit wbuf[:writeOffset+len(p)] (cap 16400)
and panicked the whole hub process. Three parts:
- tunDevice.Write rejects packets bigger than its buffers with the new
exported ErrPacketTooLarge instead of reslicing past the end. The
kernel would have refused the write anyway (tun MTU 1420), so nothing
deliverable is lost.
- Buffers follow the device MTU (max 16KB): newTunDevice(ifce, mtu),
so raising tun.mtu for legitimate jumbo UDP automatically makes such
packets writable. The relay/KCP/stream path already passes 64KB.
- fromSpoke drops an oversized datagram with a windowed count and
keeps the peer stream up; only a real device failure still tears
the stream down.
Tests: tun write guard, normal passthrough, large-MTU accept, and
stream survival with rate-limited warnings; all written first and
watched fail.
An unrouted destination under load produced one warn line per packet,
burying the event that caused the routing gap. Count misses and report
the total once per window instead; the first miss is still logged at
once.
Also log how long a peer stream lived and how much arrived on it when
it ends, so a dying peer's story (failed to connect vs failed mid-
transfer) is visible without correlating route drops.
Also ignore .memory/.
A registration handshake or keepalive echo is protocol, not a packet,
and must never reach the device. The client read loop only special-cased
the 20-byte bare echo, so a 20+N*16-byte registration frame from the
peer was misread as IPv4 ('G' is 0x47) and injected into the kernel
device as junk. The hub (p2pHub.fromSpoke) and the socket server already
filter this shape; the client now does too.
Covered by TestTransportClientDropsKeepaliveFrames, which fails against
the old shape (the device receives the frame verbatim).
The comment above newPeerTable claimed "the authorizer is an option rather
than an argument", written when NewP2PHandler did not take one. It now does,
so the comment contradicted the signature directly above it, and it still
referred to an "auther argument" that no longer exists.
Both facts are true but at different levels, which is what made the old
wording hard to read: the authorizer is a variadic option on newPeerTable
(whose other callers all mean "no address authorization"), while
NewP2PHandler takes it as an argument so a hub's owner has one place to
pass it. The auther is nil throughout, since the p2p peer allowlist has
already decided who may connect.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A tun client registers its device's addresses with the far side's tun
server by sending the handshake frame. That frame is what tells the server
which addresses this device holds, and it is the only thing that can: a
server routes by destination address, so without it the link is one-way —
traffic leaves and nothing comes back.
keepalive() has always sent it unconditionally and returned early when no
period was configured, so the period only ever decided whether it
repeated. The call site did not: it was gated on the udp network or a
configured period, which on a p2p link ("ip") meant no period, no
handshake, no registration at all. That is the shape a p2p hub needs
least — its route table has no TTL to refresh, so the repeat buys nothing
there and the one-shot is the whole of it.
The gate is gone; the handshake always runs. The period keeps its meaning
where it has one: a server reached over UDP is connectionless, so silence
is the only signal that a peer has left, and the repeat is what lets its
route expire.
Found by a user asking why the keepalive switch was needed at all. It was
not, on a p2p hub — and turning it off silently produced the one-way link
the entrypoint's own comment warns about.
p2p.go was a complete engine with nothing driving it: no handler.Handler,
so no GOST service could reach the hub and a spoke's registration frame had
no path to one at all.
NewP2PHandler takes the tun device conn and an auther, builds the peerTable
(no TTL — a stream close says a peer is gone outright, where a socket peer
has only silence to guess from), the peerRouter and the p2pHub, and starts
the device reader. Handle installs one accepted stream under its remote
address — the peer key the p2p host stamped on it, replacing whatever a
reconnect left behind — and feeds its datagrams to the hub until the read
fails. Teardown is peerGone and nothing else: it reclaims a peer's routes
only while that stream still holds its key, so a stale stream ending cannot
cut off the live connection its peer already replaced it with.
ownNets comes off the device conn's context, the two steps server.go's Handle
already uses, because a nil answer silently disables the self-route guard.
Handle returns the read error unwrapped. io.EOF, net.ErrClosed and
context.Canceled are what isServerError calls an ordinary end, so a peer
reconnecting would otherwise land in someone's error log.
The p2p half of the compat suite now runs against the real handler and the
real hub rather than reporting itself unavailable: the same six assertions,
the same two cases skipped without an auther. A second p2p case covers the
reconnect, which is the only way the suite would notice a peerGone that
dropped routes unconditionally — a hub that did so passes every shared case.
The socket server and the p2p hub are two answers to the same question, and
the design rests on a spoke being unable to tell which one it reached: the
same frame, the same magic, the same passphrase, the same registered
addresses, the same reply it refreshes its read deadline from. A spoke
configured for one is supposed to work against the other.
That claim is only worth something if the *same* assertions run against
both. Two separately-written suites would be allowed to drift, and nobody
would notice until someone deployed one against the other — which is a
field failure with nothing in any log to explain it. So the assertions are
written once here, against a compatHub that names no transport, and each
case is run once per implementation. The duplication that is left is
structural: how to build a hub and how it delivers, which is the part that
genuinely differs.
The socket half drives server.go's own transportServer over a real UDP link
rather than a copy of its loops; a compat suite that asserted against a
reimplementation would only prove the reimplementation agreed with itself.
The p2p half skips, and says why: nothing yet turns an inbound gRPC Tunnel
stream into a peerStream and feeds it to p2pHub.fromSpoke, so a spoke's
registration frame has no path to that hub. The skip is visible and the
six shared cases are named in it, because an absent assertion is not a
skipped one.
Three deliberate divergences turn the suite red, each from a real behavior
rather than a stub: registering a route for a frame the auther refused,
delivering to whichever peer spoke last, and echoing nothing after a
registration that took.
The route table answers a destination with a name, and the socket server
turns that name into a UDP address. A p2p hub needs the other answer: the
name is a peer key, and the thing it resolves to is a stream, not an
address. That is the whole of the second delivery — resolveUDPAddr is
unreachable here by construction, and putting it back on this path would be
a design error rather than a bug.
The shape is forced by the device, not chosen. tunDevice shares one read
buffer and one write buffer across every call and takes no lock: two
readers would have the second overwrite the first's target mid-call, and two
writers would copy through the same write buffer at once. It is safe today
only because the socket server happens to run one goroutine per side. A p2p
hub runs one per peer, so it has to reach the same guarantee a different
way: run() is the only goroutine that reads, and wmu is the only way to
write. This is the socket server's shape arrived at from the other side —
not a bridge per stream. Which goroutine read a packet carries no meaning,
because the route is what says where it goes.
Two locks, for two different hazards. The device's wmu guards the shared
device buffer. peerStream's own mu guards one peer's stream, whose conn
frames every write onto a byte stream and serializes only its read buffer:
two writers would put one frame's length prefix against the other's
payload, and every frame length would then be read from the wrong offset.
The -race run is what proves both are doing their job.
withdraw removes a stream only while it still holds its key. A peer that
reconnected already installed a replacement under that name, so reclaiming
routes for it on the old stream's teardown would cut off the live
connection — which is what makes a flapping peer unreachable rather than
merely briefly absent.
The keepalive is answered on the stream it arrived on, because the table
registers but never writes. Drop the echo and every keepalive:true spoke
misses the reply its read deadline is refreshed from and redials on its own
timeout, with nothing in any log: client.go's registration gate tests for
"udp" and a p2p link is "ip", so a spoke that never registers is already
indistinguishable from silence. Hence the warning naming the destination.
The concurrency tests are mutation-checked: with the device lock removed the
first fails on overlapping writes in round 0, and with the stream lock
removed the second fails on a spliced frame. Both were caught by the
assertions, not only by the race detector — the first harness was a
datagram pipe, which is atomic per message and so passed with the lock gone
until it was replaced by a byte stream, which is what the p2p conn really
is.
Its keepalive handling, route lookups and both delivery loops move onto
peerTable unchanged: the same magic-header reply, the same gateway
fallback, the same discard for an unrouted packet. Only the route's
value is now a name, resolved at the point of delivery. A test that
needed updating to pass would mean the socket path moved when it must
not have, so the line-by-line diff is the evidence here — no test in
this package exercises server.go.
The route table's reverse index is gone. It was a second copy of a fact
already in the route's name, and two maps cannot be made consistent:
concurrent set calls could leave the index naming one peer while the
route belonged to another, and dropPeer would then delete the winner's
route on the loser's teardown. dropPeer now scans the routes themselves
and deletes only entries still owned by the peer it is dropping.
A lock would have been the wrong fix: lookup runs per delivered packet
and dropPeer once per stream close, so the cost would have landed on the
hot path to settle a race that only matters at reclamation.
The keepalive protocol, the auther check, the TTL and destination lookup
are the same whichever way datagrams arrive, so they belong together and
know nothing about transport. A route's value becomes a string: a UDP
address for the socket server, a peer key for the p2p hub. net.Addr fit
neither — it is an endpoint, and one of these is not.
dropPeer is the p2p half of reclamation. The socket server expires a
route by TTL, guessing from silence that a peer is gone; a stream close
says so exactly, so the hub drops a peer's routes when its stream ends
and keeps no timer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stopping a service closes its listener, and a closed listener answers Accept
with core's listener.ErrClosed — which the guard never matched (it knew only
net.ErrClosed), so every stop reached the log at error level and read like a
crash. The tun handler logged the same shape a second time: its client wrapped a
closed device with %s, throwing the cause away, so the caller could not tell a
device closed under the reader from a real read failure. %w keeps the chain and
the handler logs the closed case at debug.
Three fixes found while wiring the p2p hub topology:
- The client keepalive was gated on network == "udp", but on a p2p link
raddr is the peer's base64 key (no port), so SplitHostPort fails and
network is "ip": the keepalive never ran, a tun server never learned the
client's route, and the read deadline was never armed. Gate on the udp
network OR a configured keepalive period; the udp branch keeps the one-shot
registration handshake it always sent.
- Liveness was refreshed only by the keepalive echo; a transient channel
hiccup can drop that single packet and redial a healthy tunnel after
3xperiod. Refresh the read deadline after any successful read instead.
- Routes were never removed: a departed client's IP -> addr stayed forever
and the device kept blackholing to a dead address. Expire a route once its
last keepalive is older than 3x the configured period, checked lazily on
the write path (no sweeper). The TTL is opt-in: without a server-side
keepalive/ttl, routes keep their current never-expiring behavior.
OpenTunnel carries a network: "udp" asks the plugin for a datagram endpoint
(one datagram per write) instead of a byte stream, which is what a tun
handler needs — a stream endpoint coalesces and splits packets. tcp stays
the default, so existing deployments are untouched.
The tun handler could not reach client mode without a service forwarder, so
a chain with no forwarder now means client mode with an empty dial address:
the link is transparent and the peer comes from the chain node (a p2p node's
addr is its peer key). That combination used to be dead config, so it is
logged. A forwarder still wins when present, and server mode is unchanged.
Alongside: the udp dialer no longer hard-asserts *net.UDPConn (a wrapped
base dialer yields a plain conn), udp joins the p2p dialer whitelist, and a
udp tunnel announces its client with an empty datagram right after dialing —
the endpoint learns where to send peer packets only from a packet it
receives, so without it a link whose peer speaks first would never come up.
Requires plugin v0.7.0 for the new field.
transportClient and transportServer each launch two goroutines for
bidirectional copy between the TUN device and the remote connection.
Previously, when one goroutine errored, the function returned
immediately while the sibling goroutine continued running — reading
from and writing to connections that would soon be closed by the
caller's deferred cleanup. In the retry loop of handleClient and
handleServer, stale goroutines accumulated across iterations, racing
with new goroutines for the TUN device and writing to closed pipes.
This caused the "io: read/write on closed pipe" error reported in
go-gost/gost#345, and more importantly, prevented recovery because
subsequent retry iterations operated on a corrupted state.
Fix:
- Derive a per-transport context in transportClient/transportServer
and check it at the top of each goroutine's loop to allow the
first-failing goroutine to signal its sibling to exit
- Replace the single-error select with a collectFirstError helper
that drains both goroutines and includes a timeout guard (5s) to
prevent deadlock when a goroutine is stuck in a non-cancelable read
- Use a per-iteration context in handleClient for the keepalive
goroutine so it is properly cancelled when an iteration ends
- Fix handleServer to use errors.Is(err, ErrTun) for consistency
- Fix transportServer's errc buffer from 1 to 2 to prevent a
goroutine leak when both goroutines error simultaneously
ginuerzh