Add aggregated NIC stats

This change also includes miscellaneous improvements:
* UnknownProtocolRcvdPackets has been separated into two stats, to
specify at which layer the unknown protocol was found (L3 or L4)
* MalformedRcvdPacket is not aggregated across every endpoint anymore.
Doing it this way did not add useful information, and it was also error-prone
(example: ipv6 forgot to increment this aggregated stat, it only
incremented its own ipv6.MalformedPacketsReceived). It is now only incremented
 the NIC.
* Removed TestStatsString test which was outdated and had no real
utility.

PiperOrigin-RevId: 375057472

9 files changed, 199 insertions, 82 deletions

diff --git a/pkg/tcpip/stack/BUILD b/pkg/tcpip/stack/BUILD
index 84aa6a9e4..395ff9a07 100644
--- a/pkg/tcpip/stack/BUILD
+++ b/pkg/tcpip/stack/BUILD
@@ -56,6 +56,7 @@ go_library(
         "neighbor_entry_list.go",
         "neighborstate_string.go",
         "nic.go",
+        "nic_stats.go",
         "nud.go",
         "packet_buffer.go",
         "packet_buffer_list.go",
diff --git a/pkg/tcpip/stack/neighbor_cache_test.go b/pkg/tcpip/stack/neighbor_cache_test.go
index 9821a18d3..a99cbebc7 100644
--- a/pkg/tcpip/stack/neighbor_cache_test.go
+++ b/pkg/tcpip/stack/neighbor_cache_test.go
@@ -95,7 +95,7 @@ func newTestNeighborResolver(nudDisp NUDDispatcher, config NUDConfigurations, cl
 			randomGenerator: rng,
 		},
 		id:    1,
-		stats: makeNICStats(),
+		stats: makeNICStats(tcpip.NICStats{}.FillIn()),
 	}, linkRes)
 	return linkRes
 }
diff --git a/pkg/tcpip/stack/neighbor_entry.go b/pkg/tcpip/stack/neighbor_entry.go
index 418c71fe5..463d017fc 100644
--- a/pkg/tcpip/stack/neighbor_entry.go
+++ b/pkg/tcpip/stack/neighbor_entry.go
@@ -361,7 +361,7 @@ func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) {
 			e.dispatchAddEventLocked()
 		case Unreachable:
 			e.dispatchChangeEventLocked()
-			e.cache.nic.stats.Neighbor.UnreachableEntryLookups.Increment()
+			e.cache.nic.stats.neighbor.unreachableEntryLookups.Increment()
 		}
 
 		config := e.nudState.Config()
diff --git a/pkg/tcpip/stack/neighbor_entry_test.go b/pkg/tcpip/stack/neighbor_entry_test.go
index 1d39ee73d..7fd152f5b 100644
--- a/pkg/tcpip/stack/neighbor_entry_test.go
+++ b/pkg/tcpip/stack/neighbor_entry_test.go
@@ -219,7 +219,7 @@ func entryTestSetup(c NUDConfigurations) (*neighborEntry, *testNUDDispatcher, *e
 			nudConfigs:      c,
 			randomGenerator: rand.New(rand.NewSource(time.Now().UnixNano())),
 		},
-		stats: makeNICStats(),
+		stats: makeNICStats(tcpip.NICStats{}.FillIn()),
 	}
 	netEP := (&testIPv6Protocol{}).NewEndpoint(&nic, nil)
 	nic.networkEndpoints = map[tcpip.NetworkProtocolNumber]NetworkEndpoint{
diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go
index dbba2c79f..378389db2 100644
--- a/pkg/tcpip/stack/nic.go
+++ b/pkg/tcpip/stack/nic.go
@@ -51,7 +51,7 @@ type nic struct {
 	name    string
 	context NICContext
 
-	stats NICStats
+	stats sharedStats
 
 	// The network endpoints themselves may be modified by calling the interface's
 	// methods, but the map reference and entries must be constant.
@@ -78,26 +78,13 @@ type nic struct {
 	}
 }
 
-// NICStats hold statistics for a NIC.
-type NICStats struct {
-	Tx DirectionStats
-	Rx DirectionStats
-
-	DisabledRx DirectionStats
-
-	Neighbor NeighborStats
-}
-
-func makeNICStats() NICStats {
-	var s NICStats
-	tcpip.InitStatCounters(reflect.ValueOf(&s).Elem())
-	return s
-}
-
-// DirectionStats includes packet and byte counts.
-type DirectionStats struct {
-	Packets *tcpip.StatCounter
-	Bytes   *tcpip.StatCounter
+// makeNICStats initializes the NIC statistics and associates them to the global
+// NIC statistics.
+func makeNICStats(global tcpip.NICStats) sharedStats {
+	var stats sharedStats
+	tcpip.InitStatCounters(reflect.ValueOf(&stats.local).Elem())
+	stats.init(&stats.local, &global)
+	return stats
 }
 
 type packetEndpointList struct {
@@ -150,7 +137,7 @@ func newNIC(stack *Stack, id tcpip.NICID, name string, ep LinkEndpoint, ctx NICC
 		id:                        id,
 		name:                      name,
 		context:                   ctx,
-		stats:                     makeNICStats(),
+		stats:                     makeNICStats(stack.Stats().NICs),
 		networkEndpoints:          make(map[tcpip.NetworkProtocolNumber]NetworkEndpoint),
 		linkAddrResolvers:         make(map[tcpip.NetworkProtocolNumber]*linkResolver),
 		duplicateAddressDetectors: make(map[tcpip.NetworkProtocolNumber]DuplicateAddressDetector),
@@ -382,8 +369,8 @@ func (n *nic) writePacket(r RouteInfo, protocol tcpip.NetworkProtocolNumber, pkt
 		return err
 	}
 
-	n.stats.Tx.Packets.Increment()
-	n.stats.Tx.Bytes.IncrementBy(uint64(numBytes))
+	n.stats.tx.packets.Increment()
+	n.stats.tx.bytes.IncrementBy(uint64(numBytes))
 	return nil
 }
 
@@ -399,13 +386,13 @@ func (n *nic) writePackets(r RouteInfo, protocol tcpip.NetworkProtocolNumber, pk
 	}
 
 	writtenPackets, err := n.LinkEndpoint.WritePackets(r, pkts, protocol)
-	n.stats.Tx.Packets.IncrementBy(uint64(writtenPackets))
+	n.stats.tx.packets.IncrementBy(uint64(writtenPackets))
 	writtenBytes := 0
 	for i, pb := 0, pkts.Front(); i < writtenPackets && pb != nil; i, pb = i+1, pb.Next() {
 		writtenBytes += pb.Size()
 	}
 
-	n.stats.Tx.Bytes.IncrementBy(uint64(writtenBytes))
+	n.stats.tx.bytes.IncrementBy(uint64(writtenBytes))
 	return writtenPackets, err
 }
 
@@ -718,18 +705,18 @@ func (n *nic) DeliverNetworkPacket(remote, local tcpip.LinkAddress, protocol tcp
 	if !enabled {
 		n.mu.RUnlock()
 
-		n.stats.DisabledRx.Packets.Increment()
-		n.stats.DisabledRx.Bytes.IncrementBy(uint64(pkt.Data().Size()))
+		n.stats.disabledRx.packets.Increment()
+		n.stats.disabledRx.bytes.IncrementBy(uint64(pkt.Data().Size()))
 		return
 	}
 
-	n.stats.Rx.Packets.Increment()
-	n.stats.Rx.Bytes.IncrementBy(uint64(pkt.Data().Size()))
+	n.stats.rx.packets.Increment()
+	n.stats.rx.bytes.IncrementBy(uint64(pkt.Data().Size()))
 
 	networkEndpoint, ok := n.networkEndpoints[protocol]
 	if !ok {
 		n.mu.RUnlock()
-		n.stack.stats.UnknownProtocolRcvdPackets.Increment()
+		n.stats.unknownL3ProtocolRcvdPackets.Increment()
 		return
 	}
 
@@ -786,7 +773,7 @@ func (n *nic) DeliverOutboundPacket(remote, local tcpip.LinkAddress, protocol tc
 func (n *nic) DeliverTransportPacket(protocol tcpip.TransportProtocolNumber, pkt *PacketBuffer) TransportPacketDisposition {
 	state, ok := n.stack.transportProtocols[protocol]
 	if !ok {
-		n.stack.stats.UnknownProtocolRcvdPackets.Increment()
+		n.stats.unknownL4ProtocolRcvdPackets.Increment()
 		return TransportPacketProtocolUnreachable
 	}
 
@@ -807,20 +794,20 @@ func (n *nic) DeliverTransportPacket(protocol tcpip.TransportProtocolNumber, pkt
 			// ICMP packets may be longer, but until icmp.Parse is implemented, here
 			// we parse it using the minimum size.
 			if _, ok := pkt.TransportHeader().Consume(transProto.MinimumPacketSize()); !ok {
-				n.stack.stats.MalformedRcvdPackets.Increment()
+				n.stats.malformedL4RcvdPackets.Increment()
 				// We consider a malformed transport packet handled because there is
 				// nothing the caller can do.
 				return TransportPacketHandled
 			}
 		} else if !transProto.Parse(pkt) {
-			n.stack.stats.MalformedRcvdPackets.Increment()
+			n.stats.malformedL4RcvdPackets.Increment()
 			return TransportPacketHandled
 		}
 	}
 
 	srcPort, dstPort, err := transProto.ParsePorts(pkt.TransportHeader().View())
 	if err != nil {
-		n.stack.stats.MalformedRcvdPackets.Increment()
+		n.stats.malformedL4RcvdPackets.Increment()
 		return TransportPacketHandled
 	}
 
@@ -852,7 +839,7 @@ func (n *nic) DeliverTransportPacket(protocol tcpip.TransportProtocolNumber, pkt
 	// If it doesn't handle it then we should do so.
 	switch res := transProto.HandleUnknownDestinationPacket(id, pkt); res {
 	case UnknownDestinationPacketMalformed:
-		n.stack.stats.MalformedRcvdPackets.Increment()
+		n.stats.malformedL4RcvdPackets.Increment()
 		return TransportPacketHandled
 	case UnknownDestinationPacketUnhandled:
 		return TransportPacketDestinationPortUnreachable
diff --git a/pkg/tcpip/stack/nic_stats.go b/pkg/tcpip/stack/nic_stats.go
new file mode 100644
index 000000000..1773d5e8d
--- /dev/null
+++ b/pkg/tcpip/stack/nic_stats.go
@@ -0,0 +1,74 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package stack
+
+import (
+	"gvisor.dev/gvisor/pkg/tcpip"
+)
+
+type sharedStats struct {
+	local tcpip.NICStats
+	multiCounterNICStats
+}
+
+// LINT.IfChange(multiCounterNICPacketStats)
+
+type multiCounterNICPacketStats struct {
+	packets tcpip.MultiCounterStat
+	bytes   tcpip.MultiCounterStat
+}
+
+func (m *multiCounterNICPacketStats) init(a, b *tcpip.NICPacketStats) {
+	m.packets.Init(a.Packets, b.Packets)
+	m.bytes.Init(a.Bytes, b.Bytes)
+}
+
+// LINT.ThenChange(../../tcpip.go:NICPacketStats)
+
+// LINT.IfChange(multiCounterNICNeighborStats)
+
+type multiCounterNICNeighborStats struct {
+	unreachableEntryLookups tcpip.MultiCounterStat
+}
+
+func (m *multiCounterNICNeighborStats) init(a, b *tcpip.NICNeighborStats) {
+	m.unreachableEntryLookups.Init(a.UnreachableEntryLookups, b.UnreachableEntryLookups)
+}
+
+// LINT.ThenChange(../../tcpip.go:NICNeighborStats)
+
+// LINT.IfChange(multiCounterNICStats)
+
+type multiCounterNICStats struct {
+	unknownL3ProtocolRcvdPackets tcpip.MultiCounterStat
+	unknownL4ProtocolRcvdPackets tcpip.MultiCounterStat
+	malformedL4RcvdPackets       tcpip.MultiCounterStat
+	tx                           multiCounterNICPacketStats
+	rx                           multiCounterNICPacketStats
+	disabledRx                   multiCounterNICPacketStats
+	neighbor                     multiCounterNICNeighborStats
+}
+
+func (m *multiCounterNICStats) init(a, b *tcpip.NICStats) {
+	m.unknownL3ProtocolRcvdPackets.Init(a.UnknownL3ProtocolRcvdPackets, b.UnknownL3ProtocolRcvdPackets)
+	m.unknownL4ProtocolRcvdPackets.Init(a.UnknownL4ProtocolRcvdPackets, b.UnknownL4ProtocolRcvdPackets)
+	m.malformedL4RcvdPackets.Init(a.MalformedL4RcvdPackets, b.MalformedL4RcvdPackets)
+	m.tx.init(&a.Tx, &b.Tx)
+	m.rx.init(&a.Rx, &b.Rx)
+	m.disabledRx.init(&a.DisabledRx, &b.DisabledRx)
+	m.neighbor.init(&a.Neighbor, &b.Neighbor)
+}
+
+// LINT.ThenChange(../../tcpip.go:NICStats)
diff --git a/pkg/tcpip/stack/nic_test.go b/pkg/tcpip/stack/nic_test.go
index 8a3005295..5cb342f78 100644
--- a/pkg/tcpip/stack/nic_test.go
+++ b/pkg/tcpip/stack/nic_test.go
@@ -15,11 +15,13 @@
 package stack
 
 import (
+	"reflect"
 	"testing"
 
 	"gvisor.dev/gvisor/pkg/tcpip"
 	"gvisor.dev/gvisor/pkg/tcpip/buffer"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
+	"gvisor.dev/gvisor/pkg/tcpip/testutil"
 )
 
 var _ AddressableEndpoint = (*testIPv6Endpoint)(nil)
@@ -171,19 +173,19 @@ func TestDisabledRxStatsWhenNICDisabled(t *testing.T) {
 	// When the NIC is disabled, the only field that matters is the stats field.
 	// This test is limited to stats counter checks.
 	nic := nic{
-		stats: makeNICStats(),
+		stats: makeNICStats(tcpip.NICStats{}.FillIn()),
 	}
 
-	if got := nic.stats.DisabledRx.Packets.Value(); got != 0 {
+	if got := nic.stats.local.DisabledRx.Packets.Value(); got != 0 {
 		t.Errorf("got DisabledRx.Packets = %d, want = 0", got)
 	}
-	if got := nic.stats.DisabledRx.Bytes.Value(); got != 0 {
+	if got := nic.stats.local.DisabledRx.Bytes.Value(); got != 0 {
 		t.Errorf("got DisabledRx.Bytes = %d, want = 0", got)
 	}
-	if got := nic.stats.Rx.Packets.Value(); got != 0 {
+	if got := nic.stats.local.Rx.Packets.Value(); got != 0 {
 		t.Errorf("got Rx.Packets = %d, want = 0", got)
 	}
-	if got := nic.stats.Rx.Bytes.Value(); got != 0 {
+	if got := nic.stats.local.Rx.Bytes.Value(); got != 0 {
 		t.Errorf("got Rx.Bytes = %d, want = 0", got)
 	}
 
@@ -195,16 +197,28 @@ func TestDisabledRxStatsWhenNICDisabled(t *testing.T) {
 		Data: buffer.View([]byte{1, 2, 3, 4}).ToVectorisedView(),
 	}))
 
-	if got := nic.stats.DisabledRx.Packets.Value(); got != 1 {
+	if got := nic.stats.local.DisabledRx.Packets.Value(); got != 1 {
 		t.Errorf("got DisabledRx.Packets = %d, want = 1", got)
 	}
-	if got := nic.stats.DisabledRx.Bytes.Value(); got != 4 {
+	if got := nic.stats.local.DisabledRx.Bytes.Value(); got != 4 {
 		t.Errorf("got DisabledRx.Bytes = %d, want = 4", got)
 	}
-	if got := nic.stats.Rx.Packets.Value(); got != 0 {
+	if got := nic.stats.local.Rx.Packets.Value(); got != 0 {
 		t.Errorf("got Rx.Packets = %d, want = 0", got)
 	}
-	if got := nic.stats.Rx.Bytes.Value(); got != 0 {
+	if got := nic.stats.local.Rx.Bytes.Value(); got != 0 {
 		t.Errorf("got Rx.Bytes = %d, want = 0", got)
 	}
 }
+
+func TestMultiCounterStatsInitialization(t *testing.T) {
+	global := tcpip.NICStats{}.FillIn()
+	nic := nic{
+		stats: makeNICStats(global),
+	}
+	multi := nic.stats.multiCounterNICStats
+	local := nic.stats.local
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&multi).Elem(), []reflect.Value{reflect.ValueOf(&local).Elem(), reflect.ValueOf(&global).Elem()}); err != nil {
+		t.Error(err)
+	}
+}
diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go
index 8814f45a6..916ebef11 100644
--- a/pkg/tcpip/stack/stack.go
+++ b/pkg/tcpip/stack/stack.go
@@ -804,7 +804,7 @@ type NICInfo struct {
 	// MTU is the maximum transmission unit.
 	MTU uint32
 
-	Stats NICStats
+	Stats tcpip.NICStats
 
 	// NetworkStats holds the stats of each NetworkEndpoint bound to the NIC.
 	NetworkStats map[tcpip.NetworkProtocolNumber]NetworkEndpointStats
@@ -856,7 +856,7 @@ func (s *Stack) NICInfo() map[tcpip.NICID]NICInfo {
 			ProtocolAddresses: nic.primaryAddresses(),
 			Flags:             flags,
 			MTU:               nic.LinkEndpoint.MTU(),
-			Stats:             nic.stats,
+			Stats:             nic.stats.local,
 			NetworkStats:      netStats,
 			Context:           nic.context,
 			ARPHardwareType:   nic.LinkEndpoint.ARPHardwareType(),
diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go
index 02d54d29b..256712446 100644
--- a/pkg/tcpip/stack/stack_test.go
+++ b/pkg/tcpip/stack/stack_test.go
@@ -2248,46 +2248,87 @@ func TestNICStats(t *testing.T) {
 	s := stack.New(stack.Options{
 		NetworkProtocols: []stack.NetworkProtocolFactory{fakeNetFactory},
 	})
-	ep1 := channel.New(10, defaultMTU, "")
-	if err := s.CreateNIC(1, ep1); err != nil {
-		t.Fatal("CreateNIC failed: ", err)
-	}
-	if err := s.AddAddress(1, fakeNetNumber, "\x01"); err != nil {
-		t.Fatal("AddAddress failed:", err)
+
+	nics := []struct {
+		addr        tcpip.Address
+		txByteCount int
+		rxByteCount int
+	}{
+		{
+			addr:        "\x01",
+			txByteCount: 30,
+			rxByteCount: 10,
+		},
+		{
+			addr:        "\x02",
+			txByteCount: 50,
+			rxByteCount: 20,
+		},
 	}
-	// Route all packets for address \x01 to NIC 1.
-	{
-		subnet, err := tcpip.NewSubnet("\x01", "\xff")
-		if err != nil {
-			t.Fatal(err)
+
+	var txBytesTotal, rxBytesTotal, txPacketsTotal, rxPacketsTotal int
+	for i, nic := range nics {
+		nicid := tcpip.NICID(i)
+		ep := channel.New(1, defaultMTU, "")
+		if err := s.CreateNIC(nicid, ep); err != nil {
+			t.Fatal("CreateNIC failed: ", err)
+		}
+		if err := s.AddAddress(nicid, fakeNetNumber, nic.addr); err != nil {
+			t.Fatal("AddAddress failed:", err)
 		}
-		s.SetRouteTable([]tcpip.Route{{Destination: subnet, Gateway: "\x00", NIC: 1}})
-	}
 
-	// Send a packet to address 1.
-	buf := buffer.NewView(30)
-	ep1.InjectInbound(fakeNetNumber, stack.NewPacketBuffer(stack.PacketBufferOptions{
-		Data: buf.ToVectorisedView(),
-	}))
-	if got, want := s.NICInfo()[1].Stats.Rx.Packets.Value(), uint64(1); got != want {
-		t.Errorf("got Rx.Packets.Value() = %d, want = %d", got, want)
-	}
+		{
+			subnet, err := tcpip.NewSubnet(nic.addr, "\xff")
+			if err != nil {
+				t.Fatal(err)
+			}
+			s.SetRouteTable([]tcpip.Route{{Destination: subnet, Gateway: "\x00", NIC: nicid}})
+		}
 
-	if got, want := s.NICInfo()[1].Stats.Rx.Bytes.Value(), uint64(len(buf)); got != want {
-		t.Errorf("got Rx.Bytes.Value() = %d, want = %d", got, want)
+		nicStats := s.NICInfo()[nicid].Stats
+
+		// Inbound packet.
+		rxBuffer := buffer.NewView(nic.rxByteCount)
+		ep.InjectInbound(fakeNetNumber, stack.NewPacketBuffer(stack.PacketBufferOptions{
+			Data: rxBuffer.ToVectorisedView(),
+		}))
+		if got, want := nicStats.Rx.Packets.Value(), uint64(1); got != want {
+			t.Errorf("got Rx.Packets.Value() = %d, want = %d", got, want)
+		}
+		if got, want := nicStats.Rx.Bytes.Value(), uint64(nic.rxByteCount); got != want {
+			t.Errorf("got Rx.Bytes.Value() = %d, want = %d", got, want)
+		}
+		rxPacketsTotal++
+		rxBytesTotal += nic.rxByteCount
+
+		// Outbound packet.
+		txBuffer := buffer.NewView(nic.txByteCount)
+		actualTxLength := nic.txByteCount + fakeNetHeaderLen
+		if err := sendTo(s, nic.addr, txBuffer); err != nil {
+			t.Fatal("sendTo failed: ", err)
+		}
+		want := ep.Drain()
+		if got := nicStats.Tx.Packets.Value(); got != uint64(want) {
+			t.Errorf("got Tx.Packets.Value() = %d, ep.Drain() = %d", got, want)
+		}
+		if got, want := nicStats.Tx.Bytes.Value(), uint64(actualTxLength); got != want {
+			t.Errorf("got Tx.Bytes.Value() = %d, want = %d", got, want)
+		}
+		txPacketsTotal += want
+		txBytesTotal += actualTxLength
 	}
 
-	payload := buffer.NewView(10)
-	// Write a packet out via the address for NIC 1
-	if err := sendTo(s, "\x01", payload); err != nil {
-		t.Fatal("sendTo failed: ", err)
+	// Now verify that each NIC stats was correctly aggregated at the stack level.
+	if got, want := s.Stats().NICs.Rx.Packets.Value(), uint64(rxPacketsTotal); got != want {
+		t.Errorf("got s.Stats().NIC.Rx.Packets.Value() = %d, want = %d", got, want)
 	}
-	want := uint64(ep1.Drain())
-	if got := s.NICInfo()[1].Stats.Tx.Packets.Value(); got != want {
-		t.Errorf("got Tx.Packets.Value() = %d, ep1.Drain() = %d", got, want)
+	if got, want := s.Stats().NICs.Rx.Bytes.Value(), uint64(rxBytesTotal); got != want {
+		t.Errorf("got s.Stats().Rx.Bytes.Value() = %d, want = %d", got, want)
 	}
-
-	if got, want := s.NICInfo()[1].Stats.Tx.Bytes.Value(), uint64(len(payload)+fakeNetHeaderLen); got != want {
+	if got, want := s.Stats().NICs.Tx.Packets.Value(), uint64(txPacketsTotal); got != want {
+		t.Errorf("got Tx.Packets.Value() = %d, ep.Drain() = %d", got, want)
+	}
+	if got, want := s.Stats().NICs.Tx.Bytes.Value(), uint64(txBytesTotal); got != want {
 		t.Errorf("got Tx.Bytes.Value() = %d, want = %d", got, want)
 	}
 }