From d30ae35c62a036a05f20032a2578069ae36bedb3 Mon Sep 17 00:00:00 2001 From: Ralph Dolmans Date: Mon, 3 Apr 2017 09:36:18 +0000 Subject: [PATCH] - Do not add current time twice to TTL before ECS cache store. - Do not touch rrset cache after ECS cache message generation. - Use LDNS_EDNS_CLIENT_SUBNET as default ECS opcode. git-svn-id: file:///svn/unbound/trunk@4086 be551aaa-1e26-0410-a405-d3ace91eadb9 --- doc/Changelog | 5 +++++ edns-subnet/subnetmod.c | 38 +++++++++++++++++++------------------- services/cache/dns.c | 5 ++++- services/cache/dns.h | 3 ++- util/config_file.c | 2 +- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/doc/Changelog b/doc/Changelog index db4e989a0..d1e4a81f1 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +3 April 2017: Ralph + - Do not add current time twice to TTL before ECS cache store. + - Do not touch rrset cache after ECS cache message generation. + - Use LDNS_EDNS_CLIENT_SUBNET as default ECS opcode. + 3 April 2017: Wouter - Fix #1217: Add metrics to unbound-control interface showing crypted, cert request, plaintext and malformed queries (from diff --git a/edns-subnet/subnetmod.c b/edns-subnet/subnetmod.c index 1ae851ba7..07328b029 100644 --- a/edns-subnet/subnetmod.c +++ b/edns-subnet/subnetmod.c @@ -306,11 +306,11 @@ update_cache(struct module_qstate *qstate, int id) struct ecs_data *edns = &sq->ecs_client_in; size_t i; - /** We already calculated hash upon lookup */ + /* We already calculated hash upon lookup */ hashvalue_type h = qstate->minfo[id] ? ((struct subnet_qstate*)qstate->minfo[id])->qinfo_hash : query_info_hash(&qstate->qinfo, qstate->query_flags); - /** Step 1, general qinfo lookup */ + /* Step 1, general qinfo lookup */ struct lruhash_entry *lru_entry = slabhash_lookup(subnet_msg_cache, h, &qstate->qinfo, 1); int acquired_lock = (lru_entry != NULL); @@ -336,7 +336,7 @@ update_cache(struct module_qstate *qstate, int id) return; } } - /** Step 2, find the correct tree */ + /* Step 2, find the correct tree */ if (!(tree = get_tree(lru_entry->data, edns, sne))) { if (acquired_lock) lock_rw_unlock(&lru_entry->lock); log_err("Subnet cache insertion failed"); @@ -360,7 +360,7 @@ update_cache(struct module_qstate *qstate, int id) addrtree_insert(tree, (addrkey_t*)edns->subnet_addr, edns->subnet_source_mask, sq->ecs_server_in.subnet_scope_mask, rep, - rep->ttl + *qstate->env->now, *qstate->env->now); + rep->ttl, *qstate->env->now); if (acquired_lock) { lock_rw_unlock(&lru_entry->lock); } else { @@ -369,7 +369,7 @@ update_cache(struct module_qstate *qstate, int id) } } -/* return true iff reply is sent. */ +/** Lookup in cache and reply true iff reply is sent. */ static int lookup_and_reply(struct module_qstate *qstate, int id, struct subnet_qstate *sq) { @@ -385,30 +385,30 @@ lookup_and_reply(struct module_qstate *qstate, int id, struct subnet_qstate *sq) memset(&sq->ecs_client_out, 0, sizeof(sq->ecs_client_out)); - if (sq) sq->qinfo_hash = h; /** Might be useful on cache miss */ + if (sq) sq->qinfo_hash = h; /* Might be useful on cache miss */ e = slabhash_lookup(sne->subnet_msg_cache, h, &qstate->qinfo, 1); - if (!e) return 0; /** qinfo not in cache */ + if (!e) return 0; /* qinfo not in cache */ data = e->data; tree = (ecs->subnet_addr_fam == EDNSSUBNET_ADDRFAM_IP4)? data->tree4 : data->tree6; - if (!tree) { /** qinfo in cache but not for this family */ + if (!tree) { /* qinfo in cache but not for this family */ lock_rw_unlock(&e->lock); return 0; } node = addrtree_find(tree, (addrkey_t*)ecs->subnet_addr, ecs->subnet_source_mask, *env->now); - if (!node) { /** plain old cache miss */ + if (!node) { /* plain old cache miss */ lock_rw_unlock(&e->lock); return 0; } - qstate->return_msg = tomsg(env, &qstate->qinfo, + qstate->return_msg = tomsg(NULL, &qstate->qinfo, (struct reply_info *)node->elem, qstate->region, *env->now, env->scratch); scope = (uint8_t)node->scope; lock_rw_unlock(&e->lock); - if (!qstate->return_msg) { /** TTL expired */ + if (!qstate->return_msg) { /* Failed allocation or expired TTL */ return 0; } @@ -452,18 +452,18 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq) if (!qstate->return_msg) return module_error; - /** We have not asked for subnet data */ + /* We have not asked for subnet data */ if (!sq->subnet_sent) { if (s_in->subnet_validdata) verbose(VERB_QUERY, "subnet: received spurious data"); - if (sq->subnet_downstream) /** Copy back to client */ + if (sq->subnet_downstream) /* Copy back to client */ cp_edns_bad_response(c_out, c_in); return module_finished; } - /** subnet sent but nothing came back */ + /* subnet sent but nothing came back */ if (!s_in->subnet_validdata) { - /** The authority indicated no support for edns subnet. As a + /* The authority indicated no support for edns subnet. As a * consequence the answer ended up in the regular cache. It * is still usefull to put it in the edns subnet cache for * when a client explicitly asks for subnet specific answer. */ @@ -476,17 +476,17 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq) return module_finished; } - /** Being here means we have asked for and got a subnet specific + /* Being here means we have asked for and got a subnet specific * answer. Also, the answer from the authority is not yet cached * anywhere. */ - /** can we accept response? */ + /* can we accept response? */ if(s_out->subnet_addr_fam != s_in->subnet_addr_fam || s_out->subnet_source_mask != s_in->subnet_source_mask || !common_prefix(s_out->subnet_addr, s_in->subnet_addr, s_out->subnet_source_mask)) { - /** we can not accept, restart query without option */ + /* we can not accept, restart query without option */ verbose(VERB_QUERY, "subnet: forged data"); s_out->subnet_validdata = 0; (void)edns_opt_list_remove(&qstate->edns_opts_back_out, @@ -500,7 +500,7 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq) lock_rw_unlock(&sne->biglock); if (sq->subnet_downstream) { - /** Client wants to see the answer, echo option back + /* Client wants to see the answer, echo option back * and adjust the scope. */ c_out->subnet_addr_fam = c_in->subnet_addr_fam; c_out->subnet_source_mask = c_in->subnet_source_mask; diff --git a/services/cache/dns.c b/services/cache/dns.c index e86c6b842..a8fde9f28 100644 --- a/services/cache/dns.c +++ b/services/cache/dns.c @@ -524,8 +524,11 @@ tomsg(struct module_env* env, struct query_info* q, struct reply_info* r, return NULL; } } - rrset_array_unlock_touch(env->rrset_cache, scratch, r->ref, + if(env) + rrset_array_unlock_touch(env->rrset_cache, scratch, r->ref, r->rrset_count); + else + rrset_array_unlock(r->ref, r->rrset_count); return msg; } diff --git a/services/cache/dns.h b/services/cache/dns.h index 5415a4728..0dfb68874 100644 --- a/services/cache/dns.h +++ b/services/cache/dns.h @@ -128,7 +128,8 @@ struct delegpt* dns_cache_find_delegation(struct module_env* env, /** * generate dns_msg from cached message - * @param env: module environment with the DNS cache. + * @param env: module environment with the DNS cache. NULL if the LRU from cache + * does not need to be touched. * @param q: query info, contains qname that will make up the dns message. * @param r: reply info that, together with qname, will make up the dns message. * @param region: where to allocate dns message. diff --git a/util/config_file.c b/util/config_file.c index 18cdbf773..5fea5d2b0 100644 --- a/util/config_file.c +++ b/util/config_file.c @@ -178,7 +178,7 @@ config_create(void) cfg->forwards = NULL; #ifdef CLIENT_SUBNET cfg->client_subnet = NULL; - cfg->client_subnet_opcode = 8; + cfg->client_subnet_opcode = LDNS_EDNS_CLIENT_SUBNET; cfg->max_client_subnet_ipv4 = 24; cfg->max_client_subnet_ipv6 = 64; #endif