Skip to content

Features/transport fixups #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wtoorop merged 3 commits into from
Jun 29, 2015
Merged

Features/transport fixups #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e20d679
Improve TCP close handling and sync connection closing
saradickinson Jun 29, 2015
e5a8094
Turn fast open on by default. Fix build warning.
saradickinson Jun 29, 2015
8bb01c4
Turn TFO off by default. Strange crash found if TCP is not available.
saradickinson Jun 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,18 @@ case "$enable_tcp_fastopen" in
;;
esac

# Not yet enabled by default as crash found when TCP fails.
# AC_ARG_ENABLE(tcp-fastopen, AC_HELP_STRING([--disable-tcp-fastopen], Disable TCP Fast Open (default=enabled if available)),
# enable_tcp_fastopen="$enableval", enable_tcp_fastopen=yes)
# if test "x$enable_tcp_fastopen" = xno; then
# AC_MSG_WARN([TCP Fast Open is disabled])
# else
# AC_CHECK_DECL([MSG_FASTOPEN], [AC_DEFINE_UNQUOTED([USE_TCP_FASTOPEN], [1], [Define this to enable TCP fast open.])],
# [AC_MSG_WARN([TCP Fast Open is not available.])], [AC_INCLUDES_DEFAULT
# #include <sys/socket.h>
# ])
# fi

AC_ARG_ENABLE(broken-native-stub-dnssec, AC_HELP_STRING([--enable-broken-native-stub-dnssec], [Enable very experimental and broken native stub DNSSEC support]))
case "$enable_broken_native_stub_dnssec" in
yes)
Expand Down
27 changes: 26 additions & 1 deletion src/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,31 @@ priv_getdns_upstreams_dereference(getdns_upstreams *upstreams)
}
}

void
priv_getdns_upstream_shutdown(getdns_upstream *upstream) {
/*There is a race condition with a new request being scheduled
while this happens so take ownership of the fd asap*/
int fd = upstream->fd;
upstream->fd = -1;
/* If the connection had a problem, but had worked this time,
* then allow re-use in the future*/
if (upstream->tcp.write_error == 1 &&
upstream->responses_received > 0)
upstream->tcp.write_error = 0;
upstream->writes_done = 0;
upstream->responses_received = 0;
if (upstream->tls_hs_state != GETDNS_HS_FAILED)
upstream->tls_hs_state = GETDNS_HS_NONE;
/* Now TLS stuff*/
if (upstream->tls_obj != NULL) {
SSL_shutdown(upstream->tls_obj);
SSL_free(upstream->tls_obj);
upstream->tls_obj = NULL;
}
if (fd != -1)
close(fd);
}

static uint8_t*
upstream_addr(getdns_upstream *upstream)
{
Expand Down Expand Up @@ -597,7 +622,7 @@ upstream_init(getdns_upstream *upstream,

/* How is this upstream doing? */
upstream->writes_done = 0;
upstream->responses_recieved = 0;
upstream->responses_received = 0;
upstream->to_retry = 2;
upstream->back_off = 1;

Expand Down
4 changes: 3 additions & 1 deletion src/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ typedef struct getdns_upstream {

/* How is this upstream doing? */
size_t writes_done;
size_t responses_recieved;
size_t responses_received;
int to_retry;
int back_off;

Expand Down Expand Up @@ -241,4 +241,6 @@ void priv_getdns_context_ub_read_cb(void *userarg);

void priv_getdns_upstreams_dereference(getdns_upstreams *upstreams);

void priv_getdns_upstream_shutdown(getdns_upstream *upstreams);

#endif /* _GETDNS_CONTEXT_H_ */
39 changes: 7 additions & 32 deletions src/stub.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,13 +505,7 @@ upstream_erred(getdns_upstream *upstream)
netreq->state = NET_REQ_FINISHED;
priv_getdns_check_dns_req_complete(netreq->owner);
}
if (upstream->tls_obj) {
SSL_shutdown(upstream->tls_obj);
SSL_free(upstream->tls_obj);
upstream->tls_obj = NULL;
}
close(upstream->fd);
upstream->fd = -1;
priv_getdns_upstream_shutdown(upstream);
}

void
Expand Down Expand Up @@ -555,31 +549,21 @@ stub_timeout_cb(void *userarg)
(void) getdns_context_request_timed_out(netreq->owner);
}


static void
upstream_idle_timeout_cb(void *userarg)
{
getdns_upstream *upstream = (getdns_upstream *)userarg;
DEBUG_STUB("*** %s: **Closing connection %d**\n",
__FUNCTION__, upstream->fd);
/*There is a race condition with a new request being scheduled while this happens
so take ownership of the fd asap*/
int fd = upstream->fd;
upstream->fd = -1;

upstream->event.timeout_cb = NULL;
upstream->event.read_cb = NULL;
upstream->event.write_cb = NULL;
GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event);
if (upstream->tls_hs_state != GETDNS_HS_FAILED)
upstream->tls_hs_state = GETDNS_HS_NONE;
if (upstream->tls_obj != NULL) {
SSL_shutdown(upstream->tls_obj);
SSL_free(upstream->tls_obj);
upstream->tls_obj = NULL;
}
close(fd);
priv_getdns_upstream_shutdown(upstream);
}


static void
upstream_tls_timeout_cb(void *userarg)
{
Expand Down Expand Up @@ -1290,7 +1274,6 @@ upstream_read_cb(void *userarg)
getdns_upstream *upstream = (getdns_upstream *)userarg;
getdns_network_req *netreq;
getdns_dns_req *dnsreq;
uint64_t idle_timeout;
int q;
uint16_t query_id;
intptr_t query_id_intptr;
Expand Down Expand Up @@ -1329,12 +1312,10 @@ upstream_read_cb(void *userarg)
netreq->response_len =
upstream->tcp.read_pos - upstream->tcp.read_buf;
upstream->tcp.read_buf = NULL;
upstream->responses_recieved++;
upstream->responses_received++;
/* TODO[TLS]: I don't think we should do this for TCP. We should stay
* on a working connection until we hit a problem.*/
upstream->upstreams->current = 0;
/* netreq may die before setting timeout*/
idle_timeout = netreq->owner->context->idle_timeout;

/* TODO: DNSSEC */
netreq->secure = 0;
Expand Down Expand Up @@ -1674,7 +1655,7 @@ fallback_on_write(getdns_network_req *netreq)

/* For sync messages we must re-schedule the events on the old upstream
* here too. Must schedule this last to make sure it is called back first! */
if (netreq->owner->loop != upstream->loop && upstream->write_queue)
if (netreq->owner->loop != upstream->loop)
upstream_reschedule_netreq_events(upstream, upstream->write_queue);

if (result != GETDNS_RETURN_GOOD)
Expand Down Expand Up @@ -1751,13 +1732,7 @@ upstream_reschedule_netreq_events(getdns_upstream *upstream,
* So we will have to be aggressive and shut the connection....*/
DEBUG_STUB("# %s: **Closing connection %d**\n",
__FUNCTION__, upstream->fd);
if (upstream->tls_obj) {
SSL_shutdown(upstream->tls_obj);
SSL_free(upstream->tls_obj);
upstream->tls_obj = NULL;
}
close(upstream->fd);
upstream->fd = -1;
priv_getdns_upstream_shutdown(upstream);
}
}

Expand Down