From 532b26145a088c3946c37040dc4731dc4edcb7cf Mon Sep 17 00:00:00 2001
From: Anton Tolchanov <anton@tailscale.com>
Date: Tue, 29 Oct 2024 13:46:34 +0000
Subject: [PATCH] wgengine/magicsock: exclude disco from throughput metrics

The user-facing metrics are intended to track data transmitted at
the overlay network level.

Updates tailscale/corp#22075

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
---
 wgengine/magicsock/derp.go      | 14 ++++++++------
 wgengine/magicsock/endpoint.go  |  3 ++-
 wgengine/magicsock/magicsock.go |  7 ++++---
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go
index 704ce3c4f..0204fa0f5 100644
--- a/wgengine/magicsock/derp.go
+++ b/wgengine/magicsock/derp.go
@@ -649,9 +649,10 @@ func (c *Conn) runDerpReader(ctx context.Context, regionID int, dc *derphttp.Cli
 }
 
 type derpWriteRequest struct {
-	addr   netip.AddrPort
-	pubKey key.NodePublic
-	b      []byte // copied; ownership passed to receiver
+	addr    netip.AddrPort
+	pubKey  key.NodePublic
+	b       []byte // copied; ownership passed to receiver
+	isDisco bool
 }
 
 // runDerpWriter runs in a goroutine for the life of a DERP
@@ -673,7 +674,7 @@ func (c *Conn) runDerpWriter(ctx context.Context, dc *derphttp.Client, ch <-chan
 			if err != nil {
 				c.logf("magicsock: derp.Send(%v): %v", wr.addr, err)
 				metricSendDERPError.Add(1)
-			} else {
+			} else if !wr.isDisco {
 				c.metrics.outboundPacketsDERPTotal.Add(1)
 				c.metrics.outboundBytesDERPTotal.Add(int64(len(wr.b)))
 			}
@@ -696,8 +697,6 @@ func (c *connBind) receiveDERP(buffs [][]byte, sizes []int, eps []conn.Endpoint)
 			// No data read occurred. Wait for another packet.
 			continue
 		}
-		c.metrics.inboundPacketsDERPTotal.Add(1)
-		c.metrics.inboundBytesDERPTotal.Add(int64(n))
 		sizes[0] = n
 		eps[0] = ep
 		return 1, nil
@@ -737,6 +736,9 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep *en
 	if stats := c.stats.Load(); stats != nil {
 		stats.UpdateRxPhysical(ep.nodeAddr, ipp, 1, dm.n)
 	}
+
+	c.metrics.inboundPacketsDERPTotal.Add(1)
+	c.metrics.inboundBytesDERPTotal.Add(int64(n))
 	return n, ep
 }
 
diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go
index 1ddde9752..5e0ada617 100644
--- a/wgengine/magicsock/endpoint.go
+++ b/wgengine/magicsock/endpoint.go
@@ -983,7 +983,8 @@ func (de *endpoint) send(buffs [][]byte) error {
 		allOk := true
 		var txBytes int
 		for _, buff := range buffs {
-			ok, _ := de.c.sendAddr(derpAddr, de.publicKey, buff)
+			const isDisco = false
+			ok, _ := de.c.sendAddr(derpAddr, de.publicKey, buff, isDisco)
 			txBytes += len(buff)
 			if !ok {
 				allOk = false
diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index 72e59a2e7..705e42d9e 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -1356,7 +1356,7 @@ func (c *Conn) sendUDPStd(addr netip.AddrPort, b []byte) (sent bool, err error)
 // An example of when they might be different: sending to an
 // IPv6 address when the local machine doesn't have IPv6 support
 // returns (false, nil); it's not an error, but nothing was sent.
-func (c *Conn) sendAddr(addr netip.AddrPort, pubKey key.NodePublic, b []byte) (sent bool, err error) {
+func (c *Conn) sendAddr(addr netip.AddrPort, pubKey key.NodePublic, b []byte, isDisco bool) (sent bool, err error) {
 	if addr.Addr() != tailcfg.DerpMagicIPAddr {
 		return c.sendUDP(addr, b)
 	}
@@ -1379,7 +1379,7 @@ func (c *Conn) sendAddr(addr netip.AddrPort, pubKey key.NodePublic, b []byte) (s
 	case <-c.donec:
 		metricSendDERPErrorClosed.Add(1)
 		return false, errConnClosed
-	case ch <- derpWriteRequest{addr, pubKey, pkt}:
+	case ch <- derpWriteRequest{addr, pubKey, pkt, isDisco}:
 		metricSendDERPQueued.Add(1)
 		return true, nil
 	default:
@@ -1577,7 +1577,8 @@ func (c *Conn) sendDiscoMessage(dst netip.AddrPort, dstKey key.NodePublic, dstDi
 
 	box := di.sharedKey.Seal(m.AppendMarshal(nil))
 	pkt = append(pkt, box...)
-	sent, err = c.sendAddr(dst, dstKey, pkt)
+	const isDisco = true
+	sent, err = c.sendAddr(dst, dstKey, pkt, isDisco)
 	if sent {
 		if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco()) {
 			node := "?"