Skip to content

Commit

Permalink
Merge pull request #1840 from pi-hole/fix/cache_recycling
Browse files Browse the repository at this point in the history
Implement DNS cache recycling
  • Loading branch information
DL6ER authored Jan 4, 2024
2 parents e1cdad4 + 7b4c0aa commit c24c1e1
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 50 deletions.
12 changes: 6 additions & 6 deletions src/database/query-table.c
Original file line number Diff line number Diff line change
Expand Up @@ -1065,19 +1065,20 @@ void DB_read_queries(void)
query->qtype = type - 100;
}
counters->querytype[query->type]++;
log_debug(DEBUG_GC, "query type %d set (database), ID = %d, new count = %d", query->type, counters->queries, counters->querytype[query->type]);
log_debug(DEBUG_STATUS, "query type %d set (database), ID = %d, new count = %d", query->type, counters->queries, counters->querytype[query->type]);

// Status is set below
query->domainID = domainID;
query->clientID = clientID;
query->upstreamID = upstreamID;
query->cacheID = findCacheID(domainID, clientID, query->type, true);
query->id = counters->queries;
query->response = 0;
query->flags.response_calculated = reply_time_avail;
query->dnssec = dnssec;
query->reply = reply;
counters->reply[query->reply]++;
log_debug(DEBUG_GC, "reply type %d set (database), ID = %d, new count = %d", query->reply, counters->queries, counters->reply[query->reply]);
log_debug(DEBUG_STATUS, "reply type %d set (database), ID = %d, new count = %d", query->reply, counters->queries, counters->reply[query->reply]);
query->response = reply_time;
query->CNAME_domainID = -1;
// Initialize flags
Expand Down Expand Up @@ -1125,8 +1126,7 @@ void DB_read_queries(void)
// Set ID of the domainlist entry that was the reason for permitting/blocking this query
// We assume the value in this field is said ID when it is not a CNAME-related domain
// (checked above) and the value of additional_info is not NULL (0 bytes storage size)
const int cacheID = findCacheID(query->domainID, query->clientID, query->type, true);
DNSCacheData *cache = getDNSCache(cacheID, true);
DNSCacheData *cache = getDNSCache(query->cacheID, true);
// Only load if
// a) we have a cache entry
// b) the value of additional_info is not NULL (0 bytes storage size)
Expand Down Expand Up @@ -1459,8 +1459,8 @@ bool queries_to_database(void)
}

// Get cache entry for this query
const int cacheID = findCacheID(query->domainID, query->clientID, query->type, false);
DNSCacheData *cache = cacheID < 0 ? NULL : getDNSCache(cacheID, true);
const int cacheID = query->cacheID >= 0 ? query->cacheID : findCacheID(query->domainID, query->clientID, query->type, false);
DNSCacheData *cache = getDNSCache(cacheID, true);

// ADDITIONAL_INFO
if(query->status == QUERY_GRAVITY_CNAME ||
Expand Down
42 changes: 25 additions & 17 deletions src/datastructure.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ int _findUpstreamID(const char *upstreamString, const in_port_t port, int line,
return upstreamID;
}

static int get_next_domainID(void)
static int get_next_free_domainID(void)
{
// Compare content of domain against known domain IP addresses
for(int domainID=0; domainID < counters->domains; domainID++)
for(int domainID = 0; domainID < counters->domains; domainID++)
{
// Get domain pointer
domainsData* domain = getDomain(domainID, false);
Expand Down Expand Up @@ -188,7 +188,7 @@ int _findDomainID(const char *domainString, const bool count, int line, const ch

// If we did not return until here, then this domain is not known
// Store ID
const int domainID = get_next_domainID();
const int domainID = get_next_free_domainID();

// Get domain pointer
domainsData* domain = _getDomain(domainID, false, line, func, file);
Expand Down Expand Up @@ -218,10 +218,10 @@ int _findDomainID(const char *domainString, const bool count, int line, const ch
return domainID;
}

static int get_next_clientID(void)
static int get_next_free_clientID(void)
{
// Compare content of client against known client IP addresses
for(int clientID=0; clientID < counters->clients; clientID++)
for(int clientID = 0; clientID < counters->clients; clientID++)
{
// Get client pointer
clientsData* client = getClient(clientID, false);
Expand Down Expand Up @@ -271,7 +271,7 @@ int _findClientID(const char *clientIP, const bool count, const bool aliasclient

// If we did not return until here, then this client is definitely new
// Store ID
const int clientID = get_next_clientID();
const int clientID = get_next_free_clientID();

// Get client pointer
clientsData* client = _getClient(clientID, false, line, func, file);
Expand Down Expand Up @@ -369,10 +369,10 @@ void change_clientcount(clientsData *client, int total, int blocked, int overTim
}
}

static int get_next_cacheID(void)
static int get_next_free_cacheID(void)
{
// Compare content of cache against known cache IP addresses
for(int cacheID=0; cacheID < counters->dns_cache_size; cacheID++)
for(int cacheID = 0; cacheID < counters->dns_cache_size; cacheID++)
{
// Get cache pointer
DNSCacheData* cache = getDNSCache(cacheID, false);
Expand Down Expand Up @@ -415,7 +415,7 @@ int _findCacheID(const int domainID, const int clientID, const enum query_type q
return -1;

// Get ID of new cache entry
const int cacheID = get_next_cacheID();
const int cacheID = get_next_free_cacheID();

// Get client pointer
DNSCacheData* dns_cache = _getDNSCache(cacheID, false, line, func, file);
Expand All @@ -426,6 +426,9 @@ int _findCacheID(const int domainID, const int clientID, const enum query_type q
return -1;
}

log_debug(DEBUG_GC, "New cache entry: domainID %d, clientID %d, query_type %d (ID %d)",
domainID, clientID, query_type, cacheID);

// Initialize cache entry
dns_cache->magic = MAGICBYTE;
dns_cache->blocking_status = UNKNOWN_BLOCKED;
Expand Down Expand Up @@ -555,12 +558,17 @@ void FTL_reset_per_client_domain_data(void)

for(int cacheID = 0; cacheID < counters->dns_cache_size; cacheID++)
{
// Reset all blocking yes/no fields for all domains and clients
// This forces a reprocessing of all available filters for any
// given domain and client the next time they are seen
DNSCacheData *dns_cache = getDNSCache(cacheID, true);
if(dns_cache != NULL)
dns_cache->blocking_status = UNKNOWN_BLOCKED;
// Get cache pointer
DNSCacheData* dns_cache = getDNSCache(cacheID, true);

// Check if the returned pointer is valid before trying to access it
if(dns_cache == NULL)
continue;

// Reset blocking status
dns_cache->blocking_status = UNKNOWN_BLOCKED;
// Reset domainlist ID
dns_cache->domainlist_id = -1;
}
}

Expand Down Expand Up @@ -1039,10 +1047,10 @@ void _query_set_status(queriesData *query, const enum query_status new_status, c
if(!init)
{
counters->status[old_status]--;
log_debug(DEBUG_GC, "status %d removed (!init), ID = %d, new count = %d", QUERY_UNKNOWN, query->id, counters->status[QUERY_UNKNOWN]);
log_debug(DEBUG_STATUS, "status %d removed (!init), ID = %d, new count = %d", QUERY_UNKNOWN, query->id, counters->status[QUERY_UNKNOWN]);
}
counters->status[new_status]++;
log_debug(DEBUG_GC, "status %d set, ID = %d, new count = %d", new_status, query->id, counters->status[new_status]);
log_debug(DEBUG_STATUS, "status %d set, ID = %d, new count = %d", new_status, query->id, counters->status[new_status]);

// ... update overTime counters, ...
const int timeidx = getOverTimeID(query->timestamp);
Expand Down
1 change: 1 addition & 0 deletions src/datastructure.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ typedef struct {
int domainID;
int clientID;
int upstreamID;
int cacheID;
int id; // the ID is a (signed) int in dnsmasq, so no need for a long int here
int CNAME_domainID; // only valid if query has a CNAME blocking status
int ede;
Expand Down
25 changes: 17 additions & 8 deletions src/dnsmasq_interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name,
query->timestamp = querytimestamp;
query->type = querytype;
counters->querytype[querytype]++;
log_debug(DEBUG_GC, "query type %d set (new query), ID = %d, new count = %d", query->type, id, counters->querytype[query->type]);
log_debug(DEBUG_STATUS, "query type %d set (new query), ID = %d, new count = %d", query->type, id, counters->querytype[query->type]);
query->qtype = qtype;
query->id = id; // Has to be set before calling query_set_status()

Expand All @@ -764,7 +764,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name,
// Initialize reply type
query->reply = REPLY_UNKNOWN;
counters->reply[REPLY_UNKNOWN]++;
log_debug(DEBUG_GC, "reply type %d set (new query), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);
log_debug(DEBUG_STATUS, "reply type %d set (new query), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);
// Store DNSSEC result for this domain
query->dnssec = DNSSEC_UNKNOWN;
query->CNAME_domainID = -1;
Expand All @@ -783,6 +783,10 @@ bool _FTL_new_query(const unsigned int flags, const char *name,
// Query extended DNS error
query->ede = EDE_UNSET;

// Initialize cache ID, may be reusing an existing one if this
// (domain,client,type) tuple was already seen before
query->cacheID = findCacheID(domainID, clientID, querytype, true);

// This query is new and not yet known to the database
query->db = -1;

Expand Down Expand Up @@ -1295,7 +1299,12 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c
}

// Get cache pointer
unsigned int cacheID = findCacheID(domainID, clientID, query->type, true);
// When this function is called with a different domain than the one
// already stored in the query, we have to re-lookup the cache ID.
// This can happen when a CNAME chain is followed and analyzed
const int cacheID = query->domainID == domainID && query->clientID == clientID ?
query->cacheID :
findCacheID(domainID, clientID, query->type, true);
DNSCacheData *dns_cache = getDNSCache(cacheID, true);
if(dns_cache == NULL)
{
Expand Down Expand Up @@ -1588,7 +1597,7 @@ bool _FTL_CNAME(const char *dst, const char *src, const int id, const char* file
else if(query->status == QUERY_REGEX)
{
// Get parent and child DNS cache entries
const int parent_cacheID = findCacheID(parent_domainID, clientID, query->type, false);
const int parent_cacheID = query->cacheID;
const int child_cacheID = findCacheID(child_domainID, clientID, query->type, false);

// Get cache pointers
Expand Down Expand Up @@ -2733,12 +2742,12 @@ static void _query_set_reply(const unsigned int flags, const enum reply_type rep

// Subtract from old reply counter
counters->reply[query->reply]--;
log_debug(DEBUG_GC, "reply type %d removed (set_reply), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);
log_debug(DEBUG_STATUS, "reply type %d removed (set_reply), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);
// Add to new reply counter
counters->reply[new_reply]++;
// Store reply type
query->reply = new_reply;
log_debug(DEBUG_GC, "reply type %d added (set_reply), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);
log_debug(DEBUG_STATUS, "reply type %d added (set_reply), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);

// Save response time
// Skipped internally if already computed
Expand Down Expand Up @@ -3355,10 +3364,10 @@ void FTL_multiple_replies(const int id, int *firstID)

// Copy relevant information over
counters->reply[duplicated_query->reply]--;
log_debug(DEBUG_GC, "duplicated_query reply type %d removed, ID = %d, new count = %d", duplicated_query->reply, duplicated_query->id, counters->reply[duplicated_query->reply]);
log_debug(DEBUG_STATUS, "duplicated_query reply type %d removed, ID = %d, new count = %d", duplicated_query->reply, duplicated_query->id, counters->reply[duplicated_query->reply]);
duplicated_query->reply = source_query->reply;
counters->reply[duplicated_query->reply]++;
log_debug(DEBUG_GC, "duplicated_query reply type %d set, ID = %d, new count = %d", duplicated_query->reply, duplicated_query->id, counters->reply[duplicated_query->reply]);
log_debug(DEBUG_STATUS, "duplicated_query reply type %d set, ID = %d, new count = %d", duplicated_query->reply, duplicated_query->id, counters->reply[duplicated_query->reply]);

duplicated_query->dnssec = source_query->dnssec;
duplicated_query->flags.complete = true;
Expand Down
37 changes: 18 additions & 19 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ static void recycle(void)
{
bool *client_used = calloc(counters->clients, sizeof(bool));
bool *domain_used = calloc(counters->domains, sizeof(bool));
bool *upstreams_used = calloc(counters->upstreams, sizeof(bool));
if(client_used == NULL || domain_used == NULL || upstreams_used == NULL)
bool *cache_used = calloc(counters->dns_cache_size, sizeof(bool));
if(client_used == NULL || domain_used == NULL || cache_used == NULL)
{
log_err("Cannot allocate memory for recycling");
return;
Expand All @@ -70,13 +70,13 @@ static void recycle(void)
client_used[query->clientID] = true;
domain_used[query->domainID] = true;

// Mark upstream as used (if any)
if(query->upstreamID > -1)
upstreams_used[query->upstreamID] = true;

// Mark CNAME domain as used (if any)
if(query->CNAME_domainID >= 0)
if(query->CNAME_domainID > -1)
domain_used[query->CNAME_domainID] = true;

// Mark cache entry as used (if any)
if(query->cacheID > -1)
cache_used[query->cacheID] = true;
}

// Recycle clients
Expand Down Expand Up @@ -128,12 +128,11 @@ static void recycle(void)
unsigned int cache_recycled = 0;
for(int cacheID = 0; cacheID < counters->dns_cache_size; cacheID++)
{
DNSCacheData *cache = getDNSCache(cacheID, true);
if(cache == NULL)
if(cache_used[cacheID])
continue;

// Skip cache entries that are still in use
if(cache->magic != 0x00)
DNSCacheData *cache = getDNSCache(cacheID, true);
if(cache == NULL)
continue;

log_debug(DEBUG_GC, "Recycling cache entry with ID %d", cacheID);
Expand All @@ -147,7 +146,7 @@ static void recycle(void)
// Free memory
free(client_used);
free(domain_used);
free(upstreams_used);
free(cache_used);

// Scan number of recycled clients and domains if in debug mode
if(config.debug.gc.v.b)
Expand Down Expand Up @@ -181,10 +180,10 @@ static void recycle(void)
free_cache++;
}

log_debug(DEBUG_GC, "Recycler summary: %u/%d (max %d) clients, %u/%d (max %d) domains and %u/%d (max %d) cache records are free",
free_clients, counters->clients, counters->clients_MAX,
free_domains, counters->domains, counters->domains_MAX,
free_cache, counters->dns_cache_size, counters->dns_cache_MAX);
log_debug(DEBUG_GC, "%d/%d clients, %d/%d domains and %d/%d cache records are free",
counters->clients_MAX + (int)free_clients - counters->clients, counters->clients_MAX,
counters->domains_MAX + (int)free_domains - counters->domains_MAX, counters->domains_MAX,
counters->dns_cache_MAX + (int)free_cache - counters->dns_cache_MAX, counters->dns_cache_MAX);

log_debug(DEBUG_GC, "Recycled additional %u clients, %u domains, and %u cache records (scanned %d queries)",
clients_recycled, domains_recycled, cache_recycled, counters->queries);
Expand Down Expand Up @@ -378,17 +377,17 @@ void runGC(const time_t now, time_t *lastGCrun, const bool flush)

// Update reply counters
counters->reply[query->reply]--;
log_debug(DEBUG_GC, "reply type %d removed (GC), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);
log_debug(DEBUG_STATUS, "reply type %d removed (GC), ID = %d, new count = %d", query->reply, query->id, counters->reply[query->reply]);

// Update type counters
counters->querytype[query->type]--;
log_debug(DEBUG_GC, "query type %d removed (GC), ID = %d, new count = %d", query->type, query->id, counters->querytype[query->type]);
log_debug(DEBUG_STATUS, "query type %d removed (GC), ID = %d, new count = %d", query->type, query->id, counters->querytype[query->type]);

// Subtract UNKNOWN from the counters before
// setting the status if different.
// Minus one here and plus one below = net zero
counters->status[QUERY_UNKNOWN]--;
log_debug(DEBUG_GC, "status %d removed (GC), ID = %d, new count = %d", QUERY_UNKNOWN, query->id, counters->status[QUERY_UNKNOWN]);
log_debug(DEBUG_STATUS, "status %d removed (GC), ID = %d, new count = %d", QUERY_UNKNOWN, query->id, counters->status[QUERY_UNKNOWN]);

// Set query again to UNKNOWN to reset the counters
query_set_status(query, QUERY_UNKNOWN);
Expand Down
1 change: 1 addition & 0 deletions src/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ void log_counter_info(void)
log_info(" -> Unknown DNS queries: %i", counters->status[QUERY_UNKNOWN]);
log_info(" -> Unique domains: %i", counters->domains);
log_info(" -> Unique clients: %i", counters->clients);
log_info(" -> DNS cache records: %i", counters->dns_cache_size);
log_info(" -> Known forward destinations: %i", counters->upstreams);
}

Expand Down

0 comments on commit c24c1e1

Please sign in to comment.