cache: fix large answers from packet cache

Atomic packets larger than both 4k and net.bufsize() could not
be fetched from cache; now that's fixed in a minimalistic way.
(Minimalistic except for nitpicks like adding comments.)

diff --git a/NEWS b/NEWS
index d76e1d1050ce772918a0f2e446840f3b3e440e2a..93e996a5701d52187acd9d1d7027c61d1ea32c62 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -289,6 +289,7 @@ Bugfixes
 - avoid SERVFAILs due to certain kind of NS dependency cycles, again
   (#374) this time seen as 'circular dependency' in verbose logs
 - policy and view modules do not overwrite result finished requests (!678)
+- cache: fix some cases of caching answers over 4 KiB (!976)
 
 Improvements
 ------------

diff --git a/daemon/worker.c b/daemon/worker.c
index ce704307ef2e476597f00973143ec5e73dc31f39..6e87229d8e7e5b145aaee142731aad838213dda8 100644 (file)
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -375,12 +375,13 @@ static void request_free(struct request_ctx *ctx)
 
 static struct qr_task *qr_task_create(struct request_ctx *ctx)
 {
-       /* How much can client handle? */
-       struct engine *engine = ctx->worker->engine;
-       size_t pktbuf_max = KR_EDNS_PAYLOAD;
-       if (engine->resolver.opt_rr) {
-               pktbuf_max = MAX(knot_edns_get_payload(engine->resolver.opt_rr),
-                                pktbuf_max);
+       /* Choose (initial) pktbuf size.  As it is now, pktbuf can be used
+        * for UDP answers from upstream *and* from cache
+        * and for sending non-UDP queries upstream (?) */
+       uint16_t pktbuf_max = KR_EDNS_PAYLOAD;
+       const knot_rrset_t *opt_our = ctx->worker->engine->resolver.opt_rr;
+       if (opt_our) {
+               pktbuf_max = MAX(pktbuf_max, knot_edns_get_payload(opt_our));
        }
 
        /* Create resolution task */

diff --git a/lib/cache/entry_pkt.c b/lib/cache/entry_pkt.c
index 986f53827d8e0fd789659c22569d81fc28aab782..4ed20474db06a9a0261e33a35683ff64c2fbf53c 100644 (file)
--- a/lib/cache/entry_pkt.c
+++ b/lib/cache/entry_pkt.c
@@ -145,14 +145,22 @@ int answer_from_pkt(kr_layer_t *ctx, knot_pkt_t *pkt, uint16_t type,
        struct kr_request *req = ctx->req;
        struct kr_query *qry = req->current_query;
 
+       const uint16_t msgid = knot_wire_get_id(pkt->wire);
+
+       /* Ensure the wire buffer is large enough.  Strategy: fit and at least double. */
        uint16_t pkt_len;
        memcpy(&pkt_len, eh->data, sizeof(pkt_len));
        if (pkt_len > pkt->max_size) {
-               return kr_error(ENOENT);
+               pkt->max_size = MIN(KNOT_WIRE_MAX_PKTSIZE,
+                                   MAX(pkt->max_size * 2, pkt_len));
+               mm_free(&ctx->req->pool, pkt->wire); /* no-op, but... */
+               pkt->wire = mm_alloc(&ctx->req->pool, pkt->max_size);
+               pkt->compr.wire = pkt->wire;
+               /* TODO: ^^ nicer way how to replace knot_pkt_t::wire ? */
        }
+       assert(pkt->max_size >= pkt_len);
 
        /* Copy answer and reparse it, but keep the original message id. */
-       uint16_t msgid = knot_wire_get_id(pkt->wire);
        knot_pkt_clear(pkt);
        memcpy(pkt->wire, eh->data + 2, pkt_len);
        pkt->size = pkt_len;

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index a4dcf136c8cd6dfcefefbabedca6cb02e557dd59..55f79abb05c8aa5d4eafb02d1ef1fbc2ffb44bb4 100644 (file)
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -1029,7 +1029,7 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
        } else
 #endif
        if (pkt->parsed <= KNOT_WIRE_HEADER_SIZE) {
-               VERBOSE_MSG("<= malformed response\n");
+               VERBOSE_MSG("<= malformed response (parsed %d)\n", (int)pkt->parsed);
                return resolve_badmsg(pkt, req, query);
        } else if (!is_paired_to_query(pkt, query)) {
                WITH_VERBOSE(query) {

diff --git a/lib/resolve.h b/lib/resolve.h
index 6abab1b92872f8f3919efac32771c50127432765..1b339ff3730ac3ee95a3cc34e3dfc60324193302 100644 (file)
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -145,7 +145,12 @@ typedef array_t(struct kr_module *) module_array_t;
 struct kr_context
 {
        struct kr_qflags options;
+
+       /** Default EDNS towards *both* clients and upstream.
+        * LATER: consider splitting the two, e.g. allow separately
+        * configured limits for UDP packet size (say, LAN is under control). */
        knot_rrset_t *opt_rr;
+
        map_t trust_anchors;
        map_t negative_anchors;
        struct kr_zonecut root_hints;
@@ -165,8 +170,8 @@ struct kr_context
 /* Kept outside, because kres-gen.lua can't handle this depth
  * (and lines here were too long anyway). */
 struct kr_request_qsource_flags {
-       bool tcp:1; /**< true if the request is on TCP (or TLS); only meaningful if (dst_addr). */
-       bool tls:1; /**< true if the request is on TLS (or HTTPS); only meaningful if (dst_addr). */
+       bool tcp:1; /**< true if the request is not on UDP; only meaningful if (dst_addr). */
+       bool tls:1; /**< true if the request is encrypted; only meaningful if (dst_addr). */
        bool http:1; /**< true if the request is on HTTP; only meaningful if (dst_addr). */
 };