Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Mikrotik LLDP discovery #7901

Merged
merged 4 commits into from Dec 16, 2017
Merged

Add Mikrotik LLDP discovery #7901

merged 4 commits into from Dec 16, 2017

Conversation

erotel
Copy link
Contributor

@erotel erotel commented Dec 11, 2017

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

RouterOS must be > 6.38

mikrotik-lldp

@laf
Copy link
Member

laf commented Dec 11, 2017

Nice work @erotel :)

out of interest can the other LLDP code further down not be re-used?

@erotel
Copy link
Contributor Author

erotel commented Dec 11, 2017

Standard LLDP can not be used because Mikrotik does not meet the LLDP standard

@laf
Copy link
Member

laf commented Dec 11, 2017

Standard LLDP can not be used because Mikrotik does not meet the LLDP standard

Oh how I laughed :(

if (($device['os'] == 'routeros') && Config::get('autodiscovery.xdp') === true) {
echo ' LLDP-MIB: ';
$lldp_array = snmpwalk_group($device, 'lldpRemEntry', 'LLDP-MIB', 3);
$lldp_ports = snmpwalk_group($device, 'mtxrInterfaceStatsName', 'MIKROTIK-MIB');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not unconditionally do 3 snmpwalks here.

If the first one fails, there is no point in doing the other two.

@murrant
Copy link
Member

murrant commented Dec 12, 2017

Looks great @erotel, looks good to merge after the small change I mentioned

@erotel
Copy link
Contributor Author

erotel commented Dec 12, 2017

@laf The Mikrotik has a lot of things wrong

standart lldp in edge core

root@speedtest:/opt/librenms# snmpwalk -v2c -c public -OQUsetX -m LLDP-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/mikrotik udp:192.168.224.224:161 lldpRemTable
lldpRemChassisIdSubtype[0][5][5] = 5
lldpRemChassisIdSubtype[0][5][9] = 4
lldpRemChassisIdSubtype[0][24][4] = 7
lldpRemChassisIdSubtype[0][24][6] = 7
lldpRemChassisIdSubtype[0][27][3] = 4
lldpRemChassisId[0][5][5] = "01 0A 85 54 C6 "
lldpRemChassisId[0][5][9] = "00 1B B1 04 B6 50 "
lldpRemChassisId[0][24][4] = "RAy17_L"
lldpRemChassisId[0][24][6] = "RAy17_U"
lldpRemChassisId[0][27][3] = "CC 37 AB C4 5D 30 "
lldpRemPortIdSubtype[0][5][5] = 3
lldpRemPortIdSubtype[0][5][9] = 5
lldpRemPortIdSubtype[0][24][4] = 7
lldpRemPortIdSubtype[0][24][6] = 7
lldpRemPortIdSubtype[0][27][3] = 3

and Mikrotik

root@speedtest:/opt/librenms# snmpwalk -v2c -c public -OQUsetX -m LLDP-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/mikrotik udp:10.133.0.8:161 lldpRemTable
lldpRemChassisIdSubtype[1] = 4
lldpRemChassisIdSubtype[2] = 4
lldpRemChassisIdSubtype[3] = 4
lldpRemChassisIdSubtype[4] = 4
lldpRemChassisIdSubtype[5] = 4
lldpRemChassisIdSubtype[6] = 4
lldpRemChassisIdSubtype[7] = 4
lldpRemChassisIdSubtype[8] = 4
lldpRemChassisIdSubtype[10] = 4
lldpRemChassisIdSubtype[11] = 4
lldpRemChassisIdSubtype[12] = 4
lldpRemChassisIdSubtype[13] = 4

Copy link
Member

@laf laf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one change from me then good to go.

if (!empty($lldp_array)) {
$lldp_ports = snmpwalk_group($device, 'mtxrInterfaceStatsName', 'MIKROTIK-MIB');
$lldp_ports_num = snmpwalk_group($device, 'mtxrNeighborInterfaceID', 'MIKROTIK-MIB');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May as well move the foreach ($lldp_array) into this if check, no point in even attempting to loop through the empty array.

@scrutinizer-notifier
Copy link

The inspection completed: 2 new issues

@laf laf merged commit c30a5e8 into librenms:master Dec 16, 2017
@lock
Copy link

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants