From 4f8cbe29f7b4e1cead138ea7d74c2b4fa98e91ce Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 25 Jun 2021 13:10:39 +0200 Subject: [PATCH] Store real over-time counts of forwarded queries. So far, we counted only the first server a query was sent to. Signed-off-by: DL6ER --- src/database/query-table.c | 1 + src/datastructure.h | 5 +- src/dnsmasq_interface.c | 52 ++++++++++----- src/gc.c | 3 + src/overTime.c | 133 ++++++++++++++++++++++++------------- 5 files changed, 128 insertions(+), 66 deletions(-) diff --git a/src/database/query-table.c b/src/database/query-table.c index e49ea550..1ba633af 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -575,6 +575,7 @@ void DB_read_queries(void) upstreamsData *upstream = getUpstream(upstreamID, true); if(upstream != NULL) { + upstream->overTime[timeidx]++; upstream->count++; upstream->lastQuery = queryTimeStamp; } diff --git a/src/datastructure.h b/src/datastructure.h index 78d568a6..32c588d6 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -56,14 +56,15 @@ ASSERT_SIZEOF(queriesData, 64, 52, 56); typedef struct { unsigned char magic; bool new; + in_addr_t port; int count; int failed; - in_addr_t port; + int overTime[OVERTIME_SLOTS]; size_t ippos; size_t namepos; time_t lastQuery; } upstreamsData; -ASSERT_SIZEOF(upstreamsData, 40, 28, 28); +ASSERT_SIZEOF(upstreamsData, 640, 628, 628); typedef struct { unsigned char magic; diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 02a7191b..0551ab42 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -59,7 +59,7 @@ static void FTL_forwarded(const unsigned int flags, const char *name, const unio static void FTL_reply(const unsigned int flags, const char *name, const union all_addr *addr, const char* arg, const int id, const char* file, const int line); static void FTL_upstream_error(const unsigned int rcode, const int id, const char* file, const int line); static void FTL_dnssec(const char *result, const int id, const char* file, const int line); -static void get_mysockaddr_ip_port(union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port); +static void mysockaddr_extract_ip_port(union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port); // Static blocking metadata static const char *blockingreason = NULL; @@ -370,6 +370,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name, // 127.0.0.1 to avoid queries originating from localhost of the // *distant* machine as queries coming from the *local* machine const sa_family_t family = addr ? addr->sa.sa_family : AF_INET; + in_port_t clientPort = daemon->port; bool internal_query = false; char clientIP[ADDRSTRLEN+1] = { 0 }; if(config.edns0_ecs && edns && edns->client_set) @@ -381,7 +382,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name, else if(addr) { // Use original requestor - get_mysockaddr_ip_port(addr, clientIP, NULL); + mysockaddr_extract_ip_port(addr, clientIP, &clientPort); } else { @@ -428,9 +429,10 @@ bool _FTL_new_query(const unsigned int flags, const char *name, { if(config.debug & DEBUG_QUERIES) { - logg("Rate-limiting %sIPv%d %s query \"%s\" from %s:%s", + logg("Rate-limiting %sIPv%d %s query \"%s\" from %s:%s#%d", proto == TCP ? "TCP " : proto == UDP ? "UDP " : "", - family == AF_INET ? 4 : 6, types, domainString, interface, clientIP); + family == AF_INET ? 4 : 6, types, domainString, interface, + clientIP, clientPort); } // Block this query @@ -444,10 +446,11 @@ bool _FTL_new_query(const unsigned int flags, const char *name, // Log new query if in debug mode if(config.debug & DEBUG_QUERIES) { - logg("**** new %sIPv%d %s query \"%s\" from %s:%s (ID %i, FTL %i, %s:%i)", + logg("**** new %sIPv%d %s query \"%s\" from %s:%s#%d (ID %i, FTL %i, %s:%i)", proto == TCP ? "TCP " : proto == UDP ? "UDP " : "", family == AF_INET ? 4 : 6, types, domainString, interface, - internal_query ? "" : clientIP, id, queryID, short_path(file), line); + internal_query ? "" : clientIP, clientPort, + id, queryID, short_path(file), line); } // Update counters @@ -1139,6 +1142,10 @@ static void FTL_forwarded(const unsigned int flags, const char *name, const unio char *upstreamIP = strdup(dest); strtolower(upstreamIP); + // Substitute "." if we are querying the root domain (e.g. DNSKEY) + if(!name || strlen(name) == 0) + name = "."; + // Debug logging if(config.debug & DEBUG_QUERIES) logg("**** forwarded %s to %s#%u (ID %i, %s:%i)", @@ -1157,15 +1164,7 @@ static void FTL_forwarded(const unsigned int flags, const char *name, const unio // Get query pointer queriesData* query = getQuery(queryID, true); - - // Proceed only if - // - current query has not been marked as replied to so far - // (it could be that answers from multiple forward - // 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) - // Use short-circuit evaluation to check if query is NULL - if(query == NULL || (query->flags.complete && query->status != QUERY_CACHE)) + if(query == NULL) { free(upstreamIP); unlock_shm(); @@ -1180,10 +1179,27 @@ static void FTL_forwarded(const unsigned int flags, const char *name, const unio upstreamsData *upstream = getUpstream(upstreamID, true); if(upstream != NULL) { + // Update overTime + upstream->overTime[query->timeidx]++; + // Update total count upstream->count++; + // Update lastQuery timestamp upstream->lastQuery = time(NULL); } + // Proceed only if + // - current query has not been marked as replied to so far + // (it could be that answers from multiple forward + // 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(query->flags.complete && query->status != QUERY_CACHE) + { + free(upstreamIP); + unlock_shm(); + return; + } + if(query->status == QUERY_CACHE) { // Detect if we cached the but need to ask the upstream @@ -1265,7 +1281,7 @@ void FTL_dnsmasq_reload(void) resolver_ready = true; } -static void get_mysockaddr_ip_port(union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port) +static void mysockaddr_extract_ip_port(union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port) { // Extract IP address inet_ntop(server->sa.sa_family, @@ -1370,10 +1386,10 @@ static void FTL_reply(const unsigned int flags, const char *name, const union al logg("**** got %s reply: %s is %s (ID %i, %s:%i)", cached ? "cache" : "upstream", name, answer, id, file, line); else { - // Log server who replied to our request + // Log server which replied to our request char ip[ADDRSTRLEN+1] = { 0 }; in_port_t port = 0; - get_mysockaddr_ip_port(&last_server, ip, &port); + mysockaddr_extract_ip_port(&last_server, ip, &port); logg("**** got %s reply from %s#%d: %s is %s (ID %i, %s:%i)", cached ? "cache" : "upstream", ip, port, name, answer, id, file, line); } diff --git a/src/gc.c b/src/gc.c index bb2d97ee..ddafa2b8 100644 --- a/src/gc.c +++ b/src/gc.c @@ -126,7 +126,10 @@ void *GC_thread(void *val) { upstreamsData* upstream = getUpstream(query->upstreamID, true); if(upstream != NULL) + { + upstream->overTime[timeidx]--; upstream->count--; + } } break; case QUERY_CACHE: diff --git a/src/overTime.c b/src/overTime.c index 56c7eaba..3acec0d2 100644 --- a/src/overTime.c +++ b/src/overTime.c @@ -51,6 +51,18 @@ static void initSlot(const unsigned int index, const time_t timestamp) client->overTime[index] = 0; } } + + // Zero overTime counter for all known upstream destinations + for(int upstreamID = 0; upstreamID < counters->upstreams; upstreamID++) + { + // Get client pointer + upstreamsData* upstream = getUpstream(upstreamID, true); + if(upstream != NULL) + { + // Set overTime data to zero + upstream->overTime[index] = 0; + } + } } void initOverTime(void) @@ -146,56 +158,85 @@ void moveOverTimeMemory(const time_t mintime) // Check if the move over amount is valid. This prevents errors if the // function is called before GC is necessary. - if(moveOverTime > 0 && moveOverTime < OVERTIME_SLOTS) + if(!(moveOverTime > 0 && moveOverTime < OVERTIME_SLOTS)) + return; + + // Move overTime memory + if(config.debug & DEBUG_OVERTIME) { - // Move overTime memory - if(config.debug & DEBUG_OVERTIME) + logg("moveOverTimeMemory(): Moving overTime %u - %u to 0 - %u", + moveOverTime, moveOverTime+remainingSlots, remainingSlots); + } + + // Move overTime memory forward to update data structure + memmove(&overTime[0], + &overTime[moveOverTime], + remainingSlots*sizeof(*overTime)); + + // Correct time indices of queries. This is necessary because we just moved the slot this index points to + for(int queryID = 0; queryID < counters->queries; queryID++) + { + // Get query pointer + queriesData* query = getQuery(queryID, true); + if(query == NULL) + continue; + + // Check if the index would become negative if we adjusted it + if(((int)query->timeidx - (int)moveOverTime) < 0) { - logg("moveOverTimeMemory(): Moving overTime %u - %u to 0 - %u", - moveOverTime, moveOverTime+remainingSlots, remainingSlots); + // This should never happen, but we print a warning if it still happens + // We don't do anything in this case + logg("WARN: moveOverTimeMemory(): overTime time index correction failed (%i: %u / %u)", + queryID, query->timeidx, moveOverTime); } - - // Move overTime memory forward to update data structure - memmove(&overTime[0], - &overTime[moveOverTime], - remainingSlots*sizeof(*overTime)); - - // Correct time indices of queries. This is necessary because we just moved the slot this index points to - for(int queryID = 0; queryID < counters->queries; queryID++) + else { - // Get query pointer - queriesData* query = getQuery(queryID, true); - if(query == NULL) - continue; - - // Check if the index would become negative if we adjusted it - if(((int)query->timeidx - (int)moveOverTime) < 0) - { - // This should never happen, but we print a warning if it still happens - // We don't do anything in this case - logg("WARN: moveOverTimeMemory(): overTime time index correction failed (%i: %u / %u)", - queryID, query->timeidx, moveOverTime); - } - else - { - query->timeidx -= moveOverTime; - } - } - - // Move client-specific overTime memory - for(int clientID = 0; clientID < counters->clients; clientID++) - { - memmove(&(getClient(clientID, true)->overTime[0]), - &(getClient(clientID, true)->overTime[moveOverTime]), - remainingSlots*sizeof(int)); - } - - // Iterate over new overTime region and initialize it - for(unsigned int timeidx = remainingSlots; timeidx < OVERTIME_SLOTS ; timeidx++) - { - // This slot is OVERTIME_INTERVAL seconds after the previous slot - const time_t timestamp = overTime[timeidx-1].timestamp + OVERTIME_INTERVAL; - initSlot(timeidx, timestamp); + query->timeidx -= moveOverTime; } } + + // Move client-specific overTime memory + for(int clientID = 0; clientID < counters->clients; clientID++) + { + clientsData *client = getClient(clientID, true); + if(!client) + continue; + memmove(&(client->overTime[0]), + &(client->overTime[moveOverTime]), + remainingSlots*sizeof(int)); + } + + // Process upstream data + for(int upstreamID = 0; upstreamID < counters->upstreams; upstreamID++) + { + upstreamsData *upstream = getUpstream(upstreamID, true); + if(!upstream) + continue; + + // Update upstream counters with overTime data we are going to move + // This is necessary because we can store inly one upstream with each query + // and garbage collection can only subtract this one when cleaning + unsigned int sum = 0; + for(unsigned int idx = 0; idx < moveOverTime; idx++) + { + sum += upstream->overTime[idx]; + } + upstream->count -= sum; + if(config.debug & DEBUG_GC) + logg("Subtracted %d from total count of upstream %s:%d, new total is %d", + sum, getstr(upstream->ippos), upstream->port, upstream->count); + + // Move upstream-specific overTime memory + memmove(&(upstream->overTime[0]), + &(upstream->overTime[moveOverTime]), + remainingSlots*sizeof(int)); + } + + // Iterate over new overTime region and initialize it + for(unsigned int timeidx = remainingSlots; timeidx < OVERTIME_SLOTS ; timeidx++) + { + // This slot is OVERTIME_INTERVAL seconds after the previous slot + const time_t timestamp = overTime[timeidx-1].timestamp + OVERTIME_INTERVAL; + initSlot(timeidx, timestamp); + } }