From 23f9d8b4cef54c2e2ddeb4ebbc12ed3ed901b8fd Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Thu, 4 Sep 2014 19:30:47 +0000 Subject: [PATCH] Add a write callback to eloop so we know when we can write to the socket. This seems to be important for our control socket as sometimes, we fail to write with EAGAIN and dhcpcd should not block on users of this socket. --- arp.c | 10 ++--- control.c | 12 +++--- dhcp.c | 12 +++--- dhcp6.c | 7 ++-- dhcpcd.c | 118 +++++++++++++++++++++++++++++++++++------------------- eloop.c | 52 ++++++++++++++++++------ eloop.h | 12 ++++-- ipv6nd.c | 9 +++-- 8 files changed, 151 insertions(+), 81 deletions(-) diff --git a/arp.c b/arp.c index 9dc86b60..eae4d58a 100644 --- a/arp.c +++ b/arp.c @@ -250,8 +250,8 @@ arp_announce(void *arg) syslog(LOG_ERR, "%s: %s: %m", __func__, ifp->name); return; } - eloop_event_add(ifp->ctx->eloop, - state->arp_fd, arp_packet, ifp); + eloop_event_add(ifp->ctx->eloop, state->arp_fd, + arp_packet, ifp, NULL, NULL); } if (++state->claims < ANNOUNCE_NUM) syslog(LOG_DEBUG, @@ -286,7 +286,7 @@ arp_announce(void *arg) timernorm(&tv); eloop_timeout_add_tv(ifp->ctx->eloop, &tv, dhcp_discover, ifp); } else { - eloop_event_delete(ifp->ctx->eloop, state->arp_fd); + eloop_event_delete(ifp->ctx->eloop, state->arp_fd, 0); close(state->arp_fd); state->arp_fd = -1; } @@ -308,7 +308,7 @@ arp_probe(void *arg) return; } eloop_event_add(ifp->ctx->eloop, - state->arp_fd, arp_packet, ifp); + state->arp_fd, arp_packet, ifp, NULL, NULL); } if (state->arping_index < ifp->options->arping_len) { @@ -378,7 +378,7 @@ arp_close(struct interface *ifp) return; if (state->arp_fd != -1) { - eloop_event_delete(ifp->ctx->eloop, state->arp_fd); + eloop_event_delete(ifp->ctx->eloop, state->arp_fd, 0); close(state->arp_fd); state->arp_fd = -1; } diff --git a/control.c b/control.c index 435e7345..8433ce25 100644 --- a/control.c +++ b/control.c @@ -64,7 +64,7 @@ control_handle_data(void *arg) last = NULL; for (lp = l->ctx->control_fds; lp; lp = lp->next) { if (lp == l) { - eloop_event_delete(lp->ctx->eloop, lp->fd); + eloop_event_delete(lp->ctx->eloop, lp->fd, 0); close(lp->fd); if (last == NULL) lp->ctx->control_fds = lp->next; @@ -137,7 +137,8 @@ control_handle(void *arg) l->listener = 0; l->next = ctx->control_fds; ctx->control_fds = l; - eloop_event_add(ctx->eloop, l->fd, control_handle_data, l); + eloop_event_add(ctx->eloop, l->fd, + control_handle_data, l, NULL, NULL); } else close(fd); } @@ -200,7 +201,8 @@ control_start(struct dhcpcd_ctx *ctx, const char *ifname) unlink(ctx->control_sock); return -1; } - eloop_event_add(ctx->eloop, ctx->control_fd, control_handle, ctx); + eloop_event_add(ctx->eloop, ctx->control_fd, + control_handle, ctx, NULL, NULL); return ctx->control_fd; } @@ -212,7 +214,7 @@ control_stop(struct dhcpcd_ctx *ctx) if (ctx->control_fd == -1) return 0; - eloop_event_delete(ctx->eloop, ctx->control_fd); + eloop_event_delete(ctx->eloop, ctx->control_fd, 0); if (shutdown(ctx->control_fd, SHUT_RDWR) == -1) retval = 1; close(ctx->control_fd); @@ -223,7 +225,7 @@ control_stop(struct dhcpcd_ctx *ctx) l = ctx->control_fds; while (l != NULL) { ctx->control_fds = l->next; - eloop_event_delete(ctx->eloop, l->fd); + eloop_event_delete(ctx->eloop, l->fd, 0); shutdown(l->fd, SHUT_RDWR); close(l->fd); free(l); diff --git a/dhcp.c b/dhcp.c index 2a46ad28..d0dcb60c 100644 --- a/dhcp.c +++ b/dhcp.c @@ -1404,12 +1404,12 @@ dhcp_close(struct interface *ifp) return; if (state->arp_fd != -1) { - eloop_event_delete(ifp->ctx->eloop, state->arp_fd); + eloop_event_delete(ifp->ctx->eloop, state->arp_fd, 0); close(state->arp_fd); state->arp_fd = -1; } if (state->raw_fd != -1) { - eloop_event_delete(ifp->ctx->eloop, state->raw_fd); + eloop_event_delete(ifp->ctx->eloop, state->raw_fd, 0); close(state->raw_fd); state->raw_fd = -1; } @@ -2704,7 +2704,7 @@ dhcp_handleudp(void *arg) * from the raw fd */ if (read(ctx->udp_fd, buffer, sizeof(buffer)) == -1) { syslog(LOG_ERR, "%s: %m", __func__); - eloop_event_delete(ctx->eloop, ctx->udp_fd); + eloop_event_delete(ctx->eloop, ctx->udp_fd, 0); close(ctx->udp_fd); ctx->udp_fd = -1; } @@ -2731,7 +2731,7 @@ dhcp_open(struct interface *ifp) return -1; } eloop_event_add(ifp->ctx->eloop, - state->raw_fd, dhcp_handlepacket, ifp); + state->raw_fd, dhcp_handlepacket, ifp, NULL, NULL); } return 0; } @@ -2791,7 +2791,7 @@ dhcp_free(struct interface *ifp) } if (ifp == NULL) { if (ctx->udp_fd != -1) { - eloop_event_delete(ctx->eloop, ctx->udp_fd); + eloop_event_delete(ctx->eloop, ctx->udp_fd, 0); close(ctx->udp_fd); ctx->udp_fd = -1; } @@ -2906,7 +2906,7 @@ dhcp_start1(void *arg) return; } eloop_event_add(ifp->ctx->eloop, - ifp->ctx->udp_fd, dhcp_handleudp, ifp->ctx); + ifp->ctx->udp_fd, dhcp_handleudp, ifp->ctx, NULL, NULL); } if (dhcp_init(ifp) == -1) { diff --git a/dhcp6.c b/dhcp6.c index 6199dc73..6a97bf2b 100644 --- a/dhcp6.c +++ b/dhcp6.c @@ -2418,7 +2418,7 @@ dhcp6_handledata(void *arg) if (bytes == -1 || bytes == 0) { syslog(LOG_ERR, "recvmsg: %m"); close(ctx->dhcp_fd); - eloop_event_delete(dhcpcd_ctx->eloop, ctx->dhcp_fd); + eloop_event_delete(dhcpcd_ctx->eloop, ctx->dhcp_fd, 0); ctx->dhcp_fd = -1; return; } @@ -2930,7 +2930,8 @@ dhcp6_open(struct dhcpcd_ctx *dctx) &n, sizeof(n)) == -1) goto errexit; - eloop_event_add(dctx->eloop, ctx->dhcp_fd, dhcp6_handledata, dctx); + eloop_event_add(dctx->eloop, ctx->dhcp_fd, + dhcp6_handledata, dctx, NULL, NULL); return 0; errexit: @@ -3184,7 +3185,7 @@ dhcp6_freedrop(struct interface *ifp, int drop, const char *reason) } if (ifp == NULL && ctx->ipv6) { if (ctx->ipv6->dhcp_fd != -1) { - eloop_event_delete(ctx->eloop, ctx->ipv6->dhcp_fd); + eloop_event_delete(ctx->eloop, ctx->ipv6->dhcp_fd, 0); close(ctx->ipv6->dhcp_fd); ctx->ipv6->dhcp_fd = -1; } diff --git a/dhcpcd.c b/dhcpcd.c index b19f22bf..130e3c52 100644 --- a/dhcpcd.c +++ b/dhcpcd.c @@ -720,7 +720,7 @@ handle_link(void *arg) ctx = arg; if (if_managelink(ctx) == -1) { syslog(LOG_ERR, "if_managelink: %m"); - eloop_event_delete(ctx->eloop, ctx->link_fd); + eloop_event_delete(ctx->eloop, ctx->link_fd, 0); close(ctx->link_fd); ctx->link_fd = -1; } @@ -1046,6 +1046,68 @@ 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) +{ + struct fd_list *fd = arg; + struct interface *ifp; + size_t len; + + len = 0; + TAILQ_FOREACH(ifp, fd->ctx->ifaces, next) { + len++; + if (D_STATE_RUNNING(ifp)) + len++; + if (D6_STATE_RUNNING(ifp)) + len++; + if (ipv6nd_hasra(ifp)) + len++; + } + if (write(fd->fd, &len, sizeof(len)) != sizeof(len)) + return; + TAILQ_FOREACH(ifp, fd->ctx->ifaces, next) { + if (send_interface(fd->fd, ifp) == -1) + syslog(LOG_ERR, "send_interface %d: %m", fd->fd); + } + eloop_event_delete(fd->ctx->eloop, fd->fd, 1); +} + int dhcpcd_handleargs(struct dhcpcd_ctx *ctx, struct fd_list *fd, int argc, char **argv) @@ -1054,53 +1116,24 @@ dhcpcd_handleargs(struct dhcpcd_ctx *ctx, struct fd_list *fd, int do_exit = 0, do_release = 0, do_reboot = 0; int opt, oi = 0; size_t len, l; - struct iovec iov[2]; char *tmp, *p; if (fd != NULL) { - /* Special commands for our control socket */ + /* Special commands for our control socket + * as the other end should be blocking until it gets the + * expected reply we should be safely able just to change the + * write callback on the fd */ if (strcmp(*argv, "--version") == 0) { - 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, "writev: %m"); - return -1; - } + eloop_event_add(fd->ctx->eloop, fd->fd, NULL, NULL, + dhcpcd_version, fd); return 0; } else if (strcmp(*argv, "--getconfigfile") == 0) { - len = strlen(ctx->cffile) + 1; - iov[0].iov_base = &len; - iov[0].iov_len = sizeof(ssize_t); - iov[1].iov_base = UNCONST(ctx->cffile); - iov[1].iov_len = len; - if (writev(fd->fd, iov, 2) == -1) { - syslog(LOG_ERR, "writev: %m"); - return -1; - } + eloop_event_add(fd->ctx->eloop, fd->fd, NULL, NULL, + dhcpcd_getconfigfile, fd); return 0; } else if (strcmp(*argv, "--getinterfaces") == 0) { - len = 0; - TAILQ_FOREACH(ifp, ctx->ifaces, next) { - len++; - if (D_STATE_RUNNING(ifp)) - len++; - if (D6_STATE_RUNNING(ifp)) - len++; - if (ipv6nd_hasra(ifp)) - len++; - } - if (write(fd->fd, &len, sizeof(len)) != - sizeof(len)) - return -1; - TAILQ_FOREACH(ifp, ctx->ifaces, next) { - if (send_interface(fd->fd, ifp) == -1) - syslog(LOG_ERR, - "send_interface %d: %m", - fd->fd); - } + eloop_event_add(fd->ctx->eloop, fd->fd, NULL, NULL, + dhcpcd_getinterfaces, fd); return 0; } else if (strcmp(*argv, "--listen") == 0) { fd->listener = 1; @@ -1598,7 +1631,8 @@ main(int argc, char **argv) if (ctx.link_fd == -1) syslog(LOG_ERR, "open_link_socket: %m"); else - eloop_event_add(ctx.eloop, ctx.link_fd, handle_link, &ctx); + eloop_event_add(ctx.eloop, ctx.link_fd, + handle_link, &ctx, NULL, NULL); /* Start any dev listening plugin which may want to * change the interface name provided by the kernel */ @@ -1688,7 +1722,7 @@ exit1: } free(ctx.duid); if (ctx.link_fd != -1) { - eloop_event_delete(ctx.eloop, ctx.link_fd); + eloop_event_delete(ctx.eloop, ctx.link_fd, 0); close(ctx.link_fd); } diff --git a/eloop.c b/eloop.c index 8841846e..91813902 100644 --- a/eloop.c +++ b/eloop.c @@ -52,7 +52,11 @@ eloop_event_setup_fds(struct eloop_ctx *ctx) i = 0; TAILQ_FOREACH(e, &ctx->events, next) { ctx->fds[i].fd = e->fd; - ctx->fds[i].events = POLLIN; + ctx->fds[i].events = 0; + if (e->read_cb) + ctx->fds[i].events |= POLLIN; + if (e->write_cb) + ctx->fds[i].events |= POLLOUT; ctx->fds[i].revents = 0; e->pollfd = &ctx->fds[i]; i++; @@ -60,8 +64,9 @@ eloop_event_setup_fds(struct eloop_ctx *ctx) } int -eloop_event_add(struct eloop_ctx *ctx, - int fd, void (*callback)(void *), void *arg) +eloop_event_add(struct eloop_ctx *ctx, int fd, + void (*read_cb)(void *), void *read_cb_arg, + void (*write_cb)(void *), void *write_cb_arg) { struct eloop_event *e; struct pollfd *nfds; @@ -69,8 +74,15 @@ eloop_event_add(struct eloop_ctx *ctx, /* We should only have one callback monitoring the fd */ TAILQ_FOREACH(e, &ctx->events, next) { if (e->fd == fd) { - e->callback = callback; - e->arg = arg; + if (read_cb) { + e->read_cb = read_cb; + e->read_cb_arg = read_cb_arg; + } + if (write_cb) { + e->write_cb = write_cb; + e->write_cb_arg = write_cb_arg; + } + eloop_event_setup_fds(ctx); return 0; } } @@ -104,8 +116,10 @@ eloop_event_add(struct eloop_ctx *ctx, /* Now populate the structure and add it to the list */ e->fd = fd; - e->callback = callback; - e->arg = arg; + e->read_cb = read_cb; + e->read_cb_arg = read_cb_arg; + e->write_cb = write_cb; + e->write_cb_arg = write_cb_arg; /* The order of events should not matter. * However, some PPP servers love to close the link right after * sending their final message. So to ensure dhcpcd processes this @@ -118,15 +132,20 @@ eloop_event_add(struct eloop_ctx *ctx, } void -eloop_event_delete(struct eloop_ctx *ctx, int fd) +eloop_event_delete(struct eloop_ctx *ctx, int fd, int write_only) { struct eloop_event *e; TAILQ_FOREACH(e, &ctx->events, next) { if (e->fd == fd) { - TAILQ_REMOVE(&ctx->events, e, next); - TAILQ_INSERT_TAIL(&ctx->free_events, e, next); - ctx->events_len--; + if (write_only) { + e->write_cb = NULL; + e->write_cb_arg = NULL; + } else { + TAILQ_REMOVE(&ctx->events, e, next); + TAILQ_INSERT_TAIL(&ctx->free_events, e, next); + ctx->events_len--; + } eloop_event_setup_fds(ctx); break; } @@ -395,8 +414,17 @@ eloop_start(struct dhcpcd_ctx *dctx) /* Process any triggered events. */ if (n > 0) { TAILQ_FOREACH(e, &ctx->events, next) { + if (e->pollfd->revents & POLLOUT && + e->write_cb) + { + e->write_cb(e->write_cb_arg); + /* We need to break here as the + * callback could destroy the next + * fd to process. */ + break; + } if (e->pollfd->revents) { - e->callback(e->arg); + e->read_cb(e->read_cb_arg); /* We need to break here as the * callback could destroy the next * fd to process. */ diff --git a/eloop.h b/eloop.h index f81eb933..7e817f82 100644 --- a/eloop.h +++ b/eloop.h @@ -41,8 +41,10 @@ struct eloop_event { TAILQ_ENTRY(eloop_event) next; int fd; - void (*callback)(void *); - void *arg; + void (*read_cb)(void *); + void *read_cb_arg; + void (*write_cb)(void *); + void *write_cb_arg; struct pollfd *pollfd; }; @@ -81,8 +83,10 @@ struct eloop_ctx { #define eloop_timeouts_delete(a, b, ...) \ eloop_q_timeouts_delete(a, ELOOP_QUEUE, b, __VA_ARGS__) -int eloop_event_add(struct eloop_ctx *, int, void (*)(void *), void *); -void eloop_event_delete(struct eloop_ctx *, int); +int eloop_event_add(struct eloop_ctx *, int, + void (*)(void *), void *, + void (*)(void *), void *); +void eloop_event_delete(struct eloop_ctx *, int, int); int eloop_q_timeout_add_sec(struct eloop_ctx *, int queue, time_t, void (*)(void *), void *); int eloop_q_timeout_add_tv(struct eloop_ctx *, int queue, diff --git a/ipv6nd.c b/ipv6nd.c index 880678f8..9a63de18 100644 --- a/ipv6nd.c +++ b/ipv6nd.c @@ -218,12 +218,13 @@ ipv6nd_open(struct dhcpcd_ctx *dctx) &filt, sizeof(filt)) == -1) goto eexit; - eloop_event_add(dctx->eloop, ctx->nd_fd, ipv6nd_handledata, dctx); + eloop_event_add(dctx->eloop, ctx->nd_fd, + ipv6nd_handledata, dctx, NULL, NULL); return ctx->nd_fd; eexit: if (ctx->nd_fd != -1) { - eloop_event_delete(dctx->eloop, ctx->nd_fd); + eloop_event_delete(dctx->eloop, ctx->nd_fd, 0); close(ctx->nd_fd); ctx->nd_fd = -1; } @@ -466,7 +467,7 @@ ipv6nd_free(struct interface *ifp) } if (ifp == NULL) { if (ctx->ipv6->nd_fd != -1) { - eloop_event_delete(ctx->eloop, ctx->ipv6->nd_fd); + eloop_event_delete(ctx->eloop, ctx->ipv6->nd_fd, 0); close(ctx->ipv6->nd_fd); ctx->ipv6->nd_fd = -1; } @@ -1454,7 +1455,7 @@ ipv6nd_handledata(void *arg) len = recvmsg(ctx->nd_fd, &ctx->rcvhdr, 0); if (len == -1 || len == 0) { syslog(LOG_ERR, "recvmsg: %m"); - eloop_event_delete(dhcpcd_ctx->eloop, ctx->nd_fd); + eloop_event_delete(dhcpcd_ctx->eloop, ctx->nd_fd, 0); close(ctx->nd_fd); ctx->nd_fd = -1; return; -- 2.47.3