From 97b6d3e91772411caabc746b981ff16b64a8b1ec Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Tue, 14 Mar 2023 13:34:04 -0700 Subject: [PATCH] sockstats: remove per-interface stats from Get They're not needed for the sockstats logger, and they're somewhat expensive to return (since they involve the creation of a map per label). We now have a separate GetInterfaces() method that returns them instead (which we can still use in the PeerAPI debug endpoint). If changing sockstatlog to sample at 10,000 Hz (instead of the default of 10Hz), the CPU usage would go up to 59% on a iPhone XS. Removing the per-interface stats drops it to 20% (a no-op implementation of Get that returns a fixed value is 16%). Updates tailscale/corp#9230 Updates #3363 Signed-off-by: Mihai Parparita --- ipn/ipnlocal/peerapi.go | 18 +++++++------ log/sockstatlog/logger_test.go | 24 ----------------- net/sockstats/sockstats.go | 46 ++++++++++++++++++++++----------- net/sockstats/sockstats_noop.go | 4 +++ net/sockstats/sockstats_tsgo.go | 31 +++++++++++++++++----- 5 files changed, 69 insertions(+), 54 deletions(-) diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index ad2f3ea51..196d0db5b 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -865,7 +865,7 @@ func (h *peerAPIHandler) handleServeSockStats(w http.ResponseWriter, r *http.Req w.Header().Set("Content-Type", "text/html; charset=utf-8") fmt.Fprintln(w, "

Socket Stats

") - stats, validation := sockstats.GetWithValidation() + stats, interfaceStats, validation := sockstats.Get(), sockstats.GetInterfaces(), sockstats.GetValidation() if stats == nil { fmt.Fprintln(w, "No socket stats available") return @@ -876,7 +876,7 @@ func (h *peerAPIHandler) handleServeSockStats(w http.ResponseWriter, r *http.Req fmt.Fprintln(w, "Label") fmt.Fprintln(w, "Tx") fmt.Fprintln(w, "Rx") - for _, iface := range stats.Interfaces { + for _, iface := range interfaceStats.Interfaces { fmt.Fprintf(w, "Tx (%s)", html.EscapeString(iface)) fmt.Fprintf(w, "Rx (%s)", html.EscapeString(iface)) } @@ -907,11 +907,13 @@ func (h *peerAPIHandler) handleServeSockStats(w http.ResponseWriter, r *http.Req txTotal += stat.TxBytes rxTotal += stat.RxBytes - for _, iface := range stats.Interfaces { - fmt.Fprintf(w, "%d", stat.TxBytesByInterface[iface]) - fmt.Fprintf(w, "%d", stat.RxBytesByInterface[iface]) - txTotalByInterface[iface] += stat.TxBytesByInterface[iface] - rxTotalByInterface[iface] += stat.RxBytesByInterface[iface] + if interfaceStat, ok := interfaceStats.Stats[label]; ok { + for _, iface := range interfaceStats.Interfaces { + fmt.Fprintf(w, "%d", interfaceStat.TxBytesByInterface[iface]) + fmt.Fprintf(w, "%d", interfaceStat.RxBytesByInterface[iface]) + txTotalByInterface[iface] += interfaceStat.TxBytesByInterface[iface] + rxTotalByInterface[iface] += interfaceStat.RxBytesByInterface[iface] + } } if validationStat, ok := validation.Stats[label]; ok && (validationStat.RxBytes > 0 || validationStat.TxBytes > 0) { @@ -932,7 +934,7 @@ func (h *peerAPIHandler) handleServeSockStats(w http.ResponseWriter, r *http.Req fmt.Fprintln(w, "Total") fmt.Fprintf(w, "%d", txTotal) fmt.Fprintf(w, "%d", rxTotal) - for _, iface := range stats.Interfaces { + for _, iface := range interfaceStats.Interfaces { fmt.Fprintf(w, "%d", txTotalByInterface[iface]) fmt.Fprintf(w, "%d", rxTotalByInterface[iface]) } diff --git a/log/sockstatlog/logger_test.go b/log/sockstatlog/logger_test.go index 9dec3fdd4..2d3e412a2 100644 --- a/log/sockstatlog/logger_test.go +++ b/log/sockstatlog/logger_test.go @@ -34,9 +34,6 @@ func TestDelta(t *testing.T) { Stats: map[sockstats.Label]sockstats.SockStat{ sockstats.LabelDERPHTTPClient: { TxBytes: 10, - TxBytesByInterface: map[string]uint64{ - "en0": 10, - }, }, }, }, @@ -44,9 +41,6 @@ func TestDelta(t *testing.T) { Stats: map[sockstats.Label]sockstats.SockStat{ sockstats.LabelDERPHTTPClient: { TxBytes: 10, - TxBytesByInterface: map[string]uint64{ - "en0": 10, - }, }, }, }, @@ -59,12 +53,8 @@ func TestDelta(t *testing.T) { Stats: map[sockstats.Label]sockstats.SockStat{ sockstats.LabelDERPHTTPClient: { TxBytes: 10, - TxBytesByInterface: map[string]uint64{ - "en0": 10, - }, }, }, - Interfaces: []string{"en0"}, }, wantStats: map[sockstats.Label]deltaStat{ sockstats.LabelDERPHTTPClient: {10, 0}, @@ -77,30 +67,16 @@ func TestDelta(t *testing.T) { sockstats.LabelDERPHTTPClient: { TxBytes: 10, RxBytes: 10, - TxBytesByInterface: map[string]uint64{ - "en0": 10, - }, - RxBytesByInterface: map[string]uint64{ - "en0": 10, - }, }, }, - Interfaces: []string{"en0"}, }, b: &sockstats.SockStats{ Stats: map[sockstats.Label]sockstats.SockStat{ sockstats.LabelDERPHTTPClient: { TxBytes: 10, RxBytes: 30, - TxBytesByInterface: map[string]uint64{ - "en0": 10, - }, - RxBytesByInterface: map[string]uint64{ - "en0": 30, - }, }, }, - Interfaces: []string{"en0"}, }, wantStats: map[sockstats.Label]deltaStat{ sockstats.LabelDERPHTTPClient: {0, 20}, diff --git a/net/sockstats/sockstats.go b/net/sockstats/sockstats.go index 99385ed67..ebfc45962 100644 --- a/net/sockstats/sockstats.go +++ b/net/sockstats/sockstats.go @@ -15,23 +15,17 @@ import ( ) // SockStats contains statistics for sockets instrumented with the -// WithSockStats() function, along with the interfaces that we have -// per-interface statistics for. +// WithSockStats() function type SockStats struct { Stats map[Label]SockStat - Interfaces []string CurrentInterfaceCellular bool } // SockStat contains the sent and received bytes for a socket instrumented with -// the WithSockStats() function. The bytes are also broken down by interface, -// though this may be a subset of the total if interfaces were added after the -// instrumented socket was created. +// the WithSockStats() function. type SockStat struct { - TxBytes uint64 - RxBytes uint64 - TxBytesByInterface map[string]uint64 - RxBytesByInterface map[string]uint64 + TxBytes uint64 + RxBytes uint64 } // Label is an identifier for a socket that stats are collected for. A finite @@ -67,6 +61,28 @@ func Get() *SockStats { return get() } +// InterfaceSockStats contains statistics for sockets instrumented with the +// WithSockStats() function, broken down by interface. The statistics may be a +// subset of the total if interfaces were added after the instrumented socket +// was created. +type InterfaceSockStats struct { + Stats map[Label]InterfaceSockStat + Interfaces []string +} + +// InterfaceSockStat contains the per-interface sent and received bytes for a +// socket instrumented with the WithSockStats() function. +type InterfaceSockStat struct { + TxBytesByInterface map[string]uint64 + RxBytesByInterface map[string]uint64 +} + +// GetWithInterfaces is a variant of Get that returns the current socket +// statistics broken down by interface. It is slightly more expensive than Get. +func GetInterfaces() *InterfaceSockStats { + return getInterfaces() +} + // ValidationSockStats contains external validation numbers for sockets // instrumented with WithSockStats. It may be a subset of the all sockets, // depending on what externa measurement mechanisms the platform supports. @@ -81,11 +97,11 @@ type ValidationSockStat struct { RxBytes uint64 } -// GetWithValidation is a variant of GetWith that returns both the current stats -// and external validation numbers for the stats. It is more expensive than -// Get and should be used in debug interfaces only. -func GetWithValidation() (*SockStats, *ValidationSockStats) { - return get(), getValidation() +// GetValidation is a variant of Get that returns external validation numbers +// for stats. It is more expensive than Get and should be used in debug +// interfaces only. +func GetValidation() *ValidationSockStats { + return getValidation() } // LinkMonitor is the interface for the parts of wgengine/mointor's Mon that we diff --git a/net/sockstats/sockstats_noop.go b/net/sockstats/sockstats_noop.go index 5e2b79ace..9a1ae1b3b 100644 --- a/net/sockstats/sockstats_noop.go +++ b/net/sockstats/sockstats_noop.go @@ -19,6 +19,10 @@ func get() *SockStats { return nil } +func getInterfaces() *InterfaceSockStats { + return nil +} + func getValidation() *ValidationSockStats { return nil } diff --git a/net/sockstats/sockstats_tsgo.go b/net/sockstats/sockstats_tsgo.go index 56069ec91..7a699fab1 100644 --- a/net/sockstats/sockstats_tsgo.go +++ b/net/sockstats/sockstats_tsgo.go @@ -157,20 +157,37 @@ func get() *SockStats { defer sockStats.mu.Unlock() r := &SockStats{ - Stats: make(map[Label]SockStat), - Interfaces: make([]string, 0, len(sockStats.usedInterfaces)), + Stats: make(map[Label]SockStat, len(sockStats.countersByLabel)), CurrentInterfaceCellular: sockStats.currentInterfaceCellular.Load(), } + + for label, counters := range sockStats.countersByLabel { + r.Stats[label] = SockStat{ + TxBytes: counters.txBytes.Load(), + RxBytes: counters.rxBytes.Load(), + } + } + + return r +} + +func getInterfaces() *InterfaceSockStats { + sockStats.mu.Lock() + defer sockStats.mu.Unlock() + + interfaceCount := len(sockStats.usedInterfaces) + r := &InterfaceSockStats{ + Stats: make(map[Label]InterfaceSockStat, len(sockStats.countersByLabel)), + Interfaces: make([]string, 0, interfaceCount), + } for iface := range sockStats.usedInterfaces { r.Interfaces = append(r.Interfaces, sockStats.knownInterfaces[iface]) } for label, counters := range sockStats.countersByLabel { - s := SockStat{ - TxBytes: counters.txBytes.Load(), - RxBytes: counters.rxBytes.Load(), - TxBytesByInterface: make(map[string]uint64), - RxBytesByInterface: make(map[string]uint64), + s := InterfaceSockStat{ + TxBytesByInterface: make(map[string]uint64, interfaceCount), + RxBytesByInterface: make(map[string]uint64, interfaceCount), } for iface, a := range counters.rxBytesByInterface { ifName := sockStats.knownInterfaces[iface]