On Sun, Aug 09, 2020 at 04:10:13PM +0200, Martijn van Duren wrote:
> I still want to move udp/tcp handling out of snmpe, so that there's
> better layering, but my previous diff never got any response and might
> do with some more polishing.
>
> For now, lets remove listen_sock from snmpd, since there's a 1:1
> correlation with struct address. This saves a couple of callocs during
> startup and 18 LoC.
>
> OK?
OK jan@
> martijn@
>
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
> retrieving revision 1.58
> diff -u -p -r1.58 parse.y
> --- parse.y 30 Jun 2020 17:11:49 -0000 1.58
> +++ parse.y 9 Aug 2020 14:08:18 -0000
> @@ -997,7 +997,6 @@ parse_config(const char *filename, u_int
> conf->sc_flags = flags;
> conf->sc_confpath = filename;
> TAILQ_INIT(&conf->sc_addresses);
> - TAILQ_INIT(&conf->sc_sockets);
> strlcpy(conf->sc_rdcommunity, "public", SNMPD_MAXCOMMUNITYLEN);
> strlcpy(conf->sc_rwcommunity, "private", SNMPD_MAXCOMMUNITYLEN);
> strlcpy(conf->sc_trcommunity, "public", SNMPD_MAXCOMMUNITYLEN);
> Index: snmpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.88
> diff -u -p -r1.88 snmpd.h
> --- snmpd.h 8 Aug 2020 13:39:57 -0000 1.88
> +++ snmpd.h 9 Aug 2020 14:08:18 -0000
> @@ -482,6 +482,9 @@ struct address {
> struct sockaddr_storage ss;
> in_port_t port;
> int ipproto;
> + int fd;
> + struct event ev;
> + struct event evt;
>
> TAILQ_ENTRY(address) entry;
>
> @@ -492,15 +495,6 @@ struct address {
> };
> TAILQ_HEAD(addresslist, address);
>
> -struct listen_sock {
> - int s_fd;
> - int s_ipproto;
> - struct event s_ev;
> - struct event s_evt;
> - TAILQ_ENTRY(listen_sock) entry;
> -};
> -TAILQ_HEAD(socklist, listen_sock);
> -
> enum usmauth {
> AUTH_NONE = 0,
> AUTH_MD5, /* HMAC-MD5-96, RFC3414 */
> @@ -545,7 +539,6 @@ struct snmpd {
>
> const char *sc_confpath;
> struct addresslist sc_addresses;
> - struct socklist sc_sockets;
> struct timeval sc_starttime;
> u_int32_t sc_engine_boots;
>
> Index: snmpe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 snmpe.c
> --- snmpe.c 8 Aug 2020 13:39:57 -0000 1.63
> +++ snmpe.c 9 Aug 2020 14:08:18 -0000
> @@ -68,7 +68,6 @@ snmpe(struct privsep *ps, struct privsep
> {
> struct snmpd *env = ps->ps_env;
> struct address *h;
> - struct listen_sock *so;
> #ifdef DEBUG
> char buf[BUFSIZ];
> struct oid *oid;
> @@ -82,14 +81,9 @@ snmpe(struct privsep *ps, struct privsep
> #endif
>
> /* bind SNMP UDP/TCP sockets */
> - TAILQ_FOREACH(h, &env->sc_addresses, entry) {
> - if ((so = calloc(1, sizeof(*so))) == NULL)
> - fatal("snmpe: %s", __func__);
> - if ((so->s_fd = snmpe_bind(h)) == -1)
> + TAILQ_FOREACH(h, &env->sc_addresses, entry)
> + if ((h->fd = snmpe_bind(h)) == -1)
> fatal("snmpe: failed to bind SNMP socket");
> - so->s_ipproto = h->ipproto;
> - TAILQ_INSERT_TAIL(&env->sc_sockets, so, entry);
> - }
>
> proc_run(ps, p, procs, nitems(procs), snmpe_init, NULL);
> }
> @@ -99,7 +93,7 @@ void
> snmpe_init(struct privsep *ps, struct privsep_proc *p, void *arg)
> {
> struct snmpd *env = ps->ps_env;
> - struct listen_sock *so;
> + struct address *h;
>
> kr_init();
> trap_init();
> @@ -107,17 +101,17 @@ snmpe_init(struct privsep *ps, struct pr
> usm_generate_keys();
>
> /* listen for incoming SNMP UDP/TCP messages */
> - TAILQ_FOREACH(so, &env->sc_sockets, entry) {
> - if (so->s_ipproto == IPPROTO_TCP) {
> - if (listen(so->s_fd, 5) < 0)
> + TAILQ_FOREACH(h, &env->sc_addresses, entry) {
> + if (h->ipproto == IPPROTO_TCP) {
> + if (listen(h->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(&h->ev, h->fd, EV_READ, snmpe_acceptcb, h);
> + evtimer_set(&h->evt, snmpe_acceptcb, h);
> } else {
> - event_set(&so->s_ev, so->s_fd, EV_READ|EV_PERSIST,
> + event_set(&h->ev, h->fd, EV_READ|EV_PERSIST,
> snmpe_recvmsg, env);
> }
> - event_add(&so->s_ev, NULL);
> + event_add(&h->ev, NULL);
> }
>
> /* no filesystem visibility */
> @@ -130,13 +124,13 @@ snmpe_init(struct privsep *ps, struct pr
> void
> snmpe_shutdown(void)
> {
> - struct listen_sock *so;
> + struct address *h;
>
> - TAILQ_FOREACH(so, &snmpd_env->sc_sockets, entry) {
> - event_del(&so->s_ev);
> - if (so->s_ipproto == IPPROTO_TCP)
> - event_del(&so->s_evt);
> - close(so->s_fd);
> + TAILQ_FOREACH(h, &snmpd_env->sc_addresses, entry) {
> + event_del(&h->ev);
> + if (h->ipproto == IPPROTO_TCP)
> + event_del(&h->evt);
> + close(h->fd);
> }
> kr_shutdown();
> }
> @@ -511,13 +505,13 @@ snmpe_parsevarbinds(struct snmp_message
> void
> snmpe_acceptcb(int fd, short type, void *arg)
> {
> - struct listen_sock *so = arg;
> - struct sockaddr_storage ss;
> - socklen_t len = sizeof(ss);
> + struct address *h = arg;
> + struct sockaddr_storage ss;
> + socklen_t len = sizeof(ss);
> struct snmp_message *msg;
> int afd;
>
> - event_add(&so->s_ev, NULL);
> + event_add(&h->ev, NULL);
> if ((type & EV_TIMEOUT))
> return;
>
> @@ -527,8 +521,8 @@ snmpe_acceptcb(int fd, short type, void
> if (errno == ENFILE || errno == EMFILE) {
> struct timeval evtpause = { 1, 0 };
>
> - event_del(&so->s_ev);
> - evtimer_add(&so->s_evt, &evtpause);
> + event_del(&h->ev);
> + evtimer_add(&h->evt, &evtpause);
> } else if (errno != EAGAIN && errno != EINTR)
> log_debug("%s: accept4", __func__);
> return;
> Index: traphandler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 traphandler.c
> --- traphandler.c 11 Mar 2020 06:53:42 -0000 1.16
> +++ traphandler.c 9 Aug 2020 14:08:18 -0000
> @@ -74,17 +74,13 @@ traphandler(struct privsep *ps, struct p
> {
> struct snmpd *env = ps->ps_env;
> struct address *h;
> - struct listen_sock *so;
>
> if (env->sc_traphandler) {
> TAILQ_FOREACH(h, &env->sc_addresses, entry) {
> if (h->ipproto != IPPROTO_UDP)
> continue;
> - if ((so = calloc(1, sizeof(*so))) == NULL)
> - fatal("%s", __func__);
> - if ((so->s_fd = traphandler_bind(h)) == -1)
> + if ((h->fd = traphandler_bind(h)) == -1)
> fatal("could not create trap listener socket");
> - TAILQ_INSERT_TAIL(&env->sc_sockets, so, entry);
> }
> }
>
> @@ -95,7 +91,7 @@ void
> traphandler_init(struct privsep *ps, struct privsep_proc *p, void *arg)
> {
> struct snmpd *env = ps->ps_env;
> - struct listen_sock *so;
> + struct address *h;
>
> if (pledge("stdio id proc recvfd exec", NULL) == -1)
> fatal("pledge");
> @@ -104,10 +100,10 @@ traphandler_init(struct privsep *ps, str
> return;
>
> /* listen for SNMP trap messages */
> - TAILQ_FOREACH(so, &env->sc_sockets, entry) {
> - event_set(&so->s_ev, so->s_fd, EV_READ|EV_PERSIST,
> + TAILQ_FOREACH(h, &env->sc_addresses, entry) {
> + event_set(&h->ev, h->fd, EV_READ|EV_PERSIST,
> traphandler_recvmsg, ps);
> - event_add(&so->s_ev, NULL);
> + event_add(&h->ev, NULL);
> }
> }
>
> @@ -141,11 +137,11 @@ traphandler_bind(struct address *addr)
> void
> traphandler_shutdown(void)
> {
> - struct listen_sock *so;
> + struct address *h;
>
> - TAILQ_FOREACH(so, &snmpd_env->sc_sockets, entry) {
> - event_del(&so->s_ev);
> - close(so->s_fd);
> + TAILQ_FOREACH(h, &snmpd_env->sc_addresses, entry) {
> + event_del(&h->ev);
> + close(h->fd);
> }
> }
>
>
