Merge pull request #405 from pi-hole/new/resolve_thread

Move resolution of client and upstream host names to a dedicated thread

FTL.h

@@ -279,3 +279,4 @@ extern pthread_t telnet_listenthreadv6;
 extern pthread_t socket_listenthread;
 extern pthread_t DBthread;
 extern pthread_t GCthread;
+extern pthread_t DNSclientthread;

database.c

@@ -548,10 +548,6 @@ void *DB_thread(void *val)
  // Update lastDBsave timer
  lastDBsave = time(NULL) - time(NULL)%config.DBinterval;
 
- // This has to be run outside of the thread locks
- // to prevent locking the resolver
- resolveNewClients();
-
  // Lock FTL's data structure, since it is
  // likely that it will be changed here
  enable_thread_lock();

dnsmasq_interface.c

@@ -853,6 +853,7 @@ pthread_t telnet_listenthreadv6;
 pthread_t socket_listenthread;
 pthread_t DBthread;
 pthread_t GCthread;
+pthread_t DNSclientthread;
 
 void FTL_fork_and_bind_sockets(struct passwd *ent_pw)
 {
@@ -907,6 +908,13 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw)
  exit(EXIT_FAILURE);
  }
 
+ // Start thread that will stay in the background until host names needs to be resolved
+ if(pthread_create( &DNSclientthread, &attr, DNSclient_thread, NULL ) != 0)
+ {
+ logg("Unable to open DNS client thread. Exiting...");
+ exit(EXIT_FAILURE);
+ }
+
  // Chown files if FTL started as user root but a dnsmasq config option
  // states to run as a different user/group (e.g. "nobody")
  if(ent_pw != NULL && getuid() == 0)

gc.c

@@ -168,11 +168,6 @@ void *GC_thread(void *val)
  // Release thread lock
  disable_thread_lock();
 
- // Reresolve client hostnames to account for changes
- // Have to this outside of the thread lock
- // to prevent locking of the resolver
- reresolveHostnames();
-
  // After storing data in the database for the next time,
  // we should scan for old entries, which will then be deleted
  // to free up pages in the database and prevent it from growing

request.c

@@ -119,7 +119,9 @@ void process_request(char *client_message, int *sock)
  // Need to release the thread lock already here to allow
  // the resolver to process the incoming PTR requests
  disable_thread_lock();
- reresolveHostnames();
+ // onlynew=false -> reresolve all host names
+ resolveClients(false);
+ resolveForwardDestinations(false);
  logg("Done re-resolving host names");
  }
  else if(command(client_message, ">recompile-regex"))

resolve.c

@@ -10,6 +10,14 @@
 
 #include "FTL.h"
 
+// Resolve new client and upstream server host names
+// once every minute
+#define RESOLVE_INTERVAL 60
+
+// Re-resolve client names
+// once every hour
+#define RERESOLVE_INTERVAL 3600
+
 char *resolveHostname(const char *addr)
 {
  // Get host name
@@ -62,68 +70,94 @@ char *resolveHostname(const char *addr)
  return hostname;
 }
 
-// This routine is run *after* garbage cleaning (default interval is once per hour)
-// to account for possibly updated hostnames
-void reresolveHostnames(void)
+// Resolve client host names
+void resolveClients(bool onlynew)
 {
- int clientID;
- for(clientID = 0; clientID < counters.clients; clientID++)
+ int i;
+ for(i = 0; i < counters.clients; i++)
  {
  // Memory validation
- validate_access("clients", clientID, true, __LINE__, __FUNCTION__, __FILE__);
+ validate_access("clients", i, true, __LINE__, __FUNCTION__, __FILE__);
 
- // Process this client only if it has at least one active query in the log
- if(clients[clientID].count < 1)
+ // If onlynew flag is set, we will only resolve new clients
+ // If not, we will try to re-resolve all known clients
+ if(onlynew && !clients[i].new)
  continue;
 
- // Get client hostname
- char *hostname = resolveHostname(clients[clientID].ip);
- if(strlen(hostname) > 0)
- {
- // Delete possibly already existing hostname pointer before storing new data
- if(clients[clientID].name != NULL)
- {
- free(clients[clientID].name);
- clients[clientID].name = NULL;
- }
-
- // Store client hostname
- clients[clientID].name = strdup(hostname);
- clients[clientID].new = false;
- }
- free(hostname);
+ char *hostname = resolveHostname(clients[i].ip);
+
+ enable_thread_lock();
+
+ if(clients[i].name != NULL)
+ free(clients[i].name);
+
+ clients[i].name = hostname;
+ clients[i].new = false;
+
+ disable_thread_lock();
  }
 }
 
-// This routine is run *before* saving to the database (default interval is once per minute)
-// to account for new clients (and forward destinations)
-void resolveNewClients(void)
+// Resolve upstream destination host names
+void resolveForwardDestinations(bool onlynew)
 {
  int i;
- for(i = 0; i < counters.clients; i++)
+ for(i = 0; i < counters.forwarded; i++)
  {
  // Memory validation
- validate_access("clients", i, true, __LINE__, __FUNCTION__, __FILE__);
+ validate_access("forwarded", i, true, __LINE__, __FUNCTION__, __FILE__);
 
- // Only try to resolve new clients
- // Note that it can happen that we are not able to find hostnames but we don't
- // want to try to resolve them every minute in this case.
- if(clients[i].new)
- {
- clients[i].name = resolveHostname(clients[i].ip);
- clients[i].new = false;
- }
+ // If onlynew flag is set, we will only resolve new upstream destinations
+ // If not, we will try to re-resolve all known upstream destinations
+ if(onlynew && !forwarded[i].new)
+ continue;
+
+ char *hostname = resolveHostname(forwarded[i].ip);
+
+ enable_thread_lock();
+
+ if(forwarded[i].name != NULL)
+ free(forwarded[i].name);
+
+ forwarded[i].name = hostname;
+ forwarded[i].new = false;
+
+ disable_thread_lock();
  }
- for(i = 0; i < counters.forwarded; i++)
+}
+
+void *DNSclient_thread(void *val)
+{
+ // Set thread name
+ prctl(PR_SET_NAME, "DNS client", 0, 0, 0);
+
+ while(!killed)
  {
- // Memory validation
- validate_access("forwarded", i, true, __LINE__, __FUNCTION__, __FILE__);
+ // Run every minute to resolve only new clients and upstream servers
+ if(time(NULL) % RESOLVE_INTERVAL == 0)
+ {
+ // Try to resolve new client host names (onlynew=true)
+ resolveClients(true);
+ // Try to resolve new upstream destination host names (onlynew=true)
+ resolveForwardDestinations(true);
+ // Prevent immediate re-run of this routine
+ sleepms(500);
+ }
 
- // Only try to resolve new forward destinations
- if(forwarded[i].new)
+ // Run every hour to update possibly changed client host names
+ if(time(NULL) % RERESOLVE_INTERVAL == 0)
  {
- forwarded[i].name = resolveHostname(forwarded[i].ip);
- forwarded[i].new = false;
+ // Try to resolve all client host names (onlynew=false)
+ resolveClients(false);
+ // Try to resolve all upstream destination host names (onlynew=false)
+ resolveForwardDestinations(false);
+ // Prevent immediate re-run of this routine
+ sleepms(500);
  }
+
+ // Idle for 0.5 sec before checking again the time criteria
+ sleepms(500);
  }
+
+ return NULL;
 }

routines.h

@@ -100,8 +100,9 @@ int main_dnsmasq(int argc, char **argv);
 void handle_signals(void);
 
 // resolve.c
-void resolveNewClients(void);
-void reresolveHostnames(void);
+void *DNSclient_thread(void *val);
+void resolveClients(bool onlynew);
+void resolveForwardDestinations(bool onlynew);
 
 // regex.c
 bool match_regex(char *input);