cache: don't change kr_layer_t::state to _DONE

I suspect this might've caused real-life issues in some edge case,
making a cached CNAME not being followed.  But in any case,
cache is not a good place to set KR_STATE_DONE, as it only creates
a (pseudo-)packed for immediate CONSUME phase; it should be
iterator who sets it in that later phase when picking those records.

I added unnecessary simplifications to code, too (all those returns).

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 306aa603dd299cebc4a681d75c87106232cf9e3f..2b0f283e3587d7e8f0fe3c8ed1fbe45dc42b2852 100644 (file)
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -325,7 +325,7 @@ knot_db_val_t key_exact_type_maypkt(struct key *k, uint16_t type)
 
 
 /** The inside for cache_peek(); implementation separated to ./peek.c */
-int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt);
+void peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt);
 /** function for .produce phase */
 int cache_peek(kr_layer_t *ctx, knot_pkt_t *pkt)
 {
@@ -353,9 +353,9 @@ int cache_peek(kr_layer_t *ctx, knot_pkt_t *pkt)
                return ctx->state;
        }
 
-       int ret = peek_nosync(ctx, pkt);
+       peek_nosync(ctx, pkt);
        kr_cache_commit(&req->ctx->cache);
-       return ret;
+       return ctx->state;
 }
 
 

diff --git a/lib/cache/peek.c b/lib/cache/peek.c
index dc7cd7fe3907466444e187da9aec8621cd210c40..b4417d2a6e5ca2fd5937a84c16b6b6320e06ce3b 100644 (file)
--- a/lib/cache/peek.c
+++ b/lib/cache/peek.c
@@ -106,9 +106,10 @@ static uint8_t get_lowest_rank(const struct kr_query *qry, const knot_dname_t *n
 
 
 /** Almost whole .produce phase for the cache module.
- * \note we don't transition to KR_STATE_FAIL even in case of "unexpected errors".
+ * \note we don't transition to KR_STATE_FAIL even in case of "unexpected errors";
+ *       we don't even change ctx->state to other values (e.g. on success).
  */
-int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
+void peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
 {
        struct kr_request *req = ctx->req;
        struct kr_query *qry = req->current_query;
@@ -118,7 +119,7 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
        int ret = kr_dname_lf(k->buf, qry->sname, false);
        if (unlikely(ret)) {
                assert(false);
-               return ctx->state;
+               return;
        }
 
        const uint8_t lowest_rank = get_lowest_rank(qry, qry->sname, qry->stype);
@@ -137,9 +138,9 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
        if (ret && ret != -abs(ENOENT)) {
                VERBOSE_MSG(qry, "=> exact hit error: %d %s\n", ret, kr_strerror(ret));
                assert(false);
-               return ctx->state;
+               return;
        } else if (!ret) {
-               return KR_STATE_DONE;
+               return;
        }
 
        /**** 1b. otherwise, find the longest prefix zone/xNAME (with OK time+rank). [...] */
@@ -147,14 +148,14 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
        ret = kr_dname_lf(k->buf, k->zname, false); /* LATER(optim.): probably remove */
        if (unlikely(ret)) {
                assert(false);
-               return ctx->state;
+               return;
        }
        entry_list_t el;
        ret = closest_NS(cache, k, el, qry, false, qry->stype == KNOT_RRTYPE_DS);
        if (ret) {
                assert(ret == kr_error(ENOENT));
                if (ret != kr_error(ENOENT) || !el[0].len) {
-                       return ctx->state;
+                       return;
                }
        }
        switch (k->type) {
@@ -163,9 +164,9 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
                assert(v.data && v.len);
                const int32_t new_ttl = get_new_ttl(v.data, qry, qry->sname,
                                                KNOT_RRTYPE_CNAME, qry->timestamp.tv_sec);
-               ret = answer_simple_hit(ctx, pkt, KNOT_RRTYPE_CNAME, v.data,
-                                       knot_db_val_bound(v), new_ttl);
-               return ret == kr_ok() ? KR_STATE_DONE : ctx->state;
+               answer_simple_hit(ctx, pkt, KNOT_RRTYPE_CNAME, v.data,
+                                 knot_db_val_bound(v), new_ttl);
+               return;
                }
        case KNOT_RRTYPE_DNAME: {
                const knot_db_val_t v = el[EL_DNAME];
@@ -173,9 +174,9 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
                /* TTL: for simplicity, we just ask for TTL of the generated CNAME. */
                const int32_t new_ttl = get_new_ttl(v.data, qry, qry->sname,
                                                KNOT_RRTYPE_CNAME, qry->timestamp.tv_sec);
-               ret = answer_dname_hit(ctx, pkt, k->zname, v.data,
-                                       knot_db_val_bound(v), new_ttl);
-               return ret == kr_ok() ? KR_STATE_DONE : ctx->state;
+               answer_dname_hit(ctx, pkt, k->zname, v.data,
+                                knot_db_val_bound(v), new_ttl);
+               return;
                }
        }
 
@@ -191,12 +192,12 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
 #if 0
        if (!eh) { /* fall back to root hints? */
                ret = kr_zonecut_set_sbelt(req->ctx, &qry->zone_cut);
-               if (ret) return ctx->state;
+               if (ret) return;
                assert(!qry->zone_cut.parent);
 
                //VERBOSE_MSG(qry, "=> using root hints\n");
                //qry->flags.AWAIT_CUT = false;
-               return ctx->state;
+               return;
        }
 
        /* Now `eh` points to the closest NS record that we've found,
@@ -204,7 +205,7 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
         * a negative proof or we may query upstream from that point. */
        kr_zonecut_set(&qry->zone_cut, k->zname);
        ret = kr_make_query(qry, pkt); // TODO: probably not yet - qname minimization
-       if (ret) return ctx->state;
+       if (ret) return;
 #endif
 
        /** Structure for collecting multiple NSEC* + RRSIG records,
@@ -238,10 +239,10 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
                                        lowest_rank, qry, cache);
                nsec_p_cleanup(&ans.nsec_p);
                if (!ret) break;
-               if (ret < 0) return ctx->state;
+               if (ret < 0) return;
        cont:
                /* Otherwise we try another nsec_p, if available. */
-               if (++i == ENTRY_APEX_NSECS_CNT) return ctx->state;
+               if (++i == ENTRY_APEX_NSECS_CNT) return;
                /* clear possible partial answers in `ans` (no need to deallocate) */
                ans.rcode = 0;
                memset(&ans.rrsets, 0, sizeof(ans.rrsets));
@@ -259,7 +260,7 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
                if (ret || !(eh = entry_h_consistent_E(val, KNOT_RRTYPE_SOA))) {
                        assert(ret); /* only want to catch `eh` failures */
                        VERBOSE_MSG(qry, "=> SOA missed\n");
-                       return ctx->state;
+                       return;
                }
                /* Check if the record is OK. */
                int32_t new_ttl = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_SOA,
@@ -268,12 +269,12 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
                        VERBOSE_MSG(qry, "=> SOA unfit %s: rank 0%.2o, new TTL %d\n",
                                        (eh->is_packet ? "packet" : "RR"),
                                        eh->rank, new_ttl);
-                       return ctx->state;
+                       return;
                }
                /* Add the SOA into the answer. */
                ret = entry2answer(&ans, AR_SOA, eh, knot_db_val_bound(val),
                                   k->zname, KNOT_RRTYPE_SOA, new_ttl);
-               if (ret) return ctx->state;
+               if (ret) return;
        }
 
        /* Find our target RCODE. */
@@ -291,14 +292,14 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
        case 0: /* i.e. nothing was found */
                /* LATER(optim.): zone cut? */
                VERBOSE_MSG(qry, "=> cache miss\n");
-               return ctx->state;
+               return;
        }
 
        if (pkt_renew(pkt, qry->sname, qry->stype)
            || knot_pkt_begin(pkt, KNOT_ANSWER)
           ) {
                assert(false);
-               return ctx->state;
+               return;
        }
        knot_wire_set_rcode(pkt->wire, real_rcode);
 
@@ -310,7 +311,7 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
                ret = pkt_append(pkt, &ans.rrsets[i], ans.rrsets[i].set.rank);
                if (ret) {
                        assert(false);
-                       return ctx->state;
+                       return;
                }
        }
 
@@ -319,8 +320,6 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt)
        qf->EXPIRING = expiring;
        qf->CACHED = true;
        qf->NO_MINIMIZE = true;
-
-       return KR_STATE_DONE;
 }
 
 /**