From: SourceForge.net <no...@so...> - 2009-03-31 13:49:43
|
Patches item #2103492, was opened at 2008-09-10 12:15 Message generated for change (Comment added) made by jsafranek You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=312694&aid=2103492&group_id=12694 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: narendra k (narendra_k_dell) Assigned to: Jan Safranek (jsafranek) Summary: Implementation of etherStatsJabbers defined in RMON-MIB Initial Comment: Hello List, Attached is the patch that implements "etherStatsJabbers" object defined by the RMON-MIB.txt. Mib2c has been used to generate the framework. The patch has been taken against net-snmp-5.4.1.2. Oses tested - Linux - Intel Processor based RHEL-5.2 32 bit and 64 bit RHEL-4.7 64 bit NIC Cards tested- Intel Intel Corporation 82541GI Gigabit Ethernet Controller. Broadcom Broadcom BCM 5708 Gigabit Ethernet Controller Signed-off-by:- nar...@de... Signed-off-by:- San...@de... Signed-off-by:- shy...@de... ---------------------------------------------------------------------- >Comment By: Jan Safranek (jsafranek) Date: 2009-03-31 15:49 Message: After consultation with other guys, we think the patch could be applied, if you fix the logging. Just go though all LOG_ERROR and make sure, they are not reported repeatedly. Of course, if malloc() fails, it's good idea to report it as error, but ioctl failures are likely to be reported always when someone calls it - like the ones I wrote in previous comment (Fedora 9, x86_64). ---------------------------------------------------------------------- Comment By: Jan Safranek (jsafranek) Date: 2009-03-31 15:29 Message: ad 3) Excess syslog messages other than the ones mentioned in my previous post dated 2008-11-20 have been cleaned up: My syslog is still full of: error getting the statistics for interface |vnet6| etherStatsTable data, operation might not be supported access:etherStatsTable,interface_ioctl_etherstats_get, no stats availablei for interface |vnet7| Again, please do not use LOG_ERR fo logging of repeated events. Anyway, the logging can be fixed quickly. What troubles me the most is that from the whole RMON-MIB you implement only one OID. That can be hardly called implementation of the RFC. The rest should be implemented too! I understand you need just this one value - you can create e.g. simple perl module for that, you don't need 220kB code with empty stubs. ---------------------------------------------------------------------- Comment By: narendra k (narendra_k_dell) Date: 2009-01-13 08:52 Message: Hello, I am attaching a revised patch addressing all the issues mentioned in the review comments from the post dated 2008-10-22. They are - 1. The patch contains #defines so that it compiles on solaris also. 2. The linux specific code has been moved to "data_access" directory. 3. Excess syslog messages other than the ones mentioned in my previous post dated 2008-11-20 have been cleaned up. Also I have made these changes to "[ 2053273 ] Etherlike-Mib Module implementation patch". It would be great if you can take a look at this report also. File Added: net-snmp-5.4.1.2-rmon-mib-revised_3.patch ---------------------------------------------------------------------- Comment By: narendra k (narendra_k_dell) Date: 2008-11-20 17:34 Message: I am attaching a revised patch containing some syslog cleanup messages. I have removed the log lines access:etherStatsTable:ioctl, _etherStats_ioctl_get error on inerface 'lo' access:etherStatsTable,interface_ioctl_etherstats_get, ETHTOOL_GETDRVINFO failed on interface |lo| and retained the line error getting the statistics for interface |lo| etherStatsTable data, operation might not be supported coming from the topmost function call "interface_ioctl_etherstats_get" to communicate that the operation on devices like lo and sit0 might not work. I suppose this would consideraly reduce the syslog messages. I have also retained the message "internal:etherStatsTable:_mfd_etherStatsTable_get_column, column 12 (etherStatsJabbers) doesn't exist for the interface with index 2 " to indicate that the variable in question might not be supported by the corresponding driver. File Added: net-snmp-5.4.1.2-rmon-mib-revised_1.patch ---------------------------------------------------------------------- Comment By: narendra k (narendra_k_dell) Date: 2008-10-26 16:41 Message: Hello, 1) > it's linux only. Net-SNMP works also on other platforms and your patch > won't compile e.g. on Solaris. Please add some #ifdefs around your code. > The usual method it to split platform independent (most of the generated > code) to a subdirectory and have platform dependent part in a separate > directory, usually named 'data_access'. Look at ip-mib for examples. > Of course, nobody wants you to implement the MIB on all platforms, just > make sure it won't be difficult for the ones who would do it. I am working on this. 2) > if I am looking right, from the whole RMON-MIB you decided to implement > only one column in one table (etherStatsJabbers in etherStatsTable). It's > too less to consider it as implementation of the MIB. That's correct. I have implemented only etherStatsJabbers object from etherStatsTable as that was important for the project i am working on. I suppose by moving the platform dependent part to data_access directory, it would not be difficult to extend the implementation to include other objects in future. > There is also example implementation of the MIB in agent/mibgroup/Rmon > which could be reused, but I don't in what state is it and if it has some > use for you. I would explore it. 3) > even as it is, your patch does not work on my machine (Fedora 9, kernel > 2.6.26.5-45.fc9.x86_64) > - some interfaces (e.g. 'lo') does not support ETHTOOL_GDRVINFO, syslog > gets full of "ETHTOOL_GETDRVINFO failed on interface |lo|" There is no driver support for ETHTOOL_GDRVINFO operation for interfaces like "lo" and "sit0". The above messages are indicating that. I would take care of this while I clean up the snmp_log messages. > - I haven't found any interface which supports ETHTOOL_GSTATS (I haven't > tested much though), the etherStatsTable is empty and I get lot of "error > getting the statistics for interface |eth0|" messages in the syslog (my NIC > has e1000e driver) The etherStatsJabbers statistics is provided by only the broadcom drivers and intel drivers don't provide this data. So that is the reson for not seeing any output when the object is queried. I have indicated this by messages like "internal:etherStatsTable:_mfd_etherStatsTable_get_column, column 12 (etherStatsJabbers) doesn't exist for the interface with index 2". I have also used #defines in the ioctl_imp_common.h such as +/* for maintainability */ + +#define BROADCOM_RECEIVE_JABBERS "rx_jabbers" + +#define ETHERSTATSJABBERS(x) strstr(x, BROADCOM_RECEIVE_JABBERS) to indicate this and it can be extended to support any new card in the future which might support this variable. > -> please don't use snmp_log(LOG_ERR, ...) for repeatedly shown messages, > DEBUGMSGTL((...)) is enough I will clean up these snmp_log messages. > -> with what HW and kernel is it supposed to work? Only the with the > cards mentioned above? It is supposed to work with other broadcom cards also even though the above mentioned cards were used for testing. ---------------------------------------------------------------------- Comment By: Jan Safranek (jsafranek) Date: 2008-10-22 14:24 Message: There are few problems with the patch 1) it's linux only. Net-SNMP works also on other platforms and your patch won't compile e.g. on Solaris. Please add some #ifdefs around your code. The usual method it to split platform independent (most of the generated code) to a subdirectory and have platform dependent part in a separate directory, usually named 'data_access'. Look at ip-mib for examples. Of course, nobody wants you to implement the MIB on all platforms, just make sure it won't be difficult for the ones who would do it. 2) if I am looking right, from the whole RMON-MIB you decided to implement only one column in one table (etherStatsJabbers in etherStatsTable). It's too less to consider it as implementation of the MIB. There is also example implementation of the MIB in agent/mibgroup/Rmon which could be reused, but I don't in what state is it and if it has some use for you. 3) even as it is, your patch does not work on my machine (Fedora 9, kernel 2.6.26.5-45.fc9.x86_64) - some interfaces (e.g. 'lo') does not support ETHTOOL_GDRVINFO, syslog gets full of "ETHTOOL_GETDRVINFO failed on interface |lo|" - I haven't found any interface which supports ETHTOOL_GSTATS (I haven't tested much though), the etherStatsTable is empty and I get lot of "error getting the statistics for interface |eth0|" messages in the syslog (my NIC has e1000e driver) -> please don't use snmp_log(LOG_ERR, ...) for repeatedly shown messages, DEBUGMSGTL((...)) is enough -> with what HW and kernel is it supposed to work? Only the with the cards mentioned above? ---------------------------------------------------------------------- Comment By: narendra k (narendra_k_dell) Date: 2008-09-17 13:33 Message: I am resubmitting the patch which addresses the compilation warnings that were reported. File Added: net-snmp-5.4.1.2-rmon-mib-revised.patch ---------------------------------------------------------------------- Comment By: narendra k (narendra_k_dell) Date: 2008-09-12 22:31 Message: I have received reports that the patch is producing compilation related warnings. I am looking into it and resubmit the patch fixing them. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=312694&aid=2103492&group_id=12694 |