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

device: Allied Telesis Wireless Support #8692

Merged
merged 14 commits into from Jul 2, 2018

Conversation

Projects
None yet
3 participants
@mattie47
Copy link
Contributor

mattie47 commented May 11, 2018

This commit provides basic info for Allied Telesis TQ Access Points

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 added some commits May 10, 2018

device: Initial support for Allied Telesis Wireless devices
This commit provides basic info for Allied Telesis TQ Access Points
device: allied wireless
Fix up mibs.
-- Moved ATKK-WLAN-ACCESS to awplus mibs folder
--- ATKK-WLAN-ACCESS mib relies on AT-SMI-MIB
-- Changed Allied-wireless to look at awplus mibs folder instead of allied
--- As above due to newer AT-SMI-MIB file
- Performed new snmprec with walk on enterprise OID
-- For some reason the enterprise and wireless specific tables are not
   walked by a default snmpwalk.
@mattie47

This comment has been minimized.

Copy link
Contributor Author

mattie47 commented May 11, 2018

So, this is pretty basic for now:

image

In terms of what the APs actually provide via SNMP, it's quite minimal, but I've added the new mib for them.

I've also attached allied-wireless-snmpwalk.txt to the commit, just for showing the other wireless info.

I was going to have a look at what's required to map up some of the wireless stuff, but I see wireless got an overhaul in #6471 and I'm probably better requesting someone looks at the wireless side as a new platform support request. Do I need to open a new device issue?

I also had '''$data_array = snmpwalk_cache_multi_oid($device, '.1.3.6.1.4.1.207', $data_array);''' in the poller, just so when I did ./scripts/collect-snmp-data.php -o allied-wireless -h wirelessap1 -d it would pick up all the wireless stuff for the snmprec file.

@mattie47

This comment has been minimized.

Copy link
Contributor Author

mattie47 commented May 11, 2018

Also - What am I doing fundamentally wrong with the snmp test stuff?

My process has been:

./scripts/collect-snmp-data.php -o allied-wireless -h wirelessap1
nano /opt/librenms/tests/snmpsim/allied-wireless.snmprec - Modify any private stuff
./scripts/save-test-data.php -o allied-wireless

Yet when I run ./scripts/pre-commit.php -o allied-wireless I get similiar results to travis but I feel like I haven't done anything wrong. It looks like it's complaining about the about in the json file not being null...

I did get the following which may have caused it:

RRD[update /opt/librenms/rrd/127.1.6.2/port-id7682.rrd N:U:U:0:0:18446744073709550758:4294932019:U:U:0:0:U:0:0:858:0]
SQL[UPDATE `ports` set `poll_time` ='1526001515',`poll_period` ='1526001515',`ifLastChange` ='6725700',`ifAlias` ='wlan0vap2',`ifAdminStatus` ='up',`ifOperStatus` ='up',`ifMtu` ='1500',`ifHighSpeed` ='0',`ifType` ='ieee80211',`ifPhysAddress` ='eccd6df2da82',`ifPromiscuousMode` ='false',`ifConnectorPresent` ='true',`ifInOctets` ='0',`ifInOctets_prev` ='0',`ifOutOctets` ='0',`ifOutOctets_prev` ='0',`ifInErrors` ='0',`ifInErrors_prev` ='0',`ifOutErrors` ='0',`ifOutErrors_prev` ='0',`ifInUcastPkts` ='18446744073709550758',`ifInUcastPkts_prev` ='0',`ifOutUcastPkts` ='4294932019',`ifOutUcastPkts_prev` ='0' WHERE `port_id` = '7682'] 
MySQL Error: Out of range value for column 'ifInUcastPkts' at row 1 (UPDATE `ports` set `poll_time` ='1526001515',`poll_period` ='1526001515',`ifLastChange` ='6725700',`ifAlias` ='wlan0vap2',`ifAdminStatus` ='up',`ifOperStatus` ='up',`ifMtu` ='1500',`ifHighSpeed` ='0',`ifType` ='ieee80211',`ifPhysAddress` ='eccd6df2da82',`ifPromiscuousMode` ='false',`ifConnectorPresent` ='true',`ifInOctets` ='0',`ifInOctets_prev` ='0',`ifOutOctets` ='0',`ifOutOctets_prev` ='0',`ifInErrors` ='0',`ifInErrors_prev` ='0',`ifOutErrors` ='0',`ifOutErrors_prev` ='0',`ifInUcastPkts` ='18446744073709550758',`ifInUcastPkts_prev` ='0',`ifOutUcastPkts` ='4294932019',`ifOutUcastPkts_prev` ='0' WHERE `port_id` = '7682')
SQL[UPDATE `ports_statistics` set `ifInNUcastPkts` ='0',`ifInNUcastPkts_prev` ='0',`ifOutNUcastPkts` ='0',`ifOutNUcastPkts_prev` ='0',`ifInDiscards` ='0',`ifInDiscards_prev` ='0',`ifOutDiscards` ='0',`ifOutDiscards_prev` ='0',`ifInUnknownProtos` ='0',`ifInUnknownProtos_prev` ='0',`ifInBroadcastPkts` ='0',`ifInBroadcastPkts_prev` ='0',`ifOutBroadcastPkts` ='0',`ifOutBroadcastPkts_prev` ='0',`ifInMulticastPkts` ='858',`ifInMulticastPkts_prev` ='0',`ifOutMulticastPkts` ='0',`ifOutMulticastPkts_prev` ='0' WHERE `port_id` = '7682'] 
1 updated
@murrant

This comment has been minimized.

Copy link
Member

murrant commented May 11, 2018

It's failing because you have a failed SQL query (remember tests always run in strict sql mode):

MySQL Error: Out of range value for column 'ifInUcastPkts' at row 1 (UPDATE `ports` set `poll_time` ='1526002098',`poll_period` ='1526002098',`ifLastChange` ='6725700',`ifAlias` ='wlan0vap2',`ifAdminStatus` ='up',`ifOperStatus` ='up',`ifMtu` ='1500',`ifHighSpeed` ='0',`ifType` ='ieee80211',`ifPhysAddress` ='eccd6df2da82',`ifPromiscuousMode` ='false',`ifConnectorPresent` ='true',`ifInOctets` ='0',`ifInOctets_prev` ='0',`ifOutOctets` ='0',`ifOutOctets_prev` ='0',`ifInErrors` ='0',`ifInErrors_prev` ='0',`ifOutErrors` ='0',`ifOutErrors_prev` ='0',`ifInUcastPkts` ='18446744073709550758',`ifInUcastPkts_prev` ='0',`ifOutUcastPkts` ='4294932019',`ifOutUcastPkts_prev` ='0' WHERE `port_id` = '14')

Yeah, so 18446744073709550758 is a really big number, but I think it is valid. Unsigned bigint max is 18446744073709551615, but we have signed in the database,which is 9223372036854775807.

@laf Do you think we need to change the port counters to unsigned bigint?

@laf

This comment has been minimized.

Copy link
Member

laf commented May 13, 2018

@laf Do you think we need to change the port counters to unsigned bigint?

@murrant If we are going to hit issues then absolutely.

@mattie47

This comment has been minimized.

Copy link
Contributor Author

mattie47 commented Jun 7, 2018

Hi @murrant,

I've tried to have a go at adding at least a tiny bit of wireless information for the Allied Telesis TQ access points, but I've hit a bit of a snag, in that snmp is really poorly implemented on them.

I was going to just add:

  • Total Number of wireless clients overall
  • Total Number of Neighbouring APs

Essentially, the mibs are done with an index based on a devices MAC:

image

In other words, there's no OID with a total count of Nodes, or Neighbouring APs.

So I have the following which works for discovery:

    public function discoverWirelessClients()
    {
        $sensors = array();
        $client_list =  snmpwalk_group($this->getDevice(), 'atkkWiAcClientAddress', array(), 'ATKK-WLAN-ACCESS');
        var_dump($client_list);
        $client_count = count($client_list);

        foreach ($client_list as $index => $data) {
            $hex = explode(':', $index);
            $mac = implode(array_map('hexdec', $hex), '.');

            $oid = '.1.3.6.1.4.1.207.8.42.6.2.1.1.1.'; //ATKK-WLAN-ACCESS::atkkWiAcClientAddress
            $sensors[] = new WirelessSensor(
                    'clients',
                    $this->getDeviceId(),
                    $oid . $mac,
                    'AlliedWireless',
                    $mac,
                    'Clients: Total',
                    $client_count
            );
        }
        d_echo($sensors);
        return $sensors;
    }

However, when this is run with the poller, the poller assumes you want to provide it a list of OIDs to poll in an array which will snmpget each OID.

My problem is that I want to provide $oid and have it do an snmpwalk rather than a get. I see the poller runs "private static function fetchSnmpData" in Sensor.php, but I've been stumped for how I really should do this :-/

https://github.com/librenms/librenms/blob/master/doc/Developing/os/Wireless-Sensors.md mentions you can potentially look at custom polling, but I still don't know if that's completely what I want to do here?

Any help is appreciated.

Thanks,

Matt

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jun 10, 2018

You cannot add client count like that in the code currently. Also, you don't want to, the poller will be very slow.

@mattie47

This comment has been minimized.

Copy link
Contributor Author

mattie47 commented Jun 12, 2018

Thanks. I'll give up on any wireless info then, given it seems the way the mibs done is simply poor. They seemed quite different from most other vendors when I tried to compare.

$version = $c;
$data_array = array();
$data_array = snmpwalk_cache_multi_oid($device, '.1.3.6.1.4.1.207', $data_array);

This comment has been minimized.

@murrant

murrant Jun 19, 2018

Member

You fetch this, but don't do anything with it. Can it be removed?

This comment has been minimized.

@mattie47

mattie47 Jun 26, 2018

Author Contributor

Yep removed. As per my original comment, I had it there just so all the snmp data would be added into the PR in hope to get some help on adding some actual wireless stats. But after some effort the mibs suck and it's not worth the time to get the stats into libre.

Use sysDescr to get Hardware, SW version*/
list($a,$b,$c) = explode(' ', $device['sysDescr']);
$hardware = $a;

This comment has been minimized.

@murrant

murrant Jun 19, 2018

Member

You could inline $hardware and $version into the list() here to avoid temporary variables.

This comment has been minimized.

@mattie47

mattie47 Jun 26, 2018

Author Contributor

Nice. Didn't think of that.

@mattie47

This comment has been minimized.

Copy link
Contributor Author

mattie47 commented Jun 26, 2018

@murrant

Travis is failing for a reason I haven't seen before:

#### Unload disco module ports ####
OS allied-wireless: Discovered ports data does not match that found in tests/data/allied-wireless.json
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
             'ifTrunk' => null
-            'ifVrf' => '0'
@@ @@
             'ifTrunk' => null
-            'ifVrf' => '0'
@@ @@
             'ifTrunk' => null
-            'ifVrf' => '0'
@@ @@

Any thoughts?

Thanks,

Matt

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jun 27, 2018

We no longer capture ifVrf for ports. (it is taken care of elsewhere). Remove it from your json files to fix.

@murrant murrant removed the Needs-Info label Jun 27, 2018

@mattie47 mattie47 force-pushed the mattie47:wireless branch from a8d5f2b to 7321c0f Jun 28, 2018

@mattie47

This comment has been minimized.

Copy link
Contributor Author

mattie47 commented Jun 28, 2018

@murrant I reran snmpsim against the device and simplified the test data stored however it's still getting the same error you were yesterday:

.......F
Time: 12.7 minutes, Memory: 64.56MB
There was 1 failure:
1) LibreNMS\Tests\YamlTest::testOSDefinitionSchema
allied-wireless.yaml does not validate. Violations:
[ifname] Integer value found, but a boolean is required

I don't get this issue when I run the same commands as travis locally.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jun 28, 2018

ifname needs to be true or false now, not 1 or 0

@laf laf added this to the 1.41 milestone Jun 29, 2018

@laf laf modified the milestones: 1.41, 1.42 Jun 29, 2018

@@ -0,0 +1,14 @@
os: allied-wireless

This comment has been minimized.

@laf

This comment has been minimized.

@mattie47

mattie47 Jul 1, 2018

Author Contributor

Hmm quite possibly.

I guess I was just trying to start something off that's generic, but perhaps being more specific is the way to go, given the wireless range I summarize below.

To summarize:

  • TQ APs are the "premium" access points which Allied Telesis make.
  • There's now also the MWS range which I think is lower in price/performance (and does have different mibs)
  • There's also the Extricom wireless range (from when AT bought Extricom) which I believe is EOL.
  • Then there's the AW+ Wireless controller with some form of different mibs support, which can manage the TQ APs.

Probably some other range I'm forgetting about....

I'll change it to allied-tq.

@mattie47 mattie47 force-pushed the mattie47:wireless branch from 13515c2 to 219cb0e Jul 1, 2018

@mattie47 mattie47 force-pushed the mattie47:wireless branch from 219cb0e to a162abe Jul 1, 2018

@laf

This comment has been minimized.

Copy link
Member

laf commented Jul 2, 2018

Thanks for that. I've updated the text description in the yaml file to be more descriptive.

@laf

laf approved these changes Jul 2, 2018

Copy link
Member

laf left a comment

LGTM

@laf laf merged commit b8a8909 into librenms:master Jul 2, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@laf

This comment has been minimized.

Copy link
Member

laf commented Jul 2, 2018

Thanks as always @mattie47 :)

@mattie47 mattie47 deleted the mattie47:wireless branch Jul 3, 2018

gs-kamnas pushed a commit to gs-kamnas/librenms that referenced this pull request Aug 1, 2018

Added Allied Telesis Wireless Support (librenms#8692)
* device: Initial support for Allied Telesis Wireless devices

This commit provides basic info for Allied Telesis TQ Access Points

* device: allied wireless initial test data

* device: allied wireless

Fix up mibs.
-- Moved ATKK-WLAN-ACCESS to awplus mibs folder
--- ATKK-WLAN-ACCESS mib relies on AT-SMI-MIB
-- Changed Allied-wireless to look at awplus mibs folder instead of allied
--- As above due to newer AT-SMI-MIB file
- Performed new snmprec with walk on enterprise OID
-- For some reason the enterprise and wireless specific tables are not
   walked by a default snmpwalk.

* add snmpwalk for analysis

* Attempt at wireless polling

* Delete allied-wireless-snmpwalk.txt

* device: fix basic allied wireless

* Update allied-wireless.json

* Simplify Allied Wireless test data

* fix allied-wireless definition

* device: allied-wireless renamed to allied-tq

* Update allied-tq.yaml

@lock lock bot locked as resolved and limited conversation to collaborators Sep 1, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.