Switch based discovery fails to find any nodes on Cisco switches due to (by default) per-VLAN SNMP contexts

[LHurst-OCF]

In _snmp_map_switch (confluent_server/confluent/networking/macmap.py), the mac walk is only done on the initial connection. On CISCO switches, mac information is returned per-context so the initial list will only see the default context.

There seems to be some comments about this problem in the code:

 # 1.3.6.1.2.1.17.7.1.2.2.1.2 - mactoindex (qbridge - preferred)
#  if not, check for cisco and if cisco, build list of all relevant vlans:
#  .1.3.6.1.4.1.9.9.46.1.6.1.1.5 - trunk port vlan map (cisco only)
#  .1.3.6.1.4.1.9.9.68.1.2.2.1.2 - access port vlan map (cisco only)
# if cisco, vlan community string indexed or snmpv3 contest for:
# 1.3.6.1.2.1.17.4.3.1.2 - mactoindx (bridge - low-end switches and cisco)
#     .1.3.6.1.2.1.17.1.4.1.2 - bridge index to if index map
# no vlan index or context for:
#  .1.3.6.1.2.1.31.1.1.1.1 - ifName... but some switches don't do it
#  .1.3.6.1.2.1.2.2.1.2 - ifDescr, usually useless, but a
#   fallback if ifName is empty

Specifically, these loops are done once with the original connection (default context), which finds nothing on a Cisco switch where nodes are not visible in the default context (typically VLAN 1):

    async for vb in conn.walk('1.3.6.1.2.1.17.7.1.2.2.1.2'):
        haveqbridge = True
        oid, bridgeport = vb
        if not bridgeport:
            continue
        oid = str(oid).rsplit('.', 6)
        # if 7, then oid[1] would be vlan id
        macaddr = '{0:02x}:{1:02x}:{2:02x}:{3:02x}:{4:02x}:{5:02x}'.format(
            *([int(x) for x in oid[-6:]])
        )
        mactobridge[macaddr] = int(bridgeport)
    if not haveqbridge:
        async for vb in conn.walk('1.3.6.1.2.1.17.4.3.1.2'):
            oid, bridgeport = vb
            if not bridgeport:
                continue
            oid = str(oid).rsplit('.', 6)
            macaddr = '{0:02x}:{1:02x}:{2:02x}:{3:02x}:{4:02x}:{5:02x}'.format(
                *([int(x) for x in oid[-6:]])
            )
            mactobridge[macaddr] = int(bridgeport)

Moving this inside the VLAN loop (which is run through at least once on all switches, by virtue of adding None to the VLANS and only Cisco switches with VLANs loop and get new connections created per VLAN context should correct this problem (note that the new connections are created without privacy_policy passed through, which I think is also an oversight?).

As there are some CISCO configurations where the macs may be returned by the default context too (I'm not sure if ONLY by the default context or as well as VLAN), I also propose changing if not vlanstocheck: vlanstocheck.add(None) to for vlan in (None, *vlanstocheck): so the initial (default) context is looped over unconditionally (on CISCO and non-CISCO) which will retain current code path in all cases but also add the per-VLAN MAC walking required for (at least some) CISCO switch configurations.

[LHurst-OCF]

(I will add a patch and/or pull request shortly.)

[LHurst-OCF]

There's a second bug in snmputil.py too, context isn't the first positional parameter to snmp.ContextData, so this line:

ctx = snmp.ContextData(self.context)

Needs to be (the or '' is because the underlying library cannot cope with None as the value):

ctx = snmp.ContextData(contextName=self.context or '')

[LHurst-OCF]

Patch for 3.15.5:

diff --git a/confluent_server/confluent/networking/macmap.py b/confluent_server/confluent/networking/macmap.py
index 0821c464..d63eda83 100644
--- a/confluent_server/confluent/networking/macmap.py
+++ b/confluent_server/confluent/networking/macmap.py
@@ -400,31 +400,10 @@ def _snmp_map_switch_relay(rqid, switch, password, user, privprotocol=None):
         sys.stdout.flush()
 
 def _snmp_map_switch(switch, password, user, privprotocol=None):
-    haveqbridge = False
     mactobridge = {}
     conn = snmp.Session(switch, password, user, privacy_protocol=privprotocol)
     ifnamemap = get_portnamemap(conn)
-    for vb in conn.walk('1.3.6.1.2.1.17.7.1.2.2.1.2'):
-        haveqbridge = True
-        oid, bridgeport = vb
-        if not bridgeport:
-            continue
-        oid = str(oid).rsplit('.', 6)
-        # if 7, then oid[1] would be vlan id
-        macaddr = '{0:02x}:{1:02x}:{2:02x}:{3:02x}:{4:02x}:{5:02x}'.format(
-            *([int(x) for x in oid[-6:]])
-        )
-        mactobridge[macaddr] = int(bridgeport)
-    if not haveqbridge:
-        for vb in conn.walk('1.3.6.1.2.1.17.4.3.1.2'):
-            oid, bridgeport = vb
-            if not bridgeport:
-                continue
-            oid = str(oid).rsplit('.', 6)
-            macaddr = '{0:02x}:{1:02x}:{2:02x}:{3:02x}:{4:02x}:{5:02x}'.format(
-                *([int(x) for x in oid[-6:]])
-            )
-            mactobridge[macaddr] = int(bridgeport)
+
     vlanstocheck = set([])
     try:
         #ciscoiftovlanmap = {}
@@ -437,17 +416,44 @@ def _snmp_map_switch(switch, password, user, privprotocol=None):
         # We might have crashed snmp on a non-cisco switch
         # in such a case, delay 8 seconds to allow recovery to complete
         eventlet.sleep(8)
-    if not vlanstocheck:
-        vlanstocheck.add(None)
+
+    # Cisco splits BRIDGE-MIB/QBRIDGE-MIB data per-VLAN behind SNMPv3
+    # contexts (or community@vlan indexing for v1/v2c).  Always check the
+    # default context too (vlan=None), since some data (e.g. QBRIDGE-MIB
+    # FDB entries, or VLAN 1 on some platforms) may only be visible there.
     bridgetoifmap = {}
-    for vlan in vlanstocheck:
+    for vlan in (None, *vlanstocheck):
         if vlan:
             if user:
-                conn = snmp.Session(switch, password, user, 'vlan-{}'.format(vlan))
+                conn = snmp.Session(switch, password, user, 'vlan-{}'.format(vlan),
+                                     privacy_protocol=privprotocol)
             else:
                 if not isinstance(password, str):
                     password = password.decode('utf8')
-                conn = snmp.Session(switch, '{}@{}'.format(password, vlan))
+                conn = snmp.Session(switch, '{}@{}'.format(password, vlan),
+                                     privacy_protocol=privprotocol)
+        haveqbridge = False
+        for vb in conn.walk('1.3.6.1.2.1.17.7.1.2.2.1.2'):
+            haveqbridge = True
+            oid, bridgeport = vb
+            if not bridgeport:
+                continue
+            oid = str(oid).rsplit('.', 6)
+            # if 7, then oid[1] would be vlan id
+            macaddr = '{0:02x}:{1:02x}:{2:02x}:{3:02x}:{4:02x}:{5:02x}'.format(
+                *([int(x) for x in oid[-6:]])
+            )
+            mactobridge[macaddr] = int(bridgeport)
+        if not haveqbridge:
+            for vb in conn.walk('1.3.6.1.2.1.17.4.3.1.2'):
+                oid, bridgeport = vb
+                if not bridgeport:
+                    continue
+                oid = str(oid).rsplit('.', 6)
+                macaddr = '{0:02x}:{1:02x}:{2:02x}:{3:02x}:{4:02x}:{5:02x}'.format(
+                    *([int(x) for x in oid[-6:]])
+                )
+                mactobridge[macaddr] = int(bridgeport)
         for vb in conn.walk('1.3.6.1.2.1.17.1.4.1.2'):
             bridgeport, ifidx = vb
             bridgeport = int(str(bridgeport).rsplit('.', 1)[1])
diff --git a/confluent_server/confluent/snmputil.py b/confluent_server/confluent/snmputil.py
index 6f03a31b..8b225188 100644
--- a/confluent_server/confluent/snmputil.py
+++ b/confluent_server/confluent/snmputil.py
@@ -124,7 +124,7 @@ class Session(object):
         # automatic behavior (e.g. DES is weak, so it's likely to be
         # overriden, but some devices only support DES)
         tp = _get_transport(self.server)
-        ctx = snmp.ContextData(self.context)
+        ctx = snmp.ContextData(contextName=self.context or '')
         resolvemib = False
         if '::' in oid:
             resolvemib = True