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.

Sign up for GitHub

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

Jump to bottom

Correct snmp function usage #12714

Merged
merged 3 commits into from
Apr 9, 2021
Merged

Correct snmp function usage #12714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e3f11c
Use snmp_translate correctly
Jellyfrog Apr 8, 2021
36a0f19
use snmp_get correctly
Jellyfrog Apr 6, 2021
7f4f65a
Update snmp.inc.php
Jellyfrog Apr 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions LibreNMS/OS/ArubaInstant.php
View file
Open in desktop
Expand Up @@ -76,7 +76,7 @@ public function discoverProcessors()
foreach ($ai_ap_data as $ai_ap => $ai_ap_oid) {
$value = $ai_ap_oid['aiAPCPUUtilization'];
$combined_oid = sprintf('%s::%s.%s', $ai_mib, 'aiAPCPUUtilization', Rewrite::oidMac($ai_ap));
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On');
$description = $ai_ap_data[$ai_ap]['aiAPSerialNum'];
$processors[] = Processor::discover('aruba-instant', $this->getDeviceId(), $oid, Rewrite::macToHex($ai_ap), $description, 1, $value);
} // end foreach
Expand Down Expand Up @@ -111,7 +111,7 @@ public function discoverWirelessClients()
// Clients Per SSID
foreach ($ssid_data as $index => $entry) {
$combined_oid = sprintf('%s::%s.%s', $ai_mib, 'aiSSIDClientNum', $index);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On');
$description = sprintf('SSID %s Clients', $entry['aiSSID']);
$oids[] = $oid;
$total_clients += $entry['aiSSIDClientNum'];
Expand All @@ -125,7 +125,7 @@ public function discoverWirelessClients()
foreach ($ap_data as $index => $entry) {
foreach ($entry['aiRadioClientNum'] as $radio => $value) {
$combined_oid = sprintf('%s::%s.%s.%s', $ai_mib, 'aiRadioClientNum', Rewrite::oidMac($index), $radio);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On');
$description = sprintf('%s Radio %s', $entry['aiAPSerialNum'], $radio);
$sensor_index = sprintf('%s.%s', Rewrite::macToHex($index), $radio);
$sensors[] = new WirelessSensor('clients', $this->getDeviceId(), $oid, 'aruba-instant', $sensor_index, $description, $value);
Expand All @@ -139,7 +139,7 @@ public function discoverWirelessClients()
$total_clients = sizeof($client_data);

$combined_oid = sprintf('%s::%s', $ai_mib, 'aiClientMACAddress');
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On');

$sensors[] = new WirelessSensor('clients', $this->getDeviceId(), $oid, 'aruba-instant', 'total-clients', 'Total Clients', $total_clients);
}
Expand All @@ -162,7 +162,7 @@ public function discoverWirelessApCount()
$total_aps = sizeof($ap_data);

$combined_oid = sprintf('%s::%s', $ai_mib, 'aiAPSerialNum');
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On');

$sensors[] = new WirelessSensor('ap-count', $this->getDeviceId(), $oid, 'aruba-instant', 'total-aps', 'Total APs', $total_aps);

Expand Down Expand Up @@ -249,7 +249,7 @@ private function discoverInstantRadio($type, $mib, $desc = '%s Radio %s')
}

$combined_oid = sprintf('%s::%s.%s.%s', $ai_mib, $mib, Rewrite::oidMac($ai_ap), $ai_ap_radio);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($combined_oid, 'ALL', 'arubaos', '-On');
$description = sprintf($desc, $ai_sg_data[$ai_ap]['aiAPSerialNum'], $ai_ap_radio);
$index = sprintf('%s.%s', Rewrite::macToHex($ai_ap), $ai_ap_radio);

Expand Down
2 changes: 1 addition & 1 deletion LibreNMS/OS/Arubaos.php
View file
Open in desktop
Expand Up @@ -87,7 +87,7 @@ public function discoverWirelessApCount()
$sensors = [];

foreach ($data as $key => $value) {
$oid = snmp_translate($mib . '::' . $key, 'ALL', 'arubaos', '-On', null);
$oid = snmp_translate($mib . '::' . $key, 'ALL', 'arubaos', '-On');
$value = intval($value);

$low_warn_const = 1; // Default warning threshold = 1 down AP
Expand Down
4 changes: 2 additions & 2 deletions LibreNMS/OS/Dlinkap.php
View file
Open in desktop
Expand Up @@ -38,8 +38,8 @@ public function discoverOS(Device $device): void
$firmware_oid = $device->sysObjectID . '.5.1.1.0';
$hardware_oid = $device->sysObjectID . '.5.1.5.0';

$device->version = snmp_get($device, $firmware_oid, '-Oqv') ?: null;
$device->hardware = trim($device->sysDescr . ' ' . snmp_get($device, $hardware_oid, '-Oqv'));
$device->version = snmp_get($this->getDeviceArray(), $firmware_oid, '-Oqv') ?: null;
$device->hardware = trim($device->sysDescr . ' ' . snmp_get($this->getDeviceArray(), $hardware_oid, '-Oqv'));
Copy link
Member Author

@Jellyfrog Jellyfrog Apr 6, 2021 •
edited

Choose a reason for hiding this comment

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

I suspect this doesn't break/fixes the tests because we don't have any real test data for it :)

}

/**
Expand Down
13 changes: 7 additions & 6 deletions includes/snmp.inc.php
View file
Open in desktop
Expand Up @@ -40,7 +40,7 @@ function prep_snmp_setting($device, $setting)
}//end prep_snmp_setting()

/**
* @param $device
* @param array $device
* @return array will contain a list of mib dirs
*/
function get_mib_dir($device)
Expand Down Expand Up @@ -78,13 +78,14 @@ function get_mib_dir($device)
* If $mibdir is empty '', return an empty string
*
* @param string $mibdir should be the name of the directory within \LibreNMS\Config::get('mib_dir')
* @param array $device
* @param array|null $device
* @return string The option string starting with -M
*/
function mibdir($mibdir = null, $device = [])
function mibdir($mibdir = null, $device = null)
{
$dirs = is_array($device) ? get_mib_dir($device) : [];

$base = Config::get('mib_dir');
$dirs = get_mib_dir($device);
$dirs[] = "$base/$mibdir";

// make sure base directory is included first
Expand Down Expand Up @@ -779,10 +780,10 @@ function snmp_gen_auth(&$device, $cmd = [], $strIndexing = null)
* @param string $mib
* @param string $mibdir the mib directory (relative to the LibreNMS mibs directory)
* @param array|string $options Options to pass to snmptranslate
* @param array $device
* @param array|null $device
* @return string
*/
function snmp_translate($oid, $mib = 'ALL', $mibdir = null, $options = null, $device = [])
function snmp_translate($oid, $mib = 'ALL', $mibdir = null, $options = null, $device = null)
Copy link
Member

Choose a reason for hiding this comment

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

This should only accept array.

Copy link
Member Author

@Jellyfrog Jellyfrog Apr 9, 2021 •
edited

Choose a reason for hiding this comment

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

No that's what we wanted to get away from.
No point in sending a empty array when we're only interested in it if it's a "device array".
It's sent to mibdir which does a is_array check

Copy link
Member Author

Choose a reason for hiding this comment

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

Best would ofc be to send the device object instead... One day. One day.

{
$cmd = [Config::get('snmptranslate', 'snmptranslate'), '-M', mibdir($mibdir, $device), '-m', $mib];

Expand Down