Fix ifSpeed/ifHighSpeed for >1Gbps interfaces

Two related bugs left interfaces faster than 1Gbps reporting wrong
values to SNMP collectors:

1. ifSpeed (.5) was silently dropped for interfaces whose link speed
   exceeded 2.5Gbps. RFC 2863 requires ifSpeed to always be present
   and saturated at uint32 max (4294967295) when the real speed
   exceeds what fits in a Gauge32; ifHighSpeed carries the actual
   Mbps value. The row is now always emitted, capped at math.MaxUint32.

2. UpdateStats only rebuilt the MIB when the interface set changed.
   On startup, the stats routine connects to VPP and runs the first
   poll before the 1-second event-monitor poll has had a chance to
   call InitializeEventWatching, so the MIB is built from an empty
   interfaceDetails map and every row defaults to 1Gbps for life.

   Fix: filterValidInterfaces (which already calls
   GetAllInterfaceDetails to filter deleted interfaces from the stats
   segment) now also pushes those details through the MIB callback
   via a new InterfaceManager.NotifyDetails method. UpdateInterface-
   Details sets a staticFieldsDirty flag whenever Speed/MAC/MTU/
   admin/oper/name actually change, and UpdateStats forces a rebuild
   on the next poll while that flag is set. Identical re-pushes do
   not dirty the cache, so AgentX session re-registration only
   happens on real change.

Adds tests for the ifSpeed branches (0/nil/1G/10G/25G/100G) and for
the dirty-flag transitions (fresh push, identical re-push, changed
Speed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/ifmib/ifmib.go

@@ -3,7 +3,9 @@
 package ifmib
 
 import (
+	"bytes"
 	"fmt"
+	"math"
 	"os"
 	"sync"
 	"time"
@@ -44,6 +46,14 @@ type InterfaceMIB struct {
 	descriptions      map[string]string
 	interfaceDetails  map[uint32]*vpp.InterfaceDetails
 
+	// staticFieldsDirty signals that interfaceDetails changed since the
+	// last rebuild and the next UpdateStats must rebuild the MIB even
+	// if the interface set looks unchanged. Without this flag the
+	// startup race between statsRoutine and eventMonitoringRoutine can
+	// rebuild the MIB before any details arrive, locking every row at
+	// the 1Gbps default forever.
+	staticFieldsDirty bool
+
 	// Counter items - direct references for fast updates
 	counterItems map[string]*agentx.ListItem
 }
@@ -104,9 +114,29 @@ func (m *InterfaceMIB) UpdateInterfaceDetails(details []vpp.InterfaceDetails) {
 	defer m.mutex.Unlock()
 
 	for _, detail := range details {
-		m.interfaceDetails[uint32(detail.SwIfIndex)] = &detail
+		d := detail
+		idx := uint32(d.SwIfIndex)
+		existing := m.interfaceDetails[idx]
+		if !sameStaticFields(existing, &d) {
+			m.staticFieldsDirty = true
+		}
+		m.interfaceDetails[idx] = &d
 	}
-	logger.Debugf("Updated interface details for %d interfaces", len(details))
+	logger.Debugf("Updated interface details for %d interfaces (dirty=%t)", len(details), m.staticFieldsDirty)
+}
+
+// sameStaticFields reports whether b carries the same MIB-relevant fields
+// as a; nil a/b counts as different so a fresh detail always dirties.
+func sameStaticFields(a, b *vpp.InterfaceDetails) bool {
+	if a == nil || b == nil {
+		return false
+	}
+	return a.Speed == b.Speed &&
+		a.AdminStatus == b.AdminStatus &&
+		a.OperStatus == b.OperStatus &&
+		a.MTU == b.MTU &&
+		a.InterfaceName == b.InterfaceName &&
+		bytes.Equal(a.MacAddress, b.MacAddress)
 }
 
 func (m *InterfaceMIB) UpdateStats(interfaceStats *api.InterfaceStats) {
@@ -120,7 +150,7 @@ func (m *InterfaceMIB) UpdateStats(interfaceStats *api.InterfaceStats) {
 	}
 
 	// Simple comparison - rebuild if different
-	needsRebuild := len(newInterfaces) != len(m.currentInterfaces)
+	needsRebuild := m.staticFieldsDirty || len(newInterfaces) != len(m.currentInterfaces)
 	if !needsRebuild {
 		for idx := range newInterfaces {
 			if !m.currentInterfaces[idx] {
@@ -131,12 +161,13 @@ func (m *InterfaceMIB) UpdateStats(interfaceStats *api.InterfaceStats) {
 	}
 
 	if needsRebuild {
-		logger.Debugf("Interface set changed, rebuilding MIB")
+		logger.Debugf("Rebuilding MIB (set changed or details dirty)")
 		if err := m.rebuildMIB(interfaceStats.Interfaces); err != nil {
 			logger.Printf("Failed to rebuild MIB: %v", err)
 			return
 		}
 		m.currentInterfaces = newInterfaces
+		m.staticFieldsDirty = false
 	} else {
 		// Fast path: just update counters
 		logger.Debugf("Updating counters for %d interfaces", len(interfaceStats.Interfaces))
@@ -246,16 +277,18 @@ func (m *InterfaceMIB) addStaticFields(handler *agentx.ListHandler, iface *api.I
 	item.Type = pdu.VariableTypeInteger
 	item.Value = mtu
 
-	// ifSpeed (.5) - Only for speeds <= 2.5Gbps
+	// ifSpeed (.5) - RFC 2863: cap at max uint32; ifHighSpeed carries the real value
-	if details != nil && details.Speed > 0 && details.Speed <= 2500000000 {
+	ifSpeedVal := uint32(1000000000)
-		item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx))
+	if details != nil && details.Speed > 0 {
-		item.Type = pdu.VariableTypeGauge32
+		if details.Speed >= math.MaxUint32 {
-		item.Value = uint32(details.Speed)
+			ifSpeedVal = math.MaxUint32
-	} else if details == nil || details.Speed == 0 {
+		} else {
-		item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx))
+			ifSpeedVal = uint32(details.Speed)
-		item.Type = pdu.VariableTypeGauge32
+		}
-		item.Value = uint32(1000000000)
 	}
+	item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx))
+	item.Type = pdu.VariableTypeGauge32
+	item.Value = ifSpeedVal
 
 	// ifPhysAddress (.6)
 	macAddr := ""

src/ifmib/ifmib_test.go

@@ -3,10 +3,16 @@
 package ifmib
 
 import (
+	"fmt"
+	"math"
 	"os"
 	"testing"
 
+	"github.com/posteo/go-agentx"
+	"github.com/posteo/go-agentx/value"
 	"go.fd.io/govpp/api"
+
+	"govpp-snmp-agentx/vpp"
 )
 
 func TestNewInterfaceMIB(t *testing.T) {
@@ -149,6 +155,96 @@ func TestLoadVPPConfigInvalidYAML(t *testing.T) {
 	}
 }
 
+func TestAddStaticFieldsIfSpeed(t *testing.T) {
+	cases := []struct {
+		name              string
+		details           *vpp.InterfaceDetails
+		wantSpeed         uint32
+		wantHighSpeedMbps uint32
+	}{
+		{"unknown speed defaults to 1Gbps", &vpp.InterfaceDetails{Speed: 0}, 1000000000, 1000},
+		{"nil details defaults to 1Gbps", nil, 1000000000, 1000},
+		{"1Gbps reported verbatim", &vpp.InterfaceDetails{Speed: 1000000000}, 1000000000, 1000},
+		{"10Gbps caps ifSpeed at uint32 max", &vpp.InterfaceDetails{Speed: 10000000000}, math.MaxUint32, 10000},
+		{"25Gbps caps ifSpeed at uint32 max", &vpp.InterfaceDetails{Speed: 25000000000}, math.MaxUint32, 25000},
+		{"100Gbps caps ifSpeed at uint32 max", &vpp.InterfaceDetails{Speed: 100000000000}, math.MaxUint32, 100000},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			mib := NewInterfaceMIB()
+			handler := &agentx.ListHandler{}
+			iface := &api.InterfaceCounters{InterfaceIndex: 0, InterfaceName: "test0"}
+			idx := 1000
+
+			mib.addStaticFields(handler, iface, idx, tc.details)
+
+			gotSpeed, ok := lookupGauge32(t, handler, fmt.Sprintf("%s.5.%d", ifEntryOID, idx))
+			if !ok {
+				t.Fatalf("ifSpeed row was not registered for idx %d", idx)
+			}
+			if gotSpeed != tc.wantSpeed {
+				t.Errorf("ifSpeed = %d, want %d", gotSpeed, tc.wantSpeed)
+			}
+
+			gotHigh, ok := lookupGauge32(t, handler, fmt.Sprintf("%s.15.%d", ifXTableOID, idx))
+			if !ok {
+				t.Fatalf("ifHighSpeed row was not registered for idx %d", idx)
+			}
+			if gotHigh != tc.wantHighSpeedMbps {
+				t.Errorf("ifHighSpeed = %d, want %d", gotHigh, tc.wantHighSpeedMbps)
+			}
+		})
+	}
+}
+
+func lookupGauge32(t *testing.T, h *agentx.ListHandler, oid string) (uint32, bool) {
+	t.Helper()
+	parsed, err := value.ParseOID(oid)
+	if err != nil {
+		t.Fatalf("parse OID %q: %v", oid, err)
+	}
+	gotOID, _, val, err := h.Get(nil, parsed)
+	if err != nil || gotOID == nil {
+		return 0, false
+	}
+	v, ok := val.(uint32)
+	if !ok {
+		t.Fatalf("OID %s value type %T, want uint32", oid, val)
+	}
+	return v, true
+}
+
+func TestUpdateInterfaceDetailsMarksDirty(t *testing.T) {
+	mib := NewInterfaceMIB()
+
+	if mib.staticFieldsDirty {
+		t.Fatal("freshly constructed MIB should not be dirty")
+	}
+
+	// First push: previously-unseen interface dirties the cache.
+	mib.UpdateInterfaceDetails([]vpp.InterfaceDetails{{SwIfIndex: 1, Speed: 25000000000}})
+	if !mib.staticFieldsDirty {
+		t.Fatal("first push of details should dirty the static-field cache")
+	}
+
+	// Simulate the rebuild that UpdateStats would do.
+	mib.staticFieldsDirty = false
+
+	// Second push with identical fields: must not re-dirty (avoids
+	// thrashing the AgentX session reregister on every stats poll).
+	mib.UpdateInterfaceDetails([]vpp.InterfaceDetails{{SwIfIndex: 1, Speed: 25000000000}})
+	if mib.staticFieldsDirty {
+		t.Fatal("identical re-push must not dirty the static-field cache")
+	}
+
+	// Third push with a changed field: must dirty.
+	mib.UpdateInterfaceDetails([]vpp.InterfaceDetails{{SwIfIndex: 1, Speed: 10000000000}})
+	if !mib.staticFieldsDirty {
+		t.Fatal("changed Speed must dirty the static-field cache")
+	}
+}
+
 func TestUpdateStatsBasic(t *testing.T) {
 	mib := NewInterfaceMIB()
 

src/vpp/vpp_iface.go

@@ -47,6 +47,18 @@ func (im *InterfaceManager) SetEventCallback(callback InterfaceEventCallback) {
 	im.eventCallback = callback
 }
 
+// NotifyDetails forwards an externally fetched interface list through
+// the configured event callback. The stats path uses this so the MIB's
+// static-field cache stays current on every poll, not just on
+// asynchronous VPP interface events — without it the first stats poll
+// can rebuild the MIB before InitializeEventWatching has populated any
+// details, leaving every row stuck at the 1Gbps default until restart.
+func (im *InterfaceManager) NotifyDetails(details []InterfaceDetails) {
+	if im.eventCallback != nil {
+		im.eventCallback(details)
+	}
+}
+
 // InitializeEventWatching starts event watching and retrieves initial interface details
 func (im *InterfaceManager) InitializeEventWatching() error {
 	if !im.client.IsConnected() {

src/vpp/vpp_stats.go

@@ -187,6 +187,13 @@ func (sm *StatsManager) filterValidInterfaces(stats *api.InterfaceStats) (*api.I
 		return stats, err
 	}
 
+	// Push the freshly fetched details through the event callback so the
+	// MIB's static-field cache (Speed, MAC, MTU, ...) is in sync with the
+	// stats it's about to receive. Otherwise the very first UpdateStats
+	// races eventMonitoringRoutine and can rebuild from an empty details
+	// map.
+	sm.interfaceManager.NotifyDetails(currentInterfaces)
+
 	// Create a map of valid sw_if_index values
 	validInterfaces := make(map[uint32]bool)
 	for _, iface := range currentInterfaces {