From: Vladimír Čunát Date: Mon, 23 Jul 2018 14:46:33 +0000 (+0200) Subject: cache.clear(name), cache.get(name): review X-Git-Tag: v3.0.0~1^2~23 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a1fa7d828aec4069ed3778d99896c5aab81f3fb1;p=thirdparty%2Fknot-resolver.git cache.clear(name), cache.get(name): review - fix some edge cases and nitpicks - static storage -> stack (for temporaries of a few kilobytes) - sync docs, including caveats of the implementation --- diff --git a/NEWS b/NEWS index 68b0cd2ee..a4034a166 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,9 @@ Bugfixes This fixes lower hit rate in NSEC3 zones (since 2.4.0). - minor TCP and TLS fixes (!623, !624, !626) +Improvements +------------ +- lua: resurrect cache.get('name') and cache.clear('name'), with caveats Knot Resolver 2.4.0 (2018-07-03) ================================ diff --git a/daemon/README.rst b/daemon/README.rst index 2f669a03b..dfabc5f13 100644 --- a/daemon/README.rst +++ b/daemon/README.rst @@ -823,23 +823,20 @@ daemons or manipulated from other processes, making for example synchronised loa .. function:: cache.prune([max_count]) - :param number max_count: maximum number of items to be pruned at once (default: 65536) - :return: ``{ pruned: int }`` - - Prune expired/invalid records. + Not implemented (anymore). .. function:: cache.get([domain]) - :return: list of matching records in cache - - Fetches matching records from cache. The **domain** can either be: - - - a domain name (e.g. ``"domain.cz"``) - - a wildcard (e.g. ``"*.domain.cz"``) + :return: table of records in cache matching the prefix - The domain name fetches all records matching this name, while the wildcard matches all records at or below that name. + .. error:: **Caveats:** - You can also use a special namespace ``"P"`` to purge NODATA/NXDOMAIN matching this name (e.g. ``"domain.cz P"``). + - the count of scanned keys is limited by 100, + so for large subtrees you may be getting just some prefix + (exact name matches go first); + - validated NSEC and NSEC3 records are not put into the table; + - CNAME and DNAME types are printed as NS; + - outdated records and unvalidated non-existence records are still output as ``true``. .. note:: This is equivalent to ``cache['domain']`` getter. @@ -847,28 +844,51 @@ daemons or manipulated from other processes, making for example synchronised loa .. code-block:: lua - -- Query cache for 'domain.cz' - cache['domain.cz'] - -- Query cache for all records at/below 'insecure.net' - cache['*.insecure.net'] + -- Query cache for 'nic.cz' + cache['nic.cz'] + + [nic.cz.] => { + [DNSKEY] => true + [SOA] => true + [AAAA] => true + [NS] => true + [A] => true + [DS] => true + } + [c.ns.nic.cz.] => { + [A] => true + [AAAA] => true + } .. function:: cache.clear([domain]) - :return: ``bool`` + :return: ``bool`` or ``int`` - Purge cache records. If the domain isn't provided, whole cache is purged. See *cache.get()* documentation for subtree matching policy. + Purge cache records. + If the domain isn't provided, whole cache is purged and ``bool`` is returned (denoting success). + + If you provide a name, only records in that subtree are purged, + and the number of removed records is returned. Examples: .. code-block:: lua - -- Clear records at/below 'bad.cz' - cache.clear('*.bad.cz') - -- Clear packet cache - cache.clear('*. P') + -- Clear records at and below 'bad.cz' + cache.clear('bad.cz') -- Clear whole cache cache.clear() + .. attention:: In case you provide a name: + + - The number of removed records is limited to 1000. + The purpose is not to block the resolver for long; if you need larger purges, + you may e.g. repeat the command with a 1ms timer until it returns a lower value. + - To minimize surprises, you may prefer to specify names that have NS/SOA records, + e.g. ``example.com``. Details: validated NSEC and NSEC3 records + (which are used for aggressive non-existence proofs) + will be removed only for zones whose **apex** is at or below the specified name. + Timers and events ^^^^^^^^^^^^^^^^^ diff --git a/daemon/bindings.c b/daemon/bindings.c index 95823d210..a409b8ffa 100644 --- a/daemon/bindings.c +++ b/daemon/bindings.c @@ -1097,25 +1097,28 @@ static int cache_remove_prefix(struct kr_cache *cache, const char *args) if (!cache || !cache->api || !cache->api->remove) { return kr_error(ENOSYS); } - static knot_db_val_t result_set[1000]; + knot_db_val_t result_set[1000]; int ret = cache_prefixed(cache, args, result_set, 1000); if (ret < 0) { return ret; } /* Duplicate result set as we're going to remove it * which will invalidate result set. */ - for (int i = 0; i < ret; ++i) { + int i; + for (i = 0; i < ret; ++i) { void *dst = malloc(result_set[i].len); if (!dst) { - return kr_error(ENOMEM); + ret = kr_error(ENOMEM); + goto cleanup; } memcpy(dst, result_set[i].data, result_set[i].len); result_set[i].data = dst; } cache->api->remove(cache->db, result_set, ret); kr_cache_sync(cache); +cleanup: /* Free keys */ - for (int i = 0; i < ret; ++i) { + while (--i >= 0) { free(result_set[i].data); } return ret; @@ -1186,10 +1189,10 @@ static int cache_clear(lua_State *L) } /** @internal Dump cache key into table on Lua stack. */ -static void cache_dump_key(lua_State *L, knot_db_val_t *key) +static void cache_dump_key(lua_State *L, knot_db_val_t key) { knot_dname_t dname[KNOT_DNAME_MAXLEN]; - char name[KNOT_DNAME_MAXLEN]; + char name[KNOT_DNAME_TXT_MAXLEN]; uint16_t type; int ret = kr_unpack_cache_key(key, dname, &type); @@ -1197,7 +1200,9 @@ static void cache_dump_key(lua_State *L, knot_db_val_t *key) return; } - knot_dname_to_str(name, dname, sizeof(dname)); + ret = !knot_dname_to_str(name, dname, sizeof(name)); + assert(!ret); + if (ret) return; /* If name typemap doesn't exist yet, create it */ lua_getfield(L, -1, name); @@ -1206,7 +1211,7 @@ static void cache_dump_key(lua_State *L, knot_db_val_t *key) lua_newtable(L); } /* Append to typemap */ - char type_buf[16] = { '\0' }; + char type_buf[KR_RRTYPE_STR_MAXLEN] = { '\0' }; knot_rrtype_to_string(type, type_buf, sizeof(type_buf)); lua_pushboolean(L, true); lua_setfield(L, -2, type_buf); @@ -1214,7 +1219,7 @@ static void cache_dump_key(lua_State *L, knot_db_val_t *key) lua_setfield(L, -2, name); } -/** Query cached records. */ +/** Query cached records. TODO: fix caveats in ./README.rst documentation? */ static int cache_get(lua_State *L) { //struct kr_cache *cache = cache_assert_open(L); // to be fixed soon @@ -1228,7 +1233,7 @@ static int cache_get(lua_State *L) /* Retrieve set of keys */ const char *args = lua_tostring(L, 1); - static knot_db_val_t result_set[100]; + knot_db_val_t result_set[100]; int ret = cache_prefixed(cache, args, result_set, 100); if (ret < 0) { format_error(L, kr_strerror(ret)); @@ -1237,7 +1242,7 @@ static int cache_get(lua_State *L) /* Format output */ lua_newtable(L); for (int i = 0; i < ret; ++i) { - cache_dump_key(L, &result_set[i]); + cache_dump_key(L, result_set[i]); } return 1; } diff --git a/lib/cache/api.c b/lib/cache/api.c index 2ed8290aa..634ced86c 100644 --- a/lib/cache/api.c +++ b/lib/cache/api.c @@ -762,8 +762,7 @@ int kr_cache_peek_exact(struct kr_cache *cache, const knot_dname_t *name, uint16 return ret; } -int kr_cache_remove(struct kr_cache *cache, const knot_dname_t *name, - uint16_t type) +int kr_cache_remove(struct kr_cache *cache, const knot_dname_t *name, uint16_t type) { if (!cache_isvalid(cache)) { return kr_error(EINVAL); @@ -796,23 +795,24 @@ int kr_cache_match(struct kr_cache *cache, const knot_dname_t *name, // use a mock type knot_db_val_t key = key_exact_type(k, KNOT_RRTYPE_A); - key.len -= sizeof((char)('E')) + sizeof(uint16_t); + key.len -= 2 /* '\0' 'E' */ + sizeof(uint16_t); /* CACHE_KEY_DEF */ + if (name[0] == '\0') ++key.len; /* the root name is special ATM */ return cache_op(cache, match, &key, keys, max); } -int kr_unpack_cache_key(knot_db_val_t *key, knot_dname_t *buf, uint16_t *type) +int kr_unpack_cache_key(knot_db_val_t key, knot_dname_t *buf, uint16_t *type) { - if (key == NULL || buf == NULL) { + if (key.data == NULL || buf == NULL || type == NULL) { return kr_error(EINVAL); } int len = -1; - const void *endp; - for (endp = key->data + 1; - endp < key->data + key->len; endp++) { - if (*(const char *)(endp-1) == '\0' && - (*(const char *)endp == 'E')) { - len = endp - key->data - 1; + const char *tag, *key_data = key.data; + for (tag = key_data + 1; tag < key_data + key.len; ++tag) { + /* CACHE_KEY_DEF */ + if (tag[-1] == '\0' && (tag == key_data + 1 || tag[-2] == '\0')) { + if (tag[0] != 'E') return kr_error(EINVAL); + len = tag - 1 - key_data; break; } } @@ -821,13 +821,13 @@ int kr_unpack_cache_key(knot_db_val_t *key, knot_dname_t *buf, uint16_t *type) return kr_error(EINVAL); } - int ret = knot_dname_lf2wire(buf, len, key->data); + int ret = knot_dname_lf2wire(buf, len, key.data); if (ret < 0) { return kr_error(ret); } - // jump over "\0 E/1" - memcpy(type, key->data + len + 2, sizeof(uint16_t)); + /* CACHE_KEY_DEF: jump over "\0 E/1" */ + memcpy(type, tag + 1, sizeof(uint16_t)); return kr_ok(); } diff --git a/lib/cache/api.h b/lib/cache/api.h index 9404fabd5..2dcb1cefc 100644 --- a/lib/cache/api.h +++ b/lib/cache/api.h @@ -143,10 +143,11 @@ int kr_cache_materialize(knot_rdataset_t *dst, const struct kr_cache_p *ref, * @param name dname * @param type rr type * @return 0 or an errcode + * @note only "exact hits" are considered ATM, moreover xNAME records + * are "hidden" as NS. (see comments in struct entry_h) */ KR_EXPORT -int kr_cache_remove(struct kr_cache *cache, const knot_dname_t *name, - uint16_t type); +int kr_cache_remove(struct kr_cache *cache, const knot_dname_t *name, uint16_t type); /** * Get keys matching a dname lf prefix @@ -154,6 +155,8 @@ int kr_cache_remove(struct kr_cache *cache, const knot_dname_t *name, * @param name dname * @param keys matched keys * @return result count or an errcode + * @note the cache keys are matched by prefix, i.e. it very much depends + * on their structure; CACHE_KEY_DEF. */ KR_EXPORT int kr_cache_match(struct kr_cache *cache, const knot_dname_t *name, @@ -165,6 +168,8 @@ int kr_cache_match(struct kr_cache *cache, const knot_dname_t *name, * @param buf output buffer of domain name in dname format * @param type output for type * @return length of dname or an errcode + * @note only "exact hits" are considered ATM, moreover xNAME records + * are "hidden" as NS. (see comments in struct entry_h) */ KR_EXPORT -int kr_unpack_cache_key(knot_db_val_t *key, knot_dname_t *buf, uint16_t *type); +int kr_unpack_cache_key(knot_db_val_t key, knot_dname_t *buf, uint16_t *type); diff --git a/lib/cache/cdb_lmdb.c b/lib/cache/cdb_lmdb.c index 2b07f9a1b..23f65261c 100644 --- a/lib/cache/cdb_lmdb.c +++ b/lib/cache/cdb_lmdb.c @@ -571,6 +571,7 @@ static int cdb_match(knot_db_t *db, knot_db_val_t *key, knot_db_val_t *val, int return ret; } + /* LATER(optim.): use txn_curs_get() instead, to save resources. */ MDB_cursor *cur = NULL; ret = mdb_cursor_open(txn, env->dbi, &cur); if (ret != 0) { diff --git a/lib/cache/impl.h b/lib/cache/impl.h index ca53bb22b..c56cb35df 100644 --- a/lib/cache/impl.h +++ b/lib/cache/impl.h @@ -37,14 +37,15 @@ /* 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. + * Code depending on the contents 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_* flags; - * - is_packet: uint16_t length, otherwise opaque and handled by ./entry_pkt.c - * - otherwise RRset + its RRSIG set (possibly empty). + * - ktype == NS: struct entry_apex - multiple types inside (NS and xNAME); + * - ktype != NS: struct entry_h + * * is_packet: uint16_t length, the rest is opaque and handled by ./entry_pkt.c + * * otherwise RRset + its RRSIG set (possibly empty). * '1' or '3' entry (NSEC or NSEC3) - * - contents is the same as for exact hit + * - struct entry_h, contents is the same as for exact hit * - flags don't make sense there */