From f2f356698048a4b1a551656104e84c941ecf995e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 25 Oct 2018 19:53:29 +0200 Subject: [PATCH 1/7] Move resolution of client and upstream host names to a dedicated thread. Fixes #404 Signed-off-by: DL6ER --- FTL.h | 1 + database.c | 4 -- dnsmasq_interface.c | 7 +++ gc.c | 5 -- request.c | 2 +- resolve.c | 121 +++++++++++++++++++++++++++++--------------- routines.h | 4 +- 7 files changed, 90 insertions(+), 54 deletions(-) diff --git a/FTL.h b/FTL.h index 979599eff..c7d76703a 100644 --- a/FTL.h +++ b/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 resolverthread; diff --git a/database.c b/database.c index 510ff22ab..80daf83bc 100644 --- a/database.c +++ b/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(); diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index f5c945d49..788085f56 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -907,6 +907,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( &resolverthread, &attr, resolver_thread, NULL ) != 0) + { + logg("Unable to open resolver 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) diff --git a/gc.c b/gc.c index 658d7096d..cfd715b8b 100644 --- a/gc.c +++ b/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 diff --git a/request.c b/request.c index b4657b556..7e229de5b 100644 --- a/request.c +++ b/request.c @@ -119,7 +119,7 @@ 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(); + resolveClients(false); // onlynew=false -> reresolve all client names logg("Done re-resolving host names"); } else if(command(client_message, ">recompile-regex")) diff --git a/resolve.c b/resolve.c index 140e03ad4..59365d1f9 100644 --- a/resolve.c +++ b/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,97 @@ 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++) + if(debug) logg("resolveClients(%s)", onlynew ? "true":"false"); + + int i; + for(i = 0; i < counters.clients; i++) { // Memory validation - validate_access("clients", clientID, true, __LINE__, __FUNCTION__, __FILE__); - - // Process this client only if it has at least one active query in the log - if(clients[clientID].count < 1) - continue; + validate_access("clients", i, true, __LINE__, __FUNCTION__, __FILE__); - // Get client hostname - char *hostname = resolveHostname(clients[clientID].ip); - if(strlen(hostname) > 0) + // 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) { - // 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; + 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(); } - free(hostname); } } -// 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 +static void resolveForwardDestinations(bool onlynew) { + if(debug) logg("resolveForwardDestinations(%s)", onlynew ? "true":"false"); + 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) + // 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) { - clients[i].name = resolveHostname(clients[i].ip); - clients[i].new = false; + 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++) +} + +pthread_t resolverthread; +void *resolver_thread(void *val) +{ + // Set thread name + prctl(PR_SET_NAME,"resolver",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); + // Prevent immediate re-run of this routine + sleepms(500); } + + // Idle for 0.5 sec before checking again the time criteria + sleepms(500); } + + return NULL; } diff --git a/routines.h b/routines.h index 00040e922..8d278fcd3 100644 --- a/routines.h +++ b/routines.h @@ -100,8 +100,8 @@ int main_dnsmasq(int argc, char **argv); void handle_signals(void); // resolve.c -void resolveNewClients(void); -void reresolveHostnames(void); +void *resolver_thread(void *val); +void resolveClients(bool onlynew); // regex.c bool match_regex(char *input); From f10b49cd6918eec1b8379f90c2b2473ca9f3821e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 25 Oct 2018 19:56:39 +0200 Subject: [PATCH 2/7] Remove obsolete debug output Signed-off-by: DL6ER --- resolve.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/resolve.c b/resolve.c index 59365d1f9..c22c6d5d0 100644 --- a/resolve.c +++ b/resolve.c @@ -73,8 +73,6 @@ char *resolveHostname(const char *addr) // Resolve client host names void resolveClients(bool onlynew) { - if(debug) logg("resolveClients(%s)", onlynew ? "true":"false"); - int i; for(i = 0; i < counters.clients; i++) { @@ -103,8 +101,6 @@ void resolveClients(bool onlynew) // Resolve upstream destination host names static void resolveForwardDestinations(bool onlynew) { - if(debug) logg("resolveForwardDestinations(%s)", onlynew ? "true":"false"); - int i; for(i = 0; i < counters.forwarded; i++) { From 513566328d1bb69a9ee0e612f18d7f2111e6b374 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 26 Oct 2018 20:17:37 +0200 Subject: [PATCH 3/7] Call to ">reresolve" also re-resolves host names of all known forward destinations Signed-off-by: DL6ER --- request.c | 4 +++- resolve.c | 2 +- routines.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/request.c b/request.c index 7e229de5b..d402559ae 100644 --- a/request.c +++ b/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(); - resolveClients(false); // onlynew=false -> reresolve all client names + // onlynew=false -> reresolve all host names + resolveClients(false); + resolveForwardDestinations(false); logg("Done re-resolving host names"); } else if(command(client_message, ">recompile-regex")) diff --git a/resolve.c b/resolve.c index c22c6d5d0..28ad6061a 100644 --- a/resolve.c +++ b/resolve.c @@ -99,7 +99,7 @@ void resolveClients(bool onlynew) } // Resolve upstream destination host names -static void resolveForwardDestinations(bool onlynew) +void resolveForwardDestinations(bool onlynew) { int i; for(i = 0; i < counters.forwarded; i++) diff --git a/routines.h b/routines.h index 8d278fcd3..3579dc99d 100644 --- a/routines.h +++ b/routines.h @@ -102,6 +102,7 @@ void handle_signals(void); // resolve.c void *resolver_thread(void *val); void resolveClients(bool onlynew); +void resolveForwardDestinations(bool onlynew); // regex.c bool match_regex(char *input); From 1a3a595e26bcd30dac06e86e10302bab085c58d0 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 27 Oct 2018 19:29:25 +0200 Subject: [PATCH 4/7] Use "DNS client" as custom thread name for the new name resolving thread Signed-off-by: DL6ER --- resolve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolve.c b/resolve.c index 28ad6061a..df016a03a 100644 --- a/resolve.c +++ b/resolve.c @@ -130,7 +130,7 @@ pthread_t resolverthread; void *resolver_thread(void *val) { // Set thread name - prctl(PR_SET_NAME,"resolver",0,0,0); + prctl(PR_SET_NAME, "DNS client", 0, 0, 0); while(!killed) { From fd53505d977502b410b93c2b29326b3b84563417 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 27 Oct 2018 20:54:45 +0200 Subject: [PATCH 5/7] Rename routines/variables to reflect "DNS client" Signed-off-by: DL6ER --- FTL.h | 2 +- dnsmasq_interface.c | 5 +++-- resolve.c | 3 +-- routines.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/FTL.h b/FTL.h index c7d76703a..7a8c977e7 100644 --- a/FTL.h +++ b/FTL.h @@ -279,4 +279,4 @@ extern pthread_t telnet_listenthreadv6; extern pthread_t socket_listenthread; extern pthread_t DBthread; extern pthread_t GCthread; -extern pthread_t resolverthread; +extern pthread_t DNSclientthread; diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 788085f56..a63257756 100644 --- a/dnsmasq_interface.c +++ b/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) { @@ -908,9 +909,9 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw) } // Start thread that will stay in the background until host names needs to be resolved - if(pthread_create( &resolverthread, &attr, resolver_thread, NULL ) != 0) + if(pthread_create( &DNSclientthread, &attr, DNSclient_thread, NULL ) != 0) { - logg("Unable to open resolver thread. Exiting..."); + logg("Unable to open DNS client thread. Exiting..."); exit(EXIT_FAILURE); } diff --git a/resolve.c b/resolve.c index df016a03a..89afc4ed2 100644 --- a/resolve.c +++ b/resolve.c @@ -126,8 +126,7 @@ void resolveForwardDestinations(bool onlynew) } } -pthread_t resolverthread; -void *resolver_thread(void *val) +void *DNSclient_thread(void *val) { // Set thread name prctl(PR_SET_NAME, "DNS client", 0, 0, 0); diff --git a/routines.h b/routines.h index 3579dc99d..a2336adfa 100644 --- a/routines.h +++ b/routines.h @@ -100,7 +100,7 @@ int main_dnsmasq(int argc, char **argv); void handle_signals(void); // resolve.c -void *resolver_thread(void *val); +void *DNSclient_thread(void *val); void resolveClients(bool onlynew); void resolveForwardDestinations(bool onlynew); From e6157da42e665718de9c63f332875a3dfe71f2b4 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 27 Oct 2018 21:06:06 +0200 Subject: [PATCH 6/7] Also re-resolve upstream host names every hour Signed-off-by: DL6ER --- resolve.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/resolve.c b/resolve.c index 89afc4ed2..eabebff1e 100644 --- a/resolve.c +++ b/resolve.c @@ -149,6 +149,8 @@ void *DNSclient_thread(void *val) { // 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); } From 9f860376b11f5c8bd25abc911bd81808cc0757de Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 27 Oct 2018 21:08:48 +0200 Subject: [PATCH 7/7] Change logic of onlynew variable: If onlynew is set and an item is NOT new, we skip this one. In all other cases, we try to resolve. Signed-off-by: DL6ER --- resolve.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/resolve.c b/resolve.c index eabebff1e..84a87155f 100644 --- a/resolve.c +++ b/resolve.c @@ -81,20 +81,20 @@ void resolveClients(bool onlynew) // 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) - { - char *hostname = resolveHostname(clients[i].ip); + if(onlynew && !clients[i].new) + continue; - enable_thread_lock(); + char *hostname = resolveHostname(clients[i].ip); - if(clients[i].name != NULL) - free(clients[i].name); + enable_thread_lock(); - clients[i].name = hostname; - clients[i].new = false; + if(clients[i].name != NULL) + free(clients[i].name); - disable_thread_lock(); - } + clients[i].name = hostname; + clients[i].new = false; + + disable_thread_lock(); } } @@ -109,20 +109,20 @@ void resolveForwardDestinations(bool onlynew) // 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) - { - char *hostname = resolveHostname(forwarded[i].ip); + if(onlynew && !forwarded[i].new) + continue; - enable_thread_lock(); + char *hostname = resolveHostname(forwarded[i].ip); - if(forwarded[i].name != NULL) - free(forwarded[i].name); + enable_thread_lock(); - forwarded[i].name = hostname; - forwarded[i].new = false; + if(forwarded[i].name != NULL) + free(forwarded[i].name); - disable_thread_lock(); - } + forwarded[i].name = hostname; + forwarded[i].new = false; + + disable_thread_lock(); } }