From: Vladimír Čunát Date: Mon, 11 Jun 2018 13:40:37 +0000 (+0200) Subject: WIP: minor code cleanups X-Git-Tag: v2.4.0~19^2~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=29fb64a020fdb35849a5aaaf87954b89736adf65;p=thirdparty%2Fknot-resolver.git WIP: minor code cleanups --- diff --git a/lib/cache/api.c b/lib/cache/api.c index afbc1c2a0..b52dcd0e1 100644 --- a/lib/cache/api.c +++ b/lib/cache/api.c @@ -51,7 +51,7 @@ /** Cache version */ -static const uint16_t CACHE_VERSION = 3; +static const uint16_t CACHE_VERSION = 4; /** Key size */ #define KEY_HSIZE (sizeof(uint8_t) + sizeof(uint16_t)) #define KEY_SIZE (KEY_HSIZE + KNOT_DNAME_MAXLEN) @@ -527,7 +527,8 @@ static int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt) return ctx->state; } /* Check if the record is OK. */ - int32_t new_ttl = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_SOA, qry->timestamp.tv_sec); + int32_t new_ttl = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_SOA, + qry->timestamp.tv_sec); if (new_ttl < 0 || eh->rank < lowest_rank || eh->is_packet) { VERBOSE_MSG(qry, "=> SOA unfit %s: rank 0%.2o, new TTL %d\n", (eh->is_packet ? "packet" : "RR"), @@ -1122,7 +1123,8 @@ static int found_exact_hit(kr_layer_t *ctx, knot_pkt_t *pkt, knot_db_val_t val, } VERBOSE_MSG(qry, "=> FEH consistent OK \n"); - int32_t new_ttl = get_new_ttl(eh, qry, qry->sname, qry->stype, qry->timestamp.tv_sec); + int32_t new_ttl = get_new_ttl(eh, qry, qry->sname, qry->stype, + qry->timestamp.tv_sec); if (new_ttl < 0 || eh->rank < lowest_rank) { /* Positive record with stale TTL or bad rank. * LATER(optim.): It's unlikely that we find a negative one, @@ -1310,7 +1312,8 @@ static int closest_NS(kr_layer_t *ctx, struct key *k, entry_list_t el) assert(false); goto next_label; } - int32_t new_ttl = get_new_ttl(eh, qry, k->zname, type, qry->timestamp.tv_sec); + int32_t new_ttl = get_new_ttl(eh, qry, k->zname, type, + qry->timestamp.tv_sec); if (new_ttl < 0 /* Not interested in negative or bogus. */ || eh->is_packet diff --git a/lib/cache/impl.h b/lib/cache/impl.h index 6529fd561..7d95e10a9 100644 --- a/lib/cache/impl.h +++ b/lib/cache/impl.h @@ -34,16 +34,20 @@ #include "lib/cache/cdb_api.h" #include "lib/resolve.h" -/** Cache entry header +/* Cache entry values - binary layout. + * + * It depends on type which is recognizable by the key. + * Code depending on the meaning of the key is marked by CACHE_KEY_DEF. * * 'E' entry (exact hit): - * - ktype == NS: entry_apex and multiple chained entry_h, based on has_* : 1 flags; + * - ktype == NS: entry_apex and multiple chained entry_h, based on has_* flags; * - is_packet: uint16_t length, otherwise opaque and handled by ./entry_pkt.c * - otherwise RRset + its RRSIG set (possibly empty). - * '1' entry (NSEC1) - * - contents is the same as for exact hit for NSEC + * '1' or '3' entry (NSEC or NSEC3) + * - contents is the same as for exact hit * - flags don't make sense there - * */ + */ + struct entry_h { uint32_t time; /**< The time of inception. */ uint32_t ttl; /**< TTL at inception moment. Assuming it fits into int32_t ATM. */ @@ -52,59 +56,54 @@ struct entry_h { bool has_optout : 1; /**< Only for packets with NSEC3. */ uint8_t data[]; }; +struct entry_apex; -enum { ENTRY_APEX_NSECS_CNT = 2 }; - -struct entry_apex { - /* ENTRY_H_FLAGS */ - bool has_ns : 1; - bool has_cname : 1; - bool has_dname : 1; - - uint8_t pad_; - int8_t nsecs[ENTRY_APEX_NSECS_CNT]; /**< values: 0: none, 1: NSEC, 3: NSEC3 */ - uint8_t data[]; - /* XXX: if not first, stamp of last being the first? - * Purpose: save cache operations if rolled the algo/params long ago. */ -}; +/** Check basic consistency of entry_h for 'E' entries, not looking into ->data. + * (for is_packet the length of data is checked) + */ +struct entry_h * entry_h_consistent(knot_db_val_t data, uint16_t type); -/** Indices for decompressed entry_list_t. */ -enum EL { - EL_NS = ENTRY_APEX_NSECS_CNT, - EL_CNAME, - EL_DNAME, - EL_LENGTH -}; -/** Note: arrays are passed "by reference" to functions (in C99). */ -typedef knot_db_val_t entry_list_t[EL_LENGTH]; +struct entry_apex * entry_apex_consistent(knot_db_val_t val); -static inline uint16_t EL2RRTYPE(enum EL i) +/** Consistency check, ATM common for NSEC and NSEC3. */ +static inline struct entry_h * entry_h_consistent_NSEC(knot_db_val_t data) { - switch (i) { - case EL_NS: return KNOT_RRTYPE_NS; - case EL_CNAME: return KNOT_RRTYPE_CNAME; - case EL_DNAME: return KNOT_RRTYPE_DNAME; - default: assert(false); return 0; - } + /* ATM it's enough to just extend the checks for exact entries. */ + const struct entry_h *eh = entry_h_consistent(data, KNOT_RRTYPE_NSEC); + bool ok = eh != NULL; + ok = ok && !eh->is_packet && !eh->has_optout; + return ok ? /*const-cast*/(struct entry_h *)eh : NULL; } + +/* nsec_p* - NSEC* chain parameters */ + static inline int nsec_p_rdlen(const uint8_t *rdata) { //TODO: the zero case? // FIXME security: overflow potential return rdata ? 5 + rdata[4] : 0; /* rfc5155 4.2 and 3.2. */ } -static const int NSEC_P_MAXLEN = sizeof(uint32_t) + 5 + 255; +static const int NSEC_P_MAXLEN = sizeof(uint32_t) + 5 + 255; // TODO: remove?? + +/** Hash of NSEC3 parameters, used as a tag to separate different chains for same zone. */ +typedef uint32_t nsec_p_hash_t; +static inline nsec_p_hash_t nsec_p_mkHash(const uint8_t *nsec_p) +{ + assert(nsec_p && !(KNOT_NSEC3_FLAG_OPT_OUT & nsec_p[1])); + return hash((const char *)nsec_p, nsec_p_rdlen(nsec_p)); +} +/** NSEC* parameters for the chain. */ +struct nsec_p { + const uint8_t *raw; /**< Pointer to raw NSEC3 parameters; NULL for NSEC. */ + nsec_p_hash_t hash; /**< Hash of `raw`, used for cache keys. */ + dnssec_nsec3_params_t libknot; /**< Format for libknot; owns malloced memory! */ +}; -/** Check basic consistency of entry_h for 'E' entries, not looking into ->data. - * (for is_packet the length of data is checked) - */ -struct entry_h * entry_h_consistent(knot_db_val_t data, uint16_t type); -struct entry_apex * entry_apex_consistent(knot_db_val_t val); -// TODO -#define KR_CACHE_KEY_MAXLEN (KNOT_DNAME_MAXLEN + 100) +/** LATER(optim.): this is overshot, but struct key usage should be cheap ATM. */ +#define KR_CACHE_KEY_MAXLEN (KNOT_DNAME_MAXLEN + 100) /* CACHE_KEY_DEF */ struct key { const knot_dname_t *zname; /**< current zone name (points within qry->sname) */ @@ -118,13 +117,6 @@ struct key { uint8_t buf[KR_CACHE_KEY_MAXLEN]; }; -/** Hash of NSEC3 parameters, used as a tag to separate different chains for same zone. */ -typedef uint32_t nsec_p_hash_t; -static inline nsec_p_hash_t nsec_p_mkHash(const uint8_t *nsec_p) -{ - assert(nsec_p && !(KNOT_NSEC3_FLAG_OPT_OUT & nsec_p[1])); - return hash((const char *)nsec_p, nsec_p_rdlen(nsec_p)); -} static inline size_t key_nwz_off(const struct key *k) { /* CACHE_KEY_DEF: zone name lf + 0 ('1' or '3'). @@ -148,8 +140,45 @@ static const int NSEC3_HASH_LEN = 20, knot_db_val_t key_exact_type_maypkt(struct key *k, uint16_t type); + /* entry_h chaining; implementation in ./entry_list.c */ +enum { ENTRY_APEX_NSECS_CNT = 2 }; + +/** Header of 'E' entry with ktype == NS. Inside is private to ./entry_list.c */ +struct entry_apex { + /* ENTRY_H_FLAGS */ + bool has_ns : 1; + bool has_cname : 1; + bool has_dname : 1; + + uint8_t pad_; /**< Weird: 1 byte + 2 bytes + x bytes; let's do 2+2+x. */ + int8_t nsecs[ENTRY_APEX_NSECS_CNT]; /**< values: 0: none, 1: NSEC, 3: NSEC3 */ + uint8_t data[]; + /* XXX: if not first, stamp of last being the first? + * Purpose: save cache operations if rolled the algo/params long ago. */ +}; + +/** Indices for decompressed entry_list_t. */ +enum EL { + EL_NS = ENTRY_APEX_NSECS_CNT, + EL_CNAME, + EL_DNAME, + EL_LENGTH +}; +/** Note: arrays are passed "by reference" to functions (in C99). */ +typedef knot_db_val_t entry_list_t[EL_LENGTH]; + +static inline uint16_t EL2RRTYPE(enum EL i) +{ + switch (i) { + case EL_NS: return KNOT_RRTYPE_NS; + case EL_CNAME: return KNOT_RRTYPE_CNAME; + case EL_DNAME: return KNOT_RRTYPE_DNAME; + default: assert(false); return 0; + } +} + /** There may be multiple entries within, so rewind `val` to the one we want. * * ATM there are multiple types only for the NS ktype - it also accommodates xNAMEs. @@ -193,6 +222,7 @@ static inline int entry_list_serial_size(const entry_list_t list) void entry_list_memcpy(struct entry_apex *ea, entry_list_t list); + /* Packet caching; implementation in ./entry_pkt.c */ /** Stash the packet into cache (if suitable, etc.) */ @@ -218,12 +248,13 @@ static inline bool is_expiring(uint32_t orig_ttl, uint32_t new_ttl) /** Returns signed result so you can inspect how much stale the RR is. * * @param owner name for stale-serving decisions. You may pass NULL to disable stale. - * FIXME: NSEC uses zone name ATM. + * @note: NSEC* uses zone name ATM; for NSEC3 the owner may not even be knowable. * @param type for stale-serving. */ int32_t get_new_ttl(const struct entry_h *entry, const struct kr_query *qry, const knot_dname_t *owner, uint16_t type, uint32_t now); + /* RRset (de)materialization; implementation in ./entry_rr.c */ /** Size of the RR count field */ @@ -232,20 +263,14 @@ int32_t get_new_ttl(const struct entry_h *entry, const struct kr_query *qry, /** Compute size of dematerialized rdataset. NULL is accepted as empty set. */ static inline int rdataset_dematerialize_size(const knot_rdataset_t *rds) { - return KR_CACHE_RR_COUNT_SIZE + (rds - ? knot_rdataset_size(rds) - 4 * rds->rr_count /*TTLs*/ - : 0); + return KR_CACHE_RR_COUNT_SIZE + (rds == NULL ? 0 + : knot_rdataset_size(rds) - 4 * rds->rr_count /*TTLs*/); } /** Dematerialize a rdataset. */ int rdataset_dematerialize(const knot_rdataset_t *rds, uint8_t * restrict data); -/** NSEC* parameters; almost nothing is meaningful for NSEC. */ -struct nsec_p { - const uint8_t *raw; /**< Pointer to raw NSEC3 parameters; NULL for NSEC. */ - nsec_p_hash_t hash; /**< Hash of `raw`, used for cache keys. */ - dnssec_nsec3_params_t libknot; /**< Format for libknot; owns malloced memory! */ -}; + /** Partially constructed answer when gathering RRsets from cache. */ struct answer { @@ -258,11 +283,11 @@ struct answer { } rrsets[1+1+3]; /**< see AR_ANSWER and friends; only required records are filled */ }; enum { - AR_ANSWER = 0, /**< Positive answer record. It might be wildcard-expanded. */ - AR_SOA, /**< SOA record. */ - AR_NSEC, /**< NSEC* covering or matching the SNAME (next closer name in NSEC3 case). */ - AR_WILD, /**< NSEC* covering or matching the source of synthesis. */ - AR_CPE, /**< NSEC3 matching the closest provable encloser. */ + AR_ANSWER = 0, /**< Positive answer record. It might be wildcard-expanded. */ + AR_SOA, /**< SOA record. */ + AR_NSEC, /**< NSEC* covering or matching the SNAME (next closer name in NSEC3 case). */ + AR_WILD, /**< NSEC* covering or matching the source of synthesis. */ + AR_CPE, /**< NSEC3 matching the closest provable encloser. */ }; /** Materialize RRset + RRSIGs into ans->rrsets[id]. @@ -316,8 +341,6 @@ int nsec1_src_synth(struct key *k, struct answer *ans, const knot_dname_t *clenc /* NSEC3 stuff. Implementation in ./nsec3.c */ - - /** Construct a string key for for NSEC3 predecessor-search, from an NSEC3 name. * \note k->zlf_len is assumed to have been correctly set */ knot_db_val_t key_NSEC3(struct key *k, const knot_dname_t *nsec3_name, @@ -333,25 +356,13 @@ int nsec3_src_synth(struct key *k, struct answer *ans, const knot_dname_t *clenc const struct kr_query *qry, struct kr_cache *cache); -#define VERBOSE_MSG(qry, fmt...) QRVERBOSE((qry), "cach", fmt) - +#define VERBOSE_MSG(qry, fmt...) QRVERBOSE((qry), "cach", fmt) /** Shorthand for operations on cache backend */ #define cache_op(cache, op, ...) (cache)->api->op((cache)->db, ## __VA_ARGS__) - -/** Consistency check, ATM common for NSEC and NSEC3. */ -static inline struct entry_h * entry_h_consistent_NSEC(knot_db_val_t data) -{ - /* ATM it's enough to just extend the checks for exact entries. */ - const struct entry_h *eh = entry_h_consistent(data, KNOT_RRTYPE_NSEC); - bool ok = eh != NULL; - ok = ok && !eh->is_packet && !eh->has_optout; - return ok ? /*const-cast*/(struct entry_h *)eh : NULL; -} - static inline uint16_t get_uint16(const void *address) { uint16_t tmp; diff --git a/lib/cache/nsec1.c b/lib/cache/nsec1.c index acf5cdeb4..b7feea7df 100644 --- a/lib/cache/nsec1.c +++ b/lib/cache/nsec1.c @@ -176,9 +176,10 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query * in case we searched before the very first one in the zone. */ return "range search found inconsistent entry"; } - /* FIXME(stale): passing just zone name instead of owner, as we don't + /* Passing just zone name instead of owner, as we don't * have it reconstructed at this point. */ - int32_t new_ttl_ = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_NSEC, qry->timestamp.tv_sec); + int32_t new_ttl_ = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_NSEC, + qry->timestamp.tv_sec); if (new_ttl_ < 0 || !kr_rank_test_noassert(eh->rank, KR_RANK_SECURE)) { return "range search found stale or insecure entry"; /* TODO: remove the stale record *and* retry, diff --git a/lib/cache/nsec3.c b/lib/cache/nsec3.c index 1b5b7ad4a..b2077d6f5 100644 --- a/lib/cache/nsec3.c +++ b/lib/cache/nsec3.c @@ -133,7 +133,7 @@ static inline bool nsec3_hash_ordered(const uint8_t *h1, const uint8_t *h2) /** NSEC3 range search. * * \param key Pass output of key_NSEC3(k, ...) - * \param value[out] The raw data of the NSEC cache record (optional; consistency checked). + * \param value[out] The raw data of the NSEC3 cache record (optional; consistency checked). * \param exact_match[out] Whether the key was matched exactly or just covered (optional). * \param hash_low[out] Output the low end hash of covering NSEC3, pointing within DB (optional). * \param new_ttl[out] New TTL of the NSEC3 (optional). @@ -179,9 +179,9 @@ static const char * find_leq_NSEC3(struct kr_cache *cache, const struct kr_query * in case we searched before the very first one in the zone. */ return "range search found inconsistent entry"; } - /* FIXME(stale): passing just zone name instead of owner, as we don't - * have it reconstructed at this point. */ - int32_t new_ttl_ = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_NSEC3, qry->timestamp.tv_sec); + /* Passing just zone name instead of owner. */ + int32_t new_ttl_ = get_new_ttl(eh, qry, k->zname, KNOT_RRTYPE_NSEC3, + qry->timestamp.tv_sec); if (new_ttl_ < 0 || !kr_rank_test_noassert(eh->rank, KR_RANK_SECURE)) { return "range search found stale or insecure entry"; /* TODO: remove the stale record *and* retry,