Print this page
NEX-18695 libfmd_snmp is unable to handle multiple OIDs in GET request
Reviewed by: Joyce McIntosh <joyce.mcintosh@nexenta.com>
Reviewed by: Cynthia Eastham <cynthia.eastham@nexenta.com>
Reviewed by: Rick McNeal <rick.mcneal@nexenta.com>
NEX-17777 snmptable produces bogus output for FM tables
Reviewed by: Cynthia Eastham <cynthia.eastham@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
NEX-3125 libfmd_snmp should compile with newer net-snmp
Reviewed by: Gordon Ross <gwr@nexenta.com>
Reviewed by: Jean McCormack <jean.mccormack@nexenta.com>
Reviewed by: Josef 'Jeff' Sipek <josef.sipek@nexenta.com>
@@ -22,19 +22,26 @@
/*
* Copyright 2009 Sun Microsystems, Inc. All rights reserved.
* Use is subject to license terms.
*/
+/*
+ * Copyright 2018 Nexenta Systems, Inc.
+ */
+
#include <fm/fmd_adm.h>
#include <fm/fmd_snmp.h>
+
#include <net-snmp/net-snmp-config.h>
#include <net-snmp/net-snmp-includes.h>
#include <net-snmp/agent/net-snmp-agent-includes.h>
-#include <pthread.h>
-#include <stddef.h>
+
#include <errno.h>
#include <libuutil.h>
+#include <pthread.h>
+#include <stddef.h>
+
#include "sunFM_impl.h"
#include "module.h"
static uu_avl_pool_t *mod_name_avl_pool;
static uu_avl_pool_t *mod_index_avl_pool;
@@ -43,49 +50,22 @@
#define VALID_AVL_STATE (mod_name_avl_pool != NULL && \
mod_index_avl_pool != NULL && mod_name_avl != NULL && \
mod_index_avl != NULL)
-#define UPDATE_WAIT_MILLIS 10 /* poll interval in milliseconds */
-
/*
* Update types. Single-index and all are mutually exclusive.
*/
#define UCT_INDEX 0x1
#define UCT_ALL 0x2
#define UCT_FLAGS 0x3
#define MODULE_DATA_VALID(d) ((d)->d_valid == valid_stamp)
-/*
- * Locking rules are straightforward. There is only one updater thread
- * for each table, and requests for update that are received while
- * another update is in progress are ignored. The single per-table lock
- * protects all the data for the table, the valid_stamp and max_index
- * tags for new data, and - importantly - all the hidden static data
- * used by the Net-SNMP library. The result return callbacks are always
- * called in the master agent thread; holding the table lock is
- * therefore sufficient since only one table's callback can be run at
- * any one time. Finer-grained locking is possible here but
- * substantially more difficult because nearly all Net-SNMP functions
- * are unsafe.
- *
- * In practice this is more than adequate, since the purpose of
- * threading out the update is to prevent excessively time-consuming
- * data collection from bottlenecking the entire agent, not to improve
- * result throughput (SNMP is not intended to be used for applications
- * requiring high throughput anyway). If the agent itself ever becomes
- * multithreaded, locking requirements become limited to our local
- * per-table data (the tree, max_index, and valid_stamp), and the
- * implementation could be revisited for better performance.
- */
-
static ulong_t max_index;
static int valid_stamp;
static pthread_mutex_t update_lock;
-static pthread_cond_t update_cv;
-static volatile enum { US_QUIET, US_NEEDED, US_INPROGRESS } update_status;
static Netsnmp_Node_Handler sunFmModuleTable_handler;
static sunFmModule_data_t *
key_build(const char *name, const ulong_t index)
@@ -268,53 +248,30 @@
fmd_adm_close(adm);
return (SNMP_ERR_NOERROR);
}
-/*ARGSUSED*/
static void
-update_thread(void *arg)
+request_update(void)
{
+ sunFmModule_update_ctx_t uc;
+
/*
* The current modinfo_update implementation offers minimal savings
* for the use of index-only updates; therefore we always do a full
* update. If it becomes advantageous to limit updates to a single
* index, the contexts can be queued by the handler instead.
*/
- sunFmModule_update_ctx_t uc;
-
uc.uc_host = NULL;
uc.uc_prog = FMD_ADM_PROGRAM;
uc.uc_version = FMD_ADM_VERSION;
-
uc.uc_index = 0;
uc.uc_type = UCT_ALL;
- for (;;) {
- (void) pthread_mutex_lock(&update_lock);
- update_status = US_QUIET;
- while (update_status == US_QUIET)
- (void) pthread_cond_wait(&update_cv, &update_lock);
- update_status = US_INPROGRESS;
- (void) pthread_mutex_unlock(&update_lock);
(void) modinfo_update(&uc);
- }
}
-static void
-request_update(void)
-{
- (void) pthread_mutex_lock(&update_lock);
- if (update_status != US_QUIET) {
- (void) pthread_mutex_unlock(&update_lock);
- return;
- }
- update_status = US_NEEDED;
- (void) pthread_cond_signal(&update_cv);
- (void) pthread_mutex_unlock(&update_lock);
-}
-
/*ARGSUSED*/
static int
module_compare_name(const void *l, const void *r, void *private)
{
sunFmModule_data_t *l_data = (sunFmModule_data_t *)l;
@@ -349,23 +306,11 @@
if ((err = pthread_mutex_init(&update_lock, NULL)) != 0) {
(void) snmp_log(LOG_ERR, MODNAME_STR ": mutex_init failure: "
"%s\n", strerror(err));
return (MIB_REGISTRATION_FAILED);
}
- if ((err = pthread_cond_init(&update_cv, NULL)) != 0) {
- (void) snmp_log(LOG_ERR, MODNAME_STR ": cond_init failure: "
- "%s\n", strerror(err));
- return (MIB_REGISTRATION_FAILED);
- }
- if ((err = pthread_create(NULL, NULL, (void *(*)(void *))update_thread,
- NULL)) != 0) {
- (void) snmp_log(LOG_ERR, MODNAME_STR ": error creating update "
- "thread: %s\n", strerror(err));
- return (MIB_REGISTRATION_FAILED);
- }
-
if ((table_info =
SNMP_MALLOC_TYPEDEF(netsnmp_table_registration_info)) == NULL)
return (MIB_REGISTRATION_FAILED);
if ((handler = netsnmp_create_handler_registration("sunFmModuleTable",
@@ -446,16 +391,16 @@
return (MIB_REGISTERED_OK);
}
/*
- * These two functions form the core of GET/GETNEXT/GETBULK handling (the
+ * These two functions form the core of GET/GETNEXT handling (the
* only kind we do). They perform two functions:
*
* - First, frob the request to set all the index variables to correspond
* to the value that's going to be returned. For GET, this is a nop;
- * for GETNEXT/GETBULK it always requires some work.
+ * for GETNEXT it always requires some work.
* - Second, find and return the fmd module information corresponding to
* the (possibly updated) indices.
*
* These should be as fast as possible; they run in the agent thread.
*/
@@ -485,17 +430,17 @@
if (build_oid(&var->name, &var->name_length, tmpoid,
reginfo->rootoid_len + 2, var) != SNMPERR_SUCCESS) {
snmp_free_varbind(var);
return (NULL);
}
- DEBUGMSGTL((MODNAME_STR, "nextmod: built fake index:\n"));
+ DEBUGMSGTL((MODNAME_STR, "nextmod: built fake index: "));
DEBUGMSGVAR((MODNAME_STR, var));
DEBUGMSG((MODNAME_STR, "\n"));
} else {
var = snmp_clone_varbind(table_info->indexes);
index = *var->val.integer;
- DEBUGMSGTL((MODNAME_STR, "nextmod: received index:\n"));
+ DEBUGMSGTL((MODNAME_STR, "nextmod: received index: "));
DEBUGMSGVAR((MODNAME_STR, var));
DEBUGMSG((MODNAME_STR, "\n"));
index++;
}
@@ -544,42 +489,33 @@
return (module_lookup_index_exact(table_info->index_oid[0]));
}
/*ARGSUSED*/
-static void
-sunFmModuleTable_return(unsigned int reg, void *arg)
+static int
+sunFmModuleTable_handler(netsnmp_mib_handler *handler,
+ netsnmp_handler_registration *reginfo, netsnmp_agent_request_info *reqinfo,
+ netsnmp_request_info *request)
{
- netsnmp_delegated_cache *cache = (netsnmp_delegated_cache *)arg;
- netsnmp_request_info *request;
- netsnmp_agent_request_info *reqinfo;
- netsnmp_handler_registration *reginfo;
netsnmp_table_request_info *table_info;
sunFmModule_data_t *data;
ulong_t modstate;
+ int ret = SNMP_ERR_NOERROR;
- ASSERT(netsnmp_handler_check_cache(cache) != NULL);
+ /*
+ * We don't support MODE_GETBULK directly, so all bulk requests should
+ * come through bulk_to_next helper. Make sure it stays that way.
+ */
+ ASSERT(reqinfo->mode == MODE_GET || reqinfo->mode == MODE_GETNEXT);
(void) pthread_mutex_lock(&update_lock);
- if (update_status != US_QUIET) {
- struct timeval tv;
+ request_update();
- tv.tv_sec = UPDATE_WAIT_MILLIS / 1000;
- tv.tv_usec = (UPDATE_WAIT_MILLIS % 1000) * 1000;
-
- (void) snmp_alarm_register_hr(tv, 0, sunFmModuleTable_return,
- cache);
- (void) pthread_mutex_unlock(&update_lock);
- return;
- }
-
- request = cache->requests;
- reqinfo = cache->reqinfo;
- reginfo = cache->reginfo;
-
+ for (; request != NULL; request = request->next) {
table_info = netsnmp_extract_table_info(request);
- request->delegated = 0;
+ if (table_info == NULL)
+ continue;
ASSERT(table_info->colnum >= SUNFMMODULE_COLMIN);
ASSERT(table_info->colnum <= SUNFMMODULE_COLMAX);
/*
@@ -594,90 +530,58 @@
* - We will never receive requests outside our table nor
* those with the first subid anything other than 1 (Entry)
* nor those without a column number. This is true even
* for GETNEXT requests.
*/
-
switch (reqinfo->mode) {
case MODE_GET:
- if ((data = sunFmModuleTable_mod(reginfo, table_info)) ==
- NULL) {
- netsnmp_free_delegated_cache(cache);
- (void) pthread_mutex_unlock(&update_lock);
- return;
- }
+ data = sunFmModuleTable_mod(reginfo, table_info);
+ if (data == NULL)
+ goto out;
break;
case MODE_GETNEXT:
- case MODE_GETBULK:
- if ((data = sunFmModuleTable_nextmod(reginfo, table_info)) ==
- NULL) {
- netsnmp_free_delegated_cache(cache);
- (void) pthread_mutex_unlock(&update_lock);
- return;
- }
+ data = sunFmModuleTable_nextmod(reginfo, table_info);
+ if (data == NULL)
+ goto out;
break;
default:
- (void) snmp_log(LOG_ERR, MODNAME_STR ": Unsupported request "
- "mode %d\n", reqinfo->mode);
- netsnmp_free_delegated_cache(cache);
- (void) pthread_mutex_unlock(&update_lock);
- return;
+ (void) snmp_log(LOG_ERR, MODNAME_STR
+ ": unsupported request mode: %d\n", reqinfo->mode);
+ ret = SNMP_ERR_GENERR;
+ goto out;
}
switch (table_info->colnum) {
case SUNFMMODULE_COL_NAME:
- (void) netsnmp_table_build_result(reginfo, request, table_info,
- ASN_OCTET_STR, (uchar_t *)data->d_ami_name,
+ (void) netsnmp_table_build_result(reginfo, request,
+ table_info, ASN_OCTET_STR,
+ (uchar_t *)data->d_ami_name,
strlen(data->d_ami_name));
break;
case SUNFMMODULE_COL_VERSION:
- (void) netsnmp_table_build_result(reginfo, request, table_info,
- ASN_OCTET_STR, (uchar_t *)data->d_ami_vers,
+ (void) netsnmp_table_build_result(reginfo, request,
+ table_info, ASN_OCTET_STR,
+ (uchar_t *)data->d_ami_vers,
strlen(data->d_ami_vers));
break;
case SUNFMMODULE_COL_STATUS:
modstate = (data->d_ami_flags & FMD_ADM_MOD_FAILED) ?
SUNFMMODULE_STATE_FAILED : SUNFMMODULE_STATE_ACTIVE;
- (void) netsnmp_table_build_result(reginfo, request, table_info,
- ASN_INTEGER, (uchar_t *)&modstate,
+ (void) netsnmp_table_build_result(reginfo, request,
+ table_info, ASN_INTEGER, (uchar_t *)&modstate,
sizeof (modstate));
break;
case SUNFMMODULE_COL_DESCRIPTION:
- (void) netsnmp_table_build_result(reginfo, request, table_info,
- ASN_OCTET_STR, (uchar_t *)data->d_ami_desc,
+ (void) netsnmp_table_build_result(reginfo, request,
+ table_info, ASN_OCTET_STR,
+ (uchar_t *)data->d_ami_desc,
strlen(data->d_ami_desc));
break;
default:
break;
}
- netsnmp_free_delegated_cache(cache);
- (void) pthread_mutex_unlock(&update_lock);
-}
-
-static int
-sunFmModuleTable_handler(netsnmp_mib_handler *handler,
- netsnmp_handler_registration *reginfo, netsnmp_agent_request_info *reqinfo,
- netsnmp_request_info *requests)
-{
- netsnmp_request_info *request;
- struct timeval tv;
-
- tv.tv_sec = UPDATE_WAIT_MILLIS / 1000;
- tv.tv_usec = (UPDATE_WAIT_MILLIS % 1000) * 1000;
-
- request_update();
-
- for (request = requests; request; request = request->next) {
- if (request->processed != 0)
- continue;
-
- if (netsnmp_extract_table_info(request) == NULL)
- continue;
-
- request->delegated = 1;
- (void) snmp_alarm_register_hr(tv, 0, sunFmModuleTable_return,
- (void *) netsnmp_create_delegated_cache(handler, reginfo,
- reqinfo, request, NULL));
}
- return (SNMP_ERR_NOERROR);
+out:
+ (void) pthread_mutex_unlock(&update_lock);
+ return (ret);
}