From 23b0746168c72eee1868944c1e4552e2fef93509 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 24 Nov 2018 09:02:00 +0100 Subject: [PATCH 1/5] Simplify upstream percentage computation. Signed-off-by: DL6ER --- api.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/api.c b/api.c index e76ccec2..615286d5 100644 --- a/api.c +++ b/api.c @@ -443,16 +443,13 @@ void getTopClients(char *client_message, int *sock) void getForwardDestinations(char *client_message, int *sock) { bool sort = true; - int i, temparray[counters.forwarded][2], forwardedsum = 0, totalqueries = 0; + int i, temparray[counters.forwarded][2], totalqueries = 0; if(command(client_message, "unsorted")) sort = false; for(i=0; i < counters.forwarded; i++) { validate_access("forwarded", i, true, __LINE__, __FUNCTION__, __FILE__); - // Compute forwardedsum - forwardedsum += forwarded[i].count; - // If we want to print a sorted output, we fill the temporary array with // the values we will use for sorting afterwards if(sort) { @@ -513,24 +510,9 @@ void getForwardDestinations(char *client_message, int *sock) else name = ""; - // Math explanation: - // A single query may result in requests being forwarded to multiple destinations - // Hence, in order to be able to give percentages here, we have to normalize the - // number of forwards to each specific destination by the total number of forward - // events. This term is done by - // a = forwarded[j].count / forwardedsum - // - // The fraction a describes now how much share an individual forward destination - // has on the total sum of sent requests. - // We also know the share of forwarded queries on the total number of queries - // b = counters.forwardedqueries / c - // where c is the number of valid queries, - // c = counters.forwardedqueries + counters.cached + counters.blocked - // - // To get the total percentage of a specific query on the total number of queries, - // we simply have to scale b by a which is what we do in the following. - if(forwardedsum > 0 && totalqueries > 0) - percentage = 1e2f * forwarded[j].count / forwardedsum * counters.forwardedqueries / totalqueries; + // Get percentage + if(totalqueries > 0) + percentage = 1e2f * forwarded[j].count / totalqueries; } // Send data: From 2ea40291229259d8c02b4ec6c5ad98dbd27d0e5c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 24 Nov 2018 09:16:32 +0100 Subject: [PATCH 2/5] Set query status to forwarded only *after* the check if it has been replied to from cache. Signed-off-by: DL6ER --- dnsmasq_interface.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 6b0eaf36..1e5d1097 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -269,9 +269,6 @@ void FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id return; } - // Set query status - queries[i].status = QUERY_FORWARDED; - // Proceed only if // - current query has not been marked as replied to so far // (it could be that answers from multiple forward @@ -332,6 +329,13 @@ void FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id // Hereby, this query is now fully determined queries[i].complete = true; } + + // Set query status to forwarded only after the + // if(queries[i].status == QUERY_CACHE) { ... } + // from above as otherwise this check will always + // be negative + queries[i].status = QUERY_FORWARDED; + // Update overTime data overTime[j].forwarded++; From f26c74a58d63e85ff8bc6eeb9a25e1876e2f4936 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 24 Nov 2018 09:24:40 +0100 Subject: [PATCH 3/5] No need to check for query completed status after the if-cond that returns early if we have seen this particular query before (and it wasn't replied to from cache) Signed-off-by: DL6ER --- dnsmasq_interface.c | 103 +++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 1e5d1097..622d40a0 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -272,7 +272,7 @@ void FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id // Proceed only if // - current query has not been marked as replied to so far // (it could be that answers from multiple forward - // destionations are coimg in for the same query) + // destinations are coming in for the same query) // - the query was formally known as cached but had to be forwarded // (this is a special case further described below) if(queries[i].complete && queries[i].status != QUERY_CACHE) @@ -287,61 +287,58 @@ void FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id int forwardID = findForwardID(forward, true); queries[i].forwardID = forwardID; - if(!queries[i].complete) + int j = queries[i].timeidx; + validate_access("overTime", j, true, __LINE__, __FUNCTION__, __FILE__); + + if(queries[i].status == QUERY_CACHE) { - int j = queries[i].timeidx; - validate_access("overTime", j, true, __LINE__, __FUNCTION__, __FILE__); + // Detect if we cached the but need to ask the upstream + // servers for the actual IPs now, we remove this query from the + // counters for cache replied queries as we had to forward a + // request for it. Example: + // Assume a domain a.com is a CNAME which is cached and has a very + // long TTL. It point to another domain server.a.com which has an + // A record but this has a much lower TTL. + // If you now query a.com and then again after some time, you end + // up in a situation where dnsmasq can answer the first level of + // the DNS result (the CNAME) from cache, hence the status of this + // query is marked as "answered from cache" in FTLDNS. However, for + // server.a.com wit the much shorter TTL, we still have to forward + // something and ask the upstream server for the final IP address. + // This code section acknowledges this by removing one entry from + // the cached counters as we will re-brand this query as having been + // forwarded in the following. + counters.cached--; + // Also correct overTime data + overTime[j].cached--; - if(queries[i].status == QUERY_CACHE) - { - // Detect if we cached the but need to ask the upstream - // servers for the actual IPs now, we remove this query from the - // counters for cache replied queries as we had to forward a - // request for it. Example: - // Assume a domain a.com is a CNAME which is cached and has a very - // long TTL. It point to another domain server.a.com which has an - // A record but this has a much lower TTL. - // If you now query a.com and then again after some time, you end - // up in a situation where dnsmasq can answer the first level of - // the DNS result (the CNAME) from cache, hence the status of this - // query is marked as "answered from cache" in FTLDNS. However, for - // server.a.com wit the much shorter TTL, we still have to forward - // something and ask the upstream server for the final IP address. - // This code section acknowledges this by removing one entry from - // the cached counters as we will re-brand this query as having been - // forwarded in the following. - counters.cached--; - // Also correct overTime data - overTime[j].cached--; - - // Correct reply timer - struct timeval response; - gettimeofday(&response, 0); - // Reset timer, shift slightly into the past to acknowledge the time - // FTLDNS needed to look up the CNAME in its cache - queries[i].response = converttimeval(response) - queries[i].response; - } - else - { - // Normal cache reply - // Query is no longer unknown - counters.unknown--; - // Hereby, this query is now fully determined - queries[i].complete = true; - } - - // Set query status to forwarded only after the - // if(queries[i].status == QUERY_CACHE) { ... } - // from above as otherwise this check will always - // be negative - queries[i].status = QUERY_FORWARDED; - - // Update overTime data - overTime[j].forwarded++; - - // Update couter for forwarded queries - counters.forwardedqueries++; + // Correct reply timer + struct timeval response; + gettimeofday(&response, 0); + // Reset timer, shift slightly into the past to acknowledge the time + // FTLDNS needed to look up the CNAME in its cache + queries[i].response = converttimeval(response) - queries[i].response; } + else + { + // Normal cache reply + // Query is no longer unknown + counters.unknown--; + // Hereby, this query is now fully determined + queries[i].complete = true; + } + + // Set query status to forwarded only after the + // if(queries[i].status == QUERY_CACHE) { ... } + // from above as otherwise this check will always + // be negative + queries[i].status = QUERY_FORWARDED; + + // Update overTime data + overTime[j].forwarded++; + + // Update counter for forwarded queries + counters.forwardedqueries++; // Release allocated memory free(forward); From cf473a384d573d92b71785a9ec7858fff8f6bbde Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 24 Nov 2018 19:23:44 +0100 Subject: [PATCH 4/5] Update comment Signed-off-by: DL6ER --- dnsmasq_interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 622d40a0..dd925cd2 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -321,7 +321,7 @@ void FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id } else { - // Normal cache reply + // Normal forwarded query (status is set below) // Query is no longer unknown counters.unknown--; // Hereby, this query is now fully determined From 5c39ec4467cb3642d53adaacd6f54f865f65fd8a Mon Sep 17 00:00:00 2001 From: Keita Suzuki Date: Fri, 30 Nov 2018 16:15:52 +0900 Subject: [PATCH 5/5] Modify type of gclastrun to avoid overflow Signed-off-by: Keita Suzuki --- gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gc.c b/gc.c index 66225312..885302bc 100644 --- a/gc.c +++ b/gc.c @@ -11,7 +11,7 @@ #include "FTL.h" bool doGC = false; -int lastGCrun = 0; +time_t lastGCrun = 0; void *GC_thread(void *val) { // Set thread name