MAJOR: dns: save a copy of the DNS response in struct resolution
Prior this patch, the DNS responses were stored in a pre-allocated
memory area (allocated at HAProxy's startup).
The problem is that this memory is erased for each new DNS responses
received and processed.
This patch removes the global memory allocation (which was not thread
safe by the way) and introduces a storage of the dns response in the
struct
resolution.
The memory in the struct resolution is also reserved at start up and is
thread safe, since each resolution structure will have its own memory
area.
For now, we simply store the response and use it atomically per
response per server.
diff --git a/include/proto/dns.h b/include/proto/dns.h
index ffb1cac..14a6af1 100644
--- a/include/proto/dns.h
+++ b/include/proto/dns.h
@@ -32,7 +32,7 @@
struct task *dns_process_resolve(struct task *t);
int dns_init_resolvers(int close_socket);
uint16_t dns_rnd16(void);
-int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, struct dns_response_packet *dns_p);
+int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, struct dns_resolution *resolution);
int dns_get_ip_from_response(struct dns_response_packet *dns_p,
struct dns_options *dns_opts, void *currentip,
short currentip_sin_family,
diff --git a/include/proto/server.h b/include/proto/server.h
index ec29230..53df241 100644
--- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -53,7 +53,7 @@
/* functions related to server name resolution */
int snr_update_srv_status(struct server *s);
-int snr_resolution_cb(struct dns_resolution *resolution, struct dns_nameserver *nameserver, struct dns_response_packet *dns_p);
+int snr_resolution_cb(struct dns_resolution *resolution, struct dns_nameserver *nameserver);
int snr_resolution_error_cb(struct dns_resolution *resolution, int error_code);
struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char *ip_family);
diff --git a/include/types/dns.h b/include/types/dns.h
index c86eb4d..54bbd02 100644
--- a/include/types/dns.h
+++ b/include/types/dns.h
@@ -219,7 +219,7 @@
struct dns_resolution {
struct list list; /* resolution list */
void *requester; /* owner of this name resolution */
- int (*requester_cb)(struct dns_resolution *, struct dns_nameserver *, struct dns_response_packet *);
+ int (*requester_cb)(struct dns_resolution *, struct dns_nameserver *);
/* requester callback for valid response */
int (*requester_error_cb)(struct dns_resolution *, int);
/* requester callback, for error management */
@@ -237,6 +237,10 @@
int try; /* current resolution try */
int try_cname; /* number of CNAME requests sent */
int nb_responses; /* count number of responses received */
+ struct dns_response_packet response; /* structure hosting the DNS response */
+ struct dns_query_item response_query_records[DNS_MAX_QUERY_RECORDS]; /* <response> query records */
+ struct dns_answer_item response_answer_records[DNS_MAX_ANSWER_RECORDS]; /* <response> answer records */
+ struct chunk response_buffer; /* buffer used as a data store for <response> above TODO: optimize the size (might be smaller) */
};
/* last resolution status code */
diff --git a/src/dns.c b/src/dns.c
index 0a1c520..bcb78bf 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -43,17 +43,6 @@
struct list dns_resolvers = LIST_HEAD_INIT(dns_resolvers);
struct dns_resolution *resolution = NULL;
-/*
- * pre-allocated memory for maximum record names in a DNS response
- * Each name is DNS_MAX_NAME_SIZE, we add 1 for the NULL character
- *
- * WARNING: this is not thread safe...
- */
-struct dns_response_packet dns_response;
-struct chunk dns_trash = { };
-struct dns_query_item dns_query_records[DNS_MAX_QUERY_RECORDS];
-struct dns_answer_item dns_answer_records[DNS_MAX_ANSWER_RECORDS];
-
static int64_t dns_query_id_seed; /* random seed */
/* proto_udp callback functions for a DNS resolution */
@@ -141,7 +130,6 @@
int fd, buflen, ret;
unsigned short query_id;
struct eb32_node *eb;
- struct dns_response_packet *dns_p = &dns_response;
fd = dgram->t.sock.fd;
@@ -200,7 +188,7 @@
/* number of responses received */
resolution->nb_responses += 1;
- ret = dns_validate_dns_response(buf, bufend, dns_p);
+ ret = dns_validate_dns_response(buf, bufend, resolution);
/* treat only errors */
switch (ret) {
@@ -250,7 +238,7 @@
/* Now let's check the query's dname corresponds to the one we sent.
* We can check only the first query of the list. We send one query at a time
* so we get one query in the response */
- query = LIST_NEXT(&dns_p->query_list, struct dns_query_item *, list);
+ query = LIST_NEXT(&resolution->response.query_list, struct dns_query_item *, list);
if (query && memcmp(query->name, resolution->hostname_dn, resolution->hostname_dn_len) != 0) {
nameserver->counters.other += 1;
resolution->requester_error_cb(resolution, DNS_RESP_WRONG_NAME);
@@ -258,7 +246,7 @@
}
nameserver->counters.valid += 1;
- resolution->requester_cb(resolution, nameserver, dns_p);
+ resolution->requester_cb(resolution, nameserver);
}
}
@@ -439,13 +427,13 @@
* Function to validate that the buffer DNS response provided in <resp> and
* finishing before <bufend> is valid from a DNS protocol point of view.
*
- * The result is stored in the structured pointed by <dns_p>.
- * It's up to the caller to allocate memory for <dns_p>.
+ * The result is stored in <resolution>' response, buf_response, response_query_records
+ * and response_answer_records members.
*
* This function returns one of the DNS_RESP_* code to indicate the type of
* error found.
*/
-int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, struct dns_response_packet *dns_p)
+int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, struct dns_resolution *resolution)
{
unsigned char *reader;
char *previous_dname, tmpname[DNS_MAX_NAME_SIZE];
@@ -453,14 +441,18 @@
int dns_query_record_id, dns_answer_record_id;
struct dns_query_item *dns_query;
struct dns_answer_item *dns_answer_record;
+ struct dns_response_packet *dns_p;
+ struct chunk *dns_response_buffer;
reader = resp;
len = 0;
previous_dname = NULL;
- /* initialization of local buffer */
+ /* initialization of response buffer and structure */
+ dns_p = &resolution->response;
+ dns_response_buffer = &resolution->response_buffer;
memset(dns_p, '\0', sizeof(struct dns_response_packet));
- chunk_reset(&dns_trash);
+ chunk_reset(dns_response_buffer);
/* query id */
if (reader + 2 >= bufend)
@@ -540,7 +532,7 @@
*/
if (dns_query_record_id > DNS_MAX_QUERY_RECORDS)
return DNS_RESP_INVALID;
- dns_query = &dns_query_records[dns_query_record_id];
+ dns_query = &resolution->response_query_records[dns_query_record_id];
LIST_ADDQ(&dns_p->query_list, &dns_query->list);
/* name is a NULL terminated string in our case, since we have
@@ -579,7 +571,7 @@
* to the record list */
if (dns_answer_record_id > DNS_MAX_ANSWER_RECORDS)
return DNS_RESP_INVALID;
- dns_answer_record = &dns_answer_records[dns_answer_record_id];
+ dns_answer_record = &resolution->response_answer_records[dns_answer_record_id];
LIST_ADDQ(&dns_p->answer_list, &dns_answer_record->list);
offset = 0;
@@ -604,11 +596,11 @@
}
- dns_answer_record->name = chunk_newstr(&dns_trash);
+ dns_answer_record->name = chunk_newstr(dns_response_buffer);
if (dns_answer_record->name == NULL)
return DNS_RESP_INVALID;
- ret = chunk_strncat(&dns_trash, tmpname, len);
+ ret = chunk_strncat(dns_response_buffer, tmpname, len);
if (ret == 0)
return DNS_RESP_INVALID;
@@ -674,11 +666,11 @@
if (len == 0)
return DNS_RESP_INVALID;
- dns_answer_record->target = chunk_newstr(&dns_trash);
+ dns_answer_record->target = chunk_newstr(dns_response_buffer);
if (dns_answer_record->target == NULL)
return DNS_RESP_INVALID;
- ret = chunk_strncat(&dns_trash, tmpname, len);
+ ret = chunk_strncat(dns_response_buffer, tmpname, len);
if (ret == 0)
return DNS_RESP_INVALID;
@@ -702,7 +694,7 @@
} /* for i 0 to ancount */
/* let's add a last \0 to close our last string */
- ret = chunk_strncat(&dns_trash, "\0", 1);
+ ret = chunk_strncat(dns_response_buffer, "\0", 1);
if (ret == 0)
return DNS_RESP_INVALID;
@@ -737,7 +729,6 @@
int j;
int rec_nb = 0;
int score, max_score;
- struct dns_response_packet *dns_response = dns_p;
family_priority = dns_opts->family_prio;
*newip = newip4 = newip6 = NULL;
@@ -745,7 +736,7 @@
*newip_sin_family = AF_UNSPEC;
/* now parsing response records */
- list_for_each_entry(record, &dns_response->answer_list, list) {
+ list_for_each_entry(record, &dns_p->answer_list, list) {
/* analyzing record content */
switch (record->type) {
case DNS_RTYPE_A:
@@ -943,19 +934,8 @@
struct dns_nameserver *curnameserver;
struct dgram_conn *dgram;
struct task *t;
- char *dns_trash_str;
int fd;
- dns_trash_str = malloc(global.tune.bufsize);
- if (dns_trash_str == NULL) {
- Alert("Starting resolvers: out of memory.\n");
- return 0;
- }
-
- /* allocate memory for the dns_trash buffer used to temporarily store
- * the records of the received response */
- chunk_init(&dns_trash, dns_trash_str, global.tune.bufsize);
-
/* give a first random value to our dns query_id seed */
dns_query_id_seed = random();
@@ -1326,20 +1306,26 @@
struct dns_resolution *dns_alloc_resolution(void)
{
struct dns_resolution *resolution = NULL;
+ char *buffer = NULL;
resolution = calloc(1, sizeof(*resolution));
+ buffer = calloc(1, global.tune.bufsize);
- if (!resolution) {
+ if (!resolution || !buffer) {
+ free(buffer);
free(resolution);
return NULL;
}
+ chunk_init(&resolution->response_buffer, buffer, global.tune.bufsize);
+
return resolution;
}
/* This function free the memory allocated to a DNS resolution */
void dns_free_resolution(struct dns_resolution *resolution)
{
+ chunk_destroy(&resolution->response_buffer);
free(resolution);
return;
diff --git a/src/server.c b/src/server.c
index 9970cde..c6e42be 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3824,7 +3824,7 @@
* 0 on error
* 1 when no error or safe ignore
*/
-int snr_resolution_cb(struct dns_resolution *resolution, struct dns_nameserver *nameserver, struct dns_response_packet *dns_p)
+int snr_resolution_cb(struct dns_resolution *resolution, struct dns_nameserver *nameserver)
{
struct server *s;
void *serverip, *firstip;
@@ -3859,7 +3859,7 @@
goto invalid;
}
- ret = dns_get_ip_from_response(dns_p, &s->dns_opts,
+ ret = dns_get_ip_from_response(&resolution->response, &s->dns_opts,
serverip, server_sin_family, &firstip,
&firstip_sin_family, s);