Switch to single session net-snmp API to ensure thread safety in the SNMP plugin

dcdbpusher/sensors/snmp/SNMPConnection.cpp

@@ -27,6 +27,7 @@ SNMPConnection::SNMPConnection() {
 	_snmpSession.peername = NULL;
 	_snmpSession.community_len = 0;
 	_snmpSession.securityNameLen = 0;
+	_snmpSessp = NULL;
 	_strand = NULL;
 }
 
@@ -60,6 +61,13 @@ void SNMPConnection::init() {
 	}
 
 	snmp_sess_init(&_snmpSession);
+	int liberr, syserr;
+	char *errstr;
+	snmp_error(&_snmpSession, &liberr, &syserr, &errstr);
+	if (liberr || syserr) {
+		LOG(error) << "SNMP: Error initializing session: " + std::string(errstr);
+		free(errstr);
+	}
 	_snmpSession.version = _version;
 	_snmpSession.peername = strdup(_host.c_str());
 
@@ -122,24 +130,43 @@ void SNMPConnection::init() {
 	_isInitialized = true;
 }
 
-int64_t SNMPConnection::issueGet(const oid* const OID, size_t OIDLen) {
+bool SNMPConnection::open() {
+	_snmpSessp = snmp_sess_open(&_snmpSession);
+	int liberr, syserr;
+	char *errstr;
+	snmp_error(&_snmpSession, &liberr, &syserr, &errstr);
+	if (liberr || syserr) {
+		LOG(error) << "SNMP-open: " << errstr;
+		free(errstr);
+		return false;
+	}
+
+	return true;
+}
+
+void SNMPConnection::close() {
+	if (_snmpSessp) {
+		snmp_sess_close(_snmpSessp);
+		_snmpSessp = NULL;
+	}
+}
+
+int64_t SNMPConnection::get(const oid* const OID, size_t OIDLen) {
 	struct snmp_session* ss;
 	struct snmp_pdu *pdu, *response;
 	struct variable_list *vp;
 	int status;
 	int64_t ret = 0;
 
-	ss = snmp_open(&_snmpSession);
-
-	if (!ss) {
-		throw std::runtime_error("Could not open SNMP Session!");
+	if (!_snmpSessp) {
+		open();
 	}
 
 	pdu = snmp_pdu_create(SNMP_MSG_GET);
 
 	snmp_add_null_var(pdu, OID, OIDLen);
 
-	status = snmp_synch_response(ss, pdu, &response);
+	status = snmp_sess_synch_response(_snmpSessp, pdu, &response);
 
 	if (status == STAT_SUCCESS && response->errstat == SNMP_ERR_NOERROR) {
 		vp = response->variables;
@@ -167,6 +194,7 @@ int64_t SNMPConnection::issueGet(const oid* const OID, size_t OIDLen) {
 			char*	err;
 			snmp_error(ss, NULL, NULL, &err);
 			LOG(error) << "SNMP-get: " << err;
+			free(err);
 		}
 
 		throw std::runtime_error("Request failed!");
@@ -175,7 +203,6 @@ int64_t SNMPConnection::issueGet(const oid* const OID, size_t OIDLen) {
 	if (response) {
 		snmp_free_pdu(response);
 	}
-	snmp_close(ss);
 	return ret;
 }
 

dcdbpusher/sensors/snmp/SNMPConnection.h

@@ -147,6 +147,16 @@ public:
 	 */
 	void init();
 
+	/**
+	 *  Open SNMP Session. Must be called once before issuing a get() call.
+	 */
+	bool open();
+	
+	/**
+	 *  Close SNMP Session.
+	 */
+	void close();
+
 	/**
 	 * Issues a get request to _host for the value specified by the OID.
 	 *
@@ -155,7 +165,7 @@ public:
 	 *
 	 * @return		The requested value
 	 */
-	int64_t issueGet(const oid* const OID, size_t OIDLen);
+	int64_t get(const oid* const OID, size_t OIDLen);
 
 private:
 	bool		_isInitialized;
@@ -175,8 +185,11 @@ private:
 	int			_securityLevel;
 	long int	_version;
 	struct snmp_session	_snmpSession;
+	void*                   _snmpSessp;
 
 	boost::asio::io_service::strand*	_strand;
+	
+	
 	LOGGER lg;
 };
 

dcdbpusher/sensors/snmp/SNMPSensorGroup.cpp

@@ -61,18 +61,21 @@ void SNMPSensorGroup::stop() {
 void SNMPSensorGroup::read() {
 	reading_t reading;
 	reading.timestamp = getTimestamp();
-
-	for(const auto& s : _sensors) {
-		try {
-			reading.value = _connection->issueGet(s->getOID(), s->getOIDLen());
+	
+	if (_connection->open()) {
+		for(const auto& s : _sensors) {
+			try {
+				reading.value = _connection->get(s->getOID(), s->getOIDLen());
 #ifdef DEBUG
-			LOG(debug) << _groupName << "::" << s->getName() << " raw reading: \"" << reading.value << "\"";
+				LOG(debug) << _groupName << "::" << s->getName() << " raw reading: \"" << reading.value << "\"";
 #endif
-			s->storeReading(reading);
-		} catch (const std::exception& e) {
-			LOG(error) << _groupName << "::" << s->getName() << " could not read value: " << e.what();
-			continue;
+				s->storeReading(reading);
+			} catch (const std::exception& e) {
+				LOG(error) << _groupName << "::" << s->getName() << " could not read value: " << e.what();
+				continue;
+			}
 		}
+		_connection->close();
 	}
 }