From: Vladimír Čunát Date: Wed, 10 Jan 2018 15:09:16 +0000 (+0100) Subject: cache: nitpicks and comments X-Git-Tag: v2.0.0~6^2~32 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c4f61dfd719dcdfc67541c715e134016b80b4b7f;p=thirdparty%2Fknot-resolver.git cache: nitpicks and comments --- diff --git a/lib/cache.c b/lib/cache.c index 7b7c5c15b..47cd84d7a 100644 --- a/lib/cache.c +++ b/lib/cache.c @@ -306,7 +306,7 @@ int cache_peek(kr_layer_t *ctx, knot_pkt_t *pkt) } /** - * \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". */ static int cache_peek_real(kr_layer_t *ctx, knot_pkt_t *pkt) { @@ -379,11 +379,7 @@ static int cache_peek_real(kr_layer_t *ctx, knot_pkt_t *pkt) val_cut.data + val_cut.len, get_new_ttl(val_cut.data, qry->timestamp.tv_sec)); /* TODO: ^^ cumbersome code */ - if (ret == kr_ok()) { - return KR_STATE_DONE; - } else { - return ctx->state; - } + return ret == kr_ok() ? KR_STATE_DONE : ctx->state; case KNOT_RRTYPE_DNAME: VERBOSE_MSG(qry, "=> DNAME not supported yet\n"); // LATER diff --git a/lib/cache/entry_pkt.c b/lib/cache/entry_pkt.c index 85743d318..44a363a83 100644 --- a/lib/cache/entry_pkt.c +++ b/lib/cache/entry_pkt.c @@ -195,11 +195,7 @@ int answer_from_pkt(kr_layer_t *ctx, knot_pkt_t *pkt, uint16_t type, } knot_wire_set_id(pkt->wire, msgid); - /* Rank-related fixups. Add rank into the additional field. */ - if (kr_rank_test(eh->rank, KR_RANK_INSECURE)) { - qry->flags.DNSSEC_INSECURE = true; - qry->flags.DNSSEC_WANT = false; - } + /* Add rank into the additional field. */ for (size_t i = 0; i < pkt->rrset_count; ++i) { assert(!pkt->rr[i].additional); uint8_t *rr_rank = mm_alloc(&pkt->mm, sizeof(*rr_rank)); diff --git a/lib/cache/impl.h b/lib/cache/impl.h index 2640f2167..fabf71401 100644 --- a/lib/cache/impl.h +++ b/lib/cache/impl.h @@ -206,11 +206,13 @@ int pkt_append(knot_pkt_t *pkt, const struct answer_rrset *rrset, uint8_t rank); /** Construct a string key for for NSEC (1) predecessor-search. - * \param add_wildcard Act as if the name was extended by "*." */ + * \param add_wildcard Act as if the name was extended by "*." + * \note k->zlf_len is assumed to have been correctly set */ knot_db_val_t key_NSEC1(struct key *k, const knot_dname_t *name, bool add_wildcard); /** Closest encloser check for NSEC (1). * To understand the interface, see the call point. + * \param k space to store key + input: zname and zlf_len * \return 0: success; >0: try other (NSEC3); <0: exit cache immediately. */ int nsec1_encloser(struct key *k, struct answer *ans, const int sname_labels, int *clencl_labels, diff --git a/lib/cache/nsec1.c b/lib/cache/nsec1.c index bbd83ee70..d40ef6374 100644 --- a/lib/cache/nsec1.c +++ b/lib/cache/nsec1.c @@ -133,15 +133,16 @@ static struct entry_h * entry_h_consistent_NSEC(knot_db_val_t data) /** NSEC1 range search. * * \param key Pass output of key_NSEC1(k, ...) - * \param val[out] The raw data of the NSEC cache record (optional; consistency checked). + * \param value[out] The raw data of the NSEC cache record (optional; consistency checked). * \param exact_match[out] Whether the key was matched exactly or just covered (optional). * \param kwz_low[out] Output the low end of covering NSEC, pointing within DB (optional). * \param kwz_high[in,out] Storage for the high end of covering NSEC (optional). * It's only set if !exact_match. + * \param new_ttl[out] New TTL of the NSEC (optional). * \return Error message or NULL. */ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query *qry, - knot_db_val_t key, const struct key *k, knot_db_val_t *value, + const knot_db_val_t key, const struct key *k, knot_db_val_t *value, bool *exact_match, knot_db_val_t *kwz_low, knot_db_val_t *kwz_high, uint32_t *new_ttl) { @@ -151,8 +152,9 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query assert(false); return "range search ERROR"; } + knot_db_val_t key_nsec = key; knot_db_val_t val = { }; - int ret = cache_op(cache, read_leq, &key, &val); + int ret = cache_op(cache, read_leq, &key_nsec, &val); if (ret < 0) { if (ret == kr_error(ENOENT)) { return "range search miss"; @@ -186,8 +188,8 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query } if (kwz_low) { *kwz_low = (knot_db_val_t){ - .data = key.data + nwz_off, - .len = key.len - nwz_off, + .data = key_nsec.data + nwz_off, + .len = key_nsec.len - nwz_off, }; /* CACHE_KEY_DEF */ } if (is_exact) { @@ -195,15 +197,12 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query return NULL; } /* The NSEC starts strictly before our target name; - * check that it's still within the zone and that - * it ends strictly after the sought name - * (or points to origin). */ - bool in_zone = key.len >= nwz_off + * now check that it still belongs into that zone. */ + const bool nsec_in_zone = key_nsec.len >= nwz_off /* CACHE_KEY_DEF */ - && memcmp(k->buf + 1, key.data, nwz_off) == 0; - if (!in_zone) { - //kr_log_verbose("key.len = %d\n", (int)key.len); - return "range search miss (!in_zone)"; + && memcmp(key.data, key_nsec.data, nwz_off) == 0; + if (!nsec_in_zone) { + return "range search miss (!nsec_in_zone)"; } /* We know it starts before sname, so let's check the other end. * 1. construct the key for the next name - kwz_hi. */ @@ -267,8 +266,8 @@ int nsec1_encloser(struct key *k, struct answer *ans, return kr_error(EINVAL); } - /* find a previous-or-equal name+NSEC in cache covering - * the QNAME, checking TTL etc. */ + /* Find a previous-or-equal name+NSEC in cache covering the QNAME, + * checking TTL etc. */ knot_db_val_t key = key_NSEC1(k, qry->sname, false); knot_db_val_t val = {}; bool exact_match; @@ -308,7 +307,8 @@ int nsec1_encloser(struct key *k, struct answer *ans, knot_nsec_bitmap(&nsec_rr->rrs, &bm, &bm_size); if (!bm || kr_nsec_bitmap_contains_type(bm, bm_size, qry->stype)) { assert(bm); - VERBOSE_MSG(qry, "=> NSEC sname: match but failed type check\n"); + VERBOSE_MSG(qry, + "=> NSEC sname: match but failed type check\n"); return ESKIP; /* exact positive answer should exist! */ } /* NODATA proven; just need to add SOA+RRSIG later */ @@ -316,34 +316,34 @@ int nsec1_encloser(struct key *k, struct answer *ans, new_ttl); ans->rcode = PKT_NODATA; return kr_ok(); + } /* else */ - } else { /* Inexact match; NXDOMAIN proven *except* for wildcards. */ - WITH_VERBOSE { - VERBOSE_MSG(qry, "=> NSEC sname: covered by: "); - kr_dname_print(nsec_rr->owner, "", " -> "); - kr_dname_print(knot_nsec_next(&nsec_rr->rrs), "", ", "); - kr_log_verbose("new TTL %d\n", new_ttl); - } - /* Find label count of the closest encloser. - * Both points in an NSEC do exist and any prefixes - * of those names as well (empty non-terminals), - * but nothing else does inside this "triangle". - */ - *clencl_labels = MAX( - knot_dname_matched_labels(nsec_rr->owner, qry->sname), - knot_dname_matched_labels(qry->sname, knot_nsec_next(&nsec_rr->rrs)) - ); - /* Empty non-terminals don't need to have - * a matching NSEC record. */ - if (sname_labels == *clencl_labels) { - ans->rcode = PKT_NODATA; - VERBOSE_MSG(qry, - "=> NSEC sname: empty non-terminal by the same RR\n"); - } else { - ans->rcode = PKT_NXDOMAIN; - } - return kr_ok(); + /* Inexact match; NXDOMAIN proven *except* for wildcards. */ + WITH_VERBOSE { + VERBOSE_MSG(qry, "=> NSEC sname: covered by: "); + kr_dname_print(nsec_rr->owner, "", " -> "); + kr_dname_print(knot_nsec_next(&nsec_rr->rrs), "", ", "); + kr_log_verbose("new TTL %d\n", new_ttl); + } + /* Find label count of the closest encloser. + * Both points in an NSEC do exist and any prefixes + * of those names as well (empty non-terminals), + * but nothing else does inside this "triangle". + */ + *clencl_labels = MAX( + knot_dname_matched_labels(nsec_rr->owner, qry->sname), + knot_dname_matched_labels(qry->sname, knot_nsec_next(&nsec_rr->rrs)) + ); + /* Empty non-terminals don't need to have + * a matching NSEC record. */ + if (sname_labels == *clencl_labels) { + ans->rcode = PKT_NODATA; + VERBOSE_MSG(qry, + "=> NSEC sname: empty non-terminal by the same RR\n"); + } else { + ans->rcode = PKT_NXDOMAIN; } + return kr_ok(); }