From 56c6301089a3d5c5c01c72a55aa99ba3fec6d978 Mon Sep 17 00:00:00 2001 From: Simon Goldschmidt Date: Tue, 16 Sep 2014 19:33:20 +0200 Subject: [PATCH] dns.c: change dns_send/dns_recv to operate on pbuf, not on contiguous buffer -> dns_payload_buffer/DNS_MSG_SIZE can be removed --- CHANGELOG | 3 + src/core/dns.c | 139 ++++++++++++++++++----------------------- src/include/lwip/opt.h | 5 -- 3 files changed, 65 insertions(+), 82 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 8fbe3cac..226be957 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,9 @@ HISTORY ++ New features: + 2014-09-16: Simon Goldschmidt + * dns.c, opt.h: reduced ram usage by parsing DNS responses in place + 2014-09-16: Simon Goldschmidt * pbuf.h/.c: added pbuf_take_at() and pbuf_put_at() diff --git a/src/core/dns.c b/src/core/dns.c index 9d545101..15b65708 100644 --- a/src/core/dns.c +++ b/src/core/dns.c @@ -293,9 +293,6 @@ static u8_t dns_seqno; static struct dns_table_entry dns_table[DNS_TABLE_SIZE]; static struct dns_req_entry dns_requests[DNS_MAX_REQUESTS]; static ip_addr_t dns_servers[DNS_MAX_SERVERS]; -/** Contiguous buffer for processing responses */ -static u8_t dns_payload_buffer[LWIP_MEM_ALIGN_BUFFER(DNS_MSG_SIZE)]; -static u8_t* dns_payload; #ifndef LWIP_DNS_STRICMP #define LWIP_DNS_STRICMP(str1, str2) dns_stricmp(str1, str2) @@ -345,8 +342,6 @@ dns_init() LWIP_ASSERT("sanity check SIZEOF_DNS_ANSWER", sizeof(struct dns_answer) <= SIZEOF_DNS_ANSWER_ASSERT); - dns_payload = (u8_t *)LWIP_MEM_ALIGN(dns_payload_buffer); - /* initialize default DNS server address */ DNS_SERVER_ADDRESS(&dnsserver); @@ -598,50 +593,53 @@ dns_lookup(const char *name) * any more). * * @param query hostname (not encoded) from the dns_table - * @param response encoded hostname in the DNS response - * @return 0: names equal; 1: names differ + * @param p pbuf containing the encoded hostname in the DNS response + * @param start_offset offset into p where the name starts + * @return 0xFFFF: names differ, other: names equal -> offset behind name */ -static char* -dns_compare_name(char *query, char *response) +static u16_t +dns_compare_name(char *query, struct pbuf* p, u16_t start_offset) { unsigned char n; + u16_t response_offset = start_offset; do { - n = *response++; + n = pbuf_get_at(p, response_offset++); /** @see RFC 1035 - 4.1.4. Message compression */ if ((n & 0xc0) == 0xc0) { /* Compressed name: cannot be equal since we don't send them */ - return NULL; + return 0xFFFF; } else { /* Not compressed name */ while (n > 0) { - if ((*query) != (*response)) { - return NULL; + if ((*query) != pbuf_get_at(p, response_offset)) { + return 0xFFFF; } - ++response; + ++response_offset; ++query; --n; }; ++query; } - } while (*response != 0); + } while (pbuf_get_at(p, response_offset) != 0); - return response + 1; + return response_offset + 1; } /** * Walk through a compact encoded DNS name and return the end of the name. * - * @param query encoded DNS name in the DNS server response - * @return end of the name + * @param p pbuf containing the name + * @param query_idx start index into p pointing to encoded DNS name in the DNS server response + * @return index to end of the name */ -static char * -dns_parse_name(char *query) +static u16_t +dns_parse_name(struct pbuf* p, u16_t query_idx) { unsigned char n; do { - n = (unsigned char) *query++; + n = pbuf_get_at(p, query_idx++); /** @see RFC 1035 - 4.1.4. Message compression */ if ((n & 0xc0) == 0xc0) { /* Compressed name */ @@ -649,13 +647,13 @@ dns_parse_name(char *query) } else { /* Not compressed name */ while (n > 0) { - ++query; + ++query_idx; --n; }; } - } while (*query != 0); + } while (pbuf_get_at(p, query_idx) != 0); - return query + 1; + return query_idx + 1; } /** @@ -668,11 +666,11 @@ static err_t dns_send(struct dns_table_entry* entry) { err_t err; - struct dns_hdr *hdr; + struct dns_hdr hdr; struct dns_query qry; struct pbuf *p; - char *query, *nptr; - const char *pHostname; + u16_t query_idx, copy_len; + const char *hostname, *hostname_part; u8_t n; u8_t pcb_idx; @@ -682,44 +680,38 @@ dns_send(struct dns_table_entry* entry) LWIP_ASSERT("dns server has no IP address set", !ip_addr_isany(&dns_servers[entry->server_idx])); /* if here, we have either a new query or a retry on a previous query to process */ - p = pbuf_alloc(PBUF_TRANSPORT, SIZEOF_DNS_HDR + DNS_MAX_NAME_LENGTH + 1 + + p = pbuf_alloc(PBUF_TRANSPORT, SIZEOF_DNS_HDR + strlen(entry->name) + 2 + SIZEOF_DNS_QUERY, PBUF_RAM); if (p != NULL) { - u16_t realloc_size; - LWIP_ASSERT("pbuf must be in one piece", p->next == NULL); /* fill dns header */ - hdr = (struct dns_hdr*)p->payload; - memset(hdr, 0, SIZEOF_DNS_HDR); - hdr->id = htons(entry->txid); - hdr->flags1 = DNS_FLAG1_RD; - hdr->numquestions = PP_HTONS(1); - query = (char*)hdr + SIZEOF_DNS_HDR; - pHostname = entry->name; - --pHostname; + memset(&hdr, 0, SIZEOF_DNS_HDR); + hdr.id = htons(entry->txid); + hdr.flags1 = DNS_FLAG1_RD; + hdr.numquestions = PP_HTONS(1); + pbuf_take(p, &hdr, SIZEOF_DNS_HDR); + hostname = entry->name; + --hostname; /* convert hostname into suitable query format. */ + query_idx = SIZEOF_DNS_HDR; do { - ++pHostname; - nptr = query; - ++query; - for(n = 0; *pHostname != '.' && *pHostname != 0; ++pHostname) { - *query = *pHostname; - ++query; + ++hostname; + hostname_part = hostname; + for(n = 0; *hostname != '.' && *hostname != 0; ++hostname) { ++n; } - *nptr = n; - } while(*pHostname != 0); - *query++='\0'; + copy_len = hostname - hostname_part; + pbuf_put_at(p, query_idx, n); + pbuf_take_at(p, hostname_part, copy_len, query_idx + 1); + query_idx += n + 1; + } while(*hostname != 0); + pbuf_put_at(p, query_idx, 0); + query_idx++; /* fill dns query */ qry.type = PP_HTONS(DNS_RRTYPE_A); qry.cls = PP_HTONS(DNS_RRCLASS_IN); - SMEMCPY(query, &qry, SIZEOF_DNS_QUERY); - - /* resize pbuf to the exact dns query */ - realloc_size = (u16_t)((query + SIZEOF_DNS_QUERY) - ((char*)(p->payload))); - LWIP_ASSERT("p->tot_len >= realloc_size", p->tot_len >= realloc_size); - pbuf_realloc(p, realloc_size); + pbuf_take_at(p, &qry, SIZEOF_DNS_QUERY, query_idx); #if ((LWIP_DNS_SECURE & LWIP_DNS_SECURE_RAND_SRC_PORT) != 0) pcb_idx = entry->pcb_idx; @@ -991,8 +983,8 @@ dns_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, ip_addr_t *addr, u16_t { u8_t i, entry_idx = DNS_TABLE_SIZE; u16_t txid; - char *ptr; - struct dns_hdr *hdr; + u16_t res_idx; + struct dns_hdr hdr; struct dns_answer ans; struct dns_query qry; u16_t nquestions, nanswers; @@ -1001,13 +993,6 @@ dns_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, ip_addr_t *addr, u16_t LWIP_UNUSED_ARG(pcb); LWIP_UNUSED_ARG(port); - /* is the dns message too big ? */ - if (p->tot_len > DNS_MSG_SIZE) { - LWIP_DEBUGF(DNS_DEBUG, ("dns_recv: pbuf too big\n")); - /* free pbuf and return */ - goto memerr; - } - /* is the dns message big enough ? */ if (p->tot_len < (SIZEOF_DNS_HDR + SIZEOF_DNS_QUERY + SIZEOF_DNS_ANSWER)) { LWIP_DEBUGF(DNS_DEBUG, ("dns_recv: pbuf too small\n")); @@ -1016,10 +1001,9 @@ dns_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, ip_addr_t *addr, u16_t } /* copy dns payload inside static buffer for processing */ - if (pbuf_copy_partial(p, dns_payload, p->tot_len, 0) == p->tot_len) { + if (pbuf_copy_partial(p, &hdr, SIZEOF_DNS_HDR, 0) == SIZEOF_DNS_HDR) { /* Match the ID in the DNS header with the name table. */ - hdr = (struct dns_hdr*)dns_payload; - txid = htons(hdr->id); + txid = htons(hdr.id); for (i = 0; i < DNS_TABLE_SIZE; i++) { struct dns_table_entry *entry = &dns_table[i]; entry_idx = i; @@ -1028,15 +1012,15 @@ dns_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, ip_addr_t *addr, u16_t u8_t dns_err; /* This entry is now completed. */ entry->state = DNS_STATE_DONE; - dns_err = hdr->flags2 & DNS_FLAG2_ERR_MASK; + dns_err = hdr.flags2 & DNS_FLAG2_ERR_MASK; /* We only care about the question(s) and the answers. The authrr and the extrarr are simply discarded. */ - nquestions = htons(hdr->numquestions); - nanswers = htons(hdr->numanswers); + nquestions = htons(hdr.numquestions); + nanswers = htons(hdr.numanswers); /* Check for error. If so, call callback to inform. */ - if (((hdr->flags1 & DNS_FLAG1_RESPONSE) == 0) || (dns_err != 0) || (nquestions != 1)) { + if (((hdr.flags1 & DNS_FLAG1_RESPONSE) == 0) || (dns_err != 0) || (nquestions != 1)) { LWIP_DEBUGF(DNS_DEBUG, ("dns_recv: \"%s\": error in flags\n", entry->name)); /* call callback to indicate error, clean up memory and return */ goto responseerr; @@ -1051,38 +1035,39 @@ dns_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, ip_addr_t *addr, u16_t /* Check if the name in the "question" part match with the name in the entry and skip it if equal. */ - ptr = dns_compare_name(entry->name, (char*)dns_payload + SIZEOF_DNS_HDR); - if (ptr == NULL) { + res_idx = dns_compare_name(entry->name, p, SIZEOF_DNS_HDR); + if (res_idx == 0xFFFF) { LWIP_DEBUGF(DNS_DEBUG, ("dns_recv: \"%s\": response not match to query\n", entry->name)); /* call callback to indicate error, clean up memory and return */ goto responseerr; } /* check if "question" part matches the request */ - SMEMCPY(&qry, ptr, SIZEOF_DNS_QUERY); + pbuf_copy_partial(p, &qry, SIZEOF_DNS_QUERY, res_idx); if((qry.type != PP_HTONS(DNS_RRTYPE_A)) || (qry.cls != PP_HTONS(DNS_RRCLASS_IN))) { LWIP_DEBUGF(DNS_DEBUG, ("dns_recv: \"%s\": response not match to query\n", entry->name)); /* call callback to indicate error, clean up memory and return */ goto responseerr; } /* skip the rest of the "question" part */ - ptr += SIZEOF_DNS_QUERY; + res_idx += SIZEOF_DNS_QUERY; while (nanswers > 0) { /* skip answer resource record's host name */ - ptr = dns_parse_name(ptr); + res_idx = dns_parse_name(p, res_idx); /* Check for IP address type and Internet class. Others are discarded. */ - SMEMCPY(&ans, ptr, SIZEOF_DNS_ANSWER); + pbuf_copy_partial(p, &ans, SIZEOF_DNS_ANSWER, res_idx); if((ans.type == PP_HTONS(DNS_RRTYPE_A)) && (ans.cls == PP_HTONS(DNS_RRCLASS_IN)) && (ans.len == PP_HTONS(sizeof(ip_addr_t))) ) { + res_idx += SIZEOF_DNS_ANSWER; /* read the answer resource record's TTL, and maximize it if needed */ entry->ttl = ntohl(ans.ttl); if (entry->ttl > DNS_MAX_TTL) { entry->ttl = DNS_MAX_TTL; } /* read the IP address after answer resource record's header */ - SMEMCPY(&(entry->ipaddr), (ptr + SIZEOF_DNS_ANSWER), sizeof(ip_addr_t)); + pbuf_copy_partial(p, &(entry->ipaddr), sizeof(entry->ipaddr), res_idx); LWIP_DEBUGF(DNS_DEBUG, ("dns_recv: \"%s\": response = ", entry->name)); ip_addr_debug_print(DNS_DEBUG, (&(entry->ipaddr))); LWIP_DEBUGF(DNS_DEBUG, ("\n")); @@ -1098,7 +1083,7 @@ dns_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, ip_addr_t *addr, u16_t /* deallocate memory and return */ goto memerr; } else { - ptr = ptr + SIZEOF_DNS_ANSWER + htons(ans.len); + res_idx = SIZEOF_DNS_ANSWER + htons(ans.len); } --nanswers; } diff --git a/src/include/lwip/opt.h b/src/include/lwip/opt.h index c0af6d02..243ba23e 100644 --- a/src/include/lwip/opt.h +++ b/src/include/lwip/opt.h @@ -901,11 +901,6 @@ #define DNS_DOES_NAME_CHECK 1 #endif -/** DNS message max. size. Default value is RFC compliant. */ -#ifndef DNS_MSG_SIZE -#define DNS_MSG_SIZE 512 -#endif - /** DNS_LOCAL_HOSTLIST: Implements a local host-to-address list. If enabled, * you have to define * #define DNS_LOCAL_HOSTLIST_INIT {{"host1", 0x123}, {"host2", 0x234}}