From: Roy Marples Date: Fri, 5 Sep 2014 12:28:05 +0000 (+0000) Subject: Implement a send queue for each control fd. X-Git-Tag: v6.4.4~41 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=923e118097c5296c5e94b5518d8b06c67c2f4b52;p=thirdparty%2Fdhcpcd.git 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 8433ce25..c129ae71 100644 --- a/control.c +++ b/control.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -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 3204b7ac..7f555977 100644 --- 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 23142094..13ece6a9 100644 --- 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 3aa59aed..f5340d15 100644 --- 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 63daa6a4..effa81d7 100644 --- 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 e1862e83..4d0f5da8 100644 --- 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