netdisco / snmp-info

Other
36 stars 31 forks source link

FWSM Support #29

Closed netdisco-automation closed 7 years ago

netdisco-automation commented 14 years ago

In the 4.x release of the software for FWSMs, Cisco added the ability to grab the ARP table via SNMP. Unfortunately, they did this by only implementing one of the tables that holds this information. I wrote this patch to derive the other tables from that one table and it has been running stable for over 6 months now.

Reported by: bldewolf

Original Ticket: snmp-info/bugs/29

netdisco-automation commented 14 years ago

Add support for FWSM 4.x devices for pulling ARP info

Original comment by: bldewolf

netdisco-automation commented 14 years ago

Hi Brian,

Thanks for the patch. Can I suggest some improvements?

1. Move the physical_at_paddr stuff into Layer3.pm where IP-MIB stuff mostly lives. In fact adding the whole ipNetToPhysicalTable (http://tools.cisco.com/Support/SNMP/do/BrowseOID.do?local=en&translate=Translate&objectInput=1.3.6.1.2.1.4.35.1) couldn't hurt as I'm sure other classes might make use of it.

2. Make a new class Layer3::CiscoFWSM which inherits (ISA) from Layer3::Cisco, then put your three subs there and remove the description() checks.

3. Change Info.pm::device_type() to put the FWSM's into this new class. That way we can easily add or remove devices to that class in one spot w/ one line. This also shield us from causing bugs on existing L3::Cisco devices as you wouldn't be changing behavior for them.

4. Update DeviceMatrix.txt :)

Again thanks for the well documented clean patch. Hopefully when one of the dev's comes out of hibernation this will get in.

-m

Original comment by: maxslug-placeholder

netdisco-automation commented 14 years ago

Original comment by: maxslug-placeholder

netdisco-automation commented 14 years ago

Original comment by: maxslug-placeholder

netdisco-automation commented 14 years ago

Updated patch with requested changes.

Original comment by: bldewolf

netdisco-automation commented 14 years ago

Here's the patch with the changes requested. Some notes: - The new SNMP IP-MIB calls are n2p-* (short for net to physical), as it doesn't make sense to name them new_at_* or some variation (since the original at_* tables would now be old_old_at_paddr). Beyond that, they are a different format. - The at_* calls in the FWSM class all emulate the at_* calls using the n2p_paddr table. My previous patch made tables that weren't really at_* formatted but happened to work. - I wanted to see if I had made changes to DeviceMatrix.txt correctly but I'm not sure how to generate the html form (or is that somewhere else?). - The class was tested live but since I am apparently running 2.00 (I'll fix that soon enough) the changes to Info.pm and Layer3.pm were done by hand. I will test on Monday to make sure there are no errors, but they weren't significant changes (but we all know how that goes...).

Also, as a sidenote, I realized that supporting the Net to Physical tables is a small step towards supporting IPv6. It may be an idea, for the future, to have a set of n2p_* methods that, if the tables aren't supporting directly, generate them from old_at_* and at_* methods and converting applications to only use the n2p tables. This way, the protocol number will always be available, even if it's usually 4...

Original comment by: bldewolf

netdisco-automation commented 14 years ago

Corrected device matrix portion

Original comment by: bldewolf

netdisco-automation commented 14 years ago

Okay, I've tested this patch entirely, and I figured out (with some help from IRC) how to generate the device matrix so that part is correct now. Let me know what you think!

Original comment by: bldewolf

netdisco-automation commented 14 years ago

Original comment by: maxslug-placeholder

netdisco-automation commented 14 years ago

Looks great. I've surreptitiously added you as a developer to SNMP::Info! I hope you're OK with that. If you are, please commit this patch. Please review the Let's use the patch approval process for future commits for the time being.

You may wish to rev the version of your all changed files to 2.02-cvs, copyright looks good, and you should change the ChangeLog (be sure to give yourself credit).

Thanks again Brian.

Original comment by: maxslug-placeholder

netdisco-automation commented 14 years ago

"Please review the " is a typo.

Original comment by: maxslug-placeholder

netdisco-automation commented 14 years ago

Okay, I've committed it. Let me know if I did anything wrong, it's my first time using CVS.

Original comment by: bldewolf

netdisco-automation commented 14 years ago

Original comment by: bldewolf