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>
pull/16989/head
James Tucker 3 months ago committed by James Tucker
parent 89fe2e1f12
commit 3b68d607be

@ -91,7 +91,7 @@ func (c *Conn) fallbackDERPRegionForPeer(peer key.NodePublic) (regionID int) {
type activeDerp struct { type activeDerp struct {
c *derphttp.Client c *derphttp.Client
cancel context.CancelFunc cancel context.CancelFunc
writeCh chan<- derpWriteRequest writeCh chan derpWriteRequest
// lastWrite is the time of the last request for its write // lastWrite is the time of the last request for its write
// channel (currently even if there was no write). // channel (currently even if there was no write).
// It is always non-nil and initialized to a non-zero Time. // 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 // It returns nil if the network is down, the Conn is closed, or the regionID is
// not known. // 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() { if c.networkDown() {
return nil return nil
} }

@ -1642,18 +1642,27 @@ func (c *Conn) sendAddr(addr netip.AddrPort, pubKey key.NodePublic, b []byte, is
// internal locks. // internal locks.
pkt := bytes.Clone(b) pkt := bytes.Clone(b)
select { wr := derpWriteRequest{addr, pubKey, pkt, isDisco}
case <-c.donec: for range 3 {
metricSendDERPErrorClosed.Add(1) select {
return false, errConnClosed case <-c.donec:
case ch <- derpWriteRequest{addr, pubKey, pkt, isDisco}: metricSendDERPErrorClosed.Add(1)
metricSendDERPQueued.Add(1) return false, errConnClosed
return true, nil case ch <- wr:
default: metricSendDERPQueued.Add(1)
metricSendDERPErrorQueue.Add(1) return true, nil
// Too many writes queued. Drop packet. default:
return false, errDropDerpPacket 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 { type receiveBatch struct {
@ -3937,6 +3946,7 @@ var (
metricSendDERPErrorChan = clientmetric.NewCounter("magicsock_send_derp_error_chan") metricSendDERPErrorChan = clientmetric.NewCounter("magicsock_send_derp_error_chan")
metricSendDERPErrorClosed = clientmetric.NewCounter("magicsock_send_derp_error_closed") metricSendDERPErrorClosed = clientmetric.NewCounter("magicsock_send_derp_error_closed")
metricSendDERPErrorQueue = clientmetric.NewCounter("magicsock_send_derp_error_queue") metricSendDERPErrorQueue = clientmetric.NewCounter("magicsock_send_derp_error_queue")
metricSendDERPDropped = clientmetric.NewCounter("magicsock_send_derp_dropped")
metricSendUDP = clientmetric.NewAggregateCounter("magicsock_send_udp") metricSendUDP = clientmetric.NewAggregateCounter("magicsock_send_udp")
metricSendUDPError = clientmetric.NewCounter("magicsock_send_udp_error") metricSendUDPError = clientmetric.NewCounter("magicsock_send_udp_error")
metricSendPeerRelay = clientmetric.NewAggregateCounter("magicsock_send_peer_relay") metricSendPeerRelay = clientmetric.NewAggregateCounter("magicsock_send_peer_relay")

Loading…
Cancel
Save