From 16bebb0ecef93f7bf417a8d19732052a0715b415 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 23 Nov 2025 10:06:26 +0100 Subject: [PATCH] Simplify the ifmib integration, stop using reflection, and move to counter updates Update tests. --- src/ifmib/ifmib.go | 596 +++++++++++++++++++--------------------- src/ifmib/ifmib_test.go | 55 ++-- 2 files changed, 309 insertions(+), 342 deletions(-) diff --git a/src/ifmib/ifmib.go b/src/ifmib/ifmib.go index 26b8928..e3c5c46 100644 --- a/src/ifmib/ifmib.go +++ b/src/ifmib/ifmib.go @@ -5,7 +5,6 @@ package ifmib import ( "fmt" "os" - "reflect" "sync" "time" @@ -19,55 +18,11 @@ import ( "govpp-snmp-agentx/vpp" ) -// IF-MIB OID bases: -// ifEntry (classic): 1.3.6.1.2.1.2.2.1 -// ifXTable (extended): 1.3.6.1.2.1.31.1.1.1 - -// ifEntry (1.3.6.1.2.1.2.2.1) - Classic Interface Table: -// ifIndex .1 - Integer32 -// ifDescr .2 - DisplayString -// ifType .3 - IANAifType -// ifMtu .4 - Integer32 -// ifSpeed .5 - Gauge32 -// ifPhysAddress .6 - PhysAddress -// ifAdminStatus .7 - INTEGER -// ifOperStatus .8 - INTEGER -// ifLastChange .9 - TimeTicks -// ifInOctets .10 - Counter32 -// ifInUcastPkts .11 - Counter32 -// ifInNUcastPkts .12 - Counter32 -// ifInDiscards .13 - Counter32 -// ifInErrors .14 - Counter32 -// ifInUnknownProtos .15 - Counter32 -// ifOutOctets .16 - Counter32 -// ifOutUcastPkts .17 - Counter32 -// ifOutNUcastPkts .18 - Counter32 -// ifOutDiscards .19 - Counter32 -// ifOutErrors .20 - Counter32 -// ifOutQLen .21 - Gauge32 -// ifSpecific .22 - OBJECT IDENTIFIER - -// ifXTable (1.3.6.1.2.1.31.1.1.1) - Extended Interface Table: -// ifName .1 - DisplayString -// ifInMulticastPkts .2 - Counter32 -// ifInBroadcastPkts .3 - Counter32 -// ifOutMulticastPkts .4 - Counter32 -// ifOutBroadcastPkts .5 - Counter32 -// ifHCInOctets .6 - Counter64 -// ifHCInUcastPkts .7 - Counter64 -// ifHCInMulticastPkts .8 - Counter64 -// ifHCInBroadcastPkts .9 - Counter64 -// ifHCOutOctets .10 - Counter64 -// ifHCOutUcastPkts .11 - Counter64 -// ifHCOutMulticastPkts .12 - Counter64 -// ifHCOutBroadcastPkts .13 - Counter64 -// ifHighSpeed .15 - Gauge32 (interface speed in Mbps) -// ifAlias .18 - DisplayString - +// IF-MIB OID bases const ifEntryOID = "1.3.6.1.2.1.2.2.1" const ifXTableOID = "1.3.6.1.2.1.31.1.1.1" -// VPP Config YAML structures +// VPP Config structures type VPPConfig struct { Interfaces map[string]VPPInterface `yaml:"interfaces"` Loopbacks map[string]VPPInterface `yaml:"loopbacks"` @@ -79,39 +34,43 @@ type VPPInterface struct { } type InterfaceMIB struct { - mutex sync.RWMutex - handler *agentx.ListHandler - ifEntrySession *agentx.Session - ifXTableSession *agentx.Session - stats map[uint32]*api.InterfaceCounters // indexed by interface index - descriptions map[string]string // interface name -> description mapping - interfaceDetails map[uint32]*vpp.InterfaceDetails // indexed by interface index + mutex sync.RWMutex + client *agentx.Client + ifEntrySession *agentx.Session + ifXTableSession *agentx.Session + + // Simple approach: track interface set and rebuild when it changes + currentInterfaces map[uint32]bool + descriptions map[string]string + interfaceDetails map[uint32]*vpp.InterfaceDetails + + // Counter items - direct references for fast updates + counterItems map[string]*agentx.ListItem } func NewInterfaceMIB() *InterfaceMIB { return &InterfaceMIB{ - handler: &agentx.ListHandler{}, - stats: make(map[uint32]*api.InterfaceCounters), - descriptions: make(map[string]string), - interfaceDetails: make(map[uint32]*vpp.InterfaceDetails), + currentInterfaces: make(map[uint32]bool), + descriptions: make(map[string]string), + interfaceDetails: make(map[uint32]*vpp.InterfaceDetails), + counterItems: make(map[string]*agentx.ListItem), } } func (m *InterfaceMIB) GetHandler() *agentx.ListHandler { - return m.handler + // Always create a new handler - this is the key simplification + return &agentx.ListHandler{} } func (m *InterfaceMIB) LoadVPPConfig(configPath string) error { m.mutex.Lock() defer m.mutex.Unlock() - // Read YAML file data, err := os.ReadFile(configPath) if err != nil { return fmt.Errorf("failed to read VPP config file: %v", err) } - // Parse YAML var config VPPConfig if err := yaml.Unmarshal(data, &config); err != nil { return fmt.Errorf("failed to parse VPP config YAML: %v", err) @@ -121,24 +80,18 @@ func (m *InterfaceMIB) LoadVPPConfig(configPath string) error { for ifName, ifConfig := range config.Interfaces { if ifConfig.Description != "" { m.descriptions[ifName] = ifConfig.Description - logger.Debugf("Loaded description for interface %s: %s", ifName, ifConfig.Description) } - - // Process sub-interfaces for subID, subConfig := range ifConfig.SubInterfaces { if subConfig.Description != "" { subIfName := fmt.Sprintf("%s.%s", ifName, subID) m.descriptions[subIfName] = subConfig.Description - logger.Debugf("Loaded description for sub-interface %s: %s", subIfName, subConfig.Description) } } } - // Extract loopback descriptions for ifName, ifConfig := range config.Loopbacks { if ifConfig.Description != "" { m.descriptions[ifName] = ifConfig.Description - logger.Debugf("Loaded description for loopback %s: %s", ifName, ifConfig.Description) } } @@ -150,316 +103,225 @@ func (m *InterfaceMIB) UpdateInterfaceDetails(details []vpp.InterfaceDetails) { m.mutex.Lock() defer m.mutex.Unlock() - logger.Debugf("Updating interface details for %d interfaces", len(details)) - - // Update interface details map for _, detail := range details { m.interfaceDetails[uint32(detail.SwIfIndex)] = &detail - logger.Debugf("Updated details for interface %d (%s): MAC=%x, Speed=%d", - detail.SwIfIndex, detail.InterfaceName, detail.MacAddress, detail.Speed) - } - - logger.Debugf("Interface details updated for %d interfaces", len(details)) -} - -func (m *InterfaceMIB) clearHandler() { - // Use reflection to access and clear the private fields of ListHandler - // since it doesn't have a Clear() method - handlerValue := reflect.ValueOf(m.handler).Elem() - - oidsField := handlerValue.FieldByName("oids") - itemsField := handlerValue.FieldByName("items") - - if oidsField.IsValid() && oidsField.CanSet() { - oidsField.Set(reflect.Zero(oidsField.Type())) - } - - if itemsField.IsValid() && itemsField.CanSet() { - itemsField.Set(reflect.Zero(itemsField.Type())) } + logger.Debugf("Updated interface details for %d interfaces", len(details)) } func (m *InterfaceMIB) UpdateStats(interfaceStats *api.InterfaceStats) { m.mutex.Lock() defer m.mutex.Unlock() - logger.Debugf("Updating IF-MIB with %d interfaces", len(interfaceStats.Interfaces)) - - // Clear existing entries while preserving the handler reference - // Since go-agentx 0.3.0 binds handlers to sessions at creation time - m.clearHandler() - m.stats = make(map[uint32]*api.InterfaceCounters) - - // Add new entries + // Check if interface set changed + newInterfaces := make(map[uint32]bool) for _, iface := range interfaceStats.Interfaces { - logger.Debugf("Processing interface %d (%s)", iface.InterfaceIndex, iface.InterfaceName) - m.stats[iface.InterfaceIndex] = &iface - m.addInterfaceToMIB(&iface) + newInterfaces[iface.InterfaceIndex] = true } - logger.Printf("Updated IF-MIB data for %d interfaces", len(m.stats)) - logger.Debugf("IF-MIB now contains %d interfaces", len(m.stats)) + // Simple comparison - rebuild if different + needsRebuild := len(newInterfaces) != len(m.currentInterfaces) + if !needsRebuild { + for idx := range newInterfaces { + if !m.currentInterfaces[idx] { + needsRebuild = true + break + } + } + } + + if needsRebuild { + logger.Debugf("Interface set changed, rebuilding MIB") + if err := m.rebuildMIB(interfaceStats.Interfaces); err != nil { + logger.Printf("Failed to rebuild MIB: %v", err) + return + } + m.currentInterfaces = newInterfaces + } else { + // Fast path: just update counters + logger.Debugf("Updating counters for %d interfaces", len(interfaceStats.Interfaces)) + for _, iface := range interfaceStats.Interfaces { + m.updateCounterValues(&iface) + } + } + + logger.Printf("Updated IF-MIB data for %d interfaces", len(interfaceStats.Interfaces)) } -func (m *InterfaceMIB) addInterfaceToMIB(iface *api.InterfaceCounters) { +func (m *InterfaceMIB) rebuildMIB(interfaces []api.InterfaceCounters) error { + // If no client is available (e.g., during testing), just track interfaces + if m.client == nil { + logger.Debugf("No AgentX client available, only tracking interface set") + return nil + } + + // Close old sessions + if m.ifEntrySession != nil { + m.ifEntrySession.Close() + m.ifEntrySession = nil + } + if m.ifXTableSession != nil { + m.ifXTableSession.Close() + m.ifXTableSession = nil + } + + // Create fresh handler + handler := &agentx.ListHandler{} + m.counterItems = make(map[string]*agentx.ListItem) + + // Build all MIB entries + for _, iface := range interfaces { + m.addInterfaceToHandler(handler, &iface) + } + + // Register new sessions + ifEntrySession, err := m.client.Session(value.MustParseOID(ifEntryOID), "ifEntry", handler) + if err != nil { + return fmt.Errorf("failed to create ifEntry session: %v", err) + } + + err = ifEntrySession.Register(127, value.MustParseOID(ifEntryOID)) + if err != nil { + ifEntrySession.Close() + return fmt.Errorf("failed to register ifEntry: %v", err) + } + + ifXTableSession, err := m.client.Session(value.MustParseOID(ifXTableOID), "ifXTable", handler) + if err != nil { + ifEntrySession.Close() + return fmt.Errorf("failed to create ifXTable session: %v", err) + } + + err = ifXTableSession.Register(127, value.MustParseOID(ifXTableOID)) + if err != nil { + ifEntrySession.Close() + ifXTableSession.Close() + return fmt.Errorf("failed to register ifXTable: %v", err) + } + + m.ifEntrySession = ifEntrySession + m.ifXTableSession = ifXTableSession + + logger.Debugf("Successfully rebuilt MIB with %d interfaces", len(interfaces)) + return nil +} + +func (m *InterfaceMIB) addInterfaceToHandler(handler *agentx.ListHandler, iface *api.InterfaceCounters) { idx := int(iface.InterfaceIndex) + *vpp.IfIndexOffset + details := m.interfaceDetails[iface.InterfaceIndex] - // Add ifEntry (classic interface table) entries - m.addIfEntry(iface, idx) + // Add static fields (these don't change frequently) + m.addStaticFields(handler, iface, idx, details) - // Add ifXTable (extended interface table) entries - m.addIfXTable(iface, idx) + // Add counter fields and store references for fast updates + m.addCounterFields(handler, iface, idx) logger.Debugf("Added interface %d (%s) to IF-MIB with SNMP index %d", iface.InterfaceIndex, iface.InterfaceName, idx) } -func (m *InterfaceMIB) addIfEntry(iface *api.InterfaceCounters, idx int) { +func (m *InterfaceMIB) addStaticFields(handler *agentx.ListHandler, iface *api.InterfaceCounters, idx int, details *vpp.InterfaceDetails) { var item *agentx.ListItem - // Get interface details if available - details := m.interfaceDetails[iface.InterfaceIndex] - // ifIndex (.1) - item = m.handler.Add(fmt.Sprintf("%s.1.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.1.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeInteger item.Value = int32(idx) // ifDescr (.2) - item = m.handler.Add(fmt.Sprintf("%s.2.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.2.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeOctetString item.Value = iface.InterfaceName // ifType (.3) - Using ethernetCsmacd(6) as default - item = m.handler.Add(fmt.Sprintf("%s.3.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.3.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeInteger item.Value = int32(6) - // ifMtu (.4) - Use real MTU if available, otherwise default to 1500 + // ifMtu (.4) mtu := int32(1500) if details != nil { mtu = int32(details.MTU) } - item = m.handler.Add(fmt.Sprintf("%s.4.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.4.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeInteger item.Value = mtu - // ifSpeed (.5) - Only populate for speeds <= 2.5Gbps (legacy field limitation) + // ifSpeed (.5) - Only for speeds <= 2.5Gbps if details != nil && details.Speed > 0 && details.Speed <= 2500000000 { - // Use real speed for interfaces <= 2.5Gbps - item = m.handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx)) + 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 { - // Default to 1Gbps when speed is unknown - item = m.handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.5.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeGauge32 item.Value = uint32(1000000000) } - // For speeds > 2.5Gbps, don't populate ifSpeed field at all - // ifPhysAddress (.6) - Use real MAC address if available + // ifPhysAddress (.6) macAddr := "" if details != nil && len(details.MacAddress) > 0 { macAddr = string(details.MacAddress) } - item = m.handler.Add(fmt.Sprintf("%s.6.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.6.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeOctetString item.Value = macAddr - // ifAdminStatus (.7) - Use real admin status if available - adminStatus := int32(1) // default up - if details != nil { - if details.AdminStatus { - adminStatus = 1 // up - } else { - adminStatus = 2 // down - } + // ifAdminStatus (.7) + adminStatus := int32(1) + if details != nil && !details.AdminStatus { + adminStatus = 2 } - item = m.handler.Add(fmt.Sprintf("%s.7.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.7.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeInteger item.Value = adminStatus - // ifOperStatus (.8) - Use real operational status if available - operStatus := int32(1) // default up - if details != nil { - if details.OperStatus { - operStatus = 1 // up - } else { - operStatus = 2 // down - } + // ifOperStatus (.8) + operStatus := int32(1) + if details != nil && !details.OperStatus { + operStatus = 2 } - item = m.handler.Add(fmt.Sprintf("%s.8.%d", ifEntryOID, idx)) + item = handler.Add(fmt.Sprintf("%s.8.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeInteger item.Value = operStatus - // ifLastChange (.9) - 0 (unknown) - item = m.handler.Add(fmt.Sprintf("%s.9.%d", ifEntryOID, idx)) + // ifLastChange (.9) + item = handler.Add(fmt.Sprintf("%s.9.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeTimeTicks item.Value = 0 * time.Second - // ifInOctets (.10) - item = m.handler.Add(fmt.Sprintf("%s.10.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.Rx.Bytes) - - // ifInUcastPkts (.11) - item = m.handler.Add(fmt.Sprintf("%s.11.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - // iface.Rx*cast.Packets is only set if "set interface feature X stats-collect-rx arc device-input" is configured - if iface.RxUnicast.Packets == 0 { - item.Value = uint32(iface.Rx.Packets) - } else { - item.Value = uint32(iface.RxUnicast.Packets) - } - - // ifInNUcastPkts (.12) - multicast + broadcast - item = m.handler.Add(fmt.Sprintf("%s.12.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.RxMulticast.Packets + iface.RxBroadcast.Packets) - - // ifInDiscards (.13) - using drops - item = m.handler.Add(fmt.Sprintf("%s.13.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.Drops) - - // ifInErrors (.14) - item = m.handler.Add(fmt.Sprintf("%s.14.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.RxErrors) - - // ifInUnknownProtos (.15) - 0 (not available) - item = m.handler.Add(fmt.Sprintf("%s.15.%d", ifEntryOID, idx)) + // ifInUnknownProtos (.15) + item = handler.Add(fmt.Sprintf("%s.15.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeCounter32 item.Value = uint32(0) - // ifOutOctets (.16) - item = m.handler.Add(fmt.Sprintf("%s.16.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.Tx.Bytes) - - // ifOutUcastPkts (.17) - item = m.handler.Add(fmt.Sprintf("%s.17.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - // iface.Tx*cast.Packets is only set if "set interface feature X stats-collect-tx arc interface-output" is configured - if iface.TxUnicast.Packets == 0 { - item.Value = uint32(iface.Tx.Packets) - } else { - item.Value = uint32(iface.TxUnicast.Packets) - } - - // ifOutNUcastPkts (.18) - multicast + broadcast - item = m.handler.Add(fmt.Sprintf("%s.18.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.TxMulticast.Packets + iface.TxBroadcast.Packets) - - // ifOutDiscards (.19) - 0 (not available) - item = m.handler.Add(fmt.Sprintf("%s.19.%d", ifEntryOID, idx)) + // ifOutDiscards (.19) + item = handler.Add(fmt.Sprintf("%s.19.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeCounter32 item.Value = uint32(0) - // ifOutErrors (.20) - item = m.handler.Add(fmt.Sprintf("%s.20.%d", ifEntryOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.TxErrors) - - // ifOutQLen (.21) - 0 (not available) - item = m.handler.Add(fmt.Sprintf("%s.21.%d", ifEntryOID, idx)) + // ifOutQLen (.21) + item = handler.Add(fmt.Sprintf("%s.21.%d", ifEntryOID, idx)) item.Type = pdu.VariableTypeGauge32 item.Value = uint32(0) - // ifSpecific (.22) - Skip this field as it's optional and causing issues -} - -func (m *InterfaceMIB) addIfXTable(iface *api.InterfaceCounters, idx int) { - var item *agentx.ListItem - + // ifXTable static fields // ifName (.1) - item = m.handler.Add(fmt.Sprintf("%s.1.%d", ifXTableOID, idx)) + item = handler.Add(fmt.Sprintf("%s.1.%d", ifXTableOID, idx)) item.Type = pdu.VariableTypeOctetString item.Value = iface.InterfaceName - // ifInMulticastPkts (.2) - item = m.handler.Add(fmt.Sprintf("%s.2.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.RxMulticast.Packets) - - // ifInBroadcastPkts (.3) - item = m.handler.Add(fmt.Sprintf("%s.3.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.RxBroadcast.Packets) - - // ifOutMulticastPkts (.4) - item = m.handler.Add(fmt.Sprintf("%s.4.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.TxMulticast.Packets) - - // ifOutBroadcastPkts (.5) - item = m.handler.Add(fmt.Sprintf("%s.5.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter32 - item.Value = uint32(iface.TxBroadcast.Packets) - - // ifHCInOctets (.6) - item = m.handler.Add(fmt.Sprintf("%s.6.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - item.Value = iface.Rx.Bytes - - // ifHCInUcastPkts (.7) - item = m.handler.Add(fmt.Sprintf("%s.7.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - if iface.RxUnicast.Packets == 0 { - item.Value = iface.Rx.Packets - } else { - item.Value = iface.RxUnicast.Packets - } - - // ifHCInMulticastPkts (.8) - item = m.handler.Add(fmt.Sprintf("%s.8.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - item.Value = iface.RxMulticast.Packets - - // ifHCInBroadcastPkts (.9) - item = m.handler.Add(fmt.Sprintf("%s.9.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - item.Value = iface.RxBroadcast.Packets - - // ifHCOutOctets (.10) - item = m.handler.Add(fmt.Sprintf("%s.10.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - item.Value = iface.Tx.Bytes - - // ifHCOutUcastPkts (.11) - item = m.handler.Add(fmt.Sprintf("%s.11.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - if iface.TxUnicast.Packets == 0 { - item.Value = iface.Tx.Packets - } else { - item.Value = iface.TxUnicast.Packets - } - - // ifHCOutMulticastPkts (.12) - item = m.handler.Add(fmt.Sprintf("%s.12.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - item.Value = iface.TxMulticast.Packets - - // ifHCOutBroadcastPkts (.13) - item = m.handler.Add(fmt.Sprintf("%s.13.%d", ifXTableOID, idx)) - item.Type = pdu.VariableTypeCounter64 - item.Value = iface.TxBroadcast.Packets - - // ifHighSpeed (.15) - Interface speed in Megabits per second - details := m.interfaceDetails[iface.InterfaceIndex] - speedMbps := uint32(1000) // default 1 Gbps = 1000 Mbps + // ifHighSpeed (.15) + speedMbps := uint32(1000) if details != nil && details.Speed > 0 { - speedMbps = uint32(details.Speed / 1000000) // Convert bps to Mbps + speedMbps = uint32(details.Speed / 1000000) } - item = m.handler.Add(fmt.Sprintf("%s.15.%d", ifXTableOID, idx)) + item = handler.Add(fmt.Sprintf("%s.15.%d", ifXTableOID, idx)) item.Type = pdu.VariableTypeGauge32 item.Value = speedMbps - // ifAlias (.18) - Interface description/alias - item = m.handler.Add(fmt.Sprintf("%s.18.%d", ifXTableOID, idx)) + // ifAlias (.18) + item = handler.Add(fmt.Sprintf("%s.18.%d", ifXTableOID, idx)) item.Type = pdu.VariableTypeOctetString - // Use description from VPP config if available, otherwise use interface name if desc, exists := m.descriptions[iface.InterfaceName]; exists { item.Value = desc } else { @@ -467,45 +329,147 @@ func (m *InterfaceMIB) addIfXTable(iface *api.InterfaceCounters, idx int) { } } -func (m *InterfaceMIB) createAndRegisterSession(client *agentx.Client, oid, name string) (*agentx.Session, error) { - session, err := client.Session(value.MustParseOID(oid), name, m.handler) - if err != nil { - return nil, fmt.Errorf("failed to create %s session: %v", name, err) +func (m *InterfaceMIB) addCounterFields(handler *agentx.ListHandler, iface *api.InterfaceCounters, idx int) { + ifIdx := iface.InterfaceIndex + + // ifEntry counters + counters := []struct { + oid string + key string + typ pdu.VariableType + }{ + {fmt.Sprintf("%s.10.%d", ifEntryOID, idx), "ifInOctets", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.11.%d", ifEntryOID, idx), "ifInUcastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.12.%d", ifEntryOID, idx), "ifInNUcastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.13.%d", ifEntryOID, idx), "ifInDiscards", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.14.%d", ifEntryOID, idx), "ifInErrors", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.16.%d", ifEntryOID, idx), "ifOutOctets", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.17.%d", ifEntryOID, idx), "ifOutUcastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.18.%d", ifEntryOID, idx), "ifOutNUcastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.20.%d", ifEntryOID, idx), "ifOutErrors", pdu.VariableTypeCounter32}, + + // ifXTable counters + {fmt.Sprintf("%s.2.%d", ifXTableOID, idx), "ifInMulticastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.3.%d", ifXTableOID, idx), "ifInBroadcastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.4.%d", ifXTableOID, idx), "ifOutMulticastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.5.%d", ifXTableOID, idx), "ifOutBroadcastPkts", pdu.VariableTypeCounter32}, + {fmt.Sprintf("%s.6.%d", ifXTableOID, idx), "ifHCInOctets", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.7.%d", ifXTableOID, idx), "ifHCInUcastPkts", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.8.%d", ifXTableOID, idx), "ifHCInMulticastPkts", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.9.%d", ifXTableOID, idx), "ifHCInBroadcastPkts", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.10.%d", ifXTableOID, idx), "ifHCOutOctets", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.11.%d", ifXTableOID, idx), "ifHCOutUcastPkts", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.12.%d", ifXTableOID, idx), "ifHCOutMulticastPkts", pdu.VariableTypeCounter64}, + {fmt.Sprintf("%s.13.%d", ifXTableOID, idx), "ifHCOutBroadcastPkts", pdu.VariableTypeCounter64}, } - err = session.Register(127, value.MustParseOID(oid)) - if err != nil { - session.Close() - return nil, fmt.Errorf("failed to register %s: %v", name, err) + for _, counter := range counters { + item := handler.Add(counter.oid) + item.Type = counter.typ + m.counterItems[fmt.Sprintf("%d_%s", ifIdx, counter.key)] = item } - return session, nil + // Set initial values + m.updateCounterValues(iface) +} + +func (m *InterfaceMIB) updateCounterValues(iface *api.InterfaceCounters) { + ifIdx := iface.InterfaceIndex + + // ifEntry counters + if item := m.counterItems[fmt.Sprintf("%d_ifInOctets", ifIdx)]; item != nil { + item.Value = uint32(iface.Rx.Bytes) + } + if item := m.counterItems[fmt.Sprintf("%d_ifInUcastPkts", ifIdx)]; item != nil { + if iface.RxUnicast.Packets == 0 { + item.Value = uint32(iface.Rx.Packets) + } else { + item.Value = uint32(iface.RxUnicast.Packets) + } + } + if item := m.counterItems[fmt.Sprintf("%d_ifInNUcastPkts", ifIdx)]; item != nil { + item.Value = uint32(iface.RxMulticast.Packets + iface.RxBroadcast.Packets) + } + if item := m.counterItems[fmt.Sprintf("%d_ifInDiscards", ifIdx)]; item != nil { + item.Value = uint32(iface.Drops) + } + if item := m.counterItems[fmt.Sprintf("%d_ifInErrors", ifIdx)]; item != nil { + item.Value = uint32(iface.RxErrors) + } + if item := m.counterItems[fmt.Sprintf("%d_ifOutOctets", ifIdx)]; item != nil { + item.Value = uint32(iface.Tx.Bytes) + } + if item := m.counterItems[fmt.Sprintf("%d_ifOutUcastPkts", ifIdx)]; item != nil { + if iface.TxUnicast.Packets == 0 { + item.Value = uint32(iface.Tx.Packets) + } else { + item.Value = uint32(iface.TxUnicast.Packets) + } + } + if item := m.counterItems[fmt.Sprintf("%d_ifOutNUcastPkts", ifIdx)]; item != nil { + item.Value = uint32(iface.TxMulticast.Packets + iface.TxBroadcast.Packets) + } + if item := m.counterItems[fmt.Sprintf("%d_ifOutErrors", ifIdx)]; item != nil { + item.Value = uint32(iface.TxErrors) + } + + // ifXTable counters + if item := m.counterItems[fmt.Sprintf("%d_ifInMulticastPkts", ifIdx)]; item != nil { + item.Value = uint32(iface.RxMulticast.Packets) + } + if item := m.counterItems[fmt.Sprintf("%d_ifInBroadcastPkts", ifIdx)]; item != nil { + item.Value = uint32(iface.RxBroadcast.Packets) + } + if item := m.counterItems[fmt.Sprintf("%d_ifOutMulticastPkts", ifIdx)]; item != nil { + item.Value = uint32(iface.TxMulticast.Packets) + } + if item := m.counterItems[fmt.Sprintf("%d_ifOutBroadcastPkts", ifIdx)]; item != nil { + item.Value = uint32(iface.TxBroadcast.Packets) + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCInOctets", ifIdx)]; item != nil { + item.Value = iface.Rx.Bytes + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCInUcastPkts", ifIdx)]; item != nil { + if iface.RxUnicast.Packets == 0 { + item.Value = iface.Rx.Packets + } else { + item.Value = iface.RxUnicast.Packets + } + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCInMulticastPkts", ifIdx)]; item != nil { + item.Value = iface.RxMulticast.Packets + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCInBroadcastPkts", ifIdx)]; item != nil { + item.Value = iface.RxBroadcast.Packets + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCOutOctets", ifIdx)]; item != nil { + item.Value = iface.Tx.Bytes + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCOutUcastPkts", ifIdx)]; item != nil { + if iface.TxUnicast.Packets == 0 { + item.Value = iface.Tx.Packets + } else { + item.Value = iface.TxUnicast.Packets + } + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCOutMulticastPkts", ifIdx)]; item != nil { + item.Value = iface.TxMulticast.Packets + } + if item := m.counterItems[fmt.Sprintf("%d_ifHCOutBroadcastPkts", ifIdx)]; item != nil { + item.Value = iface.TxBroadcast.Packets + } } func (m *InterfaceMIB) RegisterWithClient(client *agentx.Client) error { m.mutex.Lock() defer m.mutex.Unlock() - // Create and register sessions - ifEntrySession, err := m.createAndRegisterSession(client, ifEntryOID, "ifEntry") - if err != nil { - return err - } - - ifXTableSession, err := m.createAndRegisterSession(client, ifXTableOID, "ifXTable") - if err != nil { - ifEntrySession.Close() - return err - } - - m.ifEntrySession = ifEntrySession - m.ifXTableSession = ifXTableSession - - logger.Debugf("Registered IF-MIB sessions: ifEntry (%s) and ifXTable (%s)", ifEntryOID, ifXTableOID) + m.client = client + // Don't register anything yet - wait for first UpdateStats call + logger.Debugf("Stored AgentX client reference") return nil } -// Close cleans up AgentX sessions func (m *InterfaceMIB) Close() { m.mutex.Lock() defer m.mutex.Unlock() diff --git a/src/ifmib/ifmib_test.go b/src/ifmib/ifmib_test.go index 35513b2..04c55de 100644 --- a/src/ifmib/ifmib_test.go +++ b/src/ifmib/ifmib_test.go @@ -17,20 +17,24 @@ func TestNewInterfaceMIB(t *testing.T) { return } - if mib.handler == nil { - t.Error("Expected handler to be initialized") - } - - if mib.stats == nil { - t.Error("Expected stats map to be initialized") - } - if mib.descriptions == nil { t.Error("Expected descriptions map to be initialized") } - if len(mib.stats) != 0 { - t.Errorf("Expected stats map to be empty, got %d entries", len(mib.stats)) + if mib.interfaceDetails == nil { + t.Error("Expected interfaceDetails map to be initialized") + } + + if mib.currentInterfaces == nil { + t.Error("Expected currentInterfaces map to be initialized") + } + + if mib.counterItems == nil { + t.Error("Expected counterItems map to be initialized") + } + + if len(mib.currentInterfaces) != 0 { + t.Errorf("Expected currentInterfaces map to be empty, got %d entries", len(mib.currentInterfaces)) } if len(mib.descriptions) != 0 { @@ -40,14 +44,20 @@ func TestNewInterfaceMIB(t *testing.T) { func TestGetHandler(t *testing.T) { mib := NewInterfaceMIB() - handler := mib.GetHandler() + handler1 := mib.GetHandler() + handler2 := mib.GetHandler() - if handler == nil { + if handler1 == nil { t.Error("GetHandler returned nil") } - if handler != mib.handler { - t.Error("GetHandler returned different handler than expected") + if handler2 == nil { + t.Error("GetHandler returned nil") + } + + // In the new implementation, GetHandler always returns a fresh handler + if handler1 == handler2 { + t.Error("Expected GetHandler to return fresh handlers, but got the same reference") } } @@ -163,19 +173,12 @@ func TestUpdateStatsBasic(t *testing.T) { // Call UpdateStats (this will test the basic flow without AgentX sessions) mib.UpdateStats(stats) - // Check that stats were stored - if len(mib.stats) != 1 { - t.Errorf("Expected 1 interface in stats, got %d", len(mib.stats)) + // Check that interface was registered + if len(mib.currentInterfaces) != 1 { + t.Errorf("Expected 1 interface in currentInterfaces, got %d", len(mib.currentInterfaces)) } - if storedStats, exists := mib.stats[0]; !exists { - t.Error("Expected interface 0 to be stored in stats") - } else { - if storedStats.InterfaceName != "test0" { - t.Errorf("Expected interface name 'test0', got '%s'", storedStats.InterfaceName) - } - if storedStats.Rx.Packets != 100 { - t.Errorf("Expected RX packets 100, got %d", storedStats.Rx.Packets) - } + if !mib.currentInterfaces[0] { + t.Error("Expected interface 0 to be registered in currentInterfaces") } }