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

Improve legacy Allied Telesis hardware support #8071

Merged
merged 3 commits into from Jan 21, 2018

Conversation

Projects
None yet
4 participants
@mattie47
Contributor

mattie47 commented Jan 11, 2018

Refer to patch message for more details.

This patch is mostly working, with the exception of the "sysObjectId" section in includes/definitions/radlan.yaml

I've hashed it as a result. The issue is if I leave it on, generic devices will appear as "radlan" when they shouldn't.... Not sure what I'm doing wrong here.

Sorry if the code isn't pretty. I tried!

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

@mattie47

This comment has been minimized.

Contributor

mattie47 commented Jan 11, 2018

@murrant @laf

So I've given it a go and tried to add legacy support as per: #7670 (comment)

Not entirely sure what I did wrong where it's showing more than just commit 31947c9.

If you have suggestions, please feel free to edit away and I can test them.

Thanks! :-)

@murrant

This comment has been minimized.

Member

murrant commented Jan 11, 2018

sysObjectId has been renamed to sysObjectID. Fixing that should help

@mattie47

This comment has been minimized.

Contributor

mattie47 commented Jan 11, 2018

Thanks for that @murrant. That fixed things.

By the way - When I try run ./scripts/pre-commit.php, I just get:

librenms@librenmsdev:~$ ./scripts/pre-commit.php
Running unit tests... MIB search path: /opt/librenms/mibs
Cannot find module (FCMGMT-MIB): At line 28 in /opt/librenms/mibs/SW-MIB
Did not find 'connUnitPortStatEntry' in module #-1 (/opt/librenms/mibs/SW-MIB)
MIB search path: /opt/librenms/mibs
Cannot find module (FCMGMT-MIB): At line 28 in /opt/librenms/mibs/SW-MIB
Did not find 'connUnitPortStatEntry' in module #-1 (/opt/librenms/mibs/SW-MIB)
MIB search path: /opt/librenms/mibs
Cannot find module (FCMGMT-MIB): At line 28 in /opt/librenms/mibs/SW-MIB
Did not find 'connUnitPortStatEntry' in module #-1 (/opt/librenms/mibs/SW-MIB)
MIB search path: /opt/librenms/mibs
Cannot find module (FCMGMT-MIB): At line 28 in /opt/librenms/mibs/SW-MIB
Did not find 'connUnitPortStatEntry' in module #-1 (/opt/librenms/mibs/SW-MIB)
^C

Have I done something stupid, or there a bug with the pre-commit check?

Thanks,

Matt

@laf

Thanks for this as always.

You need to drop + from all the MIBs being referenced.

Also, as this will now detect more data please add some test files to cover them including the additional test data we need https://docs.librenms.org/#Developing/os/Test-Units/#example-workflow

//Legacy products: 8100S
//SNMPv2-MIB::sysDescr.0 = STRING: AlliedWare Plus (TM) 2.2.3.0
if (!$hardware && !$version && !$features) {
$hardware = snmp_get($device, '.1.3.6.1.4.1.207.8.17.1.3.1.6.1', '-OsvQU', '+AT-SMI-MIB');

This comment has been minimized.

@laf

laf Jan 11, 2018

Member

Using the MIB here doesn't need the + in front of it and you should also switch to using the named OID and use snmp_get_multi_oid for these three together.

This comment has been minimized.

@mattie47

mattie47 Jan 12, 2018

Contributor
  • Ok have removed the + throughout the file.

  • I've done some analysis into the mibs, and I actually shouldn't have been using some of these mib files. A couple of them can translate an initial part of the OID, but not the whole thing.

  • As a result, I've removed reference to the mib, and kept the static OID.

  • I also don't think I can use snmp_get_multi_oid here. I tried the following:

if (!$hardware && !$version) {
$data2 = snmp_get_multi_oid($device, '.1.3.6.1.4.1.207.8.17.1.3.1.6.1 .1.3.6.1.4.1.207.8.17.1.3.1.5.1 .1.3.6.1.4.1.207.8.17.1.3.1.8.1', '-OsvQU');

$hardware = $data2['.1.3.6.1.4.1.207.8.17.1.3.1.6.1'];
$version = $data2['.1.3.6.1.4.1.207.8.17.1.3.1.5.1'];
$serial = $data2['.1.3.6.1.4.1.207.8.17.1.3.1.8.1'];
}

However, the box appears to get overloaded when polled (these are really low end CPU products and the CPU hits 100% when I run the poller.php):

SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 .1.3.6.1.4.1.207.8.17.1.3.1.6.1 .1.3.6.1.4.1.207.8.17.1.3.1.5.1 .1.3.6.1.4.1.207.8.17.1.3.1.8.1]
"AT-8100S/48POE"
"*"
"A04481G114700071"

Hardware: 
Version: 
Features: 
Serial: 

>> Runtime for poller module 'os': 0.4109 seconds with 4136 bytes

If I run the command manually, it works:

librenms@librenmsdev:~$ snmpget -v2c -c librenms -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied 10.37.105.93 .1.3.6.1.4.1.207.8.17.1.3.1.6.1 .1.3.6.1.4.1.207.8.17.1.3.1.5.1 .1.3.6.1.4.1.207.8.17.1.3.1.8.1
Bad operator (Settings): At line 41 in /opt/librenms/mibs/allied/AT-DOS-MIB
MIB search path: /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied
Cannot find module (FCMGMT-MIB): At line 28 in /opt/librenms/mibs/SW-MIB
Did not find 'connUnitPortStatEntry' in module #-1 (/opt/librenms/mibs/SW-MIB)
"AT-8100S/48POE"
"2.2.3.0"
"A04481G114700071"

And if I switch back to:

if (!$hardware && !$version) {
    $hardware = snmp_get($device, '.1.3.6.1.4.1.207.8.17.1.3.1.6.1', '-OsvQU');
    $version  = snmp_get($device, '.1.3.6.1.4.1.207.8.17.1.3.1.5.1', '-OsvQU');
    $serial   = snmp_get($device, '.1.3.6.1.4.1.207.8.17.1.3.1.8.1', '-OsvQU');
}

then I get:

#### Load poller module os ####
SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -m AtiSwitch-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 atiswitchProductType.0 atiswitchSwVersion.0 atiswitchSw.0]
No Such Object available on this agent at this OID
No Such Object available on this agent at this OID
No Such Object available on this agent at this OID

SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -m AtiL2-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 atiL2SwProduct.0]
No Such Object available on this agent at this OID

SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -m AtiL2-MIB -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 atiL2SwVersion.0]
No Such Object available on this agent at this OID

SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 .1.3.6.1.4.1.207.8.17.1.3.1.6.1]
"AT-8100S/48POE"

SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 .1.3.6.1.4.1.207.8.17.1.3.1.5.1]
"*"

SNMP[/usr/bin/snmpget -v2c -c COMMUNITY -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied:/opt/librenms/mibs/allied udp:HOSTNAME:161 .1.3.6.1.4.1.207.8.17.1.3.1.8.1]
"A04481G114700071"

SQL[INSERT INTO `eventlog` (`host`,`device_id`,`reference`,`type`,`datetime`,`severity`,`message`,`username`)  VALUES ('19','19','NULL','system',NOW(),'3','OS Version -> Plus 2.2.3.0','')] 
SQL[INSERT INTO `eventlog` (`host`,`device_id`,`reference`,`type`,`datetime`,`severity`,`message`,`username`)  VALUES ('19','19','NULL','system',NOW(),'3','Hardware -> AT-8100S/48POE','')] 
SQL[INSERT INTO `eventlog` (`host`,`device_id`,`reference`,`type`,`datetime`,`severity`,`message`,`username`)  VALUES ('19','19','NULL','system',NOW(),'3','Serial -> A04481G114700071','')] 
Hardware: AT-8100S/48POE
Version: Plus 2.2.3.0
Features: 
Serial: A04481G114700071

Although it's possible I've done something wrong here...... I just tried to look at some of the other vendor.inc.php files to get an idea on what to do for snmp_get_multi_oid

This comment has been minimized.

@mattie47

mattie47 Jan 12, 2018

Contributor

Ignore. murrant pointed out I shouldn't be using '-OsvQU' since I'm specifying the oid manually. Works now

//Configuration file for Allied Telesis/Telesyn products NOT running Alliedware Plus 5.x.x.
//See awplus.inc.php for Alliedware Plus 5.x.x OS configuration.
/*

This comment has been minimized.

@laf

laf Jan 11, 2018

Member

Don't follow the old code, we don't need all of these comments. The data it's referencing should be made available in the snmprec files for testing anyway

This comment has been minimized.

@mattie47

mattie47 Jan 11, 2018

Contributor

So are you essentially saying get rid of pretty much all the comments e.g.


+/*
+Configuration file for Allied Telesis/Telesyn products NOT running Alliedware Plus 5.x.x.
 +See awplus.inc.php for Alliedware Plus 5.x.x OS configuration.
 + Configuration file covers:
 +- Alliedware
 +- Alliedware Plus v2.x.x.x
 +- AT-S39/41 circa 2003
 +- WebSmart OS
 +Note: ATI AT-8000S and AT-8000GS run RADLAN OS. 
 +*/ 
  
?
$hardware = snmp_getnext($device, '.1.3.6.1.2.1.47.1.1.1.1.2.64', '-OsvQU', '+ENTITY-MIB');
$version = snmp_get($device, '.1.3.6.1.4.1.89.2.4.0', '-OsvQU', '+SNMPv2-SMI');
$serial = snmp_get($device, 'ENTITY-MIB::entPhysicalSerialNum.64', '-OsvQU', '+ENTITY-MIB');

This comment has been minimized.

@laf

laf Jan 11, 2018

Member

Don't need ENTITY-MIB:: as you specify it after anyway

This comment has been minimized.

@mattie47

mattie47 Jan 12, 2018

Contributor

Have fixed this up for next commit

//legacy radlan. Not sure which devices use this.
if (!$hardware && !$version && !$serial) {
$version = snmp_get($device, 'rndBrgVersion.0', '-Ovq', 'RADLAN-MIB');

This comment has been minimized.

@laf

laf Jan 11, 2018

Member

As you now have two snmp queries, move them to snmp_get_multi_oid

This comment has been minimized.

@mattie47

mattie47 Jan 12, 2018

Contributor

As I've been fixing this up, I did some more analysis on the OIDs.

I currently have:

$hardware = snmp_getnext($device, 'entPhysicalDescr.64', '-OsvQU', 'ENTITY-MIB');
$version  = snmp_get($device, 'rndBrgVersion.0', '-OsvQU', 'RADLAN-MIB');
$serial   = snmp_get($device, 'entPhysicalSerialNum.64', '-OsvQU', 'ENTITY-MIB');

//legacy radlan. Not sure which devices use this. 

if (!$hardware && !$version && !$serial) {
$version  = snmp_get($device, 'rndBrgVersion.0', '-Ovq', 'RADLAN-MIB');
$hardware = str_replace('ATI', '', $device['sysDescr']);
$features = snmp_get($device, 'rndBaseBootVersion.00', '-Ovq', 'RADLAN-MIB');
}

Now that snmp_getnext exists in librenms, is possible that "$hardware = str_replace('ATI', '', $device['sysDescr']);" is no longer needed.

I've tested on two different models:

librenms@librenmsdev:~$ snmpgetnext -v2c -c librenms -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/radlan 10.37.105.92 SysDesc
MIB search path: /opt/librenms/mibs:/opt/librenms/mibs/radlan
48-port 10/100/1000 Ethernet Switch
librenms@librenmsdev:~$ snmpgetnext -v2c -c librenms -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/radlan 10.37.105.91 SysDesc
ATI AT-8000S

As you can see, both models while running the same OS have vastly different System Descriptions. However, both provide correct info when:

librenms@librenmsdev:~$ snmpgetnext -v2c -c librenms -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/radlan 10.37.105.91 entPhysicalDescr.64
AT-8000S/24   
librenms@librenmsdev:~$ snmpgetnext -v2c -c librenms -OsvQU -M /opt/librenms/mibs:/opt/librenms/mibs/radlan 10.37.105.92 entPhysicalDescr.64
AT-8000GS/48

Therefore, I'm just going to use the snmpgetnext instead.

After going through above, this is all a lot more simple:

@mattie47

This comment has been minimized.

Contributor

mattie47 commented Jan 16, 2018

@laf I've fixed up radlan.inc.php and allied.inc.php how I think you want.

I'm aware I still need to update snmpsim.

I should add I've thoroughly tested this with all variants:

image

@mattie47

This comment has been minimized.

Contributor

mattie47 commented Jan 16, 2018

Also I realise this is pretty horrible with commits everywhere. Once you're happy I'll try figure out how to squash all the commits into one....

devices: Improved Legacy Allied Telesis hardware support #8071
This  patch covers:

Improved SNMPrec test coverage for Allied telesis devices

allied.yaml
	- Increased device exception OID list.
		-  This list of devices are running RADLAN OS, and should not be triggered by allied.yaml
	- Minor bug fixes

allied.inc.php
	- Added updated support for Alliedware devices
		-  Version
		-  Features
		-  Hardware
		-  Serial
	- Added support for Allied Telesis Websmart switches
		-  Version
		-  Hardware
	- Added support for Alliedware Plus version 2 OS devices
		-  Hardware
		-  Version
		-  Serial
	- Simplified polling to use snmp_get_multi_oid
	- Minor bug fixes

radlan.yaml
	- Updated device support for 8000S and 8000GS.
		-  Specified device object IDs
			-  Added an exclusion also for OIDs in allied.yaml
		-  Changed icon and group to alliedtelesis

radlan.inc.php
	- Updated support for devices identifying as radlan OS.

awplus.yaml
	- Added comment to remove the sysObjectID_except list.
	- Minor bug fixes
@laf

This comment has been minimized.

Member

laf commented Jan 17, 2018

No need to squash @mattie47. We do that when we merge so we only end up with one commit after.

@laf

This comment has been minimized.

Member

laf commented Jan 17, 2018

Thanks for all of this. I've generated the rest of the test data from your snmprec files.

When you add data for tests in future, just run ./scripts/save-test-data.php -h ID against a device you have within LibreNMS. It will generate the snmprec and json file automatically.

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Jan 17, 2018

The inspection completed: No new issues

@laf

laf approved these changes Jan 17, 2018

@mattie47

This comment has been minimized.

Contributor

mattie47 commented Jan 17, 2018

@laf

Thanks for the info. I guess I forgot for that last websmart device.

No need to squash @mattie47. We do that when we merge so we only end up with one commit after.

OK no worries. I guess you guys edit a commit message accordingly to the final squash/commit?

Thanks,

Matt

@laf

This comment has been minimized.

Member

laf commented Jan 18, 2018

OK no worries. I guess you guys edit a commit message accordingly to the final squash/commit?

Yup, mostly based of the title of the PR but we might tweak if needs be.

@murrant murrant merged commit 71b4440 into librenms:master Jan 21, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@murrant

This comment has been minimized.

Member

murrant commented Jan 21, 2018

Thanks @mattie47 great work :)

@mattie47 mattie47 deleted the mattie47:legacy-allied branch Jan 21, 2018

@murrant murrant added the Device 🖥 label Jan 21, 2018

@lock

This comment has been minimized.

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.