From a5db5ef71fd1e3709fc2ed2a85c062af168b91c2 Mon Sep 17 00:00:00 2001 From: xnor Date: Tue, 8 May 2018 21:11:43 +0000 Subject: [PATCH 1/2] Fix DNS timeouts C-ares creates up to two sockets per nameserver. With up to three nameservers in resolv.conf this makes a max of six active sockets. Having only one ev_io watcher resulted in closing watchers that were still active, needlessly resulting in DNS timeouts. --- src/resolv.c | 55 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/src/resolv.c b/src/resolv.c index c2778ff5..f8a1e3ef 100644 --- a/src/resolv.c +++ b/src/resolv.c @@ -65,8 +65,12 @@ * Implement DNS resolution interface using libc-ares */ + +#define SS_NUM_IOS 6 +#define SS_INVALID_FD -1 + struct resolv_ctx { - struct ev_io io; + struct ev_io ios[SS_NUM_IOS]; struct ev_timer tw; ares_channel channel; @@ -90,7 +94,7 @@ struct resolv_query { extern int verbose; -struct resolv_ctx default_ctx; +static struct resolv_ctx default_ctx; static struct ev_loop *default_loop; enum { @@ -121,8 +125,6 @@ static void reset_timer(); static void resolv_sock_cb(EV_P_ ev_io *w, int revents) { - struct resolv_ctx *ctx = (struct resolv_ctx *)w; - ares_socket_t rfd = ARES_SOCKET_BAD, wfd = ARES_SOCKET_BAD; if (revents & EV_READ) @@ -130,7 +132,7 @@ resolv_sock_cb(EV_P_ ev_io *w, int revents) if (revents & EV_WRITE) wfd = w->fd; - ares_process_fd(ctx->channel, rfd, wfd); + ares_process_fd(default_ctx.channel, rfd, wfd); reset_timer(); } @@ -181,7 +183,9 @@ resolv_init(struct ev_loop *loop, char *nameservers, int ipv6first) FATAL("failed to set nameservers"); } - ev_init(&default_ctx.io, resolv_sock_cb); + for (int i = 0; i < SS_NUM_IOS; i++) { + ev_io_init(&default_ctx.ios[i], resolv_sock_cb, SS_INVALID_FD, 0); + } ev_timer_init(&default_ctx.tw, resolv_timeout_cb, 0.0, 0.0); return 0; @@ -190,6 +194,11 @@ resolv_init(struct ev_loop *loop, char *nameservers, int ipv6first) void resolv_shutdown(struct ev_loop *loop) { + ev_timer_stop(default_loop, &default_ctx.tw); + for (int i = 0; i < SS_NUM_IOS; i++) { + ev_io_stop(default_loop, &default_ctx.ios[i]); + } + ares_cancel(default_ctx.channel); ares_destroy(default_ctx.channel); @@ -460,13 +469,35 @@ static void resolv_sock_state_cb(void *data, int s, int read, int write) { struct resolv_ctx *ctx = (struct resolv_ctx *)data; + int events = (read ? EV_READ : 0) | (write ? EV_WRITE : 0); + + int i = 0, ffi = -1; + for (; i < SS_NUM_IOS; i++) { + if (ctx->ios[i].fd == s) { + break; + } + + if (ffi < 0 && ctx->ios[i].fd == SS_INVALID_FD) { + // first free index + ffi = i; + } + } + + if (i < SS_NUM_IOS) { + ev_io_stop(default_loop, &ctx->ios[i]); + } else if (ffi > -1) { + i = ffi; + } else { + LOGE("failed to find free I/O watcher slot for DNS query"); + // last resort: stop io and re-use slot, will cause timeout + i = 0; + ev_io_stop(default_loop, &ctx->ios[i]); + } - if (read || write) { - ev_io_stop(default_loop, &ctx->io); - ev_io_set(&ctx->io, s, (read ? EV_READ : 0) | (write ? EV_WRITE : 0)); - ev_io_start(default_loop, &ctx->io); + if (events) { + ev_io_set(&ctx->ios[i], s, events); + ev_io_start(default_loop, &ctx->ios[i]); } else { - ev_io_stop(default_loop, &ctx->io); - ev_io_set(&ctx->io, -1, 0); + ev_io_set(&ctx->ios[i], SS_INVALID_FD, 0); } } From 14b6261b403aa4db53badbba126def0d9f510a42 Mon Sep 17 00:00:00 2001 From: xnor Date: Wed, 9 May 2018 23:06:01 +0000 Subject: [PATCH 2/2] Optimize resolv timer Don't call ares_timeout since it does a search through all queries. Instead, schedule a timer that fires after 1 second. If no activity happened for over 1 second do ares timeout processing, otherwise reschedule the timer to fire after the remaining timeout. This also completely avoids the ev_timer_again calls for each single fd event. --- src/resolv.c | 51 ++++++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/src/resolv.c b/src/resolv.c index f8a1e3ef..8d78765c 100644 --- a/src/resolv.c +++ b/src/resolv.c @@ -68,10 +68,12 @@ #define SS_NUM_IOS 6 #define SS_INVALID_FD -1 +#define SS_TIMER_AFTER 1.0 struct resolv_ctx { struct ev_io ios[SS_NUM_IOS]; - struct ev_timer tw; + struct ev_timer timer; + ev_tstamp last_tick; ares_channel channel; struct ares_options options; @@ -105,7 +107,7 @@ enum { static int resolv_mode = MODE_IPV4_FIRST; static void resolv_sock_cb(struct ev_loop *, struct ev_io *, int); -static void resolv_timeout_cb(struct ev_loop *, struct ev_timer *, int); +static void resolv_timer_cb(struct ev_loop *, struct ev_timer *, int); static void resolv_sock_state_cb(void *, int, int, int); static void dns_query_v4_cb(void *, int, int, struct hostent *); @@ -117,8 +119,6 @@ static struct sockaddr *choose_ipv4_first(struct resolv_query *); static struct sockaddr *choose_ipv6_first(struct resolv_query *); static struct sockaddr *choose_any(struct resolv_query *); -static void reset_timer(); - /* * DNS UDP socket activity callback */ @@ -132,9 +132,9 @@ resolv_sock_cb(EV_P_ ev_io *w, int revents) if (revents & EV_WRITE) wfd = w->fd; - ares_process_fd(default_ctx.channel, rfd, wfd); + default_ctx.last_tick = ev_now(default_loop); - reset_timer(); + ares_process_fd(default_ctx.channel, rfd, wfd); } int @@ -186,7 +186,10 @@ resolv_init(struct ev_loop *loop, char *nameservers, int ipv6first) for (int i = 0; i < SS_NUM_IOS; i++) { ev_io_init(&default_ctx.ios[i], resolv_sock_cb, SS_INVALID_FD, 0); } - ev_timer_init(&default_ctx.tw, resolv_timeout_cb, 0.0, 0.0); + + default_ctx.last_tick = ev_now(default_loop); + ev_init(&default_ctx.timer, resolv_timer_cb); + resolv_timer_cb(default_loop, &default_ctx.timer, 0); return 0; } @@ -194,7 +197,7 @@ resolv_init(struct ev_loop *loop, char *nameservers, int ipv6first) void resolv_shutdown(struct ev_loop *loop) { - ev_timer_stop(default_loop, &default_ctx.tw); + ev_timer_stop(default_loop, &default_ctx.timer); for (int i = 0; i < SS_NUM_IOS; i++) { ev_io_stop(default_loop, &default_ctx.ios[i]); } @@ -229,8 +232,6 @@ resolv_start(const char *hostname, uint16_t port, ares_gethostbyname(default_ctx.channel, hostname, AF_INET, dns_query_v4_cb, query); ares_gethostbyname(default_ctx.channel, hostname, AF_INET6, dns_query_v6_cb, query); - - reset_timer(); } /* @@ -436,30 +437,26 @@ all_requests_are_null(struct resolv_query *query) } /* - * DNS timeout callback + * Timer callback */ static void -resolv_timeout_cb(struct ev_loop *loop, struct ev_timer *w, int revents) +resolv_timer_cb(struct ev_loop *loop, struct ev_timer *w, int revents) { - struct resolv_ctx *ctx = cork_container_of(w, struct resolv_ctx, tw); + struct resolv_ctx *ctx = cork_container_of(w, struct resolv_ctx, timer); - ares_process_fd(ctx->channel, ARES_SOCKET_BAD, ARES_SOCKET_BAD); + ev_tstamp now = ev_now(default_loop); + ev_tstamp after = ctx->last_tick - now + SS_TIMER_AFTER; - reset_timer(); -} + if (after < 0.0) { + ctx->last_tick = now; + ares_process_fd(ctx->channel, ARES_SOCKET_BAD, ARES_SOCKET_BAD); -static void -reset_timer() -{ - struct timeval tvout; - struct timeval *tv = ares_timeout(default_ctx.channel, NULL, &tvout); - if (tv == NULL) { - return; + ev_timer_set(w, SS_TIMER_AFTER, 0.0); + } else { + ev_timer_set(w, after, 0.0); } - float repeat = tv->tv_sec + tv->tv_usec / 1000000. + 1e-9; - repeat = repeat < 1.f ? 1.f : repeat; - ev_timer_set(&default_ctx.tw, repeat, repeat); - ev_timer_again(default_loop, &default_ctx.tw); + + ev_timer_start(loop, w); } /*