From 3b68d607be1e3069e9ddbd99d85966e4f059c237 Mon Sep 17 00:00:00 2001
From: James Tucker <james@tailscale.com>
Date: Thu, 28 Aug 2025 21:29:11 -0700
Subject: [PATCH] wgengine/magicsock: drop DERP queue from head rather than
 tail

If the DERP queue is full, drop the oldest item first, rather than the
youngest, on the assumption that older data is more likely to be
unanswerable.

Updates tailscale/corp#31762

Signed-off-by: James Tucker <james@tailscale.com>
---
 wgengine/magicsock/derp.go      |  4 ++--
 wgengine/magicsock/magicsock.go | 32 +++++++++++++++++++++-----------
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go
index 9c60e4893..b5fc36bb8 100644
--- a/wgengine/magicsock/derp.go
+++ b/wgengine/magicsock/derp.go
@@ -91,7 +91,7 @@ func (c *Conn) fallbackDERPRegionForPeer(peer key.NodePublic) (regionID int) {
 type activeDerp struct {
 	c       *derphttp.Client
 	cancel  context.CancelFunc
-	writeCh chan<- derpWriteRequest
+	writeCh chan derpWriteRequest
 	// lastWrite is the time of the last request for its write
 	// channel (currently even if there was no write).
 	// It is always non-nil and initialized to a non-zero Time.
@@ -302,7 +302,7 @@ const derpWriteQueueDepth = 32
 //
 // It returns nil if the network is down, the Conn is closed, or the regionID is
 // not known.
-func (c *Conn) derpWriteChanForRegion(regionID int, peer key.NodePublic) chan<- derpWriteRequest {
+func (c *Conn) derpWriteChanForRegion(regionID int, peer key.NodePublic) chan derpWriteRequest {
 	if c.networkDown() {
 		return nil
 	}
diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index a7f84e352..a11e8a1cd 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -1642,18 +1642,27 @@ func (c *Conn) sendAddr(addr netip.AddrPort, pubKey key.NodePublic, b []byte, is
 	// internal locks.
 	pkt := bytes.Clone(b)
 
-	select {
-	case <-c.donec:
-		metricSendDERPErrorClosed.Add(1)
-		return false, errConnClosed
-	case ch <- derpWriteRequest{addr, pubKey, pkt, isDisco}:
-		metricSendDERPQueued.Add(1)
-		return true, nil
-	default:
-		metricSendDERPErrorQueue.Add(1)
-		// Too many writes queued. Drop packet.
-		return false, errDropDerpPacket
+	wr := derpWriteRequest{addr, pubKey, pkt, isDisco}
+	for range 3 {
+		select {
+		case <-c.donec:
+			metricSendDERPErrorClosed.Add(1)
+			return false, errConnClosed
+		case ch <- wr:
+			metricSendDERPQueued.Add(1)
+			return true, nil
+		default:
+			select {
+			case <-ch:
+				metricSendDERPDropped.Add(1)
+			default:
+			}
+		}
 	}
+	// gave up after 3 write attempts
+	metricSendDERPErrorQueue.Add(1)
+	// Too many writes queued. Drop packet.
+	return false, errDropDerpPacket
 }
 
 type receiveBatch struct {
@@ -3937,6 +3946,7 @@ var (
 	metricSendDERPErrorChan   = clientmetric.NewCounter("magicsock_send_derp_error_chan")
 	metricSendDERPErrorClosed = clientmetric.NewCounter("magicsock_send_derp_error_closed")
 	metricSendDERPErrorQueue  = clientmetric.NewCounter("magicsock_send_derp_error_queue")
+	metricSendDERPDropped     = clientmetric.NewCounter("magicsock_send_derp_dropped")
 	metricSendUDP             = clientmetric.NewAggregateCounter("magicsock_send_udp")
 	metricSendUDPError        = clientmetric.NewCounter("magicsock_send_udp_error")
 	metricSendPeerRelay       = clientmetric.NewAggregateCounter("magicsock_send_peer_relay")