getBulk appears to have the worng 'next' behaviour - which breaks SNMP Tables

[dirkx]

Tables appear to not be quite working; given a trivial 1 entry EntityMIB table with one entry:

  snmp.addReadOnlyIntegerHandler(oidEntPhysicalIndex, 1);
  snmp.addReadOnlyIntegerHandler(entPhysicalContainedIn,0 /* top level item/container */);
  snmp.addReadOnlyIntegerHandler(oidEntPhysicalClass, 1 /* (other) */);

  snmp.addReadOnlyStaticStringHandler(oidEntPhysicalDesc, "a desc");
  snmp.addReadOnlyStaticStringHandler(oidEntPhysicalName, "a name");

  snprintf(buff, sizeof(buff), "ESP32-%016llx", ESP.getEfuseMac());
  snmp.addReadOnlyStaticStringHandler(oidEntPhysicalSerialNum, std::string(buff));

Will give the right results on an snmpwalk

  $ snmpwalk 10.11.0.181 entPhysicalTable
  ENTITY-MIB::entPhysicalIndex.1 = INTEGER: 1
  ENTITY-MIB::entPhysicalDescr.1 = STRING: a desc
  ENTITY-MIB::entPhysicalContainedIn.1 = INTEGER: 0
  ENTITY-MIB::entPhysicalClass.1 = INTEGER: other(1)
  ENTITY-MIB::entPhysicalName.1 = STRING: a name
  ENTITY-MIB::entPhysicalSerialNum.1 = STRING: ESP32-00004045651b5ae0
  $

But will give an empty/no table on an

  $ snmptable 10.11.0.181 entPhysicalTable
  ENTITY-MIB::entPhysicalTable: No entries
  $

The reason for this appears a wrong GetBulk response:

   00:00:00.000000 IP (tos 0x0, ttl 64, id 21691, offset 0, flags [none], proto UDP (17), length 74, bad cksum 0 (->102e)!)
       10.11.0.240.57576 > 10.11.0.181.161:  { SNMPv2c { GetBulk(31) R=1397163026  N=0 M=10 .1.3.6.1.2.1.47.1.1.1.1.0 } }

    00:00:00.002582 IP (tos 0x0, ttl 255, id 27, offset 0, flags [none], proto UDP (17), length 83)
       10.11.0.181.161 > 10.11.0.240.57576:  { SNMPv2c { GetResponse(37) R=1397163026  .1.3.6.1.2.1.47.1.1.1.1.0=[endOfMibView] } }

i.e. the ENTITY-MIB::entPhysicalEntry.0 does not give the right reply. If we add, as dirty workaround/hack:

   snmp.addReadOnlyIntegerHandler(oidEntPhysicalTable, 0);

to the above code; things do work:

    $ snmptable 10.11.0.181 entPhysicalTable
    SNMP table: ENTITY-MIB::entPhysicalTable

    entPhysicalDescr entPhysicalVendor ...
    a desc           ? .....
    $

With TCPDUMP showing the expected:

    10.11.0.240.60651 > 10.11.0.181.161:  { SNMPv2c { GetBulk(31) R=343496280  N=0 M=10 .1.3.6.1.2.1.47.1.1.1.1.0 } }
        00:00:00.005414 IP (tos 0x0, ttl 255, id 18, offset 0, flags [none], proto UDP (17), length 417)
    10.11.0.181.161 > 10.11.0.240.60651:  { SNMPv2c { GetResponse(367) R=343496280  .1.3.6.1.2.1.47.1.1.1.1.1.1=1 .1.3.6.1.2.1.47.1.1.1.1.2.1="Valve controller" .1.3.6.1.2.1.47.1.1.1.1.4.1=0 .1.3.6.1.2.1.47.1.1.1.1.5.1=1 .1.3.6.1.2.1.47.1.1.1.1.7.1="XXX" .1.3.6.1.2.1.47.1.1.1.1.8.1="XX" .1.3.6.1.2.1.47.1.1.1.1.9.1="XXX" .1.3.6.1.2.1.47.1.1.1.1.10.1="XXX" .1.3.6.1.2.1.47.1.1.1.1.11.1="ESP32-00004045651b5ae0" .1.3.6.1.2.1.47.1.1.1.1.12.1="XXX" } }

However - this breaks various SNMP tools - as now

     % snmpget 10.11.0.181 entPhysicalEntry.0
     ENTITY-MIB::entPhysicalEntry.0 = INTEGER: 0

instead of the expected

     ENTITY-MIB::entPhysicalEntry.0 = No Such Object available on this agent at this OID

i.e. it returns a value (0) - confusing most tools, crashing some or causing an empty table entry. So this work around is not ideal. It seems that the getBulk() handler does not do a 'getNext' enough of a search.

I suspect that ValueCallback::findCallback() is too much subtree focused; and somehow misses, on a getNext / walk=True that, for example, a get next on a nonexistent .1.3.6.1.2.1.1.9.1.0 should return, for example, .1.3.6.1.2.1.1.9.1.2.1 (i.e. the first SNMPv2-MIB::sysORID.1 of the fist SNMPv2-MIB::sysORTable).

[dirkx]

Complete/functional example that will reliably crash most SNMP tooling due to the 'fake' entry that is needed to make getBulk do the right getNext thing.

/* Wire any DS18B20 sensor up to the Sensor SNMP mib. Will generally get auto discovered
 * by most SNMP tools and shown as a temperature v.s. time graph.
 */

#include <WiFi.h>
#include <vector>

#define ONE_WIRE_PIN (5)

#define FAKE_DS

#ifndef FAKE_DS
#include <DS18B20.h>
DS18B20 ds(ONE_WIRE_PIN);
#endif

#include <WiFiUdp.h>
#include <SNMP_Agent.h>
#include <SNMPTrap.h>

// Uncomment and set as needed.
//
// #define WIFI_NETWORK "My Network"
// #define WIFI_PASSWD "Password"

#if !defined(WIFI_NETWORK) || !defined(WIFI_PASSWD)
#error "Set Wifi network name and password !"
#endif

WiFiUDP udp;
SNMPAgent snmp = SNMPAgent("public");

const char* oidSysContact = ".1.3.6.1.2.1.1.4.0";
const char* oidSysName = ".1.3.6.1.2.1.1.5.0";
const char* oidSysLocation = ".1.3.6.1.2.1.1.6.0";

#define ENTITY_SENSOR_TABLE ".1.3.6.1.2.1.99.1.1.1"
#define ENTITY_SENSOR_TYPE           ENTITY_SENSOR_TABLE ".1.%d" //  This object identifies the type of data units associated with the sensor value.
#define ENTITY_SENSOR_SCALE          ENTITY_SENSOR_TABLE ".2.%d"  // This object identifies the (power of 10) scaling factor associated with the sensor value.
#define ENTITY_SENSOR_VALUE          ENTITY_SENSOR_TABLE ".4.%d"  // Actual value
#define ENTITY_SENSOR_UNITS_DISPLAY  ENTITY_SENSOR_TABLE ".6.%d"  // Unit - e.g. Celcius in this case.

int *cachedValuesInMillis;

void setup() {
  Serial.begin(115200);
  Serial.println("\n\n\n" __FILE__ " " __DATE__ " " __TIME__);
  WiFi.begin(WIFI_NETWORK, WIFI_PASSWD);
  while (WiFi.status() != WL_CONNECTED) delay(50);

  snmp.setUDP(&udp);
  snmp.begin();

  // add some standard entries in system.
  snmp.addReadOnlyStaticStringHandler(oidSysContact, "your local admin");
  snmp.addReadOnlyStaticStringHandler(oidSysName, "this ESP32");
  snmp.addReadOnlyStaticStringHandler(oidSysLocation, "On an ESP32 near you");

  Serial.print("snmpwalk -v2c -c public ");
  Serial.print(WiFi.localIP());
  Serial.println(" system");

  // Add a (temperature) sensor table; one table entry for each sensor.
  //
#ifdef FAKE_DS
  size_t n = 5;
#else
  size_t n = ds.getNumberOfDevices();
#endif
  cachedValuesInMillis = (int *)malloc( n * sizeof(int));

  for (int i = 1; i <= n; i++) {
    cachedValuesInMillis[ i - 1 ] = 0;
    char oid[128];
    snprintf(oid, sizeof(oid), ENTITY_SENSOR_TYPE, i);
    snmp.addReadOnlyIntegerHandler(oid, 8 /* temperature */);
    snprintf(oid, sizeof(oid), ENTITY_SENSOR_SCALE, i);
    snmp.addReadOnlyIntegerHandler(oid, 8 /* milli */); // x 0.001; to allow for deci-degrees
    snprintf(oid, sizeof(oid), ENTITY_SENSOR_VALUE, i);
    snmp.addIntegerHandler(oid, &(cachedValuesInMillis[i - 1]));
    snprintf(oid, sizeof(oid), ENTITY_SENSOR_UNITS_DISPLAY, i);
    snmp.addReadOnlyStaticStringHandler(oid, "C");
  };

  // fool walk mechanish - not overly clear why we need this & crashes librenms.
  // or a bug in SNMP where it does not respont right to getbulk on this endpoint.
  //
  // The issue is that a get GetBulk(31) for 1.3.6.1.2.1.99.1.1.1.0
  // gets a endOfMibView reply if this is not in place.
  //
  // https://github.com/0neblock/Arduino_SNMP/issues/53
  //
  snmp.addReadOnlyIntegerHandler(ENTITY_SENSOR_TABLE ".0", 0);
  snmp.sortHandlers();

  Serial.print("snmpwalk -v2c -c public ");
  Serial.print(WiFi.localIP());
  Serial.println(" .1.3.6.1.2.1.99");

  Serial.print("snmptable -m +ENTITY-SENSOR-MIB  -v2c -c public ");
  Serial.print(WiFi.localIP());
  Serial.println(" entPhySensorTable");
}

void loop() {
  snmp.loop();

  // update the temperature cache every 5 seconds.
  static unsigned long lst = 0;
  if (millis() - lst > 5000) {

#ifdef FAKE_DS
    for (int i = 0; i< 5; i++)
      cachedValuesInMillis[i] = 2000 + i * 50;
#else
    ds.startConversion();
    for (int i = 0; ds.selectNext(); i++)
      cachedValuesInMillis[i] = 0.5 + ds.getTempC(false) * 1000;
#endif

    lst = millis();
  };
};