Finesse TCP timeouts for upstream connections.

Timeouts for TCP connections to non-responive servers are very long.
This in not appropriate for DNS connections.

Set timeouts for connection setup, sending data and recieving data.
The timeouts for connection setup and sending data are set at 5 seconds.
For recieving the reply this is doubled, to take into account the
time for usptream to actually get the answer.

Thanks to Petr Menšík for pointing out this problem, and finding a better
and more portable solution than the one in place heretofore.
This commit is contained in:
Simon Kelley
2024-11-25 23:18:07 +00:00
parent 481ff0ed10
commit 32a8f3e009
10 changed files with 113 additions and 85 deletions

View File

@@ -1879,11 +1879,11 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
u16 *length = (u16 *)packet;
unsigned char *payload = &packet[2];
struct dns_header *header = (struct dns_header *)payload;
unsigned char c1, c2;
unsigned int rsize;
int class, rclass, type, rtype;
unsigned char *p;
struct blockdata *saved_question;
struct timeval tv;
(void)mark;
(void)have_mark;
@@ -1901,7 +1901,7 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
while (1)
{
int data_sent = 0, timedout = 0;
int data_sent = 0, fatal = 0;
struct server *serv;
if (firstsendto == -1)
@@ -1942,12 +1942,17 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
continue;
}
#ifdef TCP_SYNCNT
/* TCP connections by default take ages to time out.
At least on Linux, we can reduce that to only two attempts
to get a reply. For DNS, that's more sensible. */
mark = 2;
setsockopt(serv->tcpfd, IPPROTO_TCP, TCP_SYNCNT, &mark, sizeof(unsigned int));
#if defined(SO_SNDTIMEO) && defined(SO_RCVTIMEO)
/* TCP connections by default take ages to time out.
Set shorter timeouts more appropriate for a DNS server.
We set the recieve timeout as twice the send timeout; we
want to fail quickly on a non-responsive server, but give it time to get an
answer. */
tv.tv_sec = TCP_TIMEOUT;
tv.tv_usec = 0;
setsockopt(serv->tcpfd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv));
tv.tv_sec += TCP_TIMEOUT;
setsockopt(serv->tcpfd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
#endif
#ifdef MSG_FASTOPEN
@@ -1955,13 +1960,13 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
if (errno == 0)
data_sent = 1;
else if (errno == ETIMEDOUT || errno == EHOSTUNREACH)
timedout = 1;
else if (errno == ETIMEDOUT || errno == EHOSTUNREACH || errno == EINPROGRESS || errno == ECONNREFUSED)
fatal = 1;
#endif
/* If fastopen failed due to lack of reply, then there's no point in
trying again in non-FASTOPEN mode. */
if (timedout || (!data_sent && connect(serv->tcpfd, &serv->addr.sa, sa_len(&serv->addr)) == -1))
if (fatal || (!data_sent && connect(serv->tcpfd, &serv->addr.sa, sa_len(&serv->addr)) == -1))
{
close(serv->tcpfd);
serv->tcpfd = -1;
@@ -1972,10 +1977,11 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
serv->flags &= ~SERV_GOT_TCP;
}
if ((!data_sent && !read_write(serv->tcpfd, packet, qsize + sizeof(u16), 0)) ||
!read_write(serv->tcpfd, &c1, 1, 1) ||
!read_write(serv->tcpfd, &c2, 1, 1) ||
!read_write(serv->tcpfd, payload, (rsize = (c1 << 8) | c2), 1))
/* We us the _ONCE veriant of read_write() here because we've set a timeout on the tcp socket
and wish to abort if the whole data is not read/written within the timeout. */
if ((!data_sent && !read_write(serv->tcpfd, (unsigned char *)packet, qsize + sizeof(u16), RW_WRITE_ONCE)) ||
!read_write(serv->tcpfd, (unsigned char *)length, sizeof (*length), RW_READ_ONCE) ||
!read_write(serv->tcpfd, payload, (rsize = ntohs(*length)), RW_READ_ONCE))
{
close(serv->tcpfd);
serv->tcpfd = -1;
@@ -2210,7 +2216,7 @@ unsigned char *tcp_request(int confd, time_t now,
/* Max TCP packet + slop + size */
unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ + sizeof(u16));
unsigned char *payload = &packet[2];
unsigned char c1, c2;
u16 tcp_len;
/* largest field in header is 16-bits, so this is still sufficiently aligned */
struct dns_header *header = (struct dns_header *)payload;
u16 *length = (u16 *)packet;
@@ -2283,9 +2289,9 @@ unsigned char *tcp_request(int confd, time_t now,
if (query_count >= TCP_MAX_QUERIES)
break;
if (!read_write(confd, &c1, 1, 1) || !read_write(confd, &c2, 1, 1) ||
!(size = c1 << 8 | c2) ||
!read_write(confd, payload, size, 1))
if (!read_write(confd, (unsigned char *)&tcp_len, sizeof(tcp_len), RW_READ) ||
!(size = ntohs(tcp_len)) ||
!read_write(confd, payload, size, RW_READ))
break;
}
@@ -2584,7 +2590,7 @@ unsigned char *tcp_request(int confd, time_t now,
if (option_bool(OPT_CMARK_ALST_EN) && have_mark && ((u32)mark & daemon->allowlist_mask))
report_addresses(header, m, mark);
#endif
if (!read_write(confd, packet, m + sizeof(u16), 0))
if (!read_write(confd, packet, m + sizeof(u16), RW_WRITE))
break;
/* If we answered with stale data, this process will now try and get fresh data into