From a485f5e7cb1854d9096d777fe2ba7ea4e697ce8a Mon Sep 17 00:00:00 2001 From: Benoit Plessis Date: Mon, 2 Sep 2019 10:16:40 +0200 Subject: [PATCH] Add support for SNMP Bulk Transfert ChangeLog: snmp plugin: support for bulk transfert has been added. As per issue #2495 we are experiencing so long collect time for switch with high number of port so i took a try at that. This is the first attempt, it's working but i may not have seen every failure opportunities, i tried to be the less disruptive as possible. This mostly modify the csnmp_read_table to switch over to 'GETBULK' PDUs if conditions are ok, and handle the extra return values with the same loop. Comments are welcome --- src/collectd-snmp.pod | 7 +++++++ src/collectd.conf.in | 1 + src/snmp.c | 39 ++++++++++++++++++++++++++++++++------- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/collectd-snmp.pod b/src/collectd-snmp.pod index 493f5ec2..efeab8e3 100644 --- a/src/collectd-snmp.pod +++ b/src/collectd-snmp.pod @@ -373,6 +373,13 @@ How long to wait for a response. The C library default is 1 second. The number of times that a query should be retried after the Timeout expires. The C library default is 5. +=item B I + +This number activate and configure snmp bulk transfers. For B data it allow +the host to send multiple values per query. Only available for SNMP v2 and greater, this +usually allow for more efficient querying and faster data transfert. +The default is 0/disabled. + =back =head1 SEE ALSO diff --git a/src/collectd.conf.in b/src/collectd.conf.in index f09f373d..180903ba 100644 --- a/src/collectd.conf.in +++ b/src/collectd.conf.in @@ -1402,6 +1402,7 @@ # Version 2 # Community "another_string" # Collect "std_traffic" "hr_users" +# Bulk_Size 100 # # # Address "192.168.0.3" diff --git a/src/snmp.c b/src/snmp.c index aeb04fdd..322e0a3e 100644 --- a/src/snmp.c +++ b/src/snmp.c @@ -99,6 +99,8 @@ struct host_definition_s { c_complain_t complaint; data_definition_t **data_list; int data_list_len; + + int bulk_size; }; typedef struct host_definition_s host_definition_t; @@ -762,6 +764,7 @@ static int csnmp_config_add_host(oconfig_item_t *ci) { /* These mean that we have not set a timeout or retry value */ hd->timeout = 0; hd->retries = -1; + hd->build_size = 0; for (int i = 0; i < ci->children_num; i++) { oconfig_item_t *option = ci->children + i; @@ -794,6 +797,8 @@ static int csnmp_config_add_host(oconfig_item_t *ci) { status = csnmp_config_add_host_security_level(hd, option); else if (strcasecmp("Context", option->key) == 0) status = cf_util_get_string(option, &hd->context); + else if (strcasecmp("BulkSize", option->key) == 0) + status = cf_util_get_int(option, &hd->bulk_size); else { WARNING( "snmp plugin: csnmp_config_add_host: Option `%s' not allowed here.", @@ -1572,7 +1577,7 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) { csnmp_oid_type_t oid_list_todo[oid_list_len]; int status; - size_t i; + size_t i,j; /* `value_list_head' and `value_cells_tail' implement a linked list for each * value. `instance_cells_head' and `instance_cells_tail' implement a linked @@ -1588,8 +1593,8 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) { csnmp_cell_value_t **value_cells_head; csnmp_cell_value_t **value_cells_tail; - DEBUG("snmp plugin: csnmp_read_table (host = %s, data = %s)", host->name, - data->name); + DEBUG("snmp plugin: csnmp_read_table (host = %s, data = %s (%ld))", host->name, + data->name, data->values_len); if (host->sess_handle == NULL) { DEBUG("snmp plugin: csnmp_read_table: host->sess_handle == NULL"); @@ -1655,7 +1660,14 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) { status = 0; while (status == 0) { - req = snmp_pdu_create(SNMP_MSG_GETNEXT); + /* If SNMP v2 and later and bulk transfert enabled, use GETBULK PDU */ + if (host->version > 1 && host->bulk_size > 0) { + req = snmp_pdu_create(SNMP_MSG_GETBULK); + req->non_repeaters = 0; + req->max_repetitions = host->bulk_size; + } else { + req = snmp_pdu_create(SNMP_MSG_GETNEXT); + } if (req == NULL) { ERROR("snmp plugin: snmp_pdu_create failed."); status = -1; @@ -1683,6 +1695,13 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) { break; } + if (req->command == SNMP_MSG_GETBULK) { + /* In bulk mode the host will send 'max_repetitions' values per + requested variable, so we need to split it per number of variable + to stay 'in budget' */ + req->max_repetitions = floor(host->bulk_size/oid_list_todo_num); + } + res = NULL; status = snmp_sess_synch_response(host->sess_handle, req, &res); @@ -1754,11 +1773,17 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) { continue; } - for (vb = res->variables, i = 0; (vb != NULL); - vb = vb->next_variable, i++) { + for (vb = res->variables, j = 0; (vb != NULL); + vb = vb->next_variable, j++) { + /* If bulk request is active convert value index of the extra value */ + if (host->version > 1 && host->bulk_size > 0) { + i = j % oid_list_todo_num; + } else { + i = j; + } /* Calculate value index from todo list */ while ((i < oid_list_len) && !oid_list_todo[i]) { - i++; + i++; j++; } if (i >= oid_list_len) { break; -- 2.11.0