The recent fix for handling large (about 16k) answers in unwind has
the downside that we are now always copying at least 16k per answer
between the resolver and frontend process.
That seems wasteful.
This re-arranges things to only copy what its needed.
Tests, OKs?
diff --git frontend.c frontend.c
index 8adbb07219b..74715087c5f 100644
--- frontend.c
+++ frontend.c
@@ -86,7 +86,8 @@ struct pending_query {
int fd;
int bogus;
int rcode_override;
- ssize_t answer_len;
+ int answer_len;
+ int received;
uint8_t *answer;
};
@@ -424,10 +425,7 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
struct imsgev *iev = bula;
struct imsgbuf *ibuf = &iev->ibuf;
struct imsg imsg;
- struct query_imsg *query_imsg;
- struct answer_imsg *answer_imsg;
int n, shut = 0, chg;
- uint8_t *p;
if (event & EV_READ) {
if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -449,45 +447,30 @@ frontend_dispatch_resolver(int fd, short event, void
*bula)
break;
switch (imsg.hdr.type) {
- case IMSG_ANSWER_HEADER:
- if (IMSG_DATA_SIZE(imsg) != sizeof(*query_imsg))
- fatalx("%s: IMSG_ANSWER_HEADER wrong length: "
- "%lu", __func__, IMSG_DATA_SIZE(imsg));
- query_imsg = (struct query_imsg *)imsg.data;
- if ((pq = find_pending_query(query_imsg->id)) ==
- NULL) {
- log_warnx("cannot find pending query %llu",
- query_imsg->id);
- break;
- }
- if (query_imsg->err) {
- send_answer(pq);
- pq = NULL;
- break;
- }
- pq->bogus = query_imsg->bogus;
- break;
- case IMSG_ANSWER:
- if (IMSG_DATA_SIZE(imsg) != sizeof(*answer_imsg))
+ case IMSG_ANSWER: {
+ struct answer_header *answer_header;
+ int data_len;
+ uint8_t *data;
+
+ if (IMSG_DATA_SIZE(imsg) < sizeof(*answer_header))
fatalx("%s: IMSG_ANSWER wrong length: "
"%lu", __func__, IMSG_DATA_SIZE(imsg));
- answer_imsg = (struct answer_imsg *)imsg.data;
- if ((pq = find_pending_query(answer_imsg->id)) ==
+ answer_header = (struct answer_header *)imsg.data;
+ data = (uint8_t *)imsg.data + sizeof(*answer_header);
+ if (answer_header->answer_len > 65535)
+ fatalx("%s: IMSG_ANSWER answer too big: %d",
+ __func__, answer_header->answer_len);
+ data_len = IMSG_DATA_SIZE(imsg) -
+ sizeof(*answer_header);
+
+ if ((pq = find_pending_query(answer_header->id)) ==
NULL) {
- log_warnx("cannot find pending query %llu",
- answer_imsg->id);
+ log_warnx("%s: cannot find pending query %llu",
+ __func__, answer_header->id);
break;
}
- p = realloc(pq->answer, pq->answer_len +
- answer_imsg->len);
-
- if (p != NULL) {
- pq->answer = p;
- memcpy(pq->answer + pq->answer_len,
- answer_imsg->answer, answer_imsg->len);
- pq->answer_len += answer_imsg->len;
- } else {
+ if (answer_header->srvfail) {
free(pq->answer);
pq->answer_len = 0;
pq->answer = NULL;
@@ -495,9 +478,32 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
send_answer(pq);
break;
}
- if (!answer_imsg->truncated)
+
+ if (pq->answer == NULL) {
+ pq->answer = malloc(answer_header->answer_len);
+ if (pq->answer == NULL) {
+ pq->answer_len = 0;
+ pq->rcode_override =
+ LDNS_RCODE_SERVFAIL;
+ send_answer(pq);
+ break;
+ }
+ pq->answer_len = answer_header->answer_len;
+ pq->received = 0;
+ pq->bogus = answer_header->bogus;
+ }
+
+ if (pq->received + data_len > pq->answer_len)
+ fatalx("%s: IMSG_ANSWER answer too big: %d",
+ __func__, data_len);
+
+ memcpy(pq->answer + pq->received, data, data_len);
+ pq->received += data_len;
+
+ if (pq->received == pq->answer_len)
send_answer(pq);
break;
+ }
case IMSG_CTL_RESOLVER_INFO:
case IMSG_CTL_AUTOCONF_RESOLVER_INFO:
case IMSG_CTL_MEM_INFO:
diff --git resolver.c resolver.c
index fb34dee87d6..c3faba08faf 100644
--- resolver.c
+++ resolver.c
@@ -884,7 +884,7 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
sldns_buffer *buf = NULL;
struct regional *region = NULL;
struct query_imsg *query_imsg;
- struct answer_imsg answer_imsg;
+ struct answer_header *answer_header;
struct running_query *rq;
struct timespec tp, elapsed;
int64_t ms;
@@ -894,12 +894,19 @@ resolve_done(struct uw_resolver *res, void *arg, int
rcode,
char rcode_buf[16];
char qclass_buf[16];
char qtype_buf[16];
- uint8_t *p;
+ uint8_t *p, *data;
+ uint8_t answer_imsg[MAX_IMSGSIZE - IMSG_HEADER_SIZE];
clock_gettime(CLOCK_MONOTONIC, &tp);
query_imsg = (struct query_imsg *)arg;
+ answer_header = (struct answer_header *)answer_imsg;
+ data = answer_imsg + sizeof(*answer_header);
+ answer_header->id = query_imsg->id;
+ answer_header->srvfail = 0;
+ answer_header->answer_len = answer_len;
+
timespecsub(&tp, &query_imsg->tp, &elapsed);
ms = elapsed.tv_sec * 1000 + elapsed.tv_nsec / 1000000;
@@ -1004,43 +1011,33 @@ resolve_done(struct uw_resolver *res, void *arg, int
rcode,
if (result->rcode == LDNS_RCODE_SERVFAIL)
goto servfail;
- query_imsg->err = 0;
-
if (sec == SECURE)
res->state = VALIDATING;
if (res->state == VALIDATING && sec == BOGUS) {
- query_imsg->bogus = !force_acceptbogus;
- if (query_imsg->bogus && why_bogus != NULL)
+ answer_header->bogus = !force_acceptbogus;
+ if (answer_header->bogus && why_bogus != NULL)
log_warnx("%s", why_bogus);
} else
- query_imsg->bogus = 0;
+ answer_header->bogus = 0;
- if (resolver_imsg_compose_frontend(IMSG_ANSWER_HEADER, 0, query_imsg,
- sizeof(*query_imsg)) == -1)
- fatalx("IMSG_ANSWER_HEADER failed for \"%s %s %s\"",
- query_imsg->qname, qclass_buf, qtype_buf);
-
- answer_imsg.id = query_imsg->id;
p = answer_packet;
- while ((size_t)answer_len > MAX_ANSWER_SIZE) {
- answer_imsg.truncated = 1;
- answer_imsg.len = MAX_ANSWER_SIZE;
- memcpy(&answer_imsg.answer, p, MAX_ANSWER_SIZE);
- if (resolver_imsg_compose_frontend(IMSG_ANSWER, 0, &answer_imsg,
- sizeof(answer_imsg)) == -1)
+ do {
+ int len;
+
+ if ((size_t)answer_len > sizeof(answer_imsg) -
+ sizeof(*answer_header))
+ len = sizeof(answer_imsg) - sizeof(*answer_header);
+ else
+ len = answer_len;
+ memcpy(data, p, len);
+ if (resolver_imsg_compose_frontend(IMSG_ANSWER, 0,
+ &answer_imsg, sizeof(*answer_header) + len) == -1)
fatalx("IMSG_ANSWER failed for \"%s %s %s\"",
query_imsg->qname, qclass_buf, qtype_buf);
- p += MAX_ANSWER_SIZE;
- answer_len -= MAX_ANSWER_SIZE;
- }
- answer_imsg.truncated = 0;
- answer_imsg.len = answer_len;
- memcpy(&answer_imsg.answer, p, answer_len);
- if (resolver_imsg_compose_frontend(IMSG_ANSWER, 0, &answer_imsg,
- sizeof(answer_imsg)) == -1)
- fatalx("IMSG_ANSWER failed for \"%s %s %s\"",
- query_imsg->qname, qclass_buf, qtype_buf);
+ answer_len -= len;
+ p += len;
+ } while (answer_len > 0);
TAILQ_REMOVE(&running_queries, rq, entry);
evtimer_del(&rq->timer_ev);
@@ -1052,9 +1049,9 @@ resolve_done(struct uw_resolver *res, void *arg, int
rcode,
/* try_next_resolver() might free rq */
if (try_next_resolver(rq) != 0 && running_res == 0) {
/* we are the last one, send SERVFAIL */
- query_imsg->err = -4; /* UB_SERVFAIL */
- resolver_imsg_compose_frontend(IMSG_ANSWER_HEADER, 0,
- query_imsg, sizeof(*query_imsg));
+ answer_header->srvfail = 1;
+ resolver_imsg_compose_frontend(IMSG_ANSWER, 0,
+ answer_imsg, sizeof(*answer_header));
}
out:
free(query_imsg);
diff --git unwind.h unwind.h
index 9ebd8f47f87..161619cd34c 100644
--- unwind.h
+++ unwind.h
@@ -116,7 +116,6 @@ enum imsg_type {
IMSG_SOCKET_IPC_FRONTEND,
IMSG_SOCKET_IPC_RESOLVER,
IMSG_QUERY,
- IMSG_ANSWER_HEADER,
IMSG_ANSWER,
IMSG_CTL_RESOLVER_INFO,
IMSG_CTL_AUTOCONF_RESOLVER_INFO,
@@ -170,18 +169,14 @@ struct query_imsg {
char qname[NI_MAXHOST];
int t;
int c;
- int err;
- int bogus;
struct timespec tp;
};
-struct answer_imsg {
-#define MAX_ANSWER_SIZE MAX_IMSGSIZE - IMSG_HEADER_SIZE -
sizeof(uint64_t) - \
- 2 * sizeof(int)
- uint64_t id;
- int truncated;
- int len;
- uint8_t answer[MAX_ANSWER_SIZE];
+struct answer_header {
+ uint64_t id;
+ int srvfail;
+ int bogus;
+ int answer_len;
};
extern uint32_t cmd_opts;
--
I'm not entirely sure you are real.
