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
refactor: Update some snmpwalks for ports polling to improve speed #6341
Conversation
Thank you for submitting a PR @laf! We have found the following @BarbarossaTM, @murrant and @ekoyle based on the history of these files to review this PR. |
@@ -66,23 +66,6 @@ | |||
'ifOutMulticastPkts', | |||
); | |||
|
|||
// From above for DB | |||
$etherlike_oids = array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We appear to not use these at all.
@@ -224,11 +210,11 @@ | |||
} | |||
} | |||
|
|||
if ($config['enable_ports_etherlike']) { | |||
echo 'dot3Stats '; | |||
$port_stats = snmpwalk_cache_oid($device, 'dot3StatsEntry', $port_stats, 'EtherLike-MIB'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use any of this table.
echo 'dot3StatsDuplexStatus'; | ||
if ($config['enable_ports_poe'] || $config['enable_ports_etherlike']) { | ||
$port_stats = snmpwalk_cache_oid($device, 'dot3StatsIndex', $port_stats, 'EtherLike-MIB'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use only this + dot3StatsDuplexStatus within poe and etherlike port polling files so only look those up.
if ($device['os_group'] == 'cisco' && $device['os'] != 'asa') { | ||
foreach ($pagp_oids as $oid) { | ||
$port_stats = snmpwalk_cache_oid($device, $oid, $port_stats, 'CISCO-PAGP-MIB'); | ||
$pagp_port_stats = snmpwalk_cache_oid($device, $oid, array(), 'CISCO-PAGP-MIB'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later on we only use the check for pagpOperationMode so we poll that first, check it has data and if so apply it to $port_stats and then poll the rest of pagp data.
@@ -280,7 +270,7 @@ | |||
$port_stats = snmpwalk_cache_oid($device, 'vlanTrunkPortEncapsulationOperType', $port_stats, 'CISCO-VTP-MIB'); | |||
$port_stats = snmpwalk_cache_oid($device, 'vlanTrunkPortNativeVlan', $port_stats, 'CISCO-VTP-MIB'); | |||
} elseif ($device['os'] != 'asa') { | |||
$port_stats = snmpwalk_cache_oid($device, 'dot1qPortVlanTable', $port_stats, 'Q-BRIDGE-MIB'); | |||
$port_stats = snmpwalk_cache_oid($device, 'dot1qPvid', $port_stats, 'Q-BRIDGE-MIB'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only used dot1qPvid out of the entire table!
Comments on both old and new changes in: https://github.com/librenms/librenms/pull/6341/files |
Auto-Deploy finished, Test PR at http://6341.ci.librenms.org or https://6341.ci.librenms.org |
The inspection completed: No new issues |
@laf Looks nice! Not sure when I will have time to test... |
You can have faith :) |
I have faith, but for some portions of the code I require myself to test before approving ;) |
Wimp :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running on production, looks good.
DO NOT DELETE THIS TEXT
Please note
Testers
If you would like to test this pull request then please run:
./scripts/github-apply <pr_id>
, i.e./scripts/github-apply 5926
I'll do some inline comments on this one but for me it drops port polling on Arista down from 160 -> 80 seconds. It also helps our Cisco switches, with about 800 devices I've dropped 20 seconds on each distributed poller.