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);