From 4d2e10df172968e9ef5b515884a3ed41f18cfc28 Mon Sep 17 00:00:00 2001 From: Leonid Evdokimov Date: Sun, 25 Mar 2012 23:56:01 +0400 Subject: [PATCH] Implement better exponential backoff in case of `accept()` failure. This commit implements two more features: * min_accept_backoff configuration option * retry accept() after some close() calls See also https://github.com/darkk/redsocks/issues/19 --- http-connect.c | 2 +- http-relay.c | 2 +- redsocks.c | 65 +++++++++++++++++++++++++++++++++---------- redsocks.conf.example | 6 ++-- redsocks.h | 6 +++- redudp.c | 12 ++++---- utils.c | 3 +- utils.h | 8 ++++++ 8 files changed, 76 insertions(+), 28 deletions(-) diff --git a/http-connect.c b/http-connect.c index e34268aa..e2435282 100644 --- a/http-connect.c +++ b/http-connect.c @@ -132,7 +132,7 @@ static void httpc_read_cb(struct bufferevent *buffev, void *_arg) } /* close relay tunnel */ - close(EVENT_FD(&client->relay->ev_write)); + redsocks_close(EVENT_FD(&client->relay->ev_write)); bufferevent_free(client->relay); /* set to initial state*/ diff --git a/http-relay.c b/http-relay.c index 4e068604..4a394518 100644 --- a/http-relay.c +++ b/http-relay.c @@ -205,7 +205,7 @@ static void httpr_relay_read_cb(struct bufferevent *buffev, void *_arg) } /* close relay tunnel */ - close(EVENT_FD(&client->relay->ev_write)); + redsocks_close(EVENT_FD(&client->relay->ev_write)); bufferevent_free(client->relay); /* set to initial state*/ diff --git a/redsocks.c b/redsocks.c index 73699b00..d085e109 100644 --- a/redsocks.c +++ b/redsocks.c @@ -65,6 +65,7 @@ static parser_entry redsocks_entries[] = { .key = "login", .type = pt_pchar }, { .key = "password", .type = pt_pchar }, { .key = "listenq", .type = pt_uint16 }, + { .key = "min_accept_backoff", .type = pt_uint16 }, { .key = "max_accept_backoff", .type = pt_uint16 }, { } }; @@ -76,14 +77,14 @@ static void tracked_event_set( void (*callback)(evutil_socket_t, short, void *), void *arg) { event_set(&tev->ev, fd, events, callback, arg); - tev->inserted = 0; + timerclear(&tev->inserted); } static int tracked_event_add(struct tracked_event *tev, const struct timeval *tv) { int ret = event_add(&tev->ev, tv); if (ret == 0) - tev->inserted = 1; + gettimeofday(&tev->inserted, NULL); return ret; } @@ -91,7 +92,7 @@ static int tracked_event_del(struct tracked_event *tev) { int ret = event_del(&tev->ev); if (ret == 0) - tev->inserted = 0; + timerclear(&tev->inserted); return ret; } @@ -120,6 +121,7 @@ static int redsocks_onenter(parser_section *section) * Linux: sysctl net.core.somaxconn * FreeBSD: sysctl kern.ipc.somaxconn */ instance->config.listenq = SOMAXCONN; + instance->config.min_backoff_ms = 100; instance->config.max_backoff_ms = 60000; for (parser_entry *entry = §ion->entries[0]; entry->key; entry++) @@ -132,6 +134,7 @@ static int redsocks_onenter(parser_section *section) (strcmp(entry->key, "login") == 0) ? (void*)&instance->config.login : (strcmp(entry->key, "password") == 0) ? (void*)&instance->config.password : (strcmp(entry->key, "listenq") == 0) ? (void*)&instance->config.listenq : + (strcmp(entry->key, "min_accept_backoff") == 0) ? (void*)&instance->config.min_backoff_ms : (strcmp(entry->key, "max_accept_backoff") == 0) ? (void*)&instance->config.max_backoff_ms : NULL; section->data = instance; @@ -170,10 +173,18 @@ static int redsocks_onexit(parser_section *section) err = "no `type` for redsocks"; } + if (!err && !instance->config.min_backoff_ms) { + err = "`min_accept_backoff` must be positive, 0 ms is too low"; + } + if (!err && !instance->config.max_backoff_ms) { err = "`max_accept_backoff` must be positive, 0 ms is too low"; } + if (!err && !(instance->config.min_backoff_ms < instance->config.max_backoff_ms)) { + err = "`min_accept_backoff` must be less than `max_accept_backoff`"; + } + if (err) parser_error(section->context, err); @@ -329,12 +340,12 @@ void redsocks_drop_client(redsocks_client *client) client->instance->relay_ss->fini(client); if (client->client) { - close(EVENT_FD(&client->client->ev_write)); + redsocks_close(EVENT_FD(&client->client->ev_write)); bufferevent_free(client->client); } if (client->relay) { - close(EVENT_FD(&client->relay->ev_write)); + redsocks_close(EVENT_FD(&client->relay->ev_write)); bufferevent_free(client->relay); } @@ -581,6 +592,32 @@ static void redsocks_accept_backoff(int fd, short what, void *_arg) log_errno(LOG_ERR, "event_add"); } +void redsocks_close_internal(int fd, const char* file, int line, const char *func) +{ + if (close(fd) == 0) { + redsocks_instance *instance = NULL; + struct timeval now; + gettimeofday(&now, NULL); + list_for_each_entry(instance, &instances, list) { + if (timerisset(&instance->accept_backoff.inserted)) { + struct timeval min_accept_backoff = { + instance->config.min_backoff_ms / 1000, + (instance->config.min_backoff_ms % 1000) * 1000}; + struct timeval time_passed; + timersub(&now, &instance->accept_backoff.inserted, &time_passed); + if (timercmp(&min_accept_backoff, &time_passed, <)) { + redsocks_accept_backoff(-1, 0, instance); + break; + } + } + } + } + else { + const int do_errno = 1; + _log_write(file, line, func, do_errno, LOG_WARNING, "close"); + } +} + static void redsocks_accept_client(int fd, short what, void *_arg) { redsocks_instance *self = _arg; @@ -599,10 +636,8 @@ static void redsocks_accept_client(int fd, short what, void *_arg) /* Different systems use different `errno` value to signal different * `lack of file descriptors` conditions. Here are most of them. */ if (errno == ENFILE || errno == EMFILE || errno == ENOBUFS || errno == ENOMEM) { - // FIXME: should I log on every attempt? self->accept_backoff_ms = (self->accept_backoff_ms << 1) + 1; - if (self->accept_backoff_ms > self->config.max_backoff_ms) - self->accept_backoff_ms = self->config.max_backoff_ms; + clamp_value(self->accept_backoff_ms, self->config.min_backoff_ms, self->config.max_backoff_ms); int delay = (random() % self->accept_backoff_ms) + 1; log_errno(LOG_WARNING, "accept: out of file descriptors, backing off for %u ms", delay); struct timeval tvdelay = { delay / 1000, (delay % 1000) * 1000 }; @@ -683,7 +718,7 @@ static void redsocks_accept_client(int fd, short what, void *_arg) redsocks_drop_client(client); } if (client_fd != -1) - close(client_fd); + redsocks_close(client_fd); } static const char *redsocks_evshut_str(unsigned short evshut) @@ -780,6 +815,8 @@ static int redsocks_init_instance(redsocks_instance *instance) } tracked_event_set(&instance->listener, fd, EV_READ | EV_PERSIST, redsocks_accept_client, instance); + fd = -1; + tracked_event_set(&instance->accept_backoff, -1, 0, redsocks_accept_backoff, instance); error = tracked_event_add(&instance->listener, NULL); @@ -794,8 +831,7 @@ static int redsocks_init_instance(redsocks_instance *instance) redsocks_fini_instance(instance); if (fd != -1) { - if (close(fd) != 0) - log_errno(LOG_WARNING, "close"); + redsocks_close(fd); } return -1; @@ -818,16 +854,15 @@ static void redsocks_fini_instance(redsocks_instance *instance) { instance->relay_ss->instance_fini(instance); if (event_initialized(&instance->listener.ev)) { - if (instance->listener.inserted) + if (timerisset(&instance->listener.inserted)) if (tracked_event_del(&instance->listener) != 0) log_errno(LOG_WARNING, "event_del"); - if (close(EVENT_FD(&instance->listener.ev)) != 0) - log_errno(LOG_WARNING, "close"); + redsocks_close(EVENT_FD(&instance->listener.ev)); memset(&instance->listener, 0, sizeof(instance->listener)); } if (event_initialized(&instance->accept_backoff.ev)) { - if (instance->accept_backoff.inserted) + if (timerisset(&instance->accept_backoff.inserted)) if (tracked_event_del(&instance->accept_backoff) != 0) log_errno(LOG_WARNING, "event_del"); memset(&instance->accept_backoff, 0, sizeof(instance->accept_backoff)); diff --git a/redsocks.conf.example b/redsocks.conf.example index d33fba64..a71eb5c3 100644 --- a/redsocks.conf.example +++ b/redsocks.conf.example @@ -49,8 +49,10 @@ redsocks { // `max_accept_backoff` is a delay to retry `accept()` after accept // failure (e.g. due to lack of file descriptors). It's measured in - // milliseconds and maximal value is 65535. Setting it to zero leads to - // busy-loop. + // milliseconds and maximal value is 65535. `min_accept_backoff` is + // used as initial backoff value and as a damper for `accept() after + // close()` logic. + // min_accept_backoff = 100; // max_accept_backoff = 60000; // `ip' and `port' are IP and tcp-port of proxy-server diff --git a/redsocks.h b/redsocks.h index 3a38c62f..4f072d56 100644 --- a/redsocks.h +++ b/redsocks.h @@ -29,13 +29,14 @@ typedef struct redsocks_config_t { char *type; char *login; char *password; + uint16_t min_backoff_ms; uint16_t max_backoff_ms; // backoff capped by 65 seconds is enough :) uint16_t listenq; } redsocks_config; struct tracked_event { struct event ev; - int inserted; + struct timeval inserted; }; typedef struct redsocks_instance_t { @@ -90,6 +91,9 @@ int redsocks_write_helper( redsocks_message_maker mkmessage, int state, size_t wm_only); +#define redsocks_close(fd) redsocks_close_internal((fd), __FILE__, __LINE__, __func__) +void redsocks_close_internal(int fd, const char* file, int line, const char *func); + #define redsocks_log_error(client, prio, msg...) \ redsocks_log_write_plain(__FILE__, __LINE__, __func__, 0, &(client)->clientaddr, &(client)->destaddr, prio, ## msg) #define redsocks_log_errno(client, prio, msg...) \ diff --git a/redudp.c b/redudp.c index 7f96edc3..0a97852c 100644 --- a/redudp.c +++ b/redudp.c @@ -96,13 +96,13 @@ static void redudp_drop_client(redudp_client *client) fd = EVENT_FD(&client->relay->ev_read); bufferevent_free(client->relay); shutdown(fd, SHUT_RDWR); - close(fd); + redsocks_close(fd); } if (event_initialized(&client->udprelay)) { fd = EVENT_FD(&client->udprelay); if (event_del(&client->udprelay) == -1) redudp_log_errno(client, LOG_ERR, "event_del"); - close(fd); + redsocks_close(fd); } list_for_each_entry_safe(q, tmp, &client->queue, list) { list_del(&q->list); @@ -255,7 +255,7 @@ static void redudp_read_assoc_reply(struct bufferevent *buffev, void *_arg) fail: if (fd != -1) - close(fd); + redsocks_close(fd); redudp_drop_client(client); } @@ -636,8 +636,7 @@ static int redudp_init_instance(redudp_instance *instance) redudp_fini_instance(instance); if (fd != -1) { - if (close(fd) != 0) - log_errno(LOG_WARNING, "close"); + redsocks_close(fd); } return -1; @@ -660,8 +659,7 @@ static void redudp_fini_instance(redudp_instance *instance) if (event_initialized(&instance->listener)) { if (event_del(&instance->listener) != 0) log_errno(LOG_WARNING, "event_del"); - if (close(EVENT_FD(&instance->listener)) != 0) - log_errno(LOG_WARNING, "close"); + redsocks_close(EVENT_FD(&instance->listener)); memset(&instance->listener, 0, sizeof(instance->listener)); } diff --git a/utils.c b/utils.c index 637fe098..c6ced518 100644 --- a/utils.c +++ b/utils.c @@ -23,6 +23,7 @@ #include #include "log.h" #include "utils.h" +#include "redsocks.h" // for redsocks_close int red_recv_udp_pkt(int fd, char *buf, size_t buflen, struct sockaddr_in *inaddr) { @@ -116,7 +117,7 @@ struct bufferevent* red_connect_relay(struct sockaddr_in *addr, evbuffercb write fail: if (relay_fd != -1) - close(relay_fd); + redsocks_close(relay_fd); if (retval) bufferevent_free(retval); return NULL; diff --git a/utils.h b/utils.h index aef1ebdf..f691b77b 100644 --- a/utils.h +++ b/utils.h @@ -31,6 +31,14 @@ struct sockaddr_in; const typeof( ((type *)0)->member ) *__mptr = (ptr); \ (type *)( (char *)__mptr - offsetof(type,member) );}) +#define clamp_value(value, min_val, max_val) do { \ + if (value < min_val) \ + value = min_val; \ + if (value > max_val) \ + value = max_val; \ +} while (0) + + time_t redsocks_time(time_t *t); char *redsocks_evbuffer_readline(struct evbuffer *buf); struct bufferevent* red_connect_relay(struct sockaddr_in *addr, evbuffercb writecb, everrorcb errorcb, void *cbarg);