- 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

diff --git a/doc/Changelog b/doc/Changelog
index db4e989a0d3128c0f61e9840005ebb7ec750a1aa..d1e4a81f1ef4e9ebcd95e9fc35654afe827f2d5c 100644 (file)
--- 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 1ae851ba74c87c5b57d01014f4c921d2cd0291d6..07328b029095b6b44c58949e7a6d59767231e36c 100644 (file)
--- 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 e86c6b8427637b3839e7e34a653088dd030909db..a8fde9f2890e8bf40d2ca77bb5f64ce4b61667df 100644 (file)
--- 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 5415a47281c1c2f64703cd8995022af92334743c..0dfb688744032348d9e982c754a4ad0d51a2b15b 100644 (file)
--- 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 18cdbf7739297b8abc99622525606efc26a13766..5fea5d2b063551dd62eb82f1099e46b07d74373e 100644 (file)
--- 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