Multiple issues in LLDP/CDP discovery #252
Comments
|
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. |
|
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... |
|
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. |
|
From @ollyg on July 14, 2017 13:1 @JeroenvIS the intention is not to mess with
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. |
|
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.
|
|
From @ollyg on July 14, 2017 15:1 Fair point, yes, we should do it that way. |
|
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. |
|
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). |
|
snmp-info/lib/SNMP/Info/CDP.pm Line 229 in 910c542 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: with this change snmp::info returns for cdp_port(): cdp_dev_port() works as expected: 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? |
|
actual example from nexus6200 & vmware 6.7: as seen on the esx host this mac address in bogus: |
|
as 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
The text was updated successfully, but these errors were encountered: