diff --git a/src/ifmib/ifmib.go b/src/ifmib/ifmib.go index e3c5c46..19899df 100644 --- a/src/ifmib/ifmib.go +++ b/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 - if details != nil && details.Speed > 0 && details.Speed <= 2500000000 { - item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeGauge32 - item.Value = uint32(details.Speed) - } else if details == nil || details.Speed == 0 { - item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeGauge32 - item.Value = uint32(1000000000) + // ifSpeed (.5) - RFC 2863: cap at max uint32; ifHighSpeed carries the real value + ifSpeedVal := uint32(1000000000) + if details != nil && details.Speed > 0 { + if details.Speed >= math.MaxUint32 { + ifSpeedVal = math.MaxUint32 + } else { + ifSpeedVal = uint32(details.Speed) + } } + item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx)) + item.Type = pdu.VariableTypeGauge32 + item.Value = ifSpeedVal // ifPhysAddress (.6) macAddr := "" diff --git a/src/ifmib/ifmib_test.go b/src/ifmib/ifmib_test.go index 04c55de..ace6838 100644 --- a/src/ifmib/ifmib_test.go +++ b/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() diff --git a/src/vpp/vpp_iface.go b/src/vpp/vpp_iface.go index 2bc5b16..dac8cef 100644 --- a/src/vpp/vpp_iface.go +++ b/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() { diff --git a/src/vpp/vpp_stats.go b/src/vpp/vpp_stats.go index 83788bb..d82b6c3 100644 --- a/src/vpp/vpp_stats.go +++ b/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 {