[PR #170] fix: send the first packet with the configured transport padding #184

New issue

Open

opened 2026-10-13 21:04:22 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 21:04:22 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amneziawg-go/pull/170
Author: @anton-vinogradov
Created: 8/3/2026
Status: 🔄 Open

Base: master ← Head: fix/transport-padding-first-packet

📝 Commits (1)

e449d12 fix: send the first packet with the configured transport padding

📊 Changes

2 files changed (+29 additions, -0 deletions)

View changed files

📝 device/device_test.go (+13 -0)
📝 device/send.go (+16 -0)

📄 Description

The problem

With S4 set, the tunnel loses its first data packet.

device.TestAWGDevicePing shows it on current master, no changes needed:

$ go test ./device/ -run TestAWGDevicePing -race=false -count=1
--- FAIL: TestAWGDevicePing (12.00s)
    --- FAIL: TestAWGDevicePing/ping_1.0.0.1 (6.00s)
        device_test.go:245: ping did not transit
    --- FAIL: TestAWGDevicePing/ping_1.0.0.2 (6.00s)
        device_test.go:248: pong did not transit

The same test passes on v0.2.19. Running one parameter at a time shows s4 is the only one that breaks it - jc, jmin, jmax, s1, s2, s3 and h1-h4 are all fine. Sending three pings in a row shows only the first is lost:

--- FAIL: TestIsoS4TwoPings/first
--- PASS: TestIsoS4TwoPings/second
--- PASS: TestIsoS4TwoPings/third

Why

RoutineReadFromTUN reads the padding once and then blocks in Read with an offset computed from it:

for {
    padding := device.paddings.transport.Load()
    offset := MessageTransportHeaderSize + int(padding)

    count, readErr = device.tun.device.Read(bufs, sizes, offset)

The routine is started by NewDevice, so it enters that first Read before IpcSet has applied S4. The packet lands at offset 16, while the peer - configured by then - looks for the message type at offset 16+S4.

Logging the receiving side confirms it. S4 is 25, and the type is at offset 0 instead of 25:

determine: size=64 transportPadding=25 headerRange=4 raw4=04 00 00 00
Received message with unknown type

Only the first packet is affected, because the next pass of the loop reads the padding again.

This is a change from 0.2.x: there the padding was applied in RoutineSequentialSender, at send time, so it was always current. The 3.0 refactor moved it to read time and put it before the blocking call.

The cost in practice is one TCP retransmit on the first connection after the device comes up, which is easy to miss but is still a lost packet on every start.

The change

Re-read the padding after Read returns, and move the packets if it changed. This covers a live S4 change over UAPI as well, not only start-up. The copy only runs when the value actually changed, so the normal path is one atomic load.

Verification

the new test fails without the change and passes with it
device.TestAWGDevicePing and device.TestTwoDevicePing pass with the change
go test ./... is green, and go test -race ./device/ is green

Found while adding AmneziaWG 3.0 support to a userspace client, where the first
connection through a fresh tunnel was reliably a second slower than the rest.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/amnezia-vpn/amneziawg-go/pull/170 **Author:** [@anton-vinogradov](https://github.com/anton-vinogradov) **Created:** 8/3/2026 **Status:** 🔄 Open **Base:** `master` ← **Head:** `fix/transport-padding-first-packet` --- ### 📝 Commits (1) - [`e449d12`](https://github.com/amnezia-vpn/amneziawg-go/commit/e449d1249ed0a9b12720d8a03486bd54333ca418) fix: send the first packet with the configured transport padding ### 📊 Changes **2 files changed** (+29 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `device/device_test.go` (+13 -0) 📝 `device/send.go` (+16 -0) </details> ### 📄 Description ## The problem With `S4` set, the tunnel loses its first data packet. `device.TestAWGDevicePing` shows it on current `master`, no changes needed: ``` $ go test ./device/ -run TestAWGDevicePing -race=false -count=1 --- FAIL: TestAWGDevicePing (12.00s) --- FAIL: TestAWGDevicePing/ping_1.0.0.1 (6.00s) device_test.go:245: ping did not transit --- FAIL: TestAWGDevicePing/ping_1.0.0.2 (6.00s) device_test.go:248: pong did not transit ``` The same test passes on `v0.2.19`. Running one parameter at a time shows `s4` is the only one that breaks it - `jc`, `jmin`, `jmax`, `s1`, `s2`, `s3` and `h1`-`h4` are all fine. Sending three pings in a row shows only the first is lost: ``` --- FAIL: TestIsoS4TwoPings/first --- PASS: TestIsoS4TwoPings/second --- PASS: TestIsoS4TwoPings/third ``` ## Why `RoutineReadFromTUN` reads the padding once and then blocks in `Read` with an offset computed from it: ```go for { padding := device.paddings.transport.Load() offset := MessageTransportHeaderSize + int(padding) count, readErr = device.tun.device.Read(bufs, sizes, offset) ``` The routine is started by `NewDevice`, so it enters that first `Read` before `IpcSet` has applied `S4`. The packet lands at offset 16, while the peer - configured by then - looks for the message type at offset `16+S4`. Logging the receiving side confirms it. `S4` is 25, and the type is at offset 0 instead of 25: ``` determine: size=64 transportPadding=25 headerRange=4 raw4=04 00 00 00 Received message with unknown type ``` Only the first packet is affected, because the next pass of the loop reads the padding again. This is a change from `0.2.x`: there the padding was applied in `RoutineSequentialSender`, at send time, so it was always current. The 3.0 refactor moved it to read time and put it before the blocking call. The cost in practice is one TCP retransmit on the first connection after the device comes up, which is easy to miss but is still a lost packet on every start. ## The change Re-read the padding after `Read` returns, and move the packets if it changed. This covers a live `S4` change over UAPI as well, not only start-up. The copy only runs when the value actually changed, so the normal path is one atomic load. ## Verification - the new test fails without the change and passes with it - `device.TestAWGDevicePing` and `device.TestTwoDevicePing` pass with the change - `go test ./...` is green, and `go test -race ./device/` is green Found while adding AmneziaWG 3.0 support to a userspace client, where the first connection through a fresh tunnel was reliably a second slower than the rest. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-13 21:04:22 +00:00

bsakaguchi referenced this issue

2026-10-13 21:04:26 +00:00

[PR #183] fix: random trailers cause ~3.5% of transport packets to be misclassified as handshakes #191

bsakaguchi referenced this issue

2026-10-13 21:04:27 +00:00

[PR #184] fix: send first TUN packet with the configured S4 padding #194