Review of msgreply.

git-svn-id: file:///svn/unbound/trunk@212 be551aaa-1e26-0410-a405-d3ace91eadb9
This commit is contained in:
Wouter Wijngaards 2007-04-02 13:58:02 +00:00
parent c6ab7d4f50
commit 5bba40f64a
6 changed files with 110 additions and 33 deletions

View file

@ -161,7 +161,7 @@ worker_handle_reply(struct comm_point* c, void* arg, int error,
memcpy(rep->reply, ldns_buffer_at(c->buffer, DNS_ID_AND_FLAGS),
rep->replysize);
reply_info_answer_iov(rep, w->query_id, w->query_flags,
&w->query_reply);
&w->query_reply, 0);
req_release(w);
/* store or update reply in the cache */
if(!(e = query_info_entrysetup(&w->qinfo, rep, w->query_hash))) {
@ -298,7 +298,7 @@ worker_handle_request(struct comm_point* c, void* arg, int error,
log_info("answer from the cache");
memcpy(&id, ldns_buffer_begin(c->buffer), sizeof(uint16_t));
reply_info_answer_iov((struct reply_info*)e->data, id,
ldns_buffer_read_u16_at(c->buffer, 2), repinfo);
ldns_buffer_read_u16_at(c->buffer, 2), repinfo, 1);
lock_rw_unlock(&e->lock);
return 0;
}
@ -316,6 +316,7 @@ worker_handle_request(struct comm_point* c, void* arg, int error,
verbose(VERB_DETAIL, "worker: too many incoming requests "
"active. dropping incoming query.");
comm_point_drop_reply(repinfo);
query_info_clear(&qinfo);
return 0;
}
worker->free_queries = w->next;

View file

@ -2,6 +2,10 @@
- check sizes of udp received messages, not too short.
- review changes. Some memmoves can be memcpys: 4byte aligned.
set id correctly on cached answers.
- review changes msgreply.c, memleak on error condition. AA flag
clear on cached reply. Lowercase queries on hashing.
unit test on lowercasing. Test AA bit not set on cached reply.
Note that no TTLs are managed.
29 March 2007: Wouter
- writev or sendmsg used when answering from cache.

View file

@ -147,6 +147,25 @@ msgreply_test()
unit_assert( query_dname_len(dname_to_buf(buff, ".")) == 1 );
unit_assert( query_dname_len(dname_to_buf(buff, "bla.foo.")) == 9 );
unit_assert( query_dname_len(dname_to_buf(buff, "x.y.z.example.com.")) == 19 );
ldns_buffer_write_at(buff, 0, "\012abCDeaBCde\003cOm\000", 16);
query_dname_tolower(ldns_buffer_begin(buff), 16);
unit_assert( memcmp(ldns_buffer_begin(buff),
"\012abcdeabcde\003com\000", 16) == 0);
ldns_buffer_write_at(buff, 0, "\001+\012abC{e-ZYXe\003NET\000", 18);
query_dname_tolower(ldns_buffer_begin(buff), 18);
unit_assert( memcmp(ldns_buffer_begin(buff),
"\001+\012abc{e-zyxe\003net\000", 18) == 0);
ldns_buffer_write_at(buff, 0, "\000", 1);
query_dname_tolower(ldns_buffer_begin(buff), 1);
unit_assert( memcmp(ldns_buffer_begin(buff), "\000", 1) == 0);
ldns_buffer_write_at(buff, 0, "\002NL\000", 4);
query_dname_tolower(ldns_buffer_begin(buff), 4);
unit_assert( memcmp(ldns_buffer_begin(buff), "\002nl\000", 4) == 0);
ldns_buffer_free(buff);
}

View file

@ -6,6 +6,7 @@ SCENARIO_BEGIN Query receives answer from the cache
STEP 1 QUERY
ENTRY_BEGIN
REPLY RD
SECTION QUESTION
www.example.com. IN A
ENTRY_END
@ -20,7 +21,8 @@ STEP 3 REPLY
ENTRY_BEGIN
MATCH opcode qtype qname
ADJUST copy_id
REPLY QR RD RA NOERROR
; authoritative answer
REPLY QR AA RD RA NOERROR
SECTION QUESTION
www.example.com. IN A
SECTION ANSWER
@ -32,27 +34,40 @@ ENTRY_BEGIN
ENTRY_END
STEP 4 CHECK_ANSWER
ENTRY_BEGIN
MATCH opcode qname qtype
MATCH all
; first reply, have AA set.
REPLY QR AA RD RA
SECTION QUESTION
www.example.com. IN A
SECTION ANSWER
www.example.com. IN A 10.20.30.40
SECTION AUTHORITY
www.example.com. IN NS ns.example.com.
SECTION ADDITIONAL
ns.example.com. IN A 10.20.30.50
ENTRY_END
; another query, same, so it must be answered frm the cache
; another query, same, so it must be answered from the cache
STEP 5 QUERY
ENTRY_BEGIN
REPLY RD
SECTION QUESTION
www.example.com. IN A
ENTRY_END
; immediate answer without an OUT_QUERY happening (checked on exit)
; also, the answer does not have AA set
STEP 6 CHECK_ANSWER
ENTRY_BEGIN
MATCH opcode qname qtype
MATCH all
REPLY QR RD RA
SECTION QUESTION
www.example.com. IN A
SECTION ANSWER
www.example.com. IN A 10.20.30.40
SECTION AUTHORITY
www.example.com. IN NS ns.example.com.
SECTION ADDITIONAL
ns.example.com. IN A 10.20.30.50
ENTRY_END
SCENARIO_END

View file

@ -68,7 +68,8 @@ query_dname_len(ldns_buffer* query)
}
}
int query_info_parse(struct query_info* m, ldns_buffer* query)
int
query_info_parse(struct query_info* m, ldns_buffer* query)
{
uint8_t* q = ldns_buffer_begin(query);
/* minimum size: header + \0 + qtype + qclass */
@ -107,7 +108,9 @@ query_info_allocqname(struct query_info* m)
if( (x) < (y) ) return -1; \
else if( (x) > (y) ) return +1; \
log_assert( (x) == (y) );
int query_info_compare(void* m1, void* m2)
int
query_info_compare(void* m1, void* m2)
{
struct query_info* msg1 = (struct query_info*)m1;
struct query_info* msg2 = (struct query_info*)m2;
@ -115,8 +118,7 @@ int query_info_compare(void* m1, void* m2)
/* from most different to least different for speed */
COMPARE_IT(msg1->qtype, msg2->qtype);
COMPARE_IT(msg1->qnamesize, msg2->qnamesize);
mc = memcmp(msg1->qname, msg2->qname, msg1->qnamesize);
if(mc != 0)
if((mc = memcmp(msg1->qname, msg2->qname, msg1->qnamesize)) != 0)
return mc;
COMPARE_IT(msg1->has_cd, msg2->has_cd);
COMPARE_IT(msg1->qclass, msg2->qclass);
@ -124,19 +126,22 @@ int query_info_compare(void* m1, void* m2)
#undef COMPARE_IT
}
void query_info_clear(struct query_info* m)
void
query_info_clear(struct query_info* m)
{
free(m->qname);
m->qname = NULL;
}
void reply_info_clear(struct reply_info* m)
void
reply_info_clear(struct reply_info* m)
{
free(m->reply);
m->reply = NULL;
}
size_t msgreply_sizefunc(void* k, void* d)
size_t
msgreply_sizefunc(void* k, void* d)
{
struct query_info* q = (struct query_info*)k;
struct reply_info* r = (struct reply_info*)d;
@ -144,7 +149,8 @@ size_t msgreply_sizefunc(void* k, void* d)
+ r->replysize + q->qnamesize;
}
void query_entry_delete(void *k, void* ATTR_UNUSED(arg))
void
query_entry_delete(void *k, void* ATTR_UNUSED(arg))
{
struct msgreply_entry* q = (struct msgreply_entry*)k;
lock_rw_destroy(&q->entry.lock);
@ -152,24 +158,45 @@ void query_entry_delete(void *k, void* ATTR_UNUSED(arg))
free(q);
}
void reply_info_delete(void* d, void* ATTR_UNUSED(arg))
void
reply_info_delete(void* d, void* ATTR_UNUSED(arg))
{
struct reply_info* r = (struct reply_info*)d;
reply_info_clear(r);
free(r);
}
hashvalue_t query_info_hash(struct query_info *q)
void
query_dname_tolower(uint8_t* dname, size_t len)
{
/* the dname is stored uncompressed */
uint8_t labellen;
log_assert(len > 0);
labellen = *dname;
while(labellen) {
dname++;
while(labellen--) {
*dname = (uint8_t)tolower((int)*dname);
dname++;
}
labellen = *dname;
}
}
hashvalue_t
query_info_hash(struct query_info *q)
{
hashvalue_t h = 0xab;
h = hashlittle(&q->qtype, sizeof(q->qtype), h);
h = hashlittle(&q->qclass, sizeof(q->qclass), h);
h = hashlittle(&q->has_cd, sizeof(q->has_cd), h);
query_dname_tolower(q->qname, q->qnamesize);
h = hashlittle(q->qname, q->qnamesize, h);
return h;
}
void reply_info_answer(struct reply_info* rep, uint16_t qflags,
void
reply_info_answer(struct reply_info* rep, uint16_t qflags,
ldns_buffer* buffer)
{
uint16_t flags;
@ -184,26 +211,32 @@ void reply_info_answer(struct reply_info* rep, uint16_t qflags,
void
reply_info_answer_iov(struct reply_info* rep, uint16_t qid,
uint16_t qflags, struct comm_reply* comrep)
uint16_t qflags, struct comm_reply* comrep, int cached)
{
uint16_t flags;
/* [0]=tcp, [1]=id, [2]=flags, [3]=message */
/* [0]=reserved for tcplen, [1]=id, [2]=flags, [3]=message */
struct iovec iov[4];
iov[1].iov_base = &qid;
iov[1].iov_len = sizeof(uint16_t);
flags = rep->flags | (qflags & 0x0100); /* copy RD bit */
log_assert(flags & 0x8000); /* QR bit must be on in our replies */
flags = htons(flags);
iov[2].iov_base = &flags;
if(!cached) {
/* original flags, copy RD bit from query. */
qflags = rep->flags | (qflags & 0x0100);
} else {
/* remove AA bit, copy RD and CD bits from query. */
qflags = (rep->flags & ~0x0400) | (qflags & 0x0110);
}
log_assert(qflags & 0x8000); /* QR bit must be on in our replies */
qflags = htons(qflags);
iov[2].iov_base = &qflags;
iov[2].iov_len = sizeof(uint16_t);
iov[3].iov_base = rep->reply;
iov[3].iov_len = rep->replysize;
comm_point_send_reply_iov(comrep, iov, 4);
}
struct msgreply_entry* query_info_entrysetup(struct query_info* q,
struct reply_info* r, hashvalue_t h)
struct msgreply_entry*
query_info_entrysetup(struct query_info* q, struct reply_info* r,
hashvalue_t h)
{
struct msgreply_entry* e = (struct msgreply_entry*)malloc(
sizeof(struct msgreply_entry));

View file

@ -88,9 +88,9 @@ struct msgreply_entry {
/**
* Parse wire query into a queryinfo structure, return 0 on parse error.
* initialises the (prealloced) queryinfo structure as well. sets reply to 0.
* initialises the (prealloced) queryinfo structure as well.
* This query structure contains a pointer back info the buffer!
* This pointer avoids memory allocation.
* This pointer avoids memory allocation. allocqname does memory allocation.
* @param m: the prealloced queryinfo structure to put query into.
* must be unused, or _clear()ed.
* @param query: the wireformat packet query. starts with ID.
@ -106,8 +106,8 @@ int query_info_parse(struct query_info* m, ldns_buffer* query);
int query_info_allocqname(struct query_info* m);
/**
* Compare two queryinfo structures, on query,
* The qname is _not_ sorted in canonical ordering.
* Compare two queryinfo structures, on query and type, class.
* It is _not_ sorted in canonical ordering.
* @param m1: struct query_info* , void* here to ease use as function pointer.
* @param m2: struct query_info* , void* here to ease use as function pointer.
* @return: 0 = same, -1 m1 is smaller, +1 m1 is larger.
@ -118,9 +118,12 @@ int query_info_compare(void* m1, void* m2);
void query_info_clear(struct query_info* m);
/** helper routine, determine length of dname in buffer, no compression ptrs
* allowed, returns 0 on failure. */
* allowed, returns 0 on parse failure. */
size_t query_dname_len(ldns_buffer* query);
/** lowercase query dname */
void query_dname_tolower(uint8_t* dname, size_t len);
/** clear out reply info structure */
void reply_info_clear(struct reply_info* m);
@ -133,7 +136,7 @@ void query_entry_delete(void *q, void* arg);
/** delete reply_info data structure */
void reply_info_delete(void* d, void* arg);
/** calculate hash value of query_info */
/** calculate hash value of query_info, lowercases the qname. */
hashvalue_t query_info_hash(struct query_info *q);
/**
@ -153,9 +156,11 @@ void reply_info_answer(struct reply_info* rep, uint16_t qflags,
* @param qid: query id, in network byte order.
* @param qflags: flags word from the query.
* @param comrep: communication reply point.
* @param cached: set true if a cached reply (so no AA bit).
* set false for the first reply.
*/
void reply_info_answer_iov(struct reply_info* rep, uint16_t qid,
uint16_t qflags, struct comm_reply* comrep);
uint16_t qflags, struct comm_reply* comrep, int cached);
/**
* Setup query info entry