No useless byteswapping.

git-svn-id: file:///svn/unbound/trunk@208 be551aaa-1e26-0410-a405-d3ace91eadb9

daemon/worker.c

@@ -57,6 +57,8 @@
 #include <netdb.h>
 #include <signal.h>
 
+/** size of ID+FLAGS in a DNS message */
+#define DNS_ID_AND_FLAGS 4
 /** timeout in seconds for UDP queries to auth servers. TODO: proper rtt */
 #define UDP_QUERY_TIMEOUT 4
 
@@ -99,7 +101,7 @@ replyerror(int r, struct work_query* w)
 	verbose(VERB_DETAIL, "reply with error");
 	
 	ldns_buffer_clear(buf);
-	ldns_buffer_write_u16(buf, w->query_id);
+	ldns_buffer_write(buf, &w->query_id, sizeof(uint16_t));
 	flags = (uint16_t)(0x8000 | r); /* QR and retcode*/
 	flags |= (w->query_flags & 0x0100); /* copy RD bit */
 	ldns_buffer_write_u16(buf, flags);
@@ -126,7 +128,8 @@ worker_handle_reply(struct comm_point* c, void* arg, int error,
 	struct work_query* w = (struct work_query*)arg;
 	struct reply_info* rep;
 	struct msgreply_entry* e;
-	verbose(VERB_DETAIL, "reply to query with stored ID %d", w->query_id);
+	verbose(VERB_DETAIL, "reply to query with stored ID %d", 
+		ntohs(w->query_id)); /* byteswapped so same as dig prints */
 	if(error != 0) {
 		replyerror(LDNS_RCODE_SERVFAIL, w);
 		return 0;
@@ -145,7 +148,8 @@ worker_handle_reply(struct comm_point* c, void* arg, int error,
 		replyerror(LDNS_RCODE_SERVFAIL, w);
 		return 0;
 	}
-	rep->replysize = ldns_buffer_limit(c->buffer) - 2; /* minus ID */
+	rep->flags = ldns_buffer_read_u16_at(c->buffer, 2);
+	rep->replysize = ldns_buffer_limit(c->buffer) - DNS_ID_AND_FLAGS;
 	log_info("got reply of size %d", rep->replysize);
 	rep->reply = (uint8_t*)malloc(rep->replysize);
 	if(!rep->reply) {
@@ -154,10 +158,10 @@ worker_handle_reply(struct comm_point* c, void* arg, int error,
 		replyerror(LDNS_RCODE_SERVFAIL, w);
 		return 0;
 	}
-	memmove(rep->reply, ldns_buffer_at(c->buffer, 2), rep->replysize);
-	ldns_buffer_write_u16_at(w->query_reply.c->buffer, 0, w->query_id);
-	reply_info_answer(rep, w->query_flags, w->query_reply.c->buffer);
-	comm_point_send_reply(&w->query_reply);
+	memmove(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);
 	req_release(w);
 	/* store or update reply in the cache */
 	if(!(e = query_info_entrysetup(&w->qinfo, rep, w->query_hash))) {
@@ -174,7 +178,7 @@ static void
 worker_process_query(struct worker* worker, struct work_query* w) 
 {
 	/* query the forwarding address */
-	verbose(VERB_DETAIL, "process_query ID %d", w->query_id);
+	verbose(VERB_DETAIL, "process_query ID %d", ntohs(w->query_id));
 	pending_udp_query(worker->back, w->query_reply.c->buffer, 
 		&worker->fwd_addr, worker->fwd_addrlen, UDP_QUERY_TIMEOUT,
 		worker_handle_reply, w, worker->rndstate);
@@ -325,7 +329,7 @@ worker_handle_request(struct comm_point* c, void* arg, int error,
 	w->query_hash = h;
 	memcpy(&w->query_reply, repinfo, sizeof(struct comm_reply));
 	memcpy(&w->qinfo, &qinfo, sizeof(struct query_info));
-	w->query_id = LDNS_ID_WIRE(ldns_buffer_begin(c->buffer));
+	memcpy(&w->query_id, ldns_buffer_begin(c->buffer), sizeof(uint16_t));
 	w->query_flags = ldns_buffer_read_u16_at(c->buffer, 2);
 
 	/* answer it */

daemon/worker.h

@@ -76,7 +76,7 @@ struct work_query {
 	struct query_info qinfo;
 	/** hash value of the query qinfo */
 	hashvalue_t query_hash;
-	/** id of query */
+	/** id of query, in network byteorder. */
 	uint16_t query_id;
 	/** flags uint16 from query */
 	uint16_t query_flags;

doc/Changelog

@@ -1,6 +1,8 @@
 29 March 2007: Wouter
 	- writev or sendmsg used when answering from cache.
 	  This avoids a copy of the data.
+	- do not do useless byteswap on query id. Store reply flags in uint16
+	  for easier access (and no repeated byteswapping).
 
 28 March 2007: Wouter
 	- new config option: num-queries-per-thread.

doc/TODO

@@ -3,4 +3,3 @@ o use real entropy to make random (ID, port) numbers more random.
 o in production mode, do not free memory on exit. In debug mode, test leaks.
 o profile memory allocation, and if performance issues, use special memory
   allocator. For example, with caches per thread.
-o do not do useless byteswap on query_id value. store qflags in uint16.

util/data/msgreply.c

@@ -175,11 +175,10 @@ void reply_info_answer(struct reply_info* rep, uint16_t qflags,
 	uint16_t flags;
 	ldns_buffer_clear(buffer);
 	ldns_buffer_skip(buffer, 2); /* ID */
-	flags = ldns_read_uint16(rep->reply);
-	flags |= (qflags & 0x0100); /* copy RD bit */
+	flags = rep->flags | (qflags & 0x0100); /* copy RD bit */
 	log_assert(flags & 0x8000); /* QR bit must be on in our replies */
 	ldns_buffer_write_u16(buffer, flags);
-	ldns_buffer_write(buffer, rep->reply+2, rep->replysize-2);
+	ldns_buffer_write(buffer, rep->reply, rep->replysize);
 	ldns_buffer_flip(buffer);
 }
 
@@ -191,17 +190,15 @@ reply_info_answer_iov(struct reply_info* rep, uint16_t qid,
 	/* [0]=tcp, [1]=id, [2]=flags, [3]=message */
 	struct iovec iov[4];
 
-	qid = htons(qid);
 	iov[1].iov_base = &qid;
 	iov[1].iov_len = sizeof(uint16_t);
-	flags = ldns_read_uint16(rep->reply);
-	flags |= (qflags & 0x0100); /* copy RD bit */
+	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;
 	iov[2].iov_len = sizeof(uint16_t);
-	iov[3].iov_base = rep->reply+2;
-	iov[3].iov_len = rep->replysize-2;
+	iov[3].iov_base = rep->reply;
+	iov[3].iov_len = rep->replysize;
 	comm_point_send_reply_iov(comrep, iov, 4);
 }
 

util/data/msgreply.h

@@ -67,10 +67,13 @@ struct query_info {
  * the query (RD,CD if not AA). prepend ID. 
  */
 struct reply_info {
-	/** the reply packet, skips ID, starts with flags/opcode/rcode word */
+	/** the reply packet, skips ID and flags, 
+	 * starts with opcode/rcode word */
 	uint8_t* reply;
 	/** the reply size */
 	size_t replysize;
+	/** the flags for the answer, host order. */
+	uint16_t flags;
 };
 
 /**
@@ -147,7 +150,7 @@ void reply_info_answer(struct reply_info* rep, uint16_t qflags,
 /**
  * Generate and send out answer from reply_info.
  * @param rep: reply to fill in.
- * @param qid: query id.
+ * @param qid: query id, in network byte order.
  * @param qflags: flags word from the query.
  * @param comrep: communication reply point.
  */