Closed netdisco-automation closed 5 years ago
netdisco-automation
commented
6 years ago From @fabled on July 13, 2017 11:47
Apparently the LLDP '0' oid prefix is LLDP's TimeFilter index. It should probably be stripped away, and re-inserted on queries where needed.
netdisco-automation
commented
6 years ago From @JeroenvIS on July 13, 2017 15:55
Please don't strip anything (like the LLDP '0' oid prefix) from the keys. Each key should already map to a local port, and all "0.x.y" and "x.y" keys should already map back to the same local port "x". So without modifying the keys it's already clear that CDP and/or LLDP report multiple neighbor records for the same local port.
The challenge is in deciding what records are "best", IMHO...
netdisco-automation
commented
6 years ago From @fabled on July 14, 2017 5:10
Yes, it is true that all the indexes map correctly. But I find the c_cap and similar functions slightly futile if caller needs to anyway do manual job combining the different protocol type results. It might be simpler for netdisco to just call lldp_cap and cdp_cap in separate loops then. Another alternative is to first resolve each index to an interface index, and have logic to figure out which index wins - and only do discovery on the winning cdp/lldp indexes or similar. In any case it will require additional logic.
netdisco-automation
commented
6 years ago From @ollyg on July 14, 2017 13:1
@JeroenvIS the intention is not to mess with lldp_* or cdp_* or the indexing, but I think we need to do one of two things with c_*:
c_* methods WILL return data incompatible with the underlying lldp or cdp methods from which they are composed.Personally I am inclined to the latter... Netdisco should ask SNMP::Info for "normalised" neighbor information and that is what the whole common neighbor table interface is for. If a user wants the actual lldp or cdp tables they are still available. The normalised table should contain the rules for selecting interface indexes and prioritising results from different protocols that conflict or contradict.
netdisco-automation
commented
6 years ago From @JeroenvIS on July 14, 2017 14:56
@ollyg Agreed, but then I'd like to suggest deprecating the c* methods and implement new methods for this purpose in SNMP::Info; for example, methods like 'neigh*'. That way other SNMP::Info users can migrate their code at their own pace.
The common topology table methods below will query the device for information from the specified topology protocols and return a single hash combining all information. As a result, there may be identical topology information returned from the two protocols causing duplicate entries. It is the calling program's responsibility to identify any duplicate entries and remove duplicates if necessary.
netdisco-automation
commented
6 years ago From @ollyg on July 14, 2017 15:1
Fair point, yes, we should do it that way.
jeneric
commented
6 years ago Plan to modify the munge routines to handle the binary values returned in 'cdp_id' and 'cdp_port', issues 1 and 2.
It was a conscious decision to keep the timefilter in the lldp index so the combined results from the c_* methods would not lose data or randomly overwrite each other due to random hash ordering.
I'm not clear on the need to create new 'neigh*' methods. Each L2 topology protocol has methods where they can be polled independently. My position is that the SNMP::Info library does not have the information to effectively merge and de-duplicate the results from multiple protocols. For example, determining which protocol best identifies the remote port. That's why the documentation states it is the calling program's responsibility to remove duplicates from the combined results of the 'c*' methods.
I have noticed that some vendors have included a new proprietary MIB leaf to identify the protocol source of the topology information when they combine multiple protocols in the same MIB structure. For example, reporting CDP information in the LLDP table. We could add a new method to indicate the source and use the leafs when available, but I'm not sure that would be any better than just using the protocol specific methods.
jeneric
commented
6 years ago The CDP class has been modified to unpack a binary MAC if present in cdp_port (issue 2).
However, I am unable to determine a reliable way to determine that we were given a packed IP in cdp_id (issue 1) since unpacking of a text system name will result in the return of what looks like an IPv4 address. The MIB states this should be a display string and it doesn't seem to be a common occurrence. So to avoid potentially returning a bogus IP, this has not been resolved.
The last issue CDP vs. LLDP (issue 3) was a conscious design decision to return the most information available to the calling application. The documentation states that it is the calling application's responsibility to remove duplicates and determine which information to keep, for example keep the entry that returns capability information or even merge the two results. The example code in the documentation illustrates that the entries should be cross referenced to the port on which the neighbor was seen. When this is done, the index becomes irrelevant.
There is the potential that a distinct CDP (or another proprietary protocol) and LLDP neighbors are present on the same port. Additionally, because protocols may use different information to represent neighbors and additional context is needed, such as correlating a MAC to a specific neighbor device and port, I think the analysis it best served by the calling application rather than the SNMP::Info library.
With this explanation I'm closing this ticket. It can be reopened if I've misunderstood the issue(s).
inphobia
commented
5 years ago 78119d6547839b6c7b010cccba2e9076a57671d5
this actually breaks a whole lot. if the portname is exactly 6 characters long the munge_mac will convert it to an convert it to a formatted mac address and break topology.
for example, all of our phones return:
CISCO-CDP-MIB::cdpCacheDevicePort.10104.2 = STRING: Port 1
CISCO-CDP-MIB::cdpCacheDevicePort.10116.155 = STRING: Port 1
CISCO-CDP-MIB::cdpCacheDevicePort.10126.143 = STRING: Port 1
CISCO-CDP-MIB::cdpCacheDevicePort.10128.149 = STRING: Port 1
CISCO-CDP-MIB::cdpCacheDevicePort.10133.156 = STRING: Port 1
CISCO-CDP-MIB::cdpCacheDevicePort.10135.145 = STRING: Port 1with this change snmp::info returns for cdp_port():
SNMP::Info::_load_attr cdp_dev_port : CISCO-CDP-MIB::cdpCacheDevicePort : .1.3.6.1.4.1.9.9.23.1.2.1.1.7
\ {
10104.2 "50:6f:72:74:20:31",
10116.155 "50:6f:72:74:20:31",
10126.143 "50:6f:72:74:20:31",
10128.149 "50:6f:72:74:20:31",
10133.156 "50:6f:72:74:20:31",
10135.145 "50:6f:72:74:20:31",
10301.141 "GigabitEthernet1/1/3"
}cdp_dev_port() works as expected:
SNMP::Info::_load_attr cdp_dev_port : CISCO-CDP-MIB::cdpCacheDevicePort : .1.3.6.1.4.1.9.9.23.1.2.1.1.7
\ {
10104.2 "Port 1",
10116.155 "Port 1",
10126.143 "Port 1",
10128.149 "Port 1",
10133.156 "Port 1",
10135.145 "Port 1",
10301.141 "GigabitEthernet1/1/3"
}i don't see an easy way to make both embedded mac addresses & valid port names work together.
as such i'm inclined to revert this part. the mib says this is a string, so just accept that.
perhaps a solution like #54 is a decent middle ground, where the fixup is only applied for certain known broken vendors/devices?
inphobia
commented
5 years ago actual example from nexus6200 & vmware 6.7:
as seen on the esx host this mac address in bogus:
Name PCI Driver Link Speed Duplex MAC Address MTU Description
vmnic0 0000:06:00.0 nenic Up 20000Mbps Full 00:25:b5:08:10:ef 9000 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic1 0000:07:00.0 nenic Up 20000Mbps Full 00:25:b5:08:21:ef 9000 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic2 0000:08:00.0 nenic Up 20000Mbps Full 00:25:b5:08:10:bf 9000 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic3 0000:09:00.0 nenic Up 20000Mbps Full 00:25:b5:08:21:bf 9000 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic4 0000:0e:00.0 nenic Up 20000Mbps Full 00:25:b5:08:10:cf 1500 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic5 0000:0f:00.0 nenic Up 20000Mbps Full 00:25:b5:08:21:cf 1500 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic6 0000:10:00.0 nenic Up 20000Mbps Full 00:25:b5:08:10:af 1500 Cisco Systems Inc Cisco VIC Ethernet NIC
vmnic7 0000:11:00.0 nenic Up 20000Mbps Full 00:25:b5:08:21:af 1500 Cisco Systems Inc Cisco VIC Ethernet NICas discussed on irc fix 2 " Unpack binary MAC if present in cdp_port " will be reverted soon since it breaks devices that actaully do the right thing.
i tried to figure out which device the original poster was talking about be it seems he randomized all macs & such, so a device specific workaround can't be added.
From @fabled on July 13, 2017 11:45
The LLDP/CDP data is not parsed correctly for proper discovery.
Find following an anonymized dump of one switch (IPs are mapped to consistent randomized IP, same goes for MACs and hostnames; the raw literal strings are anonymized inconsistently):
Observations:
Copied from original issue: netdisco/netdisco#329