From e0d711c478e335e99302a5320b43538337fa298b Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 23 Sep 2024 17:07:38 +0200 Subject: [PATCH] {net/connstats,wgengine/magicsock}: fix packet counting in connstats connstats currently increments the packet counter whenever it is called to store a length of data, however when udp batch sending was introduced we pass the length for a series of packages, and it is only incremented ones, making it count wrongly if we are on a platform supporting udp batches. Updates tailscale/corp#22075 Signed-off-by: Kristoffer Dalby --- net/connstats/stats.go | 22 +++++++++++----------- wgengine/magicsock/derp.go | 2 +- wgengine/magicsock/endpoint.go | 4 ++-- wgengine/magicsock/magicsock.go | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/net/connstats/stats.go b/net/connstats/stats.go index dbcd946b8..4e6d8e109 100644 --- a/net/connstats/stats.go +++ b/net/connstats/stats.go @@ -131,23 +131,23 @@ func (s *Statistics) updateVirtual(b []byte, receive bool) { s.virtual[conn] = cnts } -// UpdateTxPhysical updates the counters for a transmitted wireguard packet +// UpdateTxPhysical updates the counters for zero or more transmitted wireguard packets. // The src is always a Tailscale IP address, representing some remote peer. // The dst is a remote IP address and port that corresponds // with some physical peer backing the Tailscale IP address. -func (s *Statistics) UpdateTxPhysical(src netip.Addr, dst netip.AddrPort, n int) { - s.updatePhysical(src, dst, n, false) +func (s *Statistics) UpdateTxPhysical(src netip.Addr, dst netip.AddrPort, packets, bytes int) { + s.updatePhysical(src, dst, packets, bytes, false) } -// UpdateRxPhysical updates the counters for a received wireguard packet. +// UpdateRxPhysical updates the counters for zero or more received wireguard packets. // The src is always a Tailscale IP address, representing some remote peer. // The dst is a remote IP address and port that corresponds // with some physical peer backing the Tailscale IP address. -func (s *Statistics) UpdateRxPhysical(src netip.Addr, dst netip.AddrPort, n int) { - s.updatePhysical(src, dst, n, true) +func (s *Statistics) UpdateRxPhysical(src netip.Addr, dst netip.AddrPort, packets, bytes int) { + s.updatePhysical(src, dst, packets, bytes, true) } -func (s *Statistics) updatePhysical(src netip.Addr, dst netip.AddrPort, n int, receive bool) { +func (s *Statistics) updatePhysical(src netip.Addr, dst netip.AddrPort, packets, bytes int, receive bool) { conn := netlogtype.Connection{Src: netip.AddrPortFrom(src, 0), Dst: dst} s.mu.Lock() @@ -157,11 +157,11 @@ func (s *Statistics) updatePhysical(src netip.Addr, dst netip.AddrPort, n int, r return } if receive { - cnts.RxPackets++ - cnts.RxBytes += uint64(n) + cnts.RxPackets += uint64(packets) + cnts.RxBytes += uint64(bytes) } else { - cnts.TxPackets++ - cnts.TxBytes += uint64(n) + cnts.TxPackets += uint64(packets) + cnts.TxBytes += uint64(bytes) } s.physical[conn] = cnts } diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 281447ac2..bfee02f6e 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -730,7 +730,7 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep *en ep.noteRecvActivity(ipp, mono.Now()) if stats := c.stats.Load(); stats != nil { - stats.UpdateRxPhysical(ep.nodeAddr, ipp, dm.n) + stats.UpdateRxPhysical(ep.nodeAddr, ipp, 1, dm.n) } return n, ep } diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 78b9ee92a..ab9f3d47d 100644 --- a/wgengine/magicsock/endpoint.go +++ b/wgengine/magicsock/endpoint.go @@ -976,7 +976,7 @@ func (de *endpoint) send(buffs [][]byte) error { // TODO(raggi): needs updating for accuracy, as in error conditions we may have partial sends. if stats := de.c.stats.Load(); err == nil && stats != nil { - stats.UpdateTxPhysical(de.nodeAddr, udpAddr, txBytes) + stats.UpdateTxPhysical(de.nodeAddr, udpAddr, len(buffs), txBytes) } } if derpAddr.IsValid() { @@ -991,7 +991,7 @@ func (de *endpoint) send(buffs [][]byte) error { } if stats := de.c.stats.Load(); stats != nil { - stats.UpdateTxPhysical(de.nodeAddr, derpAddr, txBytes) + stats.UpdateTxPhysical(de.nodeAddr, derpAddr, 1, txBytes) } if allOk { return nil diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 2d4944baf..72e59a2e7 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -1523,7 +1523,7 @@ func (c *Conn) receiveIP(b []byte, ipp netip.AddrPort, cache *ippEndpointCache) ep.lastRecvUDPAny.StoreAtomic(now) ep.noteRecvActivity(ipp, now) if stats := c.stats.Load(); stats != nil { - stats.UpdateRxPhysical(ep.nodeAddr, ipp, len(b)) + stats.UpdateRxPhysical(ep.nodeAddr, ipp, 1, len(b)) } return ep, true }