Implement a send queue for each control fd.

Only one call to write(2) should be performed for each POLLOUT check
via poll(2) so we should never see EAGAIN when writing to control
sockets ever again.
Each fd queue is limited to 100 entries so we don't OOM with badly
written control subscribers.

diff --git a/control.c b/control.c
index 8433ce25aa430bef86ce1b551e71510072fcbf06..c129ae71d2f38d3839ba561b27693e9dabce4b39 100644 (file)
--- a/control.c
+++ b/control.c
@@ -34,6 +34,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <syslog.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -48,10 +49,48 @@
             (sizeof(*(su)) - sizeof((su)->sun_path) + strlen((su)->sun_path))
 #endif
 
+static void
+control_queue_purge(struct dhcpcd_ctx *ctx, char *data)
+{
+       int found;
+       struct fd_list *fp;
+       struct fd_data *fpd;
+
+       /* If no other fd queue has the same data, free it */
+       found = 0;
+       TAILQ_FOREACH(fp, &ctx->control_fds, next) {
+               TAILQ_FOREACH(fpd, &fp->queue, next) {
+                       if (fpd->data == data) {
+                               found = 1;
+                               break;
+                       }
+               }
+       }
+       if (!found)
+               free(data);
+}
+
+static void
+control_queue_free(struct fd_list *fd)
+{
+       struct fd_data *fdp;
+
+       while ((fdp = TAILQ_FIRST(&fd->queue))) {
+               TAILQ_REMOVE(&fd->queue, fdp, next);
+               if (fdp->freeit)
+                       control_queue_purge(fd->ctx, fdp->data);
+               free(fdp);
+       }
+       while ((fdp = TAILQ_FIRST(&fd->free_queue))) {
+               TAILQ_REMOVE(&fd->free_queue, fdp, next);
+               free(fdp);
+       }
+}
+
 static void
 control_handle_data(void *arg)
 {
-       struct fd_list *l = arg, *lp, *last;
+       struct fd_list *l = arg;
        char buffer[1024], *e, *p, *argvp[255], **ap, *a;
        ssize_t bytes;
        size_t len;
@@ -61,20 +100,11 @@ control_handle_data(void *arg)
        if (bytes == -1 || bytes == 0) {
                /* Control was closed or there was an error.
                 * Remove it from our list. */
-               last = NULL;
-               for (lp = l->ctx->control_fds; lp; lp = lp->next) {
-                       if (lp == l) {
-                               eloop_event_delete(lp->ctx->eloop, lp->fd, 0);
-                               close(lp->fd);
-                               if (last == NULL)
-                                       lp->ctx->control_fds = lp->next;
-                               else
-                                       last->next = lp->next;
-                               free(lp);
-                               break;
-                       }
-                       last = lp;
-               }
+               TAILQ_REMOVE(&l->ctx->control_fds, l, next);
+               eloop_event_delete(l->ctx->eloop, l->fd, 0);
+               close(l->fd);
+               control_queue_free(l);
+               free(l);
                return;
        }
        buffer[bytes] = '\0';
@@ -101,7 +131,8 @@ control_handle_data(void *arg)
                        }
                }
                *ap = NULL;
-               dhcpcd_handleargs(l->ctx, l, argc, argvp);
+               if (dhcpcd_handleargs(l->ctx, l, argc, argvp) == -1)
+                       syslog(LOG_ERR, "%s: dhcpcd_handleargs: %m", __func__);
        }
 }
 
@@ -135,8 +166,9 @@ control_handle(void *arg)
                l->ctx = ctx;
                l->fd = fd;
                l->listener = 0;
-               l->next = ctx->control_fds;
-               ctx->control_fds = l;
+               TAILQ_INIT(&l->queue);
+               TAILQ_INIT(&l->free_queue);
+               TAILQ_INSERT_TAIL(&ctx->control_fds, l, next);
                eloop_event_add(ctx->eloop, l->fd,
                    control_handle_data, l, NULL, NULL);
        } else
@@ -215,21 +247,17 @@ control_stop(struct dhcpcd_ctx *ctx)
        if (ctx->control_fd == -1)
                return 0;
        eloop_event_delete(ctx->eloop, ctx->control_fd, 0);
-       if (shutdown(ctx->control_fd, SHUT_RDWR) == -1)
-               retval = 1;
        close(ctx->control_fd);
        ctx->control_fd = -1;
        if (unlink(ctx->control_sock) == -1)
                retval = -1;
 
-       l = ctx->control_fds;
-       while (l != NULL) {
-               ctx->control_fds = l->next;
+       while ((l = TAILQ_FIRST(&ctx->control_fds))) {
+               TAILQ_REMOVE(&ctx->control_fds, l, next);
                eloop_event_delete(ctx->eloop, l->fd, 0);
-               shutdown(l->fd, SHUT_RDWR);
                close(l->fd);
+               control_queue_free(l);
                free(l);
-               l = ctx->control_fds;
        }
 
        return retval;
@@ -275,6 +303,65 @@ control_send(struct dhcpcd_ctx *ctx, int argc, char * const *argv)
        return write(ctx->control_fd, buffer, len);
 }
 
+static void
+control_writeone(void *arg)
+{
+       struct fd_list *fd;
+       struct iovec iov[2];
+       struct fd_data *data;
+
+       fd = arg;
+       data = TAILQ_FIRST(&fd->queue);
+       iov[0].iov_base = &data->data_len;
+       iov[0].iov_len = sizeof(size_t);
+       iov[1].iov_base = data->data;
+       iov[1].iov_len = data->data_len;
+       if (writev(fd->fd, iov, 2) == -1) {
+               syslog(LOG_ERR, "%s: writev: %m", __func__);
+               return;
+       }
+
+       TAILQ_REMOVE(&fd->queue, data, next);
+       if (data->freeit)
+               control_queue_purge(fd->ctx, data->data);
+       data->data = NULL; /* safety */
+       data->data_len = 0;
+       TAILQ_INSERT_TAIL(&fd->free_queue, data, next);
+
+       if (TAILQ_FIRST(&fd->queue) == NULL)
+               eloop_event_delete(fd->ctx->eloop, fd->fd, 1);
+}
+
+int
+control_queue(struct fd_list *fd, char *data, size_t data_len, uint8_t fit)
+{
+       struct fd_data *d;
+       size_t n;
+
+       d = TAILQ_FIRST(&fd->free_queue);
+       if (d) {
+               TAILQ_REMOVE(&fd->free_queue, d, next);
+       } else {
+               n = 0;
+               TAILQ_FOREACH(d, &fd->queue, next) {
+                       if (++n == CONTROL_QUEUE_MAX) {
+                               errno = ENOBUFS;
+                               return -1;
+                       }
+               }
+               d = malloc(sizeof(*d));
+               if (d == NULL)
+                       return -1;
+       }
+       d->data = data;
+       d->data_len = data_len;
+       d->freeit = fit;
+       TAILQ_INSERT_TAIL(&fd->queue, d, next);
+       eloop_event_add(fd->ctx->eloop, fd->fd,
+           NULL, NULL, control_writeone, fd);
+       return 0;
+}
+
 void
 control_close(struct dhcpcd_ctx *ctx)
 {

diff --git a/control.h b/control.h
index 3204b7aca930ced0e09c95b322b439c4658233d9..7f5559772a742acfc94d1e6c611302d5dcd0da2d 100644 (file)
--- a/control.h
+++ b/control.h
@@ -30,17 +30,32 @@
 
 #include "dhcpcd.h"
 
+/* Limit queue size per fd */
+#define CONTROL_QUEUE_MAX      100
+
+struct fd_data {
+       TAILQ_ENTRY(fd_data) next;
+       char *data;
+       size_t data_len;
+       uint8_t freeit;
+};
+TAILQ_HEAD(fd_data_head, fd_data);
+
 struct fd_list {
-       struct fd_list *next;
+       TAILQ_ENTRY(fd_list) next;
        struct dhcpcd_ctx *ctx;
        int fd;
        int listener;
+       struct fd_data_head queue;
+       struct fd_data_head free_queue;
 };
+TAILQ_HEAD(fd_list_head, fd_list);
 
 int control_start(struct dhcpcd_ctx *, const char *);
 int control_stop(struct dhcpcd_ctx *);
 int control_open(struct dhcpcd_ctx *, const char *);
 ssize_t control_send(struct dhcpcd_ctx *, int, char * const *);
+int control_queue(struct fd_list *fd, char *data, size_t data_len, uint8_t fit);
 void control_close(struct dhcpcd_ctx *ctx);
 
 #endif

diff --git a/dhcpcd.c b/dhcpcd.c
index 23142094019fec5daec86dab485d95c04281bff3..13ece6a96533f1fa49116fb443fa4e86836d3249 100644 (file)
--- a/dhcpcd.c
+++ b/dhcpcd.c
@@ -1046,42 +1046,6 @@ signal_init(void (*func)(int, siginfo_t *, void *), sigset_t *oldset)
 }
 #endif
 
-static void
-dhcpcd_version(void *arg)
-{
-       struct fd_list *fd = arg;
-       size_t len;
-       struct iovec iov[2];
-
-       len = strlen(VERSION) + 1;
-       iov[0].iov_base = &len;
-       iov[0].iov_len = sizeof(ssize_t);
-       iov[1].iov_base = UNCONST(VERSION);
-       iov[1].iov_len = len;
-       if (writev(fd->fd, iov, 2) == -1)
-               syslog(LOG_ERR, "%s: writev: %m", __func__);
-       else
-               eloop_event_delete(fd->ctx->eloop, fd->fd, 1);
-}
-
-static void
-dhcpcd_getconfigfile(void *arg)
-{
-       struct fd_list *fd = arg;
-       size_t len;
-       struct iovec iov[2];
-
-       len = strlen(fd->ctx->cffile) + 1;
-       iov[0].iov_base = &len;
-       iov[0].iov_len = sizeof(ssize_t);
-       iov[1].iov_base = UNCONST(fd->ctx->cffile);
-       iov[1].iov_len = len;
-       if (writev(fd->fd, iov, 2) == -1)
-               syslog(LOG_ERR, "%s: writev: %m", __func__);
-       else
-               eloop_event_delete(fd->ctx->eloop, fd->fd, 1);
-}
-
 static void
 dhcpcd_getinterfaces(void *arg)
 {
@@ -1101,11 +1065,11 @@ dhcpcd_getinterfaces(void *arg)
        }
        if (write(fd->fd, &len, sizeof(len)) != sizeof(len))
                return;
+       eloop_event_delete(fd->ctx->eloop, fd->fd, 1);
        TAILQ_FOREACH(ifp, fd->ctx->ifaces, next) {
-               if (send_interface(fd->fd, ifp) == -1)
+               if (send_interface(fd, ifp) == -1)
                        syslog(LOG_ERR, "send_interface %d: %m", fd->fd);
        }
-       eloop_event_delete(fd->ctx->eloop, fd->fd, 1);
 }
 
 int
@@ -1124,13 +1088,11 @@ dhcpcd_handleargs(struct dhcpcd_ctx *ctx, struct fd_list *fd,
                 * expected reply we should be safely able just to change the
                 * write callback on the fd */
                if (strcmp(*argv, "--version") == 0) {
-                       eloop_event_add(fd->ctx->eloop, fd->fd, NULL, NULL,
-                           dhcpcd_version, fd);
-                       return 0;
+                       return control_queue(fd, UNCONST(VERSION),
+                           strlen(VERSION) + 1, 0);
                } else if (strcmp(*argv, "--getconfigfile") == 0) {
-                       eloop_event_add(fd->ctx->eloop, fd->fd, NULL, NULL,
-                           dhcpcd_getconfigfile, fd);
-                       return 0;
+                       return control_queue(fd, UNCONST(fd->ctx->cffile),
+                           strlen(fd->ctx->cffile) + 1, 0);
                } else if (strcmp(*argv, "--getinterfaces") == 0) {
                        eloop_event_add(fd->ctx->eloop, fd->fd, NULL, NULL,
                            dhcpcd_getinterfaces, fd);
@@ -1146,10 +1108,8 @@ dhcpcd_handleargs(struct dhcpcd_ctx *ctx, struct fd_list *fd,
        for (opt = 0; opt < argc; opt++)
                len += strlen(argv[opt]) + 1;
        tmp = malloc(len);
-       if (tmp == NULL) {
-               syslog(LOG_ERR, "%s: %m", __func__);
+       if (tmp == NULL)
                return -1;
-       }
        p = tmp;
        for (opt = 0; opt < argc; opt++) {
                l = strlen(argv[opt]);
@@ -1256,6 +1216,7 @@ main(int argc, char **argv)
        ifo = NULL;
        ctx.cffile = CONFIG;
        ctx.pid_fd = ctx.control_fd = ctx.link_fd = -1;
+       TAILQ_INIT(&ctx.control_fds);
 #ifdef PLUGIN_DEV
        ctx.dev_fd = -1;
 #endif

diff --git a/dhcpcd.h b/dhcpcd.h
index 3aa59aed0fc0a89a44bbea73fa098acc0f1e8544..f5340d152f06ef8dea9a4637e6f8759eee9473ce 100644 (file)
--- a/dhcpcd.h
+++ b/dhcpcd.h
@@ -95,7 +95,7 @@ struct dhcpcd_ctx {
        struct eloop_ctx *eloop;
 
        int control_fd;
-       struct fd_list *control_fds;
+       struct fd_list_head control_fds;
        char control_sock[sizeof(CONTROLSOCKET) + IF_NAMESIZE];
        gid_t control_group;
 

diff --git a/script.c b/script.c
index 63daa6a45d4e77b3f189f6f42e08e255a142bf9d..effa81d7c8356919ec7b453a615899fdc26155d4 100644 (file)
--- a/script.c
+++ b/script.c
@@ -512,34 +512,29 @@ eexit:
        return -1;
 }
 
-static ssize_t
-send_interface1(int fd, const struct interface *iface, const char *reason)
+static int
+send_interface1(struct fd_list *fd, const struct interface *iface,
+    const char *reason)
 {
        char **env, **ep, *s;
        size_t elen;
-       struct iovec iov[2];
-       ssize_t retval;
+       int retval;
 
        if (make_env(iface, reason, &env) == -1)
                return -1;
        elen = (size_t)arraytostr((const char *const *)env, &s);
        if ((ssize_t)elen == -1)
                return -1;
-       iov[0].iov_base = &elen;
-       iov[0].iov_len = sizeof(elen);
-       iov[1].iov_base = s;
-       iov[1].iov_len = elen;
-       retval = writev(fd, iov, 2);
+       retval = control_queue(fd, s, elen, 1);
        ep = env;
        while (*ep)
                free(*ep++);
        free(env);
-       free(s);
        return retval;
 }
 
 int
-send_interface(int fd, const struct interface *ifp)
+send_interface(struct fd_list *fd, const struct interface *ifp)
 {
        const char *reason;
        int retval = 0;
@@ -562,7 +557,7 @@ send_interface(int fd, const struct interface *ifp)
                break;
        }
        if (send_interface1(fd, ifp, reason) == -1)
-                       retval = -1;
+               retval = -1;
 #ifdef INET
        if (D_STATE_RUNNING(ifp)) {
                d = D_CSTATE(ifp);
@@ -595,8 +590,7 @@ script_runreason(const struct interface *ifp, const char *reason)
        size_t e, elen = 0;
        pid_t pid;
        int status = 0;
-       const struct fd_list *fd;
-       struct iovec iov[2];
+       struct fd_list *fd;
 
        if (ifp->options->script &&
            (ifp->options->script[0] == '\0' ||
@@ -663,26 +657,21 @@ script_runreason(const struct interface *ifp, const char *reason)
 
        /* Send to our listeners */
        bigenv = NULL;
-       for (fd = ifp->ctx->control_fds; fd != NULL; fd = fd->next) {
-               if (fd->listener) {
-                       if (bigenv == NULL) {
-                               elen = (size_t)arraytostr((const char *const *)env,
-                                   &bigenv);
-                               if ((ssize_t)elen == -1) {
-                                       syslog(LOG_ERR, "%s: arraytostr: %m",
-                                           ifp->name);
-                                       continue;
-                               }
-                               iov[0].iov_base = &elen;
-                               iov[0].iov_len = sizeof(size_t);
-                               iov[1].iov_base = bigenv;
-                               iov[1].iov_len = elen;
+       TAILQ_FOREACH(fd, &ifp->ctx->control_fds, next) {
+               if (!fd->listener)
+                       continue;
+               if (bigenv == NULL) {
+                       elen = (size_t)arraytostr((const char *const *)env,
+                           &bigenv);
+                       if ((ssize_t)elen == -1) {
+                               syslog(LOG_ERR, "%s: arraytostr: %m",
+                                   ifp->name);
+                                   break;
                        }
-                       if (writev(fd->fd, iov, 2) == -1)
-                               syslog(LOG_ERR, "%s: writev: %m", __func__);
                }
+               if (control_queue(fd, bigenv, elen, 1) == -1)
+                       syslog(LOG_ERR, "%s: control_queue: %m", __func__);
        }
-       free(bigenv);
 
 out:
        /* Cleanup */

diff --git a/script.h b/script.h
index e1862e83c7de09589d63ab42676b637fd9347b0c..4d0f5da80f89699bdd70dcbdbbaa7fbb1ff3dbb2 100644 (file)
--- a/script.h
+++ b/script.h
@@ -28,8 +28,10 @@
 #ifndef SCRIPT_H
 #define SCRIPT_H
 
+#include "control.h"
+
 void if_printoptions(void);
-int send_interface(int, const struct interface *);
+int send_interface(struct fd_list *, const struct interface *);
 int script_runreason(const struct interface *, const char *);
 
 #endif