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

feature: parallel snmp-scan.py #6889

Merged
merged 8 commits into from Jul 3, 2017

Conversation

Projects
None yet
7 participants
@murrant
Member

murrant commented Jun 25, 2017

Reduces scan time of a /24 from 5 minutes to 14 seconds
Work is done by addhost.php

Just tries to addhost.php hostname/ip right now
Might need some more complexity added there, but I wasn't sure what.

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

feature: parallel snmp-scan.py
Reduces scan time of a /24 from 5 minutes to 14 seconds
Work is done by addhost.php

Just tries to addhost.php hostname/ip right now
Might need some more complexity added there, but I wasn't sure what.
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Jun 25, 2017

Thank you for submitting a PR @murrant! We have found the following @laf, @NetworkNub and @3XTron based on the history of these files to review this PR.

mention-bot commented Jun 25, 2017

Thank you for submitting a PR @murrant! We have found the following @laf, @NetworkNub and @3XTron based on the history of these files to review this PR.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jun 26, 2017

Member
-bash-4.2$ ./snmp-scan.py -l
usage: snmp-scan.py [-h] [-r NETWORK] [-t THREADS] [-l] [-v]
snmp-scan.py: error: $config['nets'] is not set in config.php, you must specify a network to scan with -r
-bash-4.2$ ./snmp-scan.py --legend
usage: snmp-scan.py [-h] [-r NETWORK] [-t THREADS] [-l] [-v]
snmp-scan.py: error: $config['nets'] is not set in config.php, you must specify a network to scan with -r

Got this trying to list legend ^^

I do wonder if we scrap the php version, I don't think it's worth having two that do the same thing one just not as good as the other.

Member

laf commented Jun 26, 2017

-bash-4.2$ ./snmp-scan.py -l
usage: snmp-scan.py [-h] [-r NETWORK] [-t THREADS] [-l] [-v]
snmp-scan.py: error: $config['nets'] is not set in config.php, you must specify a network to scan with -r
-bash-4.2$ ./snmp-scan.py --legend
usage: snmp-scan.py [-h] [-r NETWORK] [-t THREADS] [-l] [-v]
snmp-scan.py: error: $config['nets'] is not set in config.php, you must specify a network to scan with -r

Got this trying to list legend ^^

I do wonder if we scrap the php version, I don't think it's worth having two that do the same thing one just not as good as the other.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Jun 26, 2017

Member

Yes, scrap the php version, but not confident enough to do that yet ;)

That output seems appropriate laf, we don't have any networks to scan. Perhaps the wording needs to be improved.

legend isn't a one-off option, it is printed at the top of the output and then the scan commences.

Member

murrant commented Jun 26, 2017

Yes, scrap the php version, but not confident enough to do that yet ;)

That output seems appropriate laf, we don't have any networks to scan. Perhaps the wording needs to be improved.

legend isn't a one-off option, it is printed at the top of the output and then the scan commences.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jun 26, 2017

Member

Oh it's a one off in the php version. I don't care either way but do you want to mimic the php version more closely?

Member

laf commented Jun 26, 2017

Oh it's a one off in the php version. I don't care either way but do you want to mimic the php version more closely?

murrant added some commits Jun 27, 2017

Improvements in ip handling and output
Add compatibility arguments so it can be used as a drop in replacement for snmp-scan.php
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Jun 27, 2017

Member

I changed it so it may not mimic the php version, but it can be used as a drop-in replacement by changing snmp-scan.php to snmp-scan.py

Member

murrant commented Jun 27, 2017

I changed it so it may not mimic the php version, but it can be used as a drop-in replacement by changing snmp-scan.php to snmp-scan.py

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jun 27, 2017

Member

I definitely think we should ditch the php in this pull request.

Btw I think scan time depends on other factors, a /24 scan for me took 90+ seconds - still loads better than the php version.

Member

laf commented Jun 27, 2017

I definitely think we should ditch the php in this pull request.

Btw I think scan time depends on other factors, a /24 scan for me took 90+ seconds - still loads better than the php version.

@FTBZ

This comment has been minimized.

Show comment
Hide comment
@FTBZ

FTBZ Jun 27, 2017

Contributor

FYI, when scanning a /24 range with TCP not enabled it took hours for me not only 5 minutes.

Contributor

FTBZ commented Jun 27, 2017

FYI, when scanning a /24 range with TCP not enabled it took hours for me not only 5 minutes.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jun 27, 2017

Member

Using this python version?

Member

laf commented Jun 27, 2017

Using this python version?

@FTBZ

This comment has been minimized.

Show comment
Hide comment
@FTBZ

FTBZ Jun 27, 2017

Contributor

Nope, I'm saying the old snmp-scan.php. I didn't test the new one because it requiring python3.

Contributor

FTBZ commented Jun 27, 2017

Nope, I'm saying the old snmp-scan.php. I didn't test the new one because it requiring python3.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Jun 27, 2017

Member

5 minutes is the python script on a sparse network without threading it does not try tcp at this time...

Member

murrant commented Jun 27, 2017

5 minutes is the python script on a sparse network without threading it does not try tcp at this time...

@FTBZ

This comment has been minimized.

Show comment
Hide comment
@FTBZ

FTBZ Jun 27, 2017

Contributor

It works immediately better...

Scanned 254 IPs: 30 known devices, added 0 devices, failed to add 37 devices
Runtime: 74.45 seconds

Contributor

FTBZ commented Jun 27, 2017

It works immediately better...

Scanned 254 IPs: 30 known devices, added 0 devices, failed to add 37 devices
Runtime: 74.45 seconds

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Jun 27, 2017

Member

@FTBZ See how long that takes with this setting enabled in config.php $config['addhost_alwayscheckip'] = true

Member

murrant commented Jun 27, 2017

@FTBZ See how long that takes with this setting enabled in config.php $config['addhost_alwayscheckip'] = true

murrant added some commits Jun 27, 2017

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Jun 27, 2017

Member

I think this is ready to go. I'd say merge this. Make an announcement about moving from snmp-scan.php to snmp-scan.py and then dump the snmp-scan.php. I think some people run this in cron...

Member

murrant commented Jun 27, 2017

I think this is ready to go. I'd say merge this. Make an announcement about moving from snmp-scan.php to snmp-scan.py and then dump the snmp-scan.php. I think some people run this in cron...

@murrant murrant added the Discovery label Jun 28, 2017

@sorano

This comment has been minimized.

Show comment
Hide comment
@sorano

sorano Jun 29, 2017

Contributor

Tested this and seeing really nice speed improvements:

librenms@nms:~$ /opt/librenms/snmp-scan.py 172.31.5.0/24
Scanning IPs:
*******..............************..*.*..***.*.*...*...............----.**.....-.------..----------..-..-....-..**--..----.....................*-*...**************........****..----..**********.................-------.....------------**********-*--

Scanned 247 IPs: 72 known devices, added 0 devices, failed to add 57 devices, 7 ips excluded by config
Runtime: 102.01 seconds
librenms@nms:~$ time /opt/librenms/snmp-scan.php -r 172.31.5.0/24
Range: 172.31.5.0/24
|||||||..**.*****.........****.****.*****-*******.--..---..---.....-----..----.-......--.-.--------....-**.****........--------............--.-**-...*****........................*********************-----....----..--..---*******.................****.....
Scanned 254 IPs, Already known 72 Devices, Added 0 Devices, Failed to add 57 Devices.
Runtime: 723.93255090714 secs

real    12m3.964s
user    0m12.884s
sys     0m8.000s


with this setting enabled in config.php $config['addhost_alwayscheckip'] = true;

librenms@nms:~$ /opt/librenms/snmp-scan.py 172.31.5.0/24
Scanning IPs:
*******................***.*****..************....................----*-*------...-.-...--------.--...-.....-.**-----....................-.....****************......**.**...*..----***..*******.................-------.....------------*********-**--

Scanned 247 IPs: 73 known devices, added 0 devices, failed to add 57 devices, 7 ips excluded by config
Runtime: 101.61 seconds

Contributor

sorano commented Jun 29, 2017

Tested this and seeing really nice speed improvements:

librenms@nms:~$ /opt/librenms/snmp-scan.py 172.31.5.0/24
Scanning IPs:
*******..............************..*.*..***.*.*...*...............----.**.....-.------..----------..-..-....-..**--..----.....................*-*...**************........****..----..**********.................-------.....------------**********-*--

Scanned 247 IPs: 72 known devices, added 0 devices, failed to add 57 devices, 7 ips excluded by config
Runtime: 102.01 seconds
librenms@nms:~$ time /opt/librenms/snmp-scan.php -r 172.31.5.0/24
Range: 172.31.5.0/24
|||||||..**.*****.........****.****.*****-*******.--..---..---.....-----..----.-......--.-.--------....-**.****........--------............--.-**-...*****........................*********************-----....----..--..---*******.................****.....
Scanned 254 IPs, Already known 72 Devices, Added 0 Devices, Failed to add 57 Devices.
Runtime: 723.93255090714 secs

real    12m3.964s
user    0m12.884s
sys     0m8.000s


with this setting enabled in config.php $config['addhost_alwayscheckip'] = true;

librenms@nms:~$ /opt/librenms/snmp-scan.py 172.31.5.0/24
Scanning IPs:
*******................***.*****..************....................----*-*------...-.-...--------.--...-.....-.**-----....................-.....****************......**.**...*..----***..*******.................-------.....------------*********-**--

Scanned 247 IPs: 73 known devices, added 0 devices, failed to add 57 devices, 7 ips excluded by config
Runtime: 101.61 seconds

@laf

laf approved these changes Jun 29, 2017

LGTM although I would 100% remove the php version in this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

LibreNMS-CI commented Jun 30, 2017

Auto-Deploy finished, Test PR at http://6889.ci.librenms.org or https://6889.ci.librenms.org

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Jun 30, 2017

The inspection completed: No new issues

scrutinizer-notifier commented Jun 30, 2017

The inspection completed: No new issues

@librenms librenms deleted a comment from LibreNMS-CI Jul 2, 2017

@murrant murrant merged commit f810265 into librenms:master Jul 3, 2017

3 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@murrant murrant deleted the murrant:snmp-scan-parallel branch Jul 3, 2017

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 17, 2018

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

lock bot commented May 17, 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 17, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.