From: Benoit Plessis <benoit.plessis@powerboutique.com>
Date: Mon, 2 Sep 2019 08:16:40 +0000 (+0200)
Subject: Add support for SNMP Bulk Transfert
X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=a485f5e7cb1854d9096d777fe2ba7ea4e697ce8a;p=collectd.git

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
---

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<Net-SNMP> library default is 1 second.
 The number of times that a query should be retried after the Timeout expires.
 The C<Net-SNMP> library default is 5.
 
+=item B<Bulk_Size> I<Integer>
+
+This number activate and configure snmp bulk transfers. For B<Table> 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
 #   </Host>
 #   <Host "some.ups.mydomain.org">
 #       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;