Record counts of packets with unknown L3/L4 numbers

Previously, we recorded a single aggregated count. These per-protocol counts
can help us debug field issues when frames are dropped for this reason.

PiperOrigin-RevId: 405913911

pkg/sentry/socket/netstack/netstack.go

@@ -81,9 +81,7 @@ func mustCreateGauge(name, description string) *tcpip.StatCounter {
 var Metrics = tcpip.Stats{
 	DroppedPackets: mustCreateMetric("/netstack/dropped_packets", "Number of packets dropped at the transport layer."),
 	NICs: tcpip.NICStats{
-		UnknownL3ProtocolRcvdPackets: mustCreateMetric("/netstack/nic/unknown_l3_protocol_received_packets", "Number of packets received that were for an unknown or unsupported L3 protocol."),
-		UnknownL4ProtocolRcvdPackets: mustCreateMetric("/netstack/nic/unknown_l4_protocol_received_packets", "Number of packets received that were for an unknown or unsupported L4 protocol."),
-		MalformedL4RcvdPackets:       mustCreateMetric("/netstack/nic/malformed_l4_received_packets", "Number of packets received that failed L4 header parsing."),
+		MalformedL4RcvdPackets: mustCreateMetric("/netstack/nic/malformed_l4_received_packets", "Number of packets received that failed L4 header parsing."),
 		Tx: tcpip.NICPacketStats{
 			Packets: mustCreateMetric("/netstack/nic/tx/packets", "Number of packets transmitted."),
 			Bytes:   mustCreateMetric("/netstack/nic/tx/bytes", "Number of bytes transmitted."),

pkg/tcpip/network/arp/stats_test.go

@@ -45,7 +45,10 @@ func TestMultiCounterStatsInitialization(t *testing.T) {
 	// expected to be bound by a MultiCounterStat.
 	refStack := s.Stats()
 	refEP := ep.stats.localStats
-	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.arp).Elem(), []reflect.Value{reflect.ValueOf(&refEP.ARP).Elem(), reflect.ValueOf(&refStack.ARP).Elem()}); err != nil {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.arp).Elem(), []reflect.Value{reflect.ValueOf(&refEP.ARP).Elem(), reflect.ValueOf(&refStack.ARP).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: false,
+	}); err != nil {
 		t.Error(err)
 	}
 }

pkg/tcpip/network/ipv4/stats_test.go

@@ -87,13 +87,22 @@ func TestMultiCounterStatsInitialization(t *testing.T) {
 	// expected to be bound by a MultiCounterStat.
 	refStack := s.Stats()
 	refEP := ep.stats.localStats
-	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.ip).Elem(), []reflect.Value{reflect.ValueOf(&refEP.IP).Elem(), reflect.ValueOf(&refStack.IP).Elem()}); err != nil {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.ip).Elem(), []reflect.Value{reflect.ValueOf(&refEP.IP).Elem(), reflect.ValueOf(&refStack.IP).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: false,
+	}); err != nil {
 		t.Error(err)
 	}
-	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.icmp).Elem(), []reflect.Value{reflect.ValueOf(&refEP.ICMP).Elem(), reflect.ValueOf(&refStack.ICMP.V4).Elem()}); err != nil {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.icmp).Elem(), []reflect.Value{reflect.ValueOf(&refEP.ICMP).Elem(), reflect.ValueOf(&refStack.ICMP.V4).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: false,
+	}); err != nil {
 		t.Error(err)
 	}
-	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.igmp).Elem(), []reflect.Value{reflect.ValueOf(&refEP.IGMP).Elem(), reflect.ValueOf(&refStack.IGMP).Elem()}); err != nil {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.igmp).Elem(), []reflect.Value{reflect.ValueOf(&refEP.IGMP).Elem(), reflect.ValueOf(&refStack.IGMP).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: false,
+	}); err != nil {
 		t.Error(err)
 	}
 }

pkg/tcpip/network/ipv6/ipv6_test.go

@@ -3515,10 +3515,16 @@ func TestMultiCounterStatsInitialization(t *testing.T) {
 	// supposed to be bound.
 	refStack := s.Stats()
 	refEP := ep.stats.localStats
-	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.ip).Elem(), []reflect.Value{reflect.ValueOf(&refStack.IP).Elem(), reflect.ValueOf(&refEP.IP).Elem()}); err != nil {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.ip).Elem(), []reflect.Value{reflect.ValueOf(&refStack.IP).Elem(), reflect.ValueOf(&refEP.IP).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: false,
+	}); err != nil {
 		t.Error(err)
 	}
-	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.icmp).Elem(), []reflect.Value{reflect.ValueOf(&refStack.ICMP.V6).Elem(), reflect.ValueOf(&refEP.ICMP).Elem()}); err != nil {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&ep.stats.icmp).Elem(), []reflect.Value{reflect.ValueOf(&refStack.ICMP.V6).Elem(), reflect.ValueOf(&refEP.ICMP).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: false,
+	}); err != nil {
 		t.Error(err)
 	}
 }

pkg/tcpip/stack/nic.go

@@ -727,7 +727,7 @@ func (n *nic) DeliverNetworkPacket(remote, local tcpip.LinkAddress, protocol tcp
 
 	networkEndpoint, ok := n.networkEndpoints[protocol]
 	if !ok {
-		n.stats.unknownL3ProtocolRcvdPackets.Increment()
+		n.stats.unknownL3ProtocolRcvdPacketCounts.Increment(uint64(protocol))
 		return
 	}
 
@@ -827,7 +827,7 @@ func (n *nic) deliverOutboundPacket(remote tcpip.LinkAddress, pkt *PacketBuffer)
 func (n *nic) DeliverTransportPacket(protocol tcpip.TransportProtocolNumber, pkt *PacketBuffer) TransportPacketDisposition {
 	state, ok := n.stack.transportProtocols[protocol]
 	if !ok {
-		n.stats.unknownL4ProtocolRcvdPackets.Increment()
+		n.stats.unknownL4ProtocolRcvdPacketCounts.Increment(uint64(protocol))
 		return TransportPacketProtocolUnreachable
 	}
 

pkg/tcpip/stack/nic_stats.go

@@ -35,7 +35,7 @@ func (m *multiCounterNICPacketStats) init(a, b *tcpip.NICPacketStats) {
 	m.bytes.Init(a.Bytes, b.Bytes)
 }
 
-// LINT.ThenChange(../../tcpip.go:NICPacketStats)
+// LINT.ThenChange(../tcpip.go:NICPacketStats)
 
 // LINT.IfChange(multiCounterNICNeighborStats)
 
@@ -47,23 +47,23 @@ func (m *multiCounterNICNeighborStats) init(a, b *tcpip.NICNeighborStats) {
 	m.unreachableEntryLookups.Init(a.UnreachableEntryLookups, b.UnreachableEntryLookups)
 }
 
-// LINT.ThenChange(../../tcpip.go:NICNeighborStats)
+// 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
+	unknownL3ProtocolRcvdPacketCounts tcpip.MultiIntegralStatCounterMap
+	unknownL4ProtocolRcvdPacketCounts tcpip.MultiIntegralStatCounterMap
+	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.unknownL3ProtocolRcvdPacketCounts.Init(a.UnknownL3ProtocolRcvdPacketCounts, b.UnknownL3ProtocolRcvdPacketCounts)
+	m.unknownL4ProtocolRcvdPacketCounts.Init(a.UnknownL4ProtocolRcvdPacketCounts, b.UnknownL4ProtocolRcvdPacketCounts)
 	m.malformedL4RcvdPackets.Init(a.MalformedL4RcvdPackets, b.MalformedL4RcvdPackets)
 	m.tx.init(&a.Tx, &b.Tx)
 	m.rx.init(&a.Rx, &b.Rx)
@@ -71,4 +71,4 @@ func (m *multiCounterNICStats) init(a, b *tcpip.NICStats) {
 	m.neighbor.init(&a.Neighbor, &b.Neighbor)
 }
 
-// LINT.ThenChange(../../tcpip.go:NICStats)
+// LINT.ThenChange(../tcpip.go:NICStats)

pkg/tcpip/stack/nic_test.go

@@ -206,6 +206,45 @@ func TestDisabledRxStatsWhenNICDisabled(t *testing.T) {
 	}
 }
 
+func TestPacketWithUnknownNetworkProtocolNumber(t *testing.T) {
+	nic := nic{
+		stats:   makeNICStats(tcpip.NICStats{}.FillIn()),
+		enabled: 1,
+	}
+	// IPv4 isn't recognized since we haven't initialized the NIC with an IPv4
+	// endpoint.
+	nic.DeliverNetworkPacket("", "", header.IPv4ProtocolNumber, NewPacketBuffer(PacketBufferOptions{
+		Data: buffer.View([]byte{1, 2, 3, 4}).ToVectorisedView(),
+	}))
+	var count uint64
+	if got, ok := nic.stats.local.UnknownL3ProtocolRcvdPacketCounts.Get(uint64(header.IPv4ProtocolNumber)); ok {
+		count = got.Value()
+	}
+	if count != 1 {
+		t.Errorf("got UnknownL3ProtocolRcvdPacketCounts[header.IPv4ProtocolNumber] = %d, want = 1", count)
+	}
+}
+
+func TestPacketWithUnknownTransportProtocolNumber(t *testing.T) {
+	nic := nic{
+		stack:   &Stack{},
+		stats:   makeNICStats(tcpip.NICStats{}.FillIn()),
+		enabled: 1,
+	}
+	// UDP isn't recognized since we haven't initialized the NIC with a UDP
+	// protocol.
+	nic.DeliverTransportPacket(header.UDPProtocolNumber, NewPacketBuffer(PacketBufferOptions{
+		Data: buffer.View([]byte{1, 2, 3, 4}).ToVectorisedView(),
+	}))
+	var count uint64
+	if got, ok := nic.stats.local.UnknownL4ProtocolRcvdPacketCounts.Get(uint64(header.UDPProtocolNumber)); ok {
+		count = got.Value()
+	}
+	if count != 1 {
+		t.Errorf("got UnknownL4ProtocolRcvdPacketCounts[header.UDPProtocolNumber] = %d, want = 1", count)
+	}
+}
+
 func TestMultiCounterStatsInitialization(t *testing.T) {
 	global := tcpip.NICStats{}.FillIn()
 	nic := nic{
@@ -213,7 +252,10 @@ func TestMultiCounterStatsInitialization(t *testing.T) {
 	}
 	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 {
+	if err := testutil.ValidateMultiCounterStats(reflect.ValueOf(&multi).Elem(), []reflect.Value{reflect.ValueOf(&local).Elem(), reflect.ValueOf(&global).Elem()}, testutil.ValidateMultiCounterStatsOptions{
+		ExpectMultiCounterStat:            true,
+		ExpectMultiIntegralStatCounterMap: true,
+	}); err != nil {
 		t.Error(err)
 	}
 }

pkg/tcpip/tcpip.go

@@ -1301,7 +1301,8 @@ func (s *StatCounter) String() string {
 
 // A MultiCounterStat keeps track of two counters at once.
 type MultiCounterStat struct {
-	a, b *StatCounter
+	a *StatCounter
+	b *StatCounter
 }
 
 // Init sets both internal counters to point to a and b.
@@ -1923,17 +1924,89 @@ type NICPacketStats struct {
 	// LINT.ThenChange(stack/nic_stats.go:multiCounterNICPacketStats)
 }
 
+// IntegralStatCounterMap holds a map associating integral keys with
+// StatCounters.
+type IntegralStatCounterMap struct {
+	mu sync.RWMutex
+	// +checklocks:mu
+	counterMap map[uint64]*StatCounter
+}
+
+// Keys returns all keys present in the map.
+func (m *IntegralStatCounterMap) Keys() []uint64 {
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+	var keys []uint64
+	for k := range m.counterMap {
+		keys = append(keys, k)
+	}
+	return keys
+}
+
+// Get returns the counter mapped by the provided key.
+func (m *IntegralStatCounterMap) Get(key uint64) (*StatCounter, bool) {
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+	counter, ok := m.counterMap[key]
+	return counter, ok
+}
+
+// Init initializes the map.
+func (m *IntegralStatCounterMap) Init() {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.counterMap = make(map[uint64]*StatCounter)
+}
+
+// Increment increments the counter associated with the provided key.
+func (m *IntegralStatCounterMap) Increment(key uint64) {
+	m.mu.RLock()
+	counter, ok := m.counterMap[key]
+	m.mu.RUnlock()
+
+	if !ok {
+		m.mu.Lock()
+		counter, ok = m.counterMap[key]
+		if !ok {
+			counter = new(StatCounter)
+			m.counterMap[key] = counter
+		}
+		m.mu.Unlock()
+	}
+	counter.Increment()
+}
+
+// A MultiIntegralStatCounterMap keeps track of two integral counter maps at
+// once.
+type MultiIntegralStatCounterMap struct {
+	a *IntegralStatCounterMap
+	b *IntegralStatCounterMap
+}
+
+// Init sets the internal integral counter maps to point to a and b.
+func (m *MultiIntegralStatCounterMap) Init(a, b *IntegralStatCounterMap) {
+	m.a = a
+	m.b = b
+}
+
+// Increment increments the counter in each map corresponding to the
+// provided key.
+func (m *MultiIntegralStatCounterMap) Increment(key uint64) {
+	m.a.Increment(key)
+	m.b.Increment(key)
+}
+
 // NICStats holds NIC statistics.
 type NICStats struct {
 	// LINT.IfChange(NICStats)
 
-	// UnknownL3ProtocolRcvdPackets is the number of packets received that were
-	// for an unknown or unsupported network protocol.
-	UnknownL3ProtocolRcvdPackets *StatCounter
+	// UnknownL3ProtocolRcvdPacketCounts records the number of packets recieved
+	// for each unknown or unsupported netowrk protocol number.
+	UnknownL3ProtocolRcvdPacketCounts *IntegralStatCounterMap
 
-	// UnknownL4ProtocolRcvdPackets is the number of packets received that were
-	// for an unknown or unsupported transport protocol.
-	UnknownL4ProtocolRcvdPackets *StatCounter
+	// UnknownL4ProtocolRcvdPacketCounts records the number of packets recieved
+	// for each unknown or unsupported transport protocol number.
+	UnknownL4ProtocolRcvdPacketCounts *IntegralStatCounterMap
 
 	// MalformedL4RcvdPackets is the number of packets received by a NIC that
 	// could not be delivered to a transport endpoint because the L4 header could
@@ -2103,6 +2176,11 @@ func InitStatCounters(v reflect.Value) {
 			if *s == nil {
 				*s = new(StatCounter)
 			}
+		} else if s, ok := v.Addr().Interface().(**IntegralStatCounterMap); ok {
+			if *s == nil {
+				*s = new(IntegralStatCounterMap)
+				(*s).Init()
+			}
 		} else {
 			InitStatCounters(v)
 		}

pkg/tcpip/testutil/testutil.go

@@ -77,12 +77,43 @@ func validateField(ref reflect.Value, refName string, m tcpip.MultiCounterStat,
 	return nil
 }
 
-// ValidateMultiCounterStats verifies that every counter stored in multi is
-// correctly tracking its counterpart in the given counters.
-func ValidateMultiCounterStats(multi reflect.Value, counters []reflect.Value) error {
+func validateIntegralMapField(ref reflect.Value, refName string, m tcpip.MultiIntegralStatCounterMap, multiName string) error {
+	// The field names are expected to match (case insensitive).
+	if !strings.EqualFold(refName, multiName) {
+		return fmt.Errorf("wrong field name: got = %s, want = %s", multiName, refName)
+	}
+	s, ok := ref.Addr().Interface().(**tcpip.IntegralStatCounterMap)
+	if !ok {
+		return fmt.Errorf("field is not an IntegralStatCounterMap")
+	}
+
+	const key = 42
+
+	getValue := func() uint64 {
+		counter, ok := (*s).Get(key)
+		if !ok {
+			return 0
+		}
+		return counter.Value()
+	}
+
+	before := getValue()
+
+	m.Increment(key)
+
+	after := getValue()
+
+	if after != before+1 {
+		return fmt.Errorf("updates to the '%s MultiCounterStat' counters are not reflected in the '%s CounterStat'", multiName, refName)
+	}
+
+	return nil
+}
+
+func validateMultiCounterStats(multi reflect.Value, counters []reflect.Value) (foundMultiCounterStat, foundMultiIntegralStatCounterMap bool, err error) {
 	for _, c := range counters {
 		if err := checkFieldCounts(c, multi); err != nil {
-			return err
+			return false, false, err
 		}
 	}
 
@@ -90,23 +121,59 @@ func ValidateMultiCounterStats(multi reflect.Value, counters []reflect.Value) er
 		multiName := multi.Type().Field(i).Name
 		multiUnsafe := unsafeExposeUnexportedFields(multi.Field(i))
 
-		if m, ok := multiUnsafe.Addr().Interface().(*tcpip.MultiCounterStat); ok {
+		switch m := multiUnsafe.Addr().Interface().(type) {
+		case *tcpip.MultiCounterStat:
+			foundMultiCounterStat = true
 			for _, c := range counters {
 				if err := validateField(unsafeExposeUnexportedFields(c.Field(i)), c.Type().Field(i).Name, *m, multiName); err != nil {
-					return err
+					return false, false, err
 				}
 			}
-		} else {
+		case *tcpip.MultiIntegralStatCounterMap:
+			foundMultiIntegralStatCounterMap = true
+			for _, c := range counters {
+				if err := validateIntegralMapField(unsafeExposeUnexportedFields(c.Field(i)), c.Type().Field(i).Name, *m, multiName); err != nil {
+					return false, false, err
+				}
+			}
+		default:
 			var countersNextField []reflect.Value
 			for _, c := range counters {
 				countersNextField = append(countersNextField, c.Field(i))
 			}
-			if err := ValidateMultiCounterStats(multi.Field(i), countersNextField); err != nil {
-				return err
+			innerFoundMultiCounterStat, innerFoundMultiIntegralStatCounterMap, err := validateMultiCounterStats(multi.Field(i), countersNextField)
+			if err != nil {
+				return false, false, err
 			}
+			foundMultiCounterStat = foundMultiCounterStat || innerFoundMultiCounterStat
+			foundMultiIntegralStatCounterMap = foundMultiIntegralStatCounterMap || innerFoundMultiIntegralStatCounterMap
 		}
 	}
 
+	return foundMultiCounterStat, foundMultiIntegralStatCounterMap, nil
+}
+
+// ValidateMultiCounterStatsOptions holds options used when validating multi
+// counter stat structs.
+type ValidateMultiCounterStatsOptions struct {
+	ExpectMultiCounterStat            bool
+	ExpectMultiIntegralStatCounterMap bool
+}
+
+// ValidateMultiCounterStats verifies that every counter stored in multi is
+// correctly tracking its counterpart in the given counters.
+func ValidateMultiCounterStats(multi reflect.Value, counters []reflect.Value, options ValidateMultiCounterStatsOptions) error {
+	foundMultiCounterStat, foundMultiIntegralStatCounterMap, err := validateMultiCounterStats(multi, counters)
+	if err != nil {
+		return err
+	}
+	if foundMultiCounterStat != options.ExpectMultiCounterStat {
+		return fmt.Errorf("got %T presence: %t, want: %t", (*tcpip.MultiCounterStat)(nil), foundMultiCounterStat, options.ExpectMultiCounterStat)
+	}
+	if foundMultiIntegralStatCounterMap != options.ExpectMultiIntegralStatCounterMap {
+		return fmt.Errorf("got %T presence: %t, want: %t", (*tcpip.MultiIntegralStatCounterMap)(nil), foundMultiIntegralStatCounterMap, options.ExpectMultiIntegralStatCounterMap)
+	}
+
 	return nil
 }