snmpd(8) untangle transport layer and snmp engine

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

snmpd(8) untangle transport layer and snmp engine

Martijn van Duren-5
This diff does the following:
- Push the network handling to their own files.
  This helps with readability and allows for easier addition of other
  transport layers if we ever choose to (unix socket, ssh, dtls, ...)
- Allow multiple tcp packets to be handled at the same time.
  This will helps throughput with multiple backends (agentx).
- Remove sm_data from the snmp_message. This shaves of ~65k per struct.
  Right now this isn't that interesting, because we mostly handle 1
  packet at a time, but as soon as we're leveraging agentx more it
  is not unlikely that there are multiple packets in flight inside
  snmpd.

There's only one intentional function change and that is that the
outpkts is incremented once the data is pushed to the output buffer
in tcp.c, instead of when the packet has been send to the kernel.
I think this is reasonable, since the endeavour is to make the transport
layer agnostic about how snmp works.


There's more that I'd like to change, but this diff is large enough as
is.

OK?

martijn@

Index: Makefile
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/Makefile,v
retrieving revision 1.16
diff -u -p -r1.16 Makefile
--- Makefile 11 May 2019 17:46:02 -0000 1.16
+++ Makefile 15 Feb 2020 11:25:41 -0000
@@ -3,6 +3,7 @@
 PROG= snmpd
 MAN= snmpd.8 snmpd.conf.5
 SRCS= parse.y log.c control.c snmpe.c \
+    tcp.c udp.c \
     mps.c trap.c mib.c smi.c kroute.c snmpd.c timer.c \
     pf.c proc.c usm.c agentx.c traphandler.c util.c
 
Index: snmpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.86
diff -u -p -r1.86 snmpd.h
--- snmpd.h 2 Jan 2020 10:55:53 -0000 1.86
+++ snmpd.h 15 Feb 2020 11:25:41 -0000
@@ -395,23 +395,21 @@ struct pfr_buffer {
  * daemon structures
  */
 
+struct snmp_conn {
+ char sc_host[HOST_NAME_MAX+1];
+ void *sc_context;
+ TAILQ_HEAD(, snmp_message) sc_messages;
+};
+
 #define MSG_HAS_AUTH(m) (((m)->sm_flags & SNMP_MSGFLAG_AUTH) != 0)
 #define MSG_HAS_PRIV(m) (((m)->sm_flags & SNMP_MSGFLAG_PRIV) != 0)
 #define MSG_SECLEVEL(m) ((m)->sm_flags & SNMP_MSGFLAG_SECMASK)
 #define MSG_REPORT(m) (((m)->sm_flags & SNMP_MSGFLAG_REPORT) != 0)
 
 struct snmp_message {
- int sm_sock;
- struct sockaddr_storage sm_ss;
- socklen_t sm_slen;
- int sm_sock_tcp;
- struct event sm_sockev;
- char sm_host[HOST_NAME_MAX+1];
-
- struct sockaddr_storage sm_local_ss;
- socklen_t sm_local_slen;
+ struct snmp_conn *sm_conn;
+ void (*sm_reply)(struct snmp_conn *, void *, size_t);
 
- struct ber sm_ber;
  struct ber_element *sm_req;
  struct ber_element *sm_resp;
 
@@ -423,7 +421,7 @@ struct snmp_message {
  struct ber_element *sm_last;
  struct ber_element *sm_end;
 
- u_int8_t sm_data[READ_BUF_SIZE];
+ u_int8_t *sm_data;
  size_t sm_datalen;
 
  u_int sm_version;
@@ -464,6 +462,7 @@ struct snmp_message {
 
  struct ber_element *sm_varbind;
  struct ber_element *sm_varbindresp;
+ TAILQ_ENTRY(snmp_message) sm_entry;
 };
 
 /* Defined in SNMPv2-MIB.txt (RFC 3418) */
@@ -667,10 +666,19 @@ struct kroute *kroute_getaddr(in_addr_t,
 struct kif_arp *karp_first(u_short);
 struct kif_arp *karp_getaddr(struct sockaddr *, u_short, int);
 
+/* tcp.c */
+void tcp_accept(int, short, void *);
+
+/* udp.c */
+void udp_recvmsg(int, short, void *);
+
 /* snmpe.c */
 void snmpe(struct privsep *, struct privsep_proc *);
 void snmpe_shutdown(void);
 void snmpe_dispatchmsg(struct snmp_message *);
+ssize_t snmpe_extract(void *, size_t,
+    void (*)(struct snmp_conn *, void *, size_t),
+    struct snmp_conn *);
 
 /* trap.c */
 void trap_init(void);
Index: snmpe.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.61
diff -u -p -r1.61 snmpe.c
--- snmpe.c 14 Feb 2020 15:08:46 -0000 1.61
+++ snmpe.c 15 Feb 2020 11:25:41 -0000
@@ -42,22 +42,15 @@
 
 void snmpe_init(struct privsep *, struct privsep_proc *, void *);
 int snmpe_parse(struct snmp_message *);
-void snmpe_tryparse(int, struct snmp_message *);
 int snmpe_parsevarbinds(struct snmp_message *);
 void snmpe_response(struct snmp_message *);
 void snmpe_sig_handler(int sig, short, void *);
 int snmpe_dispatch_parent(int, struct privsep_proc *, struct imsg *);
 int snmpe_bind(struct address *);
-void snmpe_recvmsg(int fd, short, void *);
-void snmpe_readcb(int fd, short, void *);
-void snmpe_writecb(int fd, short, void *);
-void snmpe_acceptcb(int fd, short, void *);
-void snmpe_prepare_read(struct snmp_message *, int);
 int snmpe_encode(struct snmp_message *);
 void snmp_msgfree(struct snmp_message *);
 
 struct imsgev *iev_parent;
-static const struct timeval snmpe_tcp_timeout = { 10, 0 }; /* 10s */
 
 static struct privsep_proc procs[] = {
  { "parent", PROC_PARENT, snmpe_dispatch_parent }
@@ -111,11 +104,11 @@ snmpe_init(struct privsep *ps, struct pr
  if (so->s_ipproto == IPPROTO_TCP) {
  if (listen(so->s_fd, 5) < 0)
  fatalx("snmpe: failed to listen on socket");
- event_set(&so->s_ev, so->s_fd, EV_READ, snmpe_acceptcb, so);
- evtimer_set(&so->s_evt, snmpe_acceptcb, so);
+ event_set(&so->s_ev, so->s_fd, EV_READ, tcp_accept, so);
+ evtimer_set(&so->s_evt, tcp_accept, so);
  } else {
  event_set(&so->s_ev, so->s_fd, EV_READ|EV_PERSIST,
-    snmpe_recvmsg, env);
+    udp_recvmsg, env);
  }
  event_add(&so->s_ev, NULL);
  }
@@ -224,7 +217,6 @@ snmpe_parse(struct snmp_message *msg)
  char *comn;
  char *flagstr, *ctxname;
  size_t len;
- struct sockaddr_storage *ss = &msg->sm_ss;
  struct ber_element *root = msg->sm_req;
 
  msg->sm_errstr = "invalid message";
@@ -371,17 +363,17 @@ snmpe_parse(struct snmp_message *msg)
  msg->sm_error = errval;
  msg->sm_errorindex = erridx;
 
- print_host(ss, msg->sm_host, sizeof(msg->sm_host));
  if (msg->sm_version == SNMP_V3)
  log_debug("%s: %s: SNMPv3 context %d, flags %#x, "
     "secmodel %lld, user '%s', ctx-engine %s, ctx-name '%s', "
-    "request %lld", __func__, msg->sm_host, msg->sm_context,
-    msg->sm_flags, msg->sm_secmodel, msg->sm_username,
+    "request %lld", __func__, msg->sm_conn->sc_host,
+    msg->sm_context, msg->sm_flags, msg->sm_secmodel,
+    msg->sm_username,
     tohexstr(msg->sm_ctxengineid, msg->sm_ctxengineid_len),
     msg->sm_ctxname, msg->sm_request);
  else
  log_debug("%s: %s: SNMPv%d '%s' context %d request %lld",
-    __func__, msg->sm_host, msg->sm_version + 1,
+    __func__, msg->sm_conn->sc_host, msg->sm_version + 1,
     msg->sm_community, msg->sm_context, msg->sm_request);
 
  return (0);
@@ -389,8 +381,7 @@ snmpe_parse(struct snmp_message *msg)
  parsefail:
  stats->snmp_inasnparseerrs++;
  fail:
- print_host(ss, msg->sm_host, sizeof(msg->sm_host));
- log_debug("%s: %s: %s", __func__, msg->sm_host, msg->sm_errstr);
+ log_debug("%s: %s: %s", __func__, msg->sm_conn->sc_host, msg->sm_errstr);
  return (-1);
 }
 
@@ -434,7 +425,7 @@ snmpe_parsevarbinds(struct snmp_message
  else
  stats->snmp_intotalreqvars++;
  log_debug("%s: %s: oid %s",
-    __func__, msg->sm_host,
+    __func__, msg->sm_conn->sc_host,
     smi_oid2string(&o, buf, sizeof(buf), 0));
  break;
  case 1:
@@ -511,236 +502,65 @@ snmpe_parsevarbinds(struct snmp_message
  return (ret);
  varfail:
  log_debug("%s: %s: %s, error index %d", __func__,
-    msg->sm_host, msg->sm_errstr, msg->sm_i);
+    msg->sm_conn->sc_host, msg->sm_errstr, msg->sm_i);
  if (msg->sm_error == 0)
  msg->sm_error = SNMP_ERROR_GENERR;
  msg->sm_errorindex = msg->sm_i;
  return (-1);
 }
 
-void
-snmpe_acceptcb(int fd, short type, void *arg)
-{
- struct listen_sock *so = arg;
- struct sockaddr_storage ss;
- socklen_t len = sizeof(ss);
- struct snmp_message *msg;
- int afd;
-
- event_add(&so->s_ev, NULL);
- if ((type & EV_TIMEOUT))
- return;
-
- if ((afd = accept4(fd, (struct sockaddr *)&ss, &len,
-    SOCK_NONBLOCK|SOCK_CLOEXEC)) < 0) {
- /* Pause accept if we are out of file descriptors  */
- if (errno == ENFILE || errno == EMFILE) {
- struct timeval evtpause = { 1, 0 };
-
- event_del(&so->s_ev);
- evtimer_add(&so->s_evt, &evtpause);
- } else if (errno != EAGAIN && errno != EINTR)
- log_debug("%s: accept4", __func__);
- return;
- }
- if ((msg = calloc(1, sizeof(*msg))) == NULL)
- goto fail;
-
- snmpe_prepare_read(msg, afd);
- return;
-fail:
- free(msg);
- close(afd);
- return;
-}
-
-void
-snmpe_prepare_read(struct snmp_message *msg, int fd)
-{
- msg->sm_sock = fd;
- msg->sm_sock_tcp = 1;
- event_del(&msg->sm_sockev);
- event_set(&msg->sm_sockev, fd, EV_READ,
-    snmpe_readcb, msg);
- event_add(&msg->sm_sockev, &snmpe_tcp_timeout);
-}
-
-void
-snmpe_tryparse(int fd, struct snmp_message *msg)
+ssize_t
+snmpe_extract(void *buf, size_t buflen,
+    void (*reply)(struct snmp_conn *, void *, size_t), struct snmp_conn *conn)
 {
  struct snmp_stats *stats = &snmpd_env->sc_stats;
+ struct ber ber;
+ struct ber_element *req;
+ struct snmp_message *msg;
+ ssize_t nread;
 
- ober_set_application(&msg->sm_ber, smi_application);
- ober_set_readbuf(&msg->sm_ber, msg->sm_data, msg->sm_datalen);
- msg->sm_req = ober_read_elements(&msg->sm_ber, NULL);
- if (msg->sm_req == NULL) {
+ bzero(&ber, sizeof(ber));
+ ober_set_application(&ber, smi_application);
+ ober_set_readbuf(&ber, buf, buflen);
+ if ((req = ober_read_elements(&ber, NULL)) == NULL) {
  if (errno == ECANCELED) {
- /* short read; try again */
- snmpe_prepare_read(msg, fd);
- return;
+ /* short read */
+ return 0;
  }
- goto fail;
+ return -1;
  }
+ nread = ber.br_rptr - (u_char *)buf;
+
+ if ((msg = calloc(1, sizeof(*msg))) == NULL) {
+ log_warn("malloc");
+ return -1;
+ }
+ msg->sm_req = req;
+ msg->sm_conn = conn;
+ msg->sm_reply = reply;
+ msg->sm_data = buf;
+ msg->sm_datalen = buflen;
+ TAILQ_INSERT_TAIL(&(conn->sc_messages), msg, sm_entry);
 
  if (snmpe_parse(msg) == -1) {
+ msg->sm_data = NULL;
+ msg->sm_datalen = 0;
  if (msg->sm_usmerr && MSG_REPORT(msg)) {
  usm_make_report(msg);
  snmpe_response(msg);
- return;
+ return nread;
  } else
  goto fail;
  }
+ msg->sm_data = NULL;
+ msg->sm_datalen = 0;
  stats->snmp_inpkts++;
 
  snmpe_dispatchmsg(msg);
- return;
- fail:
- snmp_msgfree(msg);
- close(fd);
-}
-
-void
-snmpe_readcb(int fd, short type, void *arg)
-{
- struct snmp_message *msg = arg;
- ssize_t len;
-
- if (type == EV_TIMEOUT || msg->sm_datalen >= sizeof(msg->sm_data))
- goto fail;
-
- len = read(fd, msg->sm_data + msg->sm_datalen,
-    sizeof(msg->sm_data) - msg->sm_datalen);
- if (len <= 0) {
- if (errno != EAGAIN && errno != EINTR)
- goto fail;
- snmpe_prepare_read(msg, fd);
- return;
- }
-
- msg->sm_datalen += (size_t)len;
- snmpe_tryparse(fd, msg);
- return;
-
- fail:
- snmp_msgfree(msg);
- close(fd);
-}
-
-void
-snmpe_writecb(int fd, short type, void *arg)
-{
- struct snmp_stats *stats = &snmpd_env->sc_stats;
- struct snmp_message *msg = arg;
- struct snmp_message *nmsg;
- ssize_t len;
- size_t reqlen;
- struct ber *ber = &msg->sm_ber;
-
- if (type == EV_TIMEOUT)
- goto fail;
-
- len = ber->br_wend - ber->br_wbuf;
- ber->br_wptr = ber->br_wbuf;
-
- log_debug("%s: write fd %d len %zd", __func__, fd, len);
-
- len = write(fd, ber->br_wptr, len);
- if (len == -1) {
- if (errno == EAGAIN || errno == EINTR)
- return;
- else
- goto fail;
- }
-
- ber->br_wptr += len;
-
- if (ber->br_wptr < ber->br_wend) {
- event_del(&msg->sm_sockev);
- event_set(&msg->sm_sockev, msg->sm_sock, EV_WRITE,
-    snmpe_writecb, msg);
- event_add(&msg->sm_sockev, &snmpe_tcp_timeout);
- return;
- }
-
- stats->snmp_outpkts++;
-
- if ((nmsg = calloc(1, sizeof(*nmsg))) == NULL)
- goto fail;
-
- /*
- * Reuse the connection.
- * In case we already read data of the next message, copy it over.
- */
- reqlen = ober_calc_len(msg->sm_req);
- if (msg->sm_datalen > reqlen) {
- memcpy(nmsg->sm_data, msg->sm_data + reqlen,
-    msg->sm_datalen - reqlen);
- nmsg->sm_datalen = msg->sm_datalen - reqlen;
- snmp_msgfree(msg);
- snmpe_prepare_read(nmsg, fd);
- snmpe_tryparse(fd, nmsg);
- } else {
- snmp_msgfree(msg);
- snmpe_prepare_read(nmsg, fd);
- }
- return;
-
+ return nread;
  fail:
- close(fd);
  snmp_msgfree(msg);
-}
-
-void
-snmpe_recvmsg(int fd, short sig, void *arg)
-{
- struct snmpd *env = arg;
- struct snmp_stats *stats = &env->sc_stats;
- ssize_t len;
- struct snmp_message *msg;
-
- if ((msg = calloc(1, sizeof(*msg))) == NULL)
- return;
-
- msg->sm_sock = fd;
- msg->sm_slen = sizeof(msg->sm_ss);
- if ((len = recvfromto(fd, msg->sm_data, sizeof(msg->sm_data), 0,
-    (struct sockaddr *)&msg->sm_ss, &msg->sm_slen,
-    (struct sockaddr *)&msg->sm_local_ss, &msg->sm_local_slen)) < 1) {
- free(msg);
- return;
- }
-
- stats->snmp_inpkts++;
- msg->sm_datalen = (size_t)len;
-
- bzero(&msg->sm_ber, sizeof(msg->sm_ber));
- ober_set_application(&msg->sm_ber, smi_application);
- ober_set_readbuf(&msg->sm_ber, msg->sm_data, msg->sm_datalen);
-
- msg->sm_req = ober_read_elements(&msg->sm_ber, NULL);
- if (msg->sm_req == NULL) {
- stats->snmp_inasnparseerrs++;
- snmp_msgfree(msg);
- return;
- }
-
-#ifdef DEBUG
- fprintf(stderr, "recv msg:\n");
- smi_debug_elements(msg->sm_req);
-#endif
-
- if (snmpe_parse(msg) == -1) {
- if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
- usm_make_report(msg);
- snmpe_response(msg);
- return;
- } else {
- snmp_msgfree(msg);
- return;
- }
- }
-
- snmpe_dispatchmsg(msg);
+ return -1;
 }
 
 void
@@ -761,6 +581,7 @@ snmpe_response(struct snmp_message *msg)
  struct snmp_stats *stats = &snmpd_env->sc_stats;
  u_int8_t *ptr = NULL;
  ssize_t len;
+ struct ber ber;
 
  if (msg->sm_varbindresp == NULL && msg->sm_pduend != NULL)
  msg->sm_varbindresp = ober_unlink_elements(msg->sm_pduend);
@@ -790,25 +611,16 @@ snmpe_response(struct snmp_message *msg)
  if (snmpe_encode(msg) < 0)
  goto done;
 
- len = ober_write_elements(&msg->sm_ber, msg->sm_resp);
- if (ober_get_writebuf(&msg->sm_ber, (void *)&ptr) == -1)
+ bzero(&ber, sizeof(ber));
+ len = ober_write_elements(&ber, msg->sm_resp);
+ if (ober_get_writebuf(&ber, (void *)&ptr) == -1)
  goto done;
 
  usm_finalize_digest(msg, ptr, len);
- if (msg->sm_sock_tcp) {
- event_del(&msg->sm_sockev);
- event_set(&msg->sm_sockev, msg->sm_sock, EV_WRITE,
-    snmpe_writecb, msg);
- event_add(&msg->sm_sockev, &snmpe_tcp_timeout);
- return;
- } else {
- len = sendtofrom(msg->sm_sock, ptr, len, 0,
-    (struct sockaddr *)&msg->sm_ss, msg->sm_slen,
-    (struct sockaddr *)&msg->sm_local_ss, msg->sm_local_slen);
- if (len != -1)
- stats->snmp_outpkts++;
+ if (msg->sm_conn != NULL) {
+ msg->sm_reply(msg->sm_conn, ptr, len);
+ stats->snmp_outpkts++;
  }
-
  done:
  snmp_msgfree(msg);
 }
@@ -816,12 +628,12 @@ snmpe_response(struct snmp_message *msg)
 void
 snmp_msgfree(struct snmp_message *msg)
 {
- event_del(&msg->sm_sockev);
- ober_free(&msg->sm_ber);
  if (msg->sm_req != NULL)
  ober_free_elements(msg->sm_req);
  if (msg->sm_resp != NULL)
  ober_free_elements(msg->sm_resp);
+ if (msg->sm_conn != NULL)
+ TAILQ_REMOVE(&(msg->sm_conn->sc_messages), msg, sm_entry);
  free(msg);
 }
 
Index: tcp.c
===================================================================
RCS file: tcp.c
diff -N tcp.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tcp.c 15 Feb 2020 11:25:41 -0000
@@ -0,0 +1,214 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2020 Martijn van Duren <[hidden email]>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <event.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "snmpd.h"
+
+struct tcp_conn {
+ int tc_fd;
+ char *tc_rbuf;
+ size_t tc_rbuflen;
+ size_t tc_rbufsize;
+ char *tc_wbuf;
+ size_t tc_wbuflen;
+ size_t tc_wbufsize;
+ struct event tc_rev;
+ struct event tc_wev;
+ struct snmp_conn tc_sc;
+};
+
+#define TCP_MINBUF 8192
+
+static const struct timeval tcp_timeout = { 10, 0 }; /* 10s */
+
+void tcp_read(int, short, void *);
+void tcp_send(struct snmp_conn *, void *, size_t);
+void tcp_write(int, short, void *);
+void tcp_free(struct tcp_conn *);
+
+void
+tcp_accept(int lfd, short event, void *arg)
+{
+ struct listen_sock *so = arg;
+ struct sockaddr_storage ss;
+ socklen_t len = sizeof(ss);
+ struct tcp_conn *conn;
+ int fd;
+
+ event_add(&so->s_ev, NULL);
+ evtimer_del(&so->s_evt);
+ if ((event & EV_TIMEOUT))
+ return;
+
+ if ((fd = accept4(lfd, (struct sockaddr *)&ss, &len,
+    SOCK_NONBLOCK|SOCK_CLOEXEC)) < 0) {
+ /* Pause accept if we are out of file descriptors  */
+ if (errno == ENFILE || errno == EMFILE) {
+ struct timeval evtpause = { 1, 0 };
+
+ event_del(&so->s_ev);
+ evtimer_add(&so->s_evt, &evtpause);
+ } else if (errno != EAGAIN && errno != EINTR)
+ log_debug("%s: accept4", __func__);
+ return;
+ }
+ if ((conn = calloc(1, sizeof(*conn))) == NULL) {
+ close(fd);
+ return;
+ }
+ conn->tc_fd = fd;
+ print_host(&ss, conn->tc_sc.sc_host, sizeof(conn->tc_sc.sc_host));
+ conn->tc_sc.sc_context = conn;
+
+ event_set(&(conn->tc_rev), fd, EV_READ, tcp_read, conn);
+ event_set(&(conn->tc_wev), fd, EV_WRITE, tcp_write, conn);
+ event_add(&(conn->tc_rev), &tcp_timeout);
+
+ TAILQ_INIT(&(conn->tc_sc.sc_messages));
+
+ return;
+}
+
+void
+tcp_read(int fd, short event, void *cookie)
+{
+ struct tcp_conn *conn = cookie;
+ ssize_t len;
+ char *tmpbuf;
+
+ if (event == EV_TIMEOUT) {
+ tcp_free(conn);
+ return;
+ }
+
+ if (conn->tc_rbufsize - conn->tc_rbuflen == 0) {
+ tmpbuf = recallocarray(conn->tc_rbuf, conn->tc_rbufsize,
+    conn->tc_rbufsize + TCP_MINBUF, 1);
+ if (tmpbuf == NULL) {
+ log_warn("malloc");
+ tcp_free(conn);
+ return;
+ }
+ conn->tc_rbuf = tmpbuf;
+ conn->tc_rbufsize += TCP_MINBUF;
+ }
+
+ len = read(fd, conn->tc_rbuf + conn->tc_rbuflen,
+    conn->tc_rbufsize - conn->tc_rbuflen);
+ if (len <= 0) {
+ if (errno != EAGAIN && errno != EINTR) {
+ tcp_free(conn);
+ return;
+ }
+ event_add(&(conn->tc_rev), &tcp_timeout);
+ return;
+ }
+
+ conn->tc_rbuflen += (size_t)len;
+
+ while (1) {
+ len = snmpe_extract(conn->tc_rbuf, conn->tc_rbuflen, tcp_send,
+    &(conn->tc_sc));
+ switch (len) {
+ case -1:
+ tcp_free(conn);
+ return;
+ case 0:
+ event_add(&(conn->tc_rev), &tcp_timeout);
+ return;
+ default:
+ memmove(conn->tc_rbuf, conn->tc_rbuf + len,
+    conn->tc_rbuflen - len);
+ conn->tc_rbuflen -= len;
+ }
+ }
+ event_add(&(conn->tc_rev), &tcp_timeout);
+}
+
+void
+tcp_send(struct snmp_conn *sc_conn, void *buf, size_t buflen)
+{
+ struct tcp_conn *conn = sc_conn->sc_context;
+ char *tmpbuf;
+
+ if (conn->tc_wbufsize - conn->tc_wbuflen < buflen) {
+ tmpbuf = recallocarray(conn->tc_wbuf, conn->tc_wbufsize,
+    conn->tc_wbuflen + buflen, 1);
+ if (tmpbuf == NULL) {
+ log_warn("malloc");
+ tcp_free(conn);
+ return;
+ }
+ conn->tc_wbuf = tmpbuf;
+ conn->tc_wbufsize = conn->tc_wbuflen + buflen;
+ }
+
+ memcpy(conn->tc_wbuf + conn->tc_wbuflen, buf, buflen);
+ conn->tc_wbuflen += buflen;
+
+ event_add(&(conn->tc_wev), NULL);
+}
+
+void
+tcp_write(int fd, short event, void *cookie)
+{
+ struct tcp_conn *conn = cookie;
+ ssize_t nwrite;
+
+ if ((nwrite = write(conn->tc_fd, conn->tc_wbuf, conn->tc_wbuflen)) == -1) {
+ if (errno != EAGAIN && errno != EINTR) {
+ log_warn("write");
+ tcp_free(conn);
+ return;
+ }
+ event_add(&(conn->tc_wev), NULL);
+ return;
+ }
+
+ conn->tc_wbuflen -= nwrite;
+ if (conn->tc_wbuflen > 0) {
+ memmove(conn->tc_wbuf, conn->tc_wbuf + nwrite,
+    conn->tc_wbuflen);
+ event_add(&(conn->tc_wev), NULL);
+ }
+}
+
+void
+tcp_free(struct tcp_conn *conn)
+{
+ struct snmp_message *msg;
+
+ TAILQ_FOREACH(msg, &(conn->tc_sc.sc_messages), sm_entry)
+ msg->sm_conn = NULL;
+ close(conn->tc_fd);
+
+ free(conn->tc_rbuf);
+ free(conn->tc_wbuf);
+
+ event_del(&(conn->tc_rev));
+ event_del(&(conn->tc_wev));
+
+ free(conn);
+}
Index: udp.c
===================================================================
RCS file: udp.c
diff -N udp.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ udp.c 15 Feb 2020 11:25:41 -0000
@@ -0,0 +1,82 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2020 Martijn van Duren <[hidden email]>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/socket.h>
+
+#include <event.h>
+#include <stdlib.h>
+
+#include "snmpd.h"
+
+struct udp_conn {
+ int uc_fd;
+ struct sockaddr_storage uc_lss;
+ socklen_t uc_lsslen;
+ struct sockaddr_storage uc_rss;
+ socklen_t uc_rsslen;
+ struct snmp_conn uc_sc;
+};
+
+void udp_send(struct snmp_conn *, void *, size_t);
+
+void
+udp_recvmsg(int fd, short event, void *cookie)
+{
+ ssize_t nrecv, len;
+ char buf[READ_BUF_SIZE];
+ struct udp_conn *conn;
+
+ if ((conn = calloc(1, sizeof(*conn))) == NULL)
+ return;
+
+ conn->uc_fd = fd;
+ conn->uc_sc.sc_context = conn;
+ conn->uc_rsslen = sizeof(conn->uc_rss);
+ conn->uc_lsslen = sizeof(conn->uc_lss);
+ TAILQ_INIT(&(conn->uc_sc.sc_messages));
+
+ if ((nrecv = recvfromto(fd, buf, sizeof(buf), 0,
+    (struct sockaddr *)&conn->uc_rss, &(conn->uc_rsslen),
+    (struct sockaddr *)&conn->uc_lss, &(conn->uc_lsslen))) < 1) {
+ free(conn);
+ return;
+ }
+
+ print_host(&(conn->uc_rss), conn->uc_sc.sc_host,
+    sizeof(conn->uc_sc.sc_host));
+ len = snmpe_extract(buf, nrecv, udp_send, &(conn->uc_sc));
+ if (len <= 0) {
+ free(conn);
+ return;
+ }
+}
+
+void
+udp_send(struct snmp_conn *sc_conn, void *buf, size_t buflen)
+{
+ struct udp_conn *conn = sc_conn->sc_context;
+ struct snmp_message *msg;
+
+ sendtofrom(conn->uc_fd, buf, buflen, 0,
+    (struct sockaddr *)&(conn->uc_rss), conn->uc_rsslen,
+    (struct sockaddr *)&(conn->uc_lss), conn->uc_lsslen);
+
+ TAILQ_FOREACH(msg, &(conn->uc_sc.sc_messages), sm_entry)
+ msg->sm_conn = NULL;
+ free(conn);
+}

Reply | Threaded
Open this post in threaded view
|

Re: snmpd(8) untangle transport layer and snmp engine

Martijn van Duren-5
ping.

On 2/15/20 12:33 PM, Martijn van Duren wrote:

> This diff does the following:
> - Push the network handling to their own files.
>   This helps with readability and allows for easier addition of other
>   transport layers if we ever choose to (unix socket, ssh, dtls, ...)
> - Allow multiple tcp packets to be handled at the same time.
>   This will helps throughput with multiple backends (agentx).
> - Remove sm_data from the snmp_message. This shaves of ~65k per struct.
>   Right now this isn't that interesting, because we mostly handle 1
>   packet at a time, but as soon as we're leveraging agentx more it
>   is not unlikely that there are multiple packets in flight inside
>   snmpd.
>
> There's only one intentional function change and that is that the
> outpkts is incremented once the data is pushed to the output buffer
> in tcp.c, instead of when the packet has been send to the kernel.
> I think this is reasonable, since the endeavour is to make the transport
> layer agnostic about how snmp works.
>
>
> There's more that I'd like to change, but this diff is large enough as
> is.
>
> OK?
>
> martijn@
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/Makefile,v
> retrieving revision 1.16
> diff -u -p -r1.16 Makefile
> --- Makefile 11 May 2019 17:46:02 -0000 1.16
> +++ Makefile 15 Feb 2020 11:25:41 -0000
> @@ -3,6 +3,7 @@
>  PROG= snmpd
>  MAN= snmpd.8 snmpd.conf.5
>  SRCS= parse.y log.c control.c snmpe.c \
> +    tcp.c udp.c \
>      mps.c trap.c mib.c smi.c kroute.c snmpd.c timer.c \
>      pf.c proc.c usm.c agentx.c traphandler.c util.c
>  
> Index: snmpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.86
> diff -u -p -r1.86 snmpd.h
> --- snmpd.h 2 Jan 2020 10:55:53 -0000 1.86
> +++ snmpd.h 15 Feb 2020 11:25:41 -0000
> @@ -395,23 +395,21 @@ struct pfr_buffer {
>   * daemon structures
>   */
>  
> +struct snmp_conn {
> + char sc_host[HOST_NAME_MAX+1];
> + void *sc_context;
> + TAILQ_HEAD(, snmp_message) sc_messages;
> +};
> +
>  #define MSG_HAS_AUTH(m) (((m)->sm_flags & SNMP_MSGFLAG_AUTH) != 0)
>  #define MSG_HAS_PRIV(m) (((m)->sm_flags & SNMP_MSGFLAG_PRIV) != 0)
>  #define MSG_SECLEVEL(m) ((m)->sm_flags & SNMP_MSGFLAG_SECMASK)
>  #define MSG_REPORT(m) (((m)->sm_flags & SNMP_MSGFLAG_REPORT) != 0)
>  
>  struct snmp_message {
> - int sm_sock;
> - struct sockaddr_storage sm_ss;
> - socklen_t sm_slen;
> - int sm_sock_tcp;
> - struct event sm_sockev;
> - char sm_host[HOST_NAME_MAX+1];
> -
> - struct sockaddr_storage sm_local_ss;
> - socklen_t sm_local_slen;
> + struct snmp_conn *sm_conn;
> + void (*sm_reply)(struct snmp_conn *, void *, size_t);
>  
> - struct ber sm_ber;
>   struct ber_element *sm_req;
>   struct ber_element *sm_resp;
>  
> @@ -423,7 +421,7 @@ struct snmp_message {
>   struct ber_element *sm_last;
>   struct ber_element *sm_end;
>  
> - u_int8_t sm_data[READ_BUF_SIZE];
> + u_int8_t *sm_data;
>   size_t sm_datalen;
>  
>   u_int sm_version;
> @@ -464,6 +462,7 @@ struct snmp_message {
>  
>   struct ber_element *sm_varbind;
>   struct ber_element *sm_varbindresp;
> + TAILQ_ENTRY(snmp_message) sm_entry;
>  };
>  
>  /* Defined in SNMPv2-MIB.txt (RFC 3418) */
> @@ -667,10 +666,19 @@ struct kroute *kroute_getaddr(in_addr_t,
>  struct kif_arp *karp_first(u_short);
>  struct kif_arp *karp_getaddr(struct sockaddr *, u_short, int);
>  
> +/* tcp.c */
> +void tcp_accept(int, short, void *);
> +
> +/* udp.c */
> +void udp_recvmsg(int, short, void *);
> +
>  /* snmpe.c */
>  void snmpe(struct privsep *, struct privsep_proc *);
>  void snmpe_shutdown(void);
>  void snmpe_dispatchmsg(struct snmp_message *);
> +ssize_t snmpe_extract(void *, size_t,
> +    void (*)(struct snmp_conn *, void *, size_t),
> +    struct snmp_conn *);
>  
>  /* trap.c */
>  void trap_init(void);
> Index: snmpe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 snmpe.c
> --- snmpe.c 14 Feb 2020 15:08:46 -0000 1.61
> +++ snmpe.c 15 Feb 2020 11:25:41 -0000
> @@ -42,22 +42,15 @@
>  
>  void snmpe_init(struct privsep *, struct privsep_proc *, void *);
>  int snmpe_parse(struct snmp_message *);
> -void snmpe_tryparse(int, struct snmp_message *);
>  int snmpe_parsevarbinds(struct snmp_message *);
>  void snmpe_response(struct snmp_message *);
>  void snmpe_sig_handler(int sig, short, void *);
>  int snmpe_dispatch_parent(int, struct privsep_proc *, struct imsg *);
>  int snmpe_bind(struct address *);
> -void snmpe_recvmsg(int fd, short, void *);
> -void snmpe_readcb(int fd, short, void *);
> -void snmpe_writecb(int fd, short, void *);
> -void snmpe_acceptcb(int fd, short, void *);
> -void snmpe_prepare_read(struct snmp_message *, int);
>  int snmpe_encode(struct snmp_message *);
>  void snmp_msgfree(struct snmp_message *);
>  
>  struct imsgev *iev_parent;
> -static const struct timeval snmpe_tcp_timeout = { 10, 0 }; /* 10s */
>  
>  static struct privsep_proc procs[] = {
>   { "parent", PROC_PARENT, snmpe_dispatch_parent }
> @@ -111,11 +104,11 @@ snmpe_init(struct privsep *ps, struct pr
>   if (so->s_ipproto == IPPROTO_TCP) {
>   if (listen(so->s_fd, 5) < 0)
>   fatalx("snmpe: failed to listen on socket");
> - event_set(&so->s_ev, so->s_fd, EV_READ, snmpe_acceptcb, so);
> - evtimer_set(&so->s_evt, snmpe_acceptcb, so);
> + event_set(&so->s_ev, so->s_fd, EV_READ, tcp_accept, so);
> + evtimer_set(&so->s_evt, tcp_accept, so);
>   } else {
>   event_set(&so->s_ev, so->s_fd, EV_READ|EV_PERSIST,
> -    snmpe_recvmsg, env);
> +    udp_recvmsg, env);
>   }
>   event_add(&so->s_ev, NULL);
>   }
> @@ -224,7 +217,6 @@ snmpe_parse(struct snmp_message *msg)
>   char *comn;
>   char *flagstr, *ctxname;
>   size_t len;
> - struct sockaddr_storage *ss = &msg->sm_ss;
>   struct ber_element *root = msg->sm_req;
>  
>   msg->sm_errstr = "invalid message";
> @@ -371,17 +363,17 @@ snmpe_parse(struct snmp_message *msg)
>   msg->sm_error = errval;
>   msg->sm_errorindex = erridx;
>  
> - print_host(ss, msg->sm_host, sizeof(msg->sm_host));
>   if (msg->sm_version == SNMP_V3)
>   log_debug("%s: %s: SNMPv3 context %d, flags %#x, "
>      "secmodel %lld, user '%s', ctx-engine %s, ctx-name '%s', "
> -    "request %lld", __func__, msg->sm_host, msg->sm_context,
> -    msg->sm_flags, msg->sm_secmodel, msg->sm_username,
> +    "request %lld", __func__, msg->sm_conn->sc_host,
> +    msg->sm_context, msg->sm_flags, msg->sm_secmodel,
> +    msg->sm_username,
>      tohexstr(msg->sm_ctxengineid, msg->sm_ctxengineid_len),
>      msg->sm_ctxname, msg->sm_request);
>   else
>   log_debug("%s: %s: SNMPv%d '%s' context %d request %lld",
> -    __func__, msg->sm_host, msg->sm_version + 1,
> +    __func__, msg->sm_conn->sc_host, msg->sm_version + 1,
>      msg->sm_community, msg->sm_context, msg->sm_request);
>  
>   return (0);
> @@ -389,8 +381,7 @@ snmpe_parse(struct snmp_message *msg)
>   parsefail:
>   stats->snmp_inasnparseerrs++;
>   fail:
> - print_host(ss, msg->sm_host, sizeof(msg->sm_host));
> - log_debug("%s: %s: %s", __func__, msg->sm_host, msg->sm_errstr);
> + log_debug("%s: %s: %s", __func__, msg->sm_conn->sc_host, msg->sm_errstr);
>   return (-1);
>  }
>  
> @@ -434,7 +425,7 @@ snmpe_parsevarbinds(struct snmp_message
>   else
>   stats->snmp_intotalreqvars++;
>   log_debug("%s: %s: oid %s",
> -    __func__, msg->sm_host,
> +    __func__, msg->sm_conn->sc_host,
>      smi_oid2string(&o, buf, sizeof(buf), 0));
>   break;
>   case 1:
> @@ -511,236 +502,65 @@ snmpe_parsevarbinds(struct snmp_message
>   return (ret);
>   varfail:
>   log_debug("%s: %s: %s, error index %d", __func__,
> -    msg->sm_host, msg->sm_errstr, msg->sm_i);
> +    msg->sm_conn->sc_host, msg->sm_errstr, msg->sm_i);
>   if (msg->sm_error == 0)
>   msg->sm_error = SNMP_ERROR_GENERR;
>   msg->sm_errorindex = msg->sm_i;
>   return (-1);
>  }
>  
> -void
> -snmpe_acceptcb(int fd, short type, void *arg)
> -{
> - struct listen_sock *so = arg;
> - struct sockaddr_storage ss;
> - socklen_t len = sizeof(ss);
> - struct snmp_message *msg;
> - int afd;
> -
> - event_add(&so->s_ev, NULL);
> - if ((type & EV_TIMEOUT))
> - return;
> -
> - if ((afd = accept4(fd, (struct sockaddr *)&ss, &len,
> -    SOCK_NONBLOCK|SOCK_CLOEXEC)) < 0) {
> - /* Pause accept if we are out of file descriptors  */
> - if (errno == ENFILE || errno == EMFILE) {
> - struct timeval evtpause = { 1, 0 };
> -
> - event_del(&so->s_ev);
> - evtimer_add(&so->s_evt, &evtpause);
> - } else if (errno != EAGAIN && errno != EINTR)
> - log_debug("%s: accept4", __func__);
> - return;
> - }
> - if ((msg = calloc(1, sizeof(*msg))) == NULL)
> - goto fail;
> -
> - snmpe_prepare_read(msg, afd);
> - return;
> -fail:
> - free(msg);
> - close(afd);
> - return;
> -}
> -
> -void
> -snmpe_prepare_read(struct snmp_message *msg, int fd)
> -{
> - msg->sm_sock = fd;
> - msg->sm_sock_tcp = 1;
> - event_del(&msg->sm_sockev);
> - event_set(&msg->sm_sockev, fd, EV_READ,
> -    snmpe_readcb, msg);
> - event_add(&msg->sm_sockev, &snmpe_tcp_timeout);
> -}
> -
> -void
> -snmpe_tryparse(int fd, struct snmp_message *msg)
> +ssize_t
> +snmpe_extract(void *buf, size_t buflen,
> +    void (*reply)(struct snmp_conn *, void *, size_t), struct snmp_conn *conn)
>  {
>   struct snmp_stats *stats = &snmpd_env->sc_stats;
> + struct ber ber;
> + struct ber_element *req;
> + struct snmp_message *msg;
> + ssize_t nread;
>  
> - ober_set_application(&msg->sm_ber, smi_application);
> - ober_set_readbuf(&msg->sm_ber, msg->sm_data, msg->sm_datalen);
> - msg->sm_req = ober_read_elements(&msg->sm_ber, NULL);
> - if (msg->sm_req == NULL) {
> + bzero(&ber, sizeof(ber));
> + ober_set_application(&ber, smi_application);
> + ober_set_readbuf(&ber, buf, buflen);
> + if ((req = ober_read_elements(&ber, NULL)) == NULL) {
>   if (errno == ECANCELED) {
> - /* short read; try again */
> - snmpe_prepare_read(msg, fd);
> - return;
> + /* short read */
> + return 0;
>   }
> - goto fail;
> + return -1;
>   }
> + nread = ber.br_rptr - (u_char *)buf;
> +
> + if ((msg = calloc(1, sizeof(*msg))) == NULL) {
> + log_warn("malloc");
> + return -1;
> + }
> + msg->sm_req = req;
> + msg->sm_conn = conn;
> + msg->sm_reply = reply;
> + msg->sm_data = buf;
> + msg->sm_datalen = buflen;
> + TAILQ_INSERT_TAIL(&(conn->sc_messages), msg, sm_entry);
>  
>   if (snmpe_parse(msg) == -1) {
> + msg->sm_data = NULL;
> + msg->sm_datalen = 0;
>   if (msg->sm_usmerr && MSG_REPORT(msg)) {
>   usm_make_report(msg);
>   snmpe_response(msg);
> - return;
> + return nread;
>   } else
>   goto fail;
>   }
> + msg->sm_data = NULL;
> + msg->sm_datalen = 0;
>   stats->snmp_inpkts++;
>  
>   snmpe_dispatchmsg(msg);
> - return;
> - fail:
> - snmp_msgfree(msg);
> - close(fd);
> -}
> -
> -void
> -snmpe_readcb(int fd, short type, void *arg)
> -{
> - struct snmp_message *msg = arg;
> - ssize_t len;
> -
> - if (type == EV_TIMEOUT || msg->sm_datalen >= sizeof(msg->sm_data))
> - goto fail;
> -
> - len = read(fd, msg->sm_data + msg->sm_datalen,
> -    sizeof(msg->sm_data) - msg->sm_datalen);
> - if (len <= 0) {
> - if (errno != EAGAIN && errno != EINTR)
> - goto fail;
> - snmpe_prepare_read(msg, fd);
> - return;
> - }
> -
> - msg->sm_datalen += (size_t)len;
> - snmpe_tryparse(fd, msg);
> - return;
> -
> - fail:
> - snmp_msgfree(msg);
> - close(fd);
> -}
> -
> -void
> -snmpe_writecb(int fd, short type, void *arg)
> -{
> - struct snmp_stats *stats = &snmpd_env->sc_stats;
> - struct snmp_message *msg = arg;
> - struct snmp_message *nmsg;
> - ssize_t len;
> - size_t reqlen;
> - struct ber *ber = &msg->sm_ber;
> -
> - if (type == EV_TIMEOUT)
> - goto fail;
> -
> - len = ber->br_wend - ber->br_wbuf;
> - ber->br_wptr = ber->br_wbuf;
> -
> - log_debug("%s: write fd %d len %zd", __func__, fd, len);
> -
> - len = write(fd, ber->br_wptr, len);
> - if (len == -1) {
> - if (errno == EAGAIN || errno == EINTR)
> - return;
> - else
> - goto fail;
> - }
> -
> - ber->br_wptr += len;
> -
> - if (ber->br_wptr < ber->br_wend) {
> - event_del(&msg->sm_sockev);
> - event_set(&msg->sm_sockev, msg->sm_sock, EV_WRITE,
> -    snmpe_writecb, msg);
> - event_add(&msg->sm_sockev, &snmpe_tcp_timeout);
> - return;
> - }
> -
> - stats->snmp_outpkts++;
> -
> - if ((nmsg = calloc(1, sizeof(*nmsg))) == NULL)
> - goto fail;
> -
> - /*
> - * Reuse the connection.
> - * In case we already read data of the next message, copy it over.
> - */
> - reqlen = ober_calc_len(msg->sm_req);
> - if (msg->sm_datalen > reqlen) {
> - memcpy(nmsg->sm_data, msg->sm_data + reqlen,
> -    msg->sm_datalen - reqlen);
> - nmsg->sm_datalen = msg->sm_datalen - reqlen;
> - snmp_msgfree(msg);
> - snmpe_prepare_read(nmsg, fd);
> - snmpe_tryparse(fd, nmsg);
> - } else {
> - snmp_msgfree(msg);
> - snmpe_prepare_read(nmsg, fd);
> - }
> - return;
> -
> + return nread;
>   fail:
> - close(fd);
>   snmp_msgfree(msg);
> -}
> -
> -void
> -snmpe_recvmsg(int fd, short sig, void *arg)
> -{
> - struct snmpd *env = arg;
> - struct snmp_stats *stats = &env->sc_stats;
> - ssize_t len;
> - struct snmp_message *msg;
> -
> - if ((msg = calloc(1, sizeof(*msg))) == NULL)
> - return;
> -
> - msg->sm_sock = fd;
> - msg->sm_slen = sizeof(msg->sm_ss);
> - if ((len = recvfromto(fd, msg->sm_data, sizeof(msg->sm_data), 0,
> -    (struct sockaddr *)&msg->sm_ss, &msg->sm_slen,
> -    (struct sockaddr *)&msg->sm_local_ss, &msg->sm_local_slen)) < 1) {
> - free(msg);
> - return;
> - }
> -
> - stats->snmp_inpkts++;
> - msg->sm_datalen = (size_t)len;
> -
> - bzero(&msg->sm_ber, sizeof(msg->sm_ber));
> - ober_set_application(&msg->sm_ber, smi_application);
> - ober_set_readbuf(&msg->sm_ber, msg->sm_data, msg->sm_datalen);
> -
> - msg->sm_req = ober_read_elements(&msg->sm_ber, NULL);
> - if (msg->sm_req == NULL) {
> - stats->snmp_inasnparseerrs++;
> - snmp_msgfree(msg);
> - return;
> - }
> -
> -#ifdef DEBUG
> - fprintf(stderr, "recv msg:\n");
> - smi_debug_elements(msg->sm_req);
> -#endif
> -
> - if (snmpe_parse(msg) == -1) {
> - if (msg->sm_usmerr != 0 && MSG_REPORT(msg)) {
> - usm_make_report(msg);
> - snmpe_response(msg);
> - return;
> - } else {
> - snmp_msgfree(msg);
> - return;
> - }
> - }
> -
> - snmpe_dispatchmsg(msg);
> + return -1;
>  }
>  
>  void
> @@ -761,6 +581,7 @@ snmpe_response(struct snmp_message *msg)
>   struct snmp_stats *stats = &snmpd_env->sc_stats;
>   u_int8_t *ptr = NULL;
>   ssize_t len;
> + struct ber ber;
>  
>   if (msg->sm_varbindresp == NULL && msg->sm_pduend != NULL)
>   msg->sm_varbindresp = ober_unlink_elements(msg->sm_pduend);
> @@ -790,25 +611,16 @@ snmpe_response(struct snmp_message *msg)
>   if (snmpe_encode(msg) < 0)
>   goto done;
>  
> - len = ober_write_elements(&msg->sm_ber, msg->sm_resp);
> - if (ober_get_writebuf(&msg->sm_ber, (void *)&ptr) == -1)
> + bzero(&ber, sizeof(ber));
> + len = ober_write_elements(&ber, msg->sm_resp);
> + if (ober_get_writebuf(&ber, (void *)&ptr) == -1)
>   goto done;
>  
>   usm_finalize_digest(msg, ptr, len);
> - if (msg->sm_sock_tcp) {
> - event_del(&msg->sm_sockev);
> - event_set(&msg->sm_sockev, msg->sm_sock, EV_WRITE,
> -    snmpe_writecb, msg);
> - event_add(&msg->sm_sockev, &snmpe_tcp_timeout);
> - return;
> - } else {
> - len = sendtofrom(msg->sm_sock, ptr, len, 0,
> -    (struct sockaddr *)&msg->sm_ss, msg->sm_slen,
> -    (struct sockaddr *)&msg->sm_local_ss, msg->sm_local_slen);
> - if (len != -1)
> - stats->snmp_outpkts++;
> + if (msg->sm_conn != NULL) {
> + msg->sm_reply(msg->sm_conn, ptr, len);
> + stats->snmp_outpkts++;
>   }
> -
>   done:
>   snmp_msgfree(msg);
>  }
> @@ -816,12 +628,12 @@ snmpe_response(struct snmp_message *msg)
>  void
>  snmp_msgfree(struct snmp_message *msg)
>  {
> - event_del(&msg->sm_sockev);
> - ober_free(&msg->sm_ber);
>   if (msg->sm_req != NULL)
>   ober_free_elements(msg->sm_req);
>   if (msg->sm_resp != NULL)
>   ober_free_elements(msg->sm_resp);
> + if (msg->sm_conn != NULL)
> + TAILQ_REMOVE(&(msg->sm_conn->sc_messages), msg, sm_entry);
>   free(msg);
>  }
>  
> Index: tcp.c
> ===================================================================
> RCS file: tcp.c
> diff -N tcp.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ tcp.c 15 Feb 2020 11:25:41 -0000
> @@ -0,0 +1,214 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2020 Martijn van Duren <[hidden email]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/socket.h>
> +
> +#include <errno.h>
> +#include <event.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include "snmpd.h"
> +
> +struct tcp_conn {
> + int tc_fd;
> + char *tc_rbuf;
> + size_t tc_rbuflen;
> + size_t tc_rbufsize;
> + char *tc_wbuf;
> + size_t tc_wbuflen;
> + size_t tc_wbufsize;
> + struct event tc_rev;
> + struct event tc_wev;
> + struct snmp_conn tc_sc;
> +};
> +
> +#define TCP_MINBUF 8192
> +
> +static const struct timeval tcp_timeout = { 10, 0 }; /* 10s */
> +
> +void tcp_read(int, short, void *);
> +void tcp_send(struct snmp_conn *, void *, size_t);
> +void tcp_write(int, short, void *);
> +void tcp_free(struct tcp_conn *);
> +
> +void
> +tcp_accept(int lfd, short event, void *arg)
> +{
> + struct listen_sock *so = arg;
> + struct sockaddr_storage ss;
> + socklen_t len = sizeof(ss);
> + struct tcp_conn *conn;
> + int fd;
> +
> + event_add(&so->s_ev, NULL);
> + evtimer_del(&so->s_evt);
> + if ((event & EV_TIMEOUT))
> + return;
> +
> + if ((fd = accept4(lfd, (struct sockaddr *)&ss, &len,
> +    SOCK_NONBLOCK|SOCK_CLOEXEC)) < 0) {
> + /* Pause accept if we are out of file descriptors  */
> + if (errno == ENFILE || errno == EMFILE) {
> + struct timeval evtpause = { 1, 0 };
> +
> + event_del(&so->s_ev);
> + evtimer_add(&so->s_evt, &evtpause);
> + } else if (errno != EAGAIN && errno != EINTR)
> + log_debug("%s: accept4", __func__);
> + return;
> + }
> + if ((conn = calloc(1, sizeof(*conn))) == NULL) {
> + close(fd);
> + return;
> + }
> + conn->tc_fd = fd;
> + print_host(&ss, conn->tc_sc.sc_host, sizeof(conn->tc_sc.sc_host));
> + conn->tc_sc.sc_context = conn;
> +
> + event_set(&(conn->tc_rev), fd, EV_READ, tcp_read, conn);
> + event_set(&(conn->tc_wev), fd, EV_WRITE, tcp_write, conn);
> + event_add(&(conn->tc_rev), &tcp_timeout);
> +
> + TAILQ_INIT(&(conn->tc_sc.sc_messages));
> +
> + return;
> +}
> +
> +void
> +tcp_read(int fd, short event, void *cookie)
> +{
> + struct tcp_conn *conn = cookie;
> + ssize_t len;
> + char *tmpbuf;
> +
> + if (event == EV_TIMEOUT) {
> + tcp_free(conn);
> + return;
> + }
> +
> + if (conn->tc_rbufsize - conn->tc_rbuflen == 0) {
> + tmpbuf = recallocarray(conn->tc_rbuf, conn->tc_rbufsize,
> +    conn->tc_rbufsize + TCP_MINBUF, 1);
> + if (tmpbuf == NULL) {
> + log_warn("malloc");
> + tcp_free(conn);
> + return;
> + }
> + conn->tc_rbuf = tmpbuf;
> + conn->tc_rbufsize += TCP_MINBUF;
> + }
> +
> + len = read(fd, conn->tc_rbuf + conn->tc_rbuflen,
> +    conn->tc_rbufsize - conn->tc_rbuflen);
> + if (len <= 0) {
> + if (errno != EAGAIN && errno != EINTR) {
> + tcp_free(conn);
> + return;
> + }
> + event_add(&(conn->tc_rev), &tcp_timeout);
> + return;
> + }
> +
> + conn->tc_rbuflen += (size_t)len;
> +
> + while (1) {
> + len = snmpe_extract(conn->tc_rbuf, conn->tc_rbuflen, tcp_send,
> +    &(conn->tc_sc));
> + switch (len) {
> + case -1:
> + tcp_free(conn);
> + return;
> + case 0:
> + event_add(&(conn->tc_rev), &tcp_timeout);
> + return;
> + default:
> + memmove(conn->tc_rbuf, conn->tc_rbuf + len,
> +    conn->tc_rbuflen - len);
> + conn->tc_rbuflen -= len;
> + }
> + }
> + event_add(&(conn->tc_rev), &tcp_timeout);
> +}
> +
> +void
> +tcp_send(struct snmp_conn *sc_conn, void *buf, size_t buflen)
> +{
> + struct tcp_conn *conn = sc_conn->sc_context;
> + char *tmpbuf;
> +
> + if (conn->tc_wbufsize - conn->tc_wbuflen < buflen) {
> + tmpbuf = recallocarray(conn->tc_wbuf, conn->tc_wbufsize,
> +    conn->tc_wbuflen + buflen, 1);
> + if (tmpbuf == NULL) {
> + log_warn("malloc");
> + tcp_free(conn);
> + return;
> + }
> + conn->tc_wbuf = tmpbuf;
> + conn->tc_wbufsize = conn->tc_wbuflen + buflen;
> + }
> +
> + memcpy(conn->tc_wbuf + conn->tc_wbuflen, buf, buflen);
> + conn->tc_wbuflen += buflen;
> +
> + event_add(&(conn->tc_wev), NULL);
> +}
> +
> +void
> +tcp_write(int fd, short event, void *cookie)
> +{
> + struct tcp_conn *conn = cookie;
> + ssize_t nwrite;
> +
> + if ((nwrite = write(conn->tc_fd, conn->tc_wbuf, conn->tc_wbuflen)) == -1) {
> + if (errno != EAGAIN && errno != EINTR) {
> + log_warn("write");
> + tcp_free(conn);
> + return;
> + }
> + event_add(&(conn->tc_wev), NULL);
> + return;
> + }
> +
> + conn->tc_wbuflen -= nwrite;
> + if (conn->tc_wbuflen > 0) {
> + memmove(conn->tc_wbuf, conn->tc_wbuf + nwrite,
> +    conn->tc_wbuflen);
> + event_add(&(conn->tc_wev), NULL);
> + }
> +}
> +
> +void
> +tcp_free(struct tcp_conn *conn)
> +{
> + struct snmp_message *msg;
> +
> + TAILQ_FOREACH(msg, &(conn->tc_sc.sc_messages), sm_entry)
> + msg->sm_conn = NULL;
> + close(conn->tc_fd);
> +
> + free(conn->tc_rbuf);
> + free(conn->tc_wbuf);
> +
> + event_del(&(conn->tc_rev));
> + event_del(&(conn->tc_wev));
> +
> + free(conn);
> +}
> Index: udp.c
> ===================================================================
> RCS file: udp.c
> diff -N udp.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ udp.c 15 Feb 2020 11:25:41 -0000
> @@ -0,0 +1,82 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2020 Martijn van Duren <[hidden email]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/socket.h>
> +
> +#include <event.h>
> +#include <stdlib.h>
> +
> +#include "snmpd.h"
> +
> +struct udp_conn {
> + int uc_fd;
> + struct sockaddr_storage uc_lss;
> + socklen_t uc_lsslen;
> + struct sockaddr_storage uc_rss;
> + socklen_t uc_rsslen;
> + struct snmp_conn uc_sc;
> +};
> +
> +void udp_send(struct snmp_conn *, void *, size_t);
> +
> +void
> +udp_recvmsg(int fd, short event, void *cookie)
> +{
> + ssize_t nrecv, len;
> + char buf[READ_BUF_SIZE];
> + struct udp_conn *conn;
> +
> + if ((conn = calloc(1, sizeof(*conn))) == NULL)
> + return;
> +
> + conn->uc_fd = fd;
> + conn->uc_sc.sc_context = conn;
> + conn->uc_rsslen = sizeof(conn->uc_rss);
> + conn->uc_lsslen = sizeof(conn->uc_lss);
> + TAILQ_INIT(&(conn->uc_sc.sc_messages));
> +
> + if ((nrecv = recvfromto(fd, buf, sizeof(buf), 0,
> +    (struct sockaddr *)&conn->uc_rss, &(conn->uc_rsslen),
> +    (struct sockaddr *)&conn->uc_lss, &(conn->uc_lsslen))) < 1) {
> + free(conn);
> + return;
> + }
> +
> + print_host(&(conn->uc_rss), conn->uc_sc.sc_host,
> +    sizeof(conn->uc_sc.sc_host));
> + len = snmpe_extract(buf, nrecv, udp_send, &(conn->uc_sc));
> + if (len <= 0) {
> + free(conn);
> + return;
> + }
> +}
> +
> +void
> +udp_send(struct snmp_conn *sc_conn, void *buf, size_t buflen)
> +{
> + struct udp_conn *conn = sc_conn->sc_context;
> + struct snmp_message *msg;
> +
> + sendtofrom(conn->uc_fd, buf, buflen, 0,
> +    (struct sockaddr *)&(conn->uc_rss), conn->uc_rsslen,
> +    (struct sockaddr *)&(conn->uc_lss), conn->uc_lsslen);
> +
> + TAILQ_FOREACH(msg, &(conn->uc_sc.sc_messages), sm_entry)
> + msg->sm_conn = NULL;
> + free(conn);
> +}
>