fixed SNMPv3 retries algorithm inconsistencies (#106)

Author: etingof

CHANGES.txt

@@ -11,6 +11,7 @@ Revision 4.4.2, released 2017-11-XX
 - Example scripts rearranged in a way that IPv6 requirement is
   clearly encoded in the script's name
 - Fixed non-implied-OID encoding in SNMP table indices
+- Fixed inconsistent SNMPv3 discovery and retrying algorithm
 
 Revision 4.4.1, released 2017-10-23
 -----------------------------------

pysnmp/entity/rfc3413/cmdgen.py

@@ -43,7 +43,8 @@ def getNextVarBinds(varBinds, origVarBinds=None):
 class CommandGenerator(object):
     _null = univ.Null('')
 
-    def __init__(self):
+    def __init__(self, **options):
+        self.__options = options
         self.__pendingReqs = {}
 
     def processResponsePdu(self, snmpEngine, messageProcessingModel,
@@ -61,26 +62,28 @@ def processResponsePdu(self, snmpEngine, messageProcessingModel,
          origSecurityName, origSecurityLevel, origContextEngineId,
          origContextName, origPduVersion, origPdu,
          origTimeout, origRetryCount,
-         origRetries) = self.__pendingReqs.pop(sendPduHandle)
+         origRetries, origDiscoveryRetries) = self.__pendingReqs.pop(sendPduHandle)
 
         snmpEngine.transportDispatcher.jobFinished(id(self))
 
         # 3.1.3
         if statusInformation:
             debug.logger & debug.flagApp and debug.logger(
                 'processResponsePdu: sendPduHandle %s, statusInformation %s' % (sendPduHandle, statusInformation))
+
             errorIndication = statusInformation['errorIndication']
 
-            # SNMP engine discovery will take extra retries, allow that
-            if (errorIndication in (errind.notInTimeWindow,
-                                    errind.unknownEngineID) and
-                    origRetries == origRetryCount + 2 or
-                    errorIndication not in (errind.notInTimeWindow, errind.unknownEngineID) and
-                    origRetries == origRetryCount):
+            if errorIndication in (errind.notInTimeWindow, errind.unknownEngineID):
+                origDiscoveryRetries += 1
+                origRetries = 0
+            else:
+                origDiscoveryRetries = 0
+                origRetries += 1
+
+            if origRetries > origRetryCount or origDiscoveryRetries > self.__options.get('discoveryRetries', 4):
                 debug.logger & debug.flagApp and debug.logger(
                     'processResponsePdu: sendPduHandle %s, retry count %d exceeded' % (sendPduHandle, origRetries))
-                cbFun(snmpEngine, origSendRequestHandle,
-                      statusInformation['errorIndication'], None, cbCtx)
+                cbFun(snmpEngine, origSendRequestHandle, errorIndication, None, cbCtx)
                 return
 
             # User-side API assumes SMIv2
@@ -107,7 +110,7 @@ def processResponsePdu(self, snmpEngine, messageProcessingModel,
                     origMessageProcessingModel, origSecurityModel,
                     origSecurityName, origSecurityLevel, origContextEngineId,
                     origContextName, origPduVersion, origPdu, origTimeout,
-                    origRetryCount, origRetries + 1
+                    origRetryCount, origRetries, origDiscoveryRetries
                 )
                 return
 
@@ -192,7 +195,7 @@ def sendPdu(self, snmpEngine, targetName, contextEngineId,
             transportDomain, transportAddress, messageProcessingModel,
             securityModel, securityName, securityLevel, contextEngineId,
             contextName, pduVersion, origPDU, timeoutInTicks,
-            retryCount, 0
+            retryCount, 0, 0
         )
 
         debug.logger & debug.flagApp and debug.logger(

pysnmp/entity/rfc3413/ntforg.py

@@ -10,7 +10,7 @@
 from pysnmp.proto.proxy import rfc2576
 from pysnmp.proto import rfc3411
 from pysnmp.proto.api import v2c
-from pysnmp.proto import error
+from pysnmp.proto import errind, error
 from pysnmp.smi import view, rfc1902
 from pysnmp import nextid
 from pysnmp import debug
@@ -21,10 +21,11 @@
 class NotificationOriginator(object):
     acmID = 3  # default MIB access control method to use
 
-    def __init__(self, snmpContext=None):
+    def __init__(self, **options):
         self.__pendingReqs = {}
         self.__pendingNotifications = {}
-        self.snmpContext = snmpContext  # this is deprecated
+        self.snmpContext = options.pop('snmpContext', None)  # this is deprecated
+        self.__options = options
 
     def processResponsePdu(self, snmpEngine, messageProcessingModel,
                            securityModel, securityName, securityLevel,
@@ -40,20 +41,29 @@ def processResponsePdu(self, snmpEngine, messageProcessingModel,
          origMessageProcessingModel, origSecurityModel,
          origSecurityName, origSecurityLevel, origContextEngineId,
          origContextName, origPdu, origTimeout,
-         origRetryCount, origRetries) = self.__pendingReqs.pop(sendPduHandle)
+         origRetryCount, origRetries, origDiscoveryRetries) = self.__pendingReqs.pop(sendPduHandle)
 
         snmpEngine.transportDispatcher.jobFinished(id(self))
 
         if statusInformation:
             debug.logger & debug.flagApp and debug.logger(
                 'processResponsePdu: sendRequestHandle %s, sendPduHandle %s statusInformation %s' % (
                     sendRequestHandle, sendPduHandle, statusInformation))
-            if origRetries == origRetryCount:
+
+            errorIndication = statusInformation['errorIndication']
+
+            if errorIndication in (errind.notInTimeWindow, errind.unknownEngineID):
+                origDiscoveryRetries += 1
+                origRetries = 0
+            else:
+                origDiscoveryRetries = 0
+                origRetries += 1
+
+            if origRetries > origRetryCount or origDiscoveryRetries > self.__options.get('discoveryRetries', 4):
                 debug.logger & debug.flagApp and debug.logger(
                     'processResponsePdu: sendRequestHandle %s, sendPduHandle %s retry count %d exceeded' % (
                         sendRequestHandle, sendPduHandle, origRetries))
-                cbFun(snmpEngine, sendRequestHandle,
-                      statusInformation['errorIndication'], None, cbCtx)
+                cbFun(snmpEngine, sendRequestHandle, errorIndication, None, cbCtx)
                 return
 
             # Convert timeout in seconds into timeout in timer ticks
@@ -98,7 +108,7 @@ def processResponsePdu(self, snmpEngine, messageProcessingModel,
                 origMessageProcessingModel, origSecurityModel,
                 origSecurityName, origSecurityLevel,
                 origContextEngineId, origContextName, origPdu,
-                origTimeout, origRetryCount, origRetries + 1
+                origTimeout, origRetryCount, origRetries, origDiscoveryRetries
             )
             return
 
@@ -148,7 +158,7 @@ def sendPdu(self, snmpEngine, targetName, contextEngineId,
             self.__pendingReqs[sendPduHandle] = (
                 transportDomain, transportAddress, messageProcessingModel,
                 securityModel, securityName, securityLevel, contextEngineId,
-                contextName, pdu, timeout, retryCount, True
+                contextName, pdu, timeout, retryCount, 0, 0
             )
             snmpEngine.transportDispatcher.jobStarted(id(self))
         else: