@cryptotaxi247 / netdata-1 / commits / 514d58d05

refactor(go.d/snmp): recreate client on SNMPv3 "packet is not authentic" errors (#20897)

Ilya Mashchenko committed Aug 28, 2025 at 13:10 UTC 514d58d05d31ff494a300b6bdc1bca3494421519
5 files changed +103 -92
src/go/plugin/go.d/collector/snmp/collect.go
+49 -12
@@ -25,6 +25,17 @@ import (
25 const oidSysUptime = "1.3.6.1.2.1.1.3.0"
26
27 func (c *Collector) collect() (map[string]int64, error) {
28 + if c.snmpClient == nil {
29 + snmpClient, err := c.initAndConnectSNMPClient()
30 + if err != nil {
31 + return nil, err
32 + }
33 + c.snmpClient = snmpClient
34 + if c.ddSnmpColl != nil {
35 + c.ddSnmpColl.SetSNMPClient(snmpClient)
36 + }
37 + }
38 +
39 if c.sysInfo == nil {
40 si, err := snmputils.GetSysInfo(c.snmpClient)
41 if err != nil {
@@ -99,13 +110,6 @@ func (c *Collector) collectSysUptime(mx map[string]int64) error {
110 return nil
111 }
112
102 -func (c *Collector) walkAll(rootOid string) ([]gosnmp.SnmpPDU, error) {
103 - if c.snmpClient.Version() == gosnmp.Version1 {
104 - return c.snmpClient.WalkAll(rootOid)
105 - }
106 - return c.snmpClient.BulkWalkAll(rootOid)
107 -}
108 -
113 func (c *Collector) setupVnode(si *snmputils.SysInfo, deviceMeta map[string]ddsnmp.MetaTag) *vnodes.VirtualNode {
114 if c.Vnode.GUID == "" {
115 c.Vnode.GUID = uuid.NewSHA1(uuid.NameSpaceDNS, []byte(c.Hostname)).String()
@@ -181,7 +185,33 @@ func (c *Collector) setupProfiles(sysObjectID string) []*ddsnmp.Profile {
185 return snmpProfiles
186 }
187
184 -func (c *Collector) adjustMaxRepetitions() (bool, error) {
188 +func (c *Collector) initAndConnectSNMPClient() (gosnmp.Handler, error) {
189 + snmpClient, err := c.initSNMPClient()
190 + if err != nil {
191 + return nil, fmt.Errorf("init: %w", err)
192 + }
193 +
194 + if err := snmpClient.Connect(); err != nil {
195 + return nil, fmt.Errorf("connect: %w", err)
196 + }
197 +
198 + if c.adjMaxRepetitions != 0 {
199 + snmpClient.SetMaxRepetitions(c.adjMaxRepetitions)
200 + } else {
201 + ok, err := c.adjustMaxRepetitions(snmpClient)
202 + if err != nil {
203 + return nil, fmt.Errorf("re-adjust max repetitions SNMP client: %w", err)
204 + }
205 + if !ok {
206 + c.Warningf("SNMP bulk walk disabled: table metrics collection unavailable (device may not support GETBULK or max-repetitions adjustment failed)")
207 + }
208 + c.adjMaxRepetitions = snmpClient.MaxRepetitions()
209 + c.snmpBulkWalkOk = ok
210 + }
211 +
212 + return snmpClient, nil
213 +}
214 +func (c *Collector) adjustMaxRepetitions(snmpClient gosnmp.Handler) (bool, error) {
215 orig := c.Config.Options.MaxRepetitions
216 maxReps := c.Config.Options.MaxRepetitions
217 attempts := 0
@@ -190,7 +220,7 @@ func (c *Collector) adjustMaxRepetitions() (bool, error) {
220 for maxReps > 0 && attempts < maxAttempts {
221 attempts++
222
193 - v, err := c.walkAll(snmputils.RootOidMibSystem)
223 + v, err := walkAll(snmpClient, snmputils.RootOidMibSystem)
224 if err != nil {
225 return false, err
226 }
@@ -218,21 +248,28 @@ func (c *Collector) adjustMaxRepetitions() (bool, error) {
248 maxReps = max(0, maxReps) // Ensure non-negative
249
250 c.Debugf("max_repetitions=%d returned no data, trying %d", prevMaxReps, maxReps)
221 - c.snmpClient.SetMaxRepetitions(uint32(maxReps))
251 + snmpClient.SetMaxRepetitions(uint32(maxReps))
252 }
253
254 // Restore original value since nothing worked
225 - c.snmpClient.SetMaxRepetitions(uint32(orig))
255 + snmpClient.SetMaxRepetitions(uint32(orig))
256 c.Debugf("unable to find working max_repetitions value after %d attempts", attempts)
257 return false, nil
258 }
259
260 +func walkAll(snmpClient gosnmp.Handler, rootOid string) ([]gosnmp.SnmpPDU, error) {
261 + if snmpClient.Version() == gosnmp.Version1 {
262 + return snmpClient.WalkAll(rootOid)
263 + }
264 + return snmpClient.BulkWalkAll(rootOid)
265 +}
266 +
267 func pduToInt(pdu gosnmp.SnmpPDU) (int64, error) {
268 switch pdu.Type {
269 case gosnmp.Counter32, gosnmp.Counter64, gosnmp.Integer, gosnmp.Gauge32, gosnmp.TimeTicks:
270 return gosnmp.ToBigInt(pdu.Value).Int64(), nil
271 default:
235 - return 0, fmt.Errorf("unussported type: '%v'", pdu.Type)
272 + return 0, fmt.Errorf("unsupported type: '%v'", pdu.Type)
273 }
274 }
275
src/go/plugin/go.d/collector/snmp/collect_if_mib.go
+2 -2
@@ -20,12 +20,12 @@ const (
20 )
21
22 func (c *Collector) collectNetworkInterfaces(mx map[string]int64) error {
23 - ifMibTable, err := c.walkAll(rootOidIfMibIfTable)
23 + ifMibTable, err := walkAll(c.snmpClient, rootOidIfMibIfTable)
24 if err != nil {
25 return err
26 }
27
28 - ifMibXTable, err := c.walkAll(rootOidIfMibIfXTable)
28 + ifMibXTable, err := walkAll(c.snmpClient, rootOidIfMibIfXTable)
29 if err != nil {
30 return err
31 }
src/go/plugin/go.d/collector/snmp/collector.go
+36 -41
@@ -6,6 +6,7 @@ import (
6 "context"
7 _ "embed"
8 "fmt"
9 + "strings"
10
11 "github.com/gosnmp/gosnmp"
12
@@ -54,18 +55,15 @@ func New() *Collector {
55 },
56 },
57
57 - charts: &module.Charts{},
58 -
59 - seenMetrics: make(map[string]bool),
58 + charts: &module.Charts{},
59 + seenScalarMetrics: make(map[string]bool),
60 + seenTableMetrics: make(map[string]bool),
61
62 newSnmpClient: gosnmp.NewHandler,
63
64 snmpBulkWalkOk: true,
65 netInterfaces: make(map[string]*netInterface),
66 collectIfMib: true,
66 -
67 - seenScalarMetrics: make(map[string]bool),
68 - seenTableMetrics: make(map[string]bool),
67 }
68 }
69
@@ -75,29 +73,26 @@ type Collector struct {
73
74 vnode *vnodes.VirtualNode
75
78 - charts *module.Charts
79 - seenMetrics map[string]bool
76 + charts *module.Charts
77 + seenScalarMetrics map[string]bool
78 + seenTableMetrics map[string]bool
79
80 newSnmpClient func() gosnmp.Handler
81 snmpClient gosnmp.Handler
82 ddSnmpColl *ddsnmpcollector.Collector
83
85 - netIfaceFilterByName matcher.Matcher
86 - netIfaceFilterByType matcher.Matcher
87 -
88 - snmpBulkWalkOk bool
89 - collectIfMib bool // only for tests
90 -
91 - netInterfaces map[string]*netInterface
92 -
93 - sysInfo *snmputils.SysInfo
94 -
95 - customOids []string
96 -
84 + sysInfo *snmputils.SysInfo
85 snmpProfiles []*ddsnmp.Profile
86
99 - seenScalarMetrics map[string]bool
100 - seenTableMetrics map[string]bool
87 + adjMaxRepetitions uint32
88 + snmpBulkWalkOk bool
89 +
90 + // legacy data collection parameters
91 + netIfaceFilterByName matcher.Matcher
92 + netIfaceFilterByType matcher.Matcher
93 + collectIfMib bool // only for tests
94 + netInterfaces map[string]*netInterface
95 + customOids []string
96 }
97
98 func (c *Collector) Configuration() any {
@@ -105,22 +100,14 @@ func (c *Collector) Configuration() any {
100 }
101
102 func (c *Collector) Init(context.Context) error {
108 - err := c.validateConfig()
109 - if err != nil {
103 + if err := c.validateConfig(); err != nil {
104 return fmt.Errorf("config validation failed: %v", err)
105 }
106
113 - snmpClient, err := c.initSNMPClient()
114 - if err != nil {
107 + if _, err := c.initSNMPClient(); err != nil {
108 return fmt.Errorf("failed to initialize SNMP client: %v", err)
109 }
110
118 - err = snmpClient.Connect()
119 - if err != nil {
120 - return fmt.Errorf("SNMP client connection failed: %v", err)
121 - }
122 - c.snmpClient = snmpClient
123 -
111 byName, byType, err := c.initNetIfaceFilters()
112 if err != nil {
113 return fmt.Errorf("failed to initialize network interface filters: %v", err)
@@ -140,17 +127,17 @@ func (c *Collector) Init(context.Context) error {
127 }
128
129 func (c *Collector) Check(context.Context) error {
143 - if _, err := snmputils.GetSysInfo(c.snmpClient); err != nil {
144 - return err
130 + if c.snmpClient == nil {
131 + snmpClient, err := c.initAndConnectSNMPClient()
132 + if err != nil {
133 + return fmt.Errorf("failed to init and connect SNMP client: %v", err)
134 + }
135 + c.snmpClient = snmpClient
136 }
146 - ok, err := c.adjustMaxRepetitions()
147 - if err != nil {
137 +
138 + if _, err := snmputils.GetSysInfo(c.snmpClient); err != nil {
139 return err
140 }
150 - if !ok {
151 - c.Warningf("SNMP bulk walk disabled: table metrics collection unavailable (device may not support GETBULK or max-repetitions adjustment failed)")
152 - }
153 - c.snmpBulkWalkOk = ok
141
142 return nil
143 }
@@ -159,10 +146,18 @@ func (c *Collector) Charts() *module.Charts {
146 return c.charts
147 }
148
162 -func (c *Collector) Collect(context.Context) map[string]int64 {
149 +func (c *Collector) Collect(ctx context.Context) map[string]int64 {
150 mx, err := c.collect()
151 if err != nil {
152 c.Error(err)
153 + // Some buggy SNMPv3 devices occasionally get stuck with
154 + // "packet is not authentic" errors. Closing and dropping
155 + // the client here forces a reconnect on the next scrape,
156 + // which usually recovers the session.
157 + if strings.Contains(err.Error(), "packet is not authentic") {
158 + c.Cleanup(ctx)
159 + c.snmpClient = nil
160 + }
161 }
162
163 if len(mx) == 0 {
src/go/plugin/go.d/collector/snmp/collector_test.go
+1 -14
@@ -102,20 +102,6 @@ func TestCollector_Cleanup(t *testing.T) {
102 tests := map[string]struct {
103 prepareSNMP func(t *testing.T, m *snmpmock.MockHandler) *Collector
104 }{
105 - "cleanup call if snmpClient initialized": {
106 - prepareSNMP: func(t *testing.T, m *snmpmock.MockHandler) *Collector {
107 - collr := New()
108 - collr.Config = prepareV2Config()
109 - collr.newSnmpClient = func() gosnmp.Handler { return m }
110 - setMockClientInitExpect(m)
111 -
112 - require.NoError(t, collr.Init(context.Background()))
113 -
114 - m.EXPECT().Close().Times(1)
115 -
116 - return collr
117 - },
118 - },
105 "cleanup call does not panic if snmpClient not initialized": {
106 prepareSNMP: func(t *testing.T, m *snmpmock.MockHandler) *Collector {
107 collr := New()
@@ -583,6 +569,7 @@ func setMockClientInitExpect(m *snmpmock.MockHandler) {
569 m.EXPECT().SetMsgFlags(gomock.Any()).AnyTimes()
570 m.EXPECT().SetSecurityParameters(gomock.Any()).AnyTimes()
571 m.EXPECT().Connect().Return(nil).AnyTimes()
572 + m.EXPECT().MaxRepetitions().Return(uint32(25)).AnyTimes()
573 }
574
575 func setMockClientSysObjectidExpect(m *snmpmock.MockHandler) {
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector.go
+15 -23
@@ -21,7 +21,6 @@ import (
21 func New(snmpClient gosnmp.Handler, profiles []*ddsnmp.Profile, log *logger.Logger, sysobjectid string) *Collector {
22 coll := &Collector{
23 log: log.With(slog.String("ddsnmp", "collector")),
24 - snmpClient: snmpClient,
24 profiles: make(map[string]*profileState),
25 missingOIDs: make(map[string]bool),
26 tableCache: newTableCache(30*time.Minute, 1),
@@ -45,7 +44,6 @@ func New(snmpClient gosnmp.Handler, profiles []*ddsnmp.Profile, log *logger.Logg
44 type (
45 Collector struct {
46 log *logger.Logger
48 - snmpClient gosnmp.Handler
47 profiles map[string]*profileState
48 missingOIDs map[string]bool
49 tableCache *tableCache
@@ -120,6 +118,21 @@ func (c *Collector) Collect() ([]*ddsnmp.ProfileMetrics, error) {
118 return metrics, nil
119 }
120
121 +func (c *Collector) SetSNMPClient(snmpClient gosnmp.Handler) {
122 + if c.globalTagsCollector != nil {
123 + c.globalTagsCollector.snmpClient = snmpClient
124 + }
125 + if c.deviceMetadataCollector != nil {
126 + c.deviceMetadataCollector.snmpClient = snmpClient
127 + }
128 + if c.scalarCollector != nil {
129 + c.scalarCollector.snmpClient = snmpClient
130 + }
131 + if c.tableCollector != nil {
132 + c.tableCollector.snmpClient = snmpClient
133 + }
134 +}
135 +
136 func (c *Collector) collectProfile(ps *profileState) (*ddsnmp.ProfileMetrics, error) {
137 if !ps.initialized {
138 globalTag, err := c.globalTagsCollector.Collect(ps.profile)
@@ -184,27 +197,6 @@ func (c *Collector) updateProfileMetrics(pm *ddsnmp.ProfileMetrics) {
197 }
198 }
199
187 -func (c *Collector) snmpGet(oids []string) (map[string]gosnmp.SnmpPDU, error) {
188 - pdus := make(map[string]gosnmp.SnmpPDU)
189 -
190 - for chunk := range slices.Chunk(oids, c.snmpClient.MaxOids()) {
191 - result, err := c.snmpClient.Get(chunk)
192 - if err != nil {
193 - return nil, err
194 - }
195 -
196 - for _, pdu := range result.Variables {
197 - if !isPduWithData(pdu) {
198 - c.missingOIDs[trimOID(pdu.Name)] = true
199 - continue
200 - }
201 - pdus[trimOID(pdu.Name)] = pdu
202 - }
203 - }
204 -
205 - return pdus, nil
206 -}
207 -
200 var metricMetaReplacer = strings.NewReplacer(
201 "'", "",
202 "\n", " ",