lots of useless recvfrom() calls

[Lennert Buytenhek]

On Mon, May 21, 2018 at 09:41:11AM +0300, Lennert Buytenhek wrote:

> > > > I am using the patch below to avoid one useless recvfrom() call (that
> > > > returns EAGAIN) for every call to dns_ioevent().  Under low load, where
> > > > every call to dns_ioevent() processes one DNS reply, this saves half of
> > > > the total number of recvfrom() calls.
> > > 
> > > Is it really a problem under low load? If the load is low, it doesn't
> > > really matter how many recvfrom() system calls are made, I think.
> > 
> > 'low load' here is 'a low expected number of reply packets per POLLIN
> > cycle', but even for a large number of queries per second and multiple
> > parallel queries, I would not expect there to be more than one reply
> > packet per POLLIN cycle very often.
> 
> Another option is to try to recv() a packet up to as many times as we
> have an outstanding DNS query on this context.  So if there is one
> outstanding DNS query, always return after one recv() call, if there
> are five, then try up to five times (but return as soon as we get
> EAGAIN), etc.

This patch does that, and it seems to work, but it does break the
implicit guarantee that dns_ioevent() can be used with an
edge-triggered poll method. :(



diff --git a/udns_resolver.c b/udns_resolver.c
index be8ce8a..5711921 100644
--- a/udns_resolver.c
+++ b/udns_resolver.c
@@ -167,7 +167,8 @@ struct dns_ctx {		/* resolver context */
   unsigned dnsc_nextid;			/* next queue ID to use if !0 */
   int dnsc_udpsock;			/* UDP socket */
   struct dns_qlist dnsc_qactive;	/* active list sorted by deadline */
-  int dnsc_nactive;			/* number entries in dnsc_qactive */
+  unsigned dnsc_nqueued;		/* number entries in dnsc_qactive */
+  unsigned dnsc_ndequeued;
   dnsc_t *dnsc_pbuf;			/* packet buffer (udpbuf size) */
   int dnsc_qstatus;			/* last query status value */
 };
@@ -200,7 +201,8 @@ struct dns_ctx dns_defctx;
 #define CTXINITED(ctx) (ctx->dnsc_flags & DNS_INITED)
 #define SETCTXFRESH(ctx) SETCTXINITED(ctx); assert(!CTXOPEN(ctx))
 #define SETCTXINACTIVE(ctx) \
-		SETCTXINITED(ctx); assert(!ctx->dnsc_nactive)
+		SETCTXINITED(ctx); \
+		assert(ctx->dnsc_nqueued == ctx->dnsc_ndequeued)
 #define SETCTXOPEN(ctx) SETCTXINITED(ctx); assert(CTXOPEN(ctx))
 #define CTXOPEN(ctx) (ctx->dnsc_udpsock >= 0)
 
@@ -218,7 +220,7 @@ static void dns_assert_ctx(const struct dns_ctx *ctx) {
                  q->dnsq_prev->dnsq_next : ctx->dnsc_qactive.head));
     ++nactive;
   }
-  assert(nactive == ctx->dnsc_nactive);
+  assert(nactive == ctx->dnsc_nqueued - ctx->dnsc_ndequeued);
 }
 #endif
 
@@ -431,7 +433,7 @@ void dns_close(struct dns_ctx *ctx) {
       free(p);
     }
     qlist_init(&ctx->dnsc_qactive);
-    ctx->dnsc_nactive = 0;
+    ctx->dnsc_ndequeued = ctx->dnsc_nqueued;
     dns_drop_utm(ctx);
   }
 }
@@ -462,7 +464,8 @@ struct dns_ctx *dns_new(const struct dns_ctx *copy) {
   *ctx = *copy;
   ctx->dnsc_udpsock = -1;
   qlist_init(&ctx->dnsc_qactive);
-  ctx->dnsc_nactive = 0;
+  ctx->dnsc_nqueued = 0;
+  ctx->dnsc_ndequeued = 0;
   ctx->dnsc_pbuf = NULL;
   ctx->dnsc_qstatus = 0;
   ctx->dnsc_srchend = ctx->dnsc_srchbuf +
@@ -588,7 +591,7 @@ int dns_sock(const struct dns_ctx *ctx) {
 int dns_active(const struct dns_ctx *ctx) {
   SETCTXINITED(ctx);
   dns_assert_ctx(ctx);
-  return ctx->dnsc_nactive;
+  return ctx->dnsc_nqueued - ctx->dnsc_ndequeued;
 }
 
 int dns_status(const struct dns_ctx *ctx) {
@@ -611,8 +614,8 @@ dns_end_query(struct dns_ctx *ctx, struct dns_query *q,
   ctx->dnsc_qstatus = status;
   assert((status < 0 && result == 0) || (status >= 0 && result != 0));
   assert(cbck != 0);	/*XXX callback may be NULL */
-  assert(ctx->dnsc_nactive > 0);
-  --ctx->dnsc_nactive;
+  assert(ctx->dnsc_nqueued > ctx->dnsc_ndequeued);
+  ctx->dnsc_ndequeued++;
   qlist_remove(&ctx->dnsc_qactive, q);
   /* force the query to be unconnected */
   /*memset(q, 0, sizeof(*q));*/
@@ -925,7 +928,7 @@ dns_submit_dn(struct dns_ctx *ctx,
    * will actually send it.
    */
   qlist_add_head(&ctx->dnsc_qactive, q);
-  ++ctx->dnsc_nactive;
+  ++ctx->dnsc_nqueued;
   dns_request_utm(ctx, 0);
 
   return q;
@@ -1188,6 +1191,8 @@ static int dns_ioevent_internal(struct dns_ctx *ctx, time_t now) {
  * loop forever if an error IS fatal.
  */
 void dns_ioevent(struct dns_ctx *ctx, time_t now) {
+  unsigned nqueued;
+
   SETCTX(ctx);
   if (!CTXOPEN(ctx))
     return;
@@ -1195,7 +1200,8 @@ void dns_ioevent(struct dns_ctx *ctx, time_t now) {
 
   if (!now) now = time(NULL);
 
-  while (dns_ioevent_internal(ctx, now));
+  nqueued = ctx->dnsc_nqueued;
+  while (nqueued > ctx->dnsc_ndequeued && dns_ioevent_internal(ctx, now));
 
   dns_request_utm(ctx, now);
 }
@@ -1317,7 +1323,7 @@ int dns_cancel(struct dns_ctx *ctx, struct dns_query *q) {
   if (q->dnsq_cbck == dns_resolve_cb)
     return (ctx->dnsc_qstatus = DNS_E_BADQUERY);
   qlist_remove(&ctx->dnsc_qactive, q);
-  --ctx->dnsc_nactive;
+  ++ctx->dnsc_ndequeued;
   dns_request_utm(ctx, 0);
   return 0;
 }