mirror of
https://github.com/netbirdio/wireguard-go.git
synced 2026-09-23 00:28:31 -07:00
Zoltan Papp
8bf8fa968f
The keepalive deadlock fixed in #20 was one instance of a class the kernel module rules out by construction. This commit adopts the same two rules instead of working around the symptom. 1. Nothing reached from a timer callback may wait. In the kernel the callback runs in softirq: alloc_skb is GFP_ATOMIC and gives up, and wg_queue_enqueue_per_device_and_peer fails with -ENOSPC/-EPIPE and the packet is dropped or marked PACKET_STATE_DEAD. #20 made the keepalive allocation non-blocking but left the rest of the path waiting: a container allocation on the nonce-overflow branch and the two bounded queue sends in SendStagedPackets. Any of them, parked while holding the timer's runningLock, wedges Timer.DelSync and with it Peer.Stop, RemovePeer and Device.Close. SendKeepalive now runs sendStagedPackets(wait=false): a full pool or queue drops the batch; if the sequential sender already holds the batch it is emptied and unlocked so the sender passes over it. The public SendStagedPackets keeps waiting, so the TUN reader keeps its backpressure. 2. A peer holds at most MAX_STAGED_PACKETS (128) packets while waiting for a handshake; wg_xmit drops the oldest before adding a batch. Our staged queue was bounded in batches, not packets, and a GSO batch can carry 128 packets, so one peer that never completes its handshake could pin thousands of buffers and drain a capped pool for every other peer. StagePackets now enforces the packet bound with a per-peer counter, dropping the oldest batches first. That, not the handshake retry policy, is how the kernel keeps a dead peer from hurting the rest of the device. With the bound in place the handshakeAttempts change from #20 is not needed and is reverted: outbound traffic resets the counter again, as it does upstream and in wg_packet_send_queued_handshake_initiation, and SendHandshakeInitiation goes back to the upstream isRetry signature so these lines no longer diverge on merges. The revert also removes a problem that change introduced. It relied on the give-up branch of expiredRetransmitHandshake to release a dead peer's buffers, and that branch only runs after MaxTimerHandshakes failed retries - about 90 seconds. Until then the pool stayed drained, the TUN reader stayed parked and no peer on the device could send. And once the branch had run, nothing reset the counter again while traffic kept flowing: the next packet sent one initiation and re-armed the retransmit timer, which found the counter already past the limit and gave up after a single try, flushing the staged queue, deleting the keepalive timer and logging "giving up" every five seconds for as long as the peer was down and had traffic. Upstream and the kernel give each burst of new traffic a full 90-second budget instead, and now so do we. Tests: TestStagedSendKeepsHandshakeAttempts is replaced by TestStagedSendResetsHandshakeAttempts; TestStagePacketsBoundedPerPeer checks the packet bound and that dropped buffers return to the pool; TestSendKeepaliveWithFullOutboundQueue wedges the sequential sender and checks the keepalive still returns.