problems with v3 informs
Erik Anggard
I have found two things with v3 informs that cause a bit of trouble for
us (and probably others that use net-snmp in a similar way):
1. When snmpd.conf is read and a trapsess line is being parsed its
callback function snmpd_parse_config_trapsess will be called. It will do
an snmp_parse_args and then an snmp_open to start the trap session. In
the case of v3 informs snmp_open will result in a call to
snmpv3_engineID_probe to get the engine ID of the inform receiver. The
problem is that snmpv3_engineID_probe isn't asynchronous. This means
that if you have several inform receivers, high retry count and/or long
timeouts and the receivers are down or not reachable you will get a very
long startup time for snmpd.
2. If the engine ID probe fails (i.e. the inform receiver happens to be
down or the connection is broken when snmpd is started) the receiver
will not receive any future informs unless snmpd is restarted (or
kill-huped).
I've modified agent_trap.c and snmp_api.c to solve these two problems.
This what I did:
- Changed snmpd_parse_config_trapsess so it doesn't call snmp_open.
Instead it will add the session to a list called newsinks. This list
will be checked every time a new trap/inform is sent.
- Changed snmpv3_engineID_probe to be asynchronous. It will be handed a
callback function that will call add_trap_session for the session if the
engine ID probe succeed. If it failed the session will be put back on
the newsinks list (so it will be tried for again the next time a
trap/inform is sent).
This seems to work quite well. It would be very nice if something along
these lines could be added to net-snmp. I'd be happy to supply anyone
with patches of the work I did, but it would probably be better if
someone on the net-snmp team rewrote it since I probably haven't
followed your coding standards.
BTW the version I did this work on was 5.0.pre3 but the two problems
seem to exist in -current also.
Best regards,
Erik Anggard
PacketFront Sweden AB
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Net-snmp-coders mailing list
Net-snm...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
Wes Hardaker
Erik> 1. When snmpd.conf is read and a trapsess line is being parsed its
Erik> callback function snmpd_parse_config_trapsess will be called. It will
Erik> do an snmp_parse_args and then an snmp_open to start the trap
Erik> session. In the case of v3 informs snmp_open will result in a call to
Erik> snmpv3_engineID_probe to get the engine ID of the inform receiver. The
Erik> problem is that snmpv3_engineID_probe isn't asynchronous. This means
Erik> that if you have several inform receivers, high retry count and/or
Erik> long timeouts and the receivers are down or not reachable you will get
Erik> a very long startup time for snmpd.
Yep. It's a known problem. However, you can specify the engineid on
the trapsess line which will at least make the probing not happen.
Erik> 2. If the engine ID probe fails (i.e. the inform receiver happens to
Erik> be down or the connection is broken when snmpd is started) the
Erik> receiver will not receive any future informs unless snmpd is restarted
Erik> (or kill-huped).
Yep.
Erik> - Changed snmpd_parse_config_trapsess so it doesn't call
Erik> snmp_open. Instead it will add the session to a list called
Erik> newsinks. This list will be checked every time a new trap/inform is
Erik> sent.
Erik> - Changed snmpv3_engineID_probe to be asynchronous. It will be handed
Erik> a callback function that will call add_trap_session for the session if
Erik> the engine ID probe succeed. If it failed the session will be put back
Erik> on the newsinks list (so it will be tried for again the next time a
Erik> trap/inform is sent).
I'm interested in seeing the patch. The first solution is obviously
great, the second I'd have to see as it likely has possibly unknown
repercussions till I see how it's handled.
Please mail all replies to net-snm...@lists.sourceforge.net
Erik Anggard
--------------030005020506030403080308
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Ok, here you have them (three attached files), but they aren't pretty!
:) I few things to note:
- They are patches against 5.0.pre3.
- I'm not completely familiar with the net-snmp code, I've only dug
through the source files that seemed relevant to these problems, so as
you said, I might very well have missed something critical. I haven't
done any really extensive testing but it has worked fine in the few
tests I've done so far.
- If a v3 notifiy receiver isn't up when snmpd is started the first
inform sent after it does come up will also be lost (since it will only
trigger a new async engine id probe) but the subsequent ones will
succeed. This could probably be solved by storing the last trap/inform
pdu and when an async engine id probe succeeds it will resend that pdu
to the new sink.
- I haven't used any locks since we don't use threads on our system.
- There should probably some more logging.
/Erik
Wes Hardaker wrote:
--------------030005020506030403080308
Content-Type: text/plain;
name="agent_trap.c.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="agent_trap.c.patch"
--- agent/agent_trap.c.old 8 May 2002 12:00:02
+++ agent/agent_trap.c 20 Sep 2002 17:04:11
@@ -48,11 +48,17 @@
#endif
#include <net-snmp/net-snmp-includes.h>
+#include <net-snmp/agent/snmp_agent.h>
#include <net-snmp/agent/agent_trap.h>
#include <net-snmp/agent/agent_callbacks.h>
#include <net-snmp/agent/mib_module_config.h>
+/*
+ * this must be standardized somewhere, right?
+ */
+#define MAX_ARGS 128
+
struct trap_sink {
netsnmp_session *sesp;
struct trap_sink *next;
@@ -60,7 +66,17 @@
int version;
};
+struct new_sink {
+ netsnmp_session *sesp;
+ int argn;
+ char *argv[MAX_ARGS];
+ int traptype;
+ struct new_sink *next;
+ int fail_cnt;
+};
+
struct trap_sink *sinks = NULL;
+struct new_sink *newsinks = NULL;
extern struct timeval starttime;
@@ -105,6 +121,10 @@
* static void send_v1_trap (netsnmp_session *, int, int);
* static void send_v2_trap (netsnmp_session *, int, int, int);
*/
+int newsink_callback(int status, netsnmp_session *ss, int reqid,
+ netsnmp_pdu *response, void *cb_magic);
+int add_new_sink(netsnmp_session * ss, int argn, char **argv, int trapt);
+void check_newsinks(void);
/*******************
@@ -260,11 +280,26 @@
snmpd_free_trapsinks(void)
{
struct trap_sink *sp = sinks;
+ struct new_sink *sn = newsinks;
+ char **argv;
+ int argn;
+
while (sp) {
sinks = sinks->next;
free_trap_session(sp);
sp = sinks;
}
+
+ while (sn) {
+ newsinks = newsinks->next;
+ argv = sn->argv;
+ for (argn = sn->argn; argn > 0; argn--) {
+ free(argv[argn - 1]);
+ }
+ free(sn->sesp);
+ free(sn);
+ sn = newsinks;
+ }
}
/*******************
@@ -483,6 +518,7 @@
* Now loop through the list of trap sinks,
* sending an appropriately formatted PDU to each
*/
+ check_newsinks();
for (sink = sinks; sink; sink = sink->next) {
if (sink->version == SNMP_VERSION_1 && trap == -1)
continue; /* Skip v1 sinks for v2 only traps */
@@ -744,11 +780,6 @@
}
}
-/*
- * this must be standardized somewhere, right?
- */
-#define MAX_ARGS 128
-
static int traptype;
static void
@@ -776,7 +807,7 @@
{
char *argv[MAX_ARGS], *cp = cptr, tmp[SPRINT_MAX_LEN];
int argn, arg;
- netsnmp_session session, *ss;
+ netsnmp_session session;
/*
* inform or trap? default to trap
@@ -793,25 +824,8 @@
}
arg = snmp_parse_args(argn, argv, &session, "C:", trapOptProc);
- ss = snmp_open(&session);
-
- for (; argn > 0; argn--) {
- free(argv[argn - 1]);
- }
-
- if (!ss) {
- config_perror
- ("snmpd: failed to parse this line or the remote trap receiver is down. Possible cause:");
- snmp_sess_perror("snmpd: snmpd_parse_config_trapsess()", &session);
- return;
- }
-
- if (ss->version == SNMP_VERSION_1) {
- add_trap_session(ss, SNMP_MSG_TRAP, 0, SNMP_VERSION_1);
- } else {
- add_trap_session(ss, traptype, (traptype == SNMP_MSG_INFORM),
- ss->version);
- }
+ add_new_sink(&session, argn, argv, traptype);
+ check_newsinks(); /* open session or start async eid probing */
}
void
@@ -834,3 +848,130 @@
snmp_trapcommunity = NULL;
}
}
+
+
+/*
+ * newsink_callback: called by eid_probe_callback once an engine ID probe
+ * has finished successfully or failed (e.g. timed out).
+ */
+int
+newsink_callback(int status, netsnmp_session *ss, int reqid,
+ netsnmp_pdu *response, void *cb_magic)
+{
+ struct new_sink *n = cb_magic;
+ char **argv;
+ int argn;
+ int trapt;
+
+
+ if (status == SNMPERR_SUCCESS) {
+ trapt = n->traptype;
+ /* snmp_open ok, add to sinks list and remove from newsinks. */
+ if (ss->version == SNMP_VERSION_1) {
+ add_trap_session(ss, SNMP_MSG_TRAP, 0, SNMP_VERSION_1);
+ } else {
+ add_trap_session(ss, trapt, (trapt == SNMP_MSG_INFORM),
+ ss->version);
+ }
+ argv = n->argv;
+ for (argn = n->argn; argn > 0; argn--) {
+ free(argv[argn - 1]);
+ }
+ free(n->sesp);
+ free(n);
+ ss->callback = NULL;
+ ss->callback_magic = NULL;
+ ss->flags &= ~SNMP_FLAGS_ASYNC_PROBE;
+ } else {
+ n->fail_cnt++;
+ /* Put it back on the newsinks list. */
+ n->next = newsinks;
+ newsinks = n;
+ }
+
+ return 1;
+}
+
+/*
+ * add_new_sink: add a new sink to list, if it is a v3 inform it will
+ * stay on the list until the engine ID has been probed successfully.
+ */
+int
+add_new_sink(netsnmp_session * ss, int argn, char **argv, int trapt)
+{
+ struct new_sink *n;
+ netsnmp_session *ses;
+
+ n = (struct new_sink *) malloc(sizeof(*n));
+ ses = (netsnmp_session *) malloc(sizeof(*ses));
+ if (n == NULL || ses == NULL)
+ return 0;
+
+ /* copy data returned by snmp_parse_args */
+ *ses = *ss;
+ n->sesp = ses;
+ n->argn = argn;
+ n->traptype = trapt;
+ memcpy(n->argv, argv, MAX_ARGS*sizeof(char *));
+
+ /* if version 3 set up for async engine ID probe */
+ if (ses->version == SNMP_VERSION_3) {
+ ses->flags |= SNMP_FLAGS_ASYNC_PROBE;
+ ses->callback = newsink_callback;
+ ses->callback_magic = n;
+ }
+
+ n->fail_cnt = 0;
+ n->next = newsinks;
+ newsinks = n;
+
+ return 1;
+}
+
+/*
+ * check_newsinks: check list of new sinks. For v3 session that don't have
+ * an engine ID an async probe will be started. Other types of sessions will
+ * be started directly.
+ */
+void
+check_newsinks(void)
+{
+ netsnmp_session *ss;
+ struct new_sink *n, **prevnext;
+ char **argv;
+ int argn;
+ int trapt;
+
+ for (n = newsinks, prevnext = &newsinks; n != NULL; ) {
+
+ ss = snmp_open(n->sesp);
+
+ if (!ss) {
+ /* snmp_open failed, keep on newsinks list. */
+ n->fail_cnt++;
+ /* TODO: add logging? */
+ prevnext = &n->next;
+ n = n->next;
+ } else {
+ *prevnext = n->next;
+ if (!(n->sesp->flags & SNMP_FLAGS_ASYNC_PROBE)) {
+ trapt = n->traptype;
+ /* snmp_open ok, add to sinks list and remove from newsinks. */
+ if (ss->version == SNMP_VERSION_1) {
+ add_trap_session(ss, SNMP_MSG_TRAP, 0, SNMP_VERSION_1);
+ } else {
+ add_trap_session(ss, trapt, (trapt == SNMP_MSG_INFORM),
+ ss->version);
+ }
+ argv = n->argv;
+ for (argn = n->argn; argn > 0; argn--) {
+ free(argv[argn - 1]);
+ }
+ free(n->sesp);
+ free(n);
+ }
+ n = *prevnext;
+ }
+ }
+}
+
--------------030005020506030403080308
Content-Type: text/plain;
name="snmp_api.c.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="snmp_api.c.patch"
--- snmplib/snmp_api.c.old 8 May 2002 12:00:07
+++ snmplib/snmp_api.c 20 Sep 2002 16:47:57
@@ -342,6 +342,9 @@
int snmp_get_errno(void);
void snmp_synch_reset(netsnmp_session * notused);
void snmp_synch_setup(netsnmp_session * notused);
+static int eid_probe_callback(int op,
+ netsnmp_session *session,
+ int reqid, netsnmp_pdu *pdu, void *magic);
#ifndef HAVE_STRERROR
const char *
@@ -1144,6 +1147,10 @@
}
+typedef struct eid_probe_state_s {
+ int reqid;
+} eid_probe_state;
+
int
snmpv3_engineID_probe(struct session_list *slp,
@@ -1152,7 +1159,8 @@
netsnmp_pdu *pdu = NULL, *response = NULL;
netsnmp_session *session;
unsigned int i;
- int status;
+ int status, rc;
+ eid_probe_state *state;
if (slp == NULL || slp->session == NULL) {
return 0;
@@ -1176,7 +1184,21 @@
return 0;
}
DEBUGMSGTL(("snmp_api", "probing for engineID...\n"));
- status = snmp_sess_synch_response(slp, pdu, &response);
+
+ if (in_session->flags & SNMP_FLAGS_ASYNC_PROBE) {
+ if ((state = malloc(sizeof(*state))) == NULL)
+ return 0;
+ rc = snmp_sess_async_send(slp, pdu, eid_probe_callback, state);
+ if (rc == 0) {
+ free(state);
+ snmp_free_pdu(pdu);
+ return 0;
+ }
+ state->reqid = rc;
+ return 1;
+ }
+
+ status = snmp_sess_synch_response(slp, pdu, &response);
if ((response == NULL) && (status == STAT_SUCCESS)) {
status = STAT_ERROR;
@@ -1239,6 +1261,67 @@
}
return 1;
+}
+
+
+static int
+eid_probe_callback(int op,
+ netsnmp_session *sp,
+ int reqid, netsnmp_pdu *pdu, void *magic)
+{
+ eid_probe_state *state = (eid_probe_state *)magic;
+ int rpt_type, res;
+
+ if (reqid != state->reqid && pdu && pdu->command != SNMP_MSG_REPORT) {
+ return 0;
+ }
+
+ if (op == NETSNMP_CALLBACK_OP_RECEIVED_MESSAGE) {
+ rpt_type = snmpv3_get_report_type(pdu);
+ if (SNMPV3_IGNORE_UNAUTH_REPORTS ||
+ rpt_type == SNMPERR_NOT_IN_TIME_WINDOW) {
+ return 1;
+ }
+ res = SNMPERR_SUCCESS;
+ } else if (op == NETSNMP_CALLBACK_OP_TIMED_OUT)
+ res = SNMPERR_TIMEOUT;
+ else
+ res = SNMPERR_INVALID_MSG;
+
+ /*
+ * Handle engineID discovery.
+ */
+ if (!sp->securityEngineIDLen && pdu->securityEngineIDLen) {
+ sp->securityEngineID = (u_char *) malloc(pdu->securityEngineIDLen);
+ memcpy(sp->securityEngineID, pdu->securityEngineID,
+ pdu->securityEngineIDLen);
+ sp->securityEngineIDLen = pdu->securityEngineIDLen;
+ if (!sp->contextEngineIDLen) {
+ sp->contextEngineID = (u_char *) malloc(pdu->securityEngineIDLen);
+ memcpy(sp->contextEngineID, pdu->securityEngineID,
+ pdu->securityEngineIDLen);
+ sp->contextEngineIDLen = pdu->securityEngineIDLen;
+ }
+ }
+
+ if (res == SNMPERR_SUCCESS && sp->securityEngineIDLen == 0)
+ res = SNMPERR_INVALID_MSG;
+
+ if (res == SNMPERR_SUCCESS && (sp->engineBoots || sp->engineTime)) {
+ set_enginetime(sp->securityEngineID,
+ sp->securityEngineIDLen,
+ sp->engineBoots, sp->engineTime,
+ TRUE);
+ }
+
+ if (res == SNMPERR_SUCCESS)
+ res = create_user_from_session(sp);
+
+ if (sp->callback)
+ sp->callback(res, sp, 0, NULL, sp->callback_magic);
+
+ return 1;
+
}
--------------030005020506030403080308
Content-Type: text/plain;
name="snmp_api.h.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="snmp_api.h.patch"
--- include/net-snmp/library/snmp_api.h.old 8 May 2002 12:00:05
+++ include/net-snmp/library/snmp_api.h 20 Sep 2002 16:39:23
@@ -292,6 +292,7 @@
#define SNMP_DETAIL_SIZE 512
#define SNMP_FLAGS_DONT_PROBE 0x100 /* don't probe for an engineID */
+#define SNMP_FLAGS_ASYNC_PROBE 0x200 /* async probe of engine ID */
#define SNMP_FLAGS_STREAM_SOCKET 0x80
#define SNMP_FLAGS_LISTENING 0x40 /* Server stream sockets only */
#define SNMP_FLAGS_SUBSESSION 0x20
--------------030005020506030403080308--