llmnrd: merge rtnl interface event loop into main select() loops

Instead of spawming a thread for the sole purpose of watching the rtnl
for (presumably) seldom events, just merge the select() for all sockets
together in one single main loop.

This reduces unnecessary complexity and makes llmnrd no longer require
any locking.

It also allows us to handle signals in a thread-safe manner (as there
aren't any ;) and thus the race condition on exit reported in #20

Closes #20
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

6 files changed, 116 insertions, 178 deletions

diff --git a/Makefile b/Makefile
index 0de1919..0f53165 100644
--- a/Makefile
+++ b/Makefile
@@ -7,7 +7,7 @@ VERSION = 0.2.1
 # llmnrd binary
 D_P 	= llmnrd
 D_OBJS	= llmnr.o iface.o socket.o util.o llmnrd.o
-D_LIBS	= -lpthread
+D_LIBS	=
 
 # llmnr-query binary
 Q_P 	= llmnr-query
diff --git a/iface.c b/iface.c
index 333a142..83dc377 100644
--- a/iface.c
+++ b/iface.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2015-2017 Tobias Klauser <tklauser@distanz.ch>
  *
  * This file is part of llmnrd.
  *
@@ -19,7 +19,6 @@
 #include <assert.h>
 #include <errno.h>
 #include <netdb.h>
-#include <pthread.h>
 #include <stdint.h>
 #include <string.h>
 #include <unistd.h>
@@ -40,10 +39,7 @@
 
 #include "iface.h"
 
-static bool iface_running = true;
-static bool iface_ipv6 = false;
 static unsigned int iface_ifindex = 0;
-static pthread_t iface_thread;
 static iface_event_handler_t iface_event_handler;
 
 struct iface_record {
@@ -54,7 +50,6 @@ struct iface_record {
 };
 
 static struct list_head iface_list_head;
-static pthread_mutex_t iface_list_mutex;
 
 size_t iface_addr_lookup(unsigned int ifindex, unsigned char family,
 			 struct sockaddr_storage *addrs, size_t addrs_size)
@@ -65,8 +60,6 @@ size_t iface_addr_lookup(unsigned int ifindex, unsigned char family,
 	if (!addrs)
 		return 0;
 
-	pthread_mutex_lock(&iface_list_mutex);
-
 	list_for_each_entry(rec, &iface_list_head, list) {
 		if (rec->index == ifindex) {
 			size_t i;
@@ -81,8 +74,6 @@ size_t iface_addr_lookup(unsigned int ifindex, unsigned char family,
 		}
 	}
 
-	pthread_mutex_unlock(&iface_list_mutex);
-
 	return n;
 }
 
@@ -182,8 +173,6 @@ static void iface_addr_add(unsigned int index, unsigned char family, const void
 
 	fill_sockaddr_storage(&sst, family, addr);
 
-	pthread_mutex_lock(&iface_list_mutex);
-
 	list_for_each_entry(rec, &iface_list_head, list)
 		if (rec->index == index)
 			goto add;
@@ -195,7 +184,6 @@ static void iface_addr_add(unsigned int index, unsigned char family, const void
 	list_add_tail(&rec->list, &iface_list_head);
 add:
 	iface_record_addr_add(rec, &sst);
-	pthread_mutex_unlock(&iface_list_mutex);
 }
 
 static void iface_addr_del(unsigned int index, unsigned char family, const void *addr)
@@ -205,16 +193,12 @@ static void iface_addr_del(unsigned int index, unsigned char family, const void
 
 	fill_sockaddr_storage(&sst, family, addr);
 
-	pthread_mutex_lock(&iface_list_mutex);
-
 	list_for_each_entry(rec, &iface_list_head, list) {
 		if (rec->index == index) {
 			iface_record_addr_del(rec, &sst);
 			break;
 		}
 	}
-
-	pthread_mutex_unlock(&iface_list_mutex);
 }
 
 static void iface_nlmsg_change_link(const struct nlmsghdr *nlh __unused)
@@ -342,75 +326,33 @@ static int iface_rtnl_enumerate(int sock, uint16_t type, unsigned char family)
 	return iface_nlmsg_process((const struct nlmsghdr *)pktbuf, recvlen);
 }
 
-void iface_register_event_handler(iface_event_handler_t event_handler)
+void iface_init(int sock, const char *iface, bool ipv6,
+		iface_event_handler_t event_handler)
 {
-	iface_event_handler = event_handler;
-}
-
-int iface_run(void)
-{
-	int ret = -1;
-	int sock;
-
 	INIT_LIST_HEAD(&iface_list_head);
-	if (pthread_mutex_init(&iface_list_mutex, NULL) != 0) {
-		log_err("Failed to initialize interface list mutex\n");
-		return -1;
-	}
-
-	sock = socket_open_rtnl(iface_ipv6);
-	if (sock < 0)
-		return -1;
+	iface_event_handler = event_handler;
+	if (iface)
+		iface_ifindex = if_nametoindex(iface);
 
 	/* send RTM_GETADDR request to initially populate the interface list */
 	if (iface_rtnl_enumerate(sock, RTM_GETADDR, AF_INET) < 0)
-		goto out;
-	if (iface_ipv6) {
+		log_err("Failed to enumerate rtnl interfaces: %s\n", strerror(errno));
+	if (ipv6)
 		if (iface_rtnl_enumerate(sock, RTM_GETADDR, AF_INET6) < 0)
-			goto out;
-	}
-
-	while (iface_running) {
-		ssize_t recvlen;
-		uint8_t pktbuf[8192];
-
-		if ((recvlen = recv(sock, pktbuf, sizeof(pktbuf), 0)) < 0) {
-			if (errno != EINTR)
-				log_err("Failed to receive netlink message: %s\n", strerror(errno));
-			goto out;
-		}
-
-		if (iface_nlmsg_process((const struct nlmsghdr *)pktbuf, recvlen) < 0)
-			log_warn("Error processing netlink message\n");
-	}
-
-	pthread_mutex_destroy(&iface_list_mutex);
-	ret = 0;
-out:
-	close(sock);
-	return ret;
-}
-
-static void* iface_run_wrapper(void *data __unused)
-{
-	return ERR_PTR(iface_run());
+			log_err("Failed to enumerate rtnl interfaces: %s\n", strerror(errno));
 }
 
-int iface_start_thread(bool ipv6, const char *iface)
+void iface_recv(int sock)
 {
-	iface_ipv6 = ipv6;
-	if (iface)
-		iface_ifindex = if_nametoindex(iface);
+	ssize_t recvlen;
+	uint8_t pktbuf[8192];
 
-	if (pthread_create(&iface_thread, NULL, iface_run_wrapper, NULL) < 0) {
-		log_err("Failed to start interface monitoring thread\n");
-		return -1;
+	if ((recvlen = recv(sock, pktbuf, sizeof(pktbuf), 0)) < 0) {
+		if (errno != EINTR)
+			log_err("Failed to receive netlink message: %s\n", strerror(errno));
+		return;
 	}
 
-	return 0;
-}
-
-void iface_stop(void)
-{
-	iface_running = false;
+	if (iface_nlmsg_process((const struct nlmsghdr *)pktbuf, recvlen) < 0)
+		log_warn("Error processing netlink message\n");
 }
diff --git a/iface.h b/iface.h
index 11c49e7..2203e92 100644
--- a/iface.h
+++ b/iface.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2015-2017 Tobias Klauser <tklauser@distanz.ch>
  *
  * This file is part of llmnrd.
  *
@@ -30,9 +30,9 @@ enum iface_event_type {
 typedef void (*iface_event_handler_t)(enum iface_event_type, unsigned char af,
 				      unsigned int ifindex);
 
-void iface_register_event_handler(iface_event_handler_t event_handler);
-int iface_start_thread(bool ipv6, const char *iface);
-void iface_stop(void);
+void iface_init(int sock, const char *iface, bool ipv6,
+		iface_event_handler_t event_handler);
+void iface_recv(int sock);
 
 size_t iface_addr_lookup(unsigned int ifindex, unsigned char family,
 			 struct sockaddr_storage *addrs, size_t addrs_size);
diff --git a/llmnr.c b/llmnr.c
index 17926a7..73f5168 100644
--- a/llmnr.c
+++ b/llmnr.c
@@ -37,52 +37,17 @@
 #include "llmnr-packet.h"
 #include "llmnr.h"
 
-static int llmnr_sock_ipv4 = -1;
-static int llmnr_sock_ipv6 = -1;
-static bool llmnr_running = true;
-/*
- * Host name in DNS name format (length octet + name + 0 byte)
- */
+static bool llmnr_ipv6 = false;
+/* Host name in DNS name format (length octet + name + 0 byte) */
 static char llmnr_hostname[LLMNR_LABEL_MAX_SIZE + 2];
 
-static void llmnr_iface_event_handle(enum iface_event_type type, unsigned char af,
-				     unsigned int ifindex)
-{
-	switch (af) {
-	case AF_INET:
-		socket_mcast_group_ipv4(llmnr_sock_ipv4, ifindex, type == IFACE_ADD);
-		break;
-	case AF_INET6:
-		socket_mcast_group_ipv6(llmnr_sock_ipv6, ifindex, type == IFACE_ADD);
-		break;
-	default:
-		/* ignore */
-		break;
-	}
-}
-
-int llmnr_init(const char *hostname, uint16_t port, bool ipv6, const char *iface)
+void llmnr_init(const char *hostname, bool ipv6)
 {
 	llmnr_hostname[0] = strlen(hostname);
 	strncpy(&llmnr_hostname[1], hostname, LLMNR_LABEL_MAX_SIZE);
 	llmnr_hostname[LLMNR_LABEL_MAX_SIZE + 1] = '\0';
-	log_info("Starting llmnrd on port %u, hostname %s\n", port, hostname);
-	if (iface)
-		log_info("Binding to interface %s\n", iface);
-
-	llmnr_sock_ipv4 = socket_open_ipv4(port, iface);
-	if (llmnr_sock_ipv4 < 0)
-		return -1;
-
-	if (ipv6) {
-		llmnr_sock_ipv6 = socket_open_ipv6(port, iface);
-		if (llmnr_sock_ipv6 < 0)
-			return -1;
-	}
-
-	iface_register_event_handler(&llmnr_iface_event_handle);
 
-	return 0;
+	llmnr_ipv6 = ipv6;
 }
 
 static bool llmnr_name_matches(const uint8_t *query)
@@ -134,7 +99,7 @@ static void llmnr_respond(unsigned int ifindex, const struct llmnr_hdr *hdr,
 		return;
 
 	/* No AAAA responses if IPv6 is disabled */
-	if (llmnr_sock_ipv6 < 0 && qtype == LLMNR_QTYPE_AAAA)
+	if (llmnr_ipv6 < 0 && qtype == LLMNR_QTYPE_AAAA)
 		return;
 
 	switch (qtype) {
@@ -254,7 +219,7 @@ static void llmnr_packet_process(unsigned int ifindex, const uint8_t *pktbuf, si
 		llmnr_respond(ifindex, hdr, query, query_len, sock, sst);
 }
 
-static void llmnr_recv(int sock)
+void llmnr_recv(int sock)
 {
 	uint8_t pktbuf[2048], aux[128];
 	struct msghdr msg;
@@ -297,45 +262,3 @@ static void llmnr_recv(int sock)
 	else
 		log_warn("Could not get interface of incoming packet\n");
 }
-
-int llmnr_run(void)
-{
-	int ret = -1;
-
-	while (llmnr_running) {
-		fd_set rfds;
-		int nfds, ret;
-
-		FD_ZERO(&rfds);
-		FD_SET(llmnr_sock_ipv4, &rfds);
-		if (llmnr_sock_ipv6 >= 0) {
-			FD_SET(llmnr_sock_ipv6, &rfds);
-			nfds = max(llmnr_sock_ipv4, llmnr_sock_ipv6) + 1;
-		} else
-			nfds = llmnr_sock_ipv4 + 1;
-
-		ret = select(nfds, &rfds, NULL, NULL, NULL);
-		if (ret < 0) {
-			if (errno != EINTR)
-				log_err("Failed to select() on socket: %s\n", strerror(errno));
-			goto out;
-		} else if (ret) {
-			if (FD_ISSET(llmnr_sock_ipv4, &rfds))
-				llmnr_recv(llmnr_sock_ipv4);
-			if (llmnr_sock_ipv6 >= 0 && FD_ISSET(llmnr_sock_ipv6, &rfds))
-				llmnr_recv(llmnr_sock_ipv6);
-		}
-	}
-
-	ret = 0;
-out:
-	close(llmnr_sock_ipv4);
-	if (llmnr_sock_ipv6 >= 0)
-		close(llmnr_sock_ipv6);
-	return ret;
-}
-
-void llmnr_stop(void)
-{
-	llmnr_running = false;
-}
diff --git a/llmnr.h b/llmnr.h
index 620e539..892ba24 100644
--- a/llmnr.h
+++ b/llmnr.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2015-2017 Tobias Klauser <tklauser@distanz.ch>
  *
  * This file is part of llmnrd.
  *
@@ -20,10 +20,8 @@
 #define LLMNR_H
 
 #include <stdbool.h>
-#include <stdint.h>
 
-int llmnr_init(const char *hostname, uint16_t port, bool ipv6, const char *iface);
-int llmnr_run(void);
-void llmnr_stop(void);
+void llmnr_init(const char *hostname, bool ipv6);
+void llmnr_recv(int sock);
 
 #endif /* LLMNR_H */
diff --git a/llmnrd.c b/llmnrd.c
index 2bc4a20..66a9f04 100644
--- a/llmnrd.c
+++ b/llmnrd.c
@@ -1,7 +1,7 @@
 /*
  * llmnrd -- LLMNR (RFC 4705) responder daemon.
  *
- * Copyright (C) 2014-2016 Tobias Klauser <tklauser@distanz.ch>
+ * Copyright (C) 2014-2017 Tobias Klauser <tklauser@distanz.ch>
  *
  * This file is part of llmnrd.
  *
@@ -38,6 +38,11 @@
 #include "iface.h"
 #include "llmnr.h"
 #include "llmnr-packet.h"
+#include "socket.h"
+
+static bool llmnrd_running = true;
+static int llmnrd_sock_ipv4 = -1;
+static int llmnrd_sock_ipv6 = -1;
 
 static const char *short_opts = "H:i:p:6dhV";
 static const struct option long_opts[] = {
@@ -82,8 +87,7 @@ static void signal_handler(int sig)
 	case SIGQUIT:
 	case SIGTERM:
 		log_info("Interrupt received. Stopping llmnrd.\n");
-		iface_stop();
-		llmnr_stop();
+		llmnrd_running = false;
 		break;
 	case SIGHUP:
 	default:
@@ -109,14 +113,32 @@ static void register_signal(int sig, void (*handler)(int))
 	}
 }
 
+static void iface_event_handle(enum iface_event_type type, unsigned char af,
+			       unsigned int ifindex)
+{
+	switch (af) {
+	case AF_INET:
+		socket_mcast_group_ipv4(llmnrd_sock_ipv4, ifindex, type == IFACE_ADD);
+		break;
+	case AF_INET6:
+		socket_mcast_group_ipv6(llmnrd_sock_ipv6, ifindex, type == IFACE_ADD);
+		break;
+	default:
+		/* ignore */
+		break;
+	}
+}
+
 int main(int argc, char **argv)
 {
-	int c, ret = EXIT_FAILURE;
+	int c, ret = -1;
 	long num_arg;
 	bool daemonize = false, ipv6 = false;
 	char *hostname = NULL;
 	char *iface = NULL;
 	uint16_t port = LLMNR_UDP_PORT;
+	int llmnrd_sock_rtnl = -1;
+	int nfds;
 
 	while ((c = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) {
 		switch (c) {
@@ -166,18 +188,71 @@ int main(int argc, char **argv)
 	if (daemonize) {
 		if (daemon(0, 0) != 0) {
 			log_err("Failed to daemonize process: %s\n", strerror(errno));
-			return EXIT_FAILURE;
+			goto out;
 		}
 	}
 
-	if (llmnr_init(hostname, port, ipv6, iface) < 0)
+	log_info("Starting llmnrd on port %u, hostname %s\n", port, hostname);
+	if (iface)
+		log_info("Binding to interface %s\n", iface);
+
+	llmnrd_sock_ipv4 = socket_open_ipv4(port, iface);
+	if (llmnrd_sock_ipv4 < 0)
 		goto out;
 
-	if (iface_start_thread(ipv6, iface) < 0)
+	if (ipv6) {
+		llmnrd_sock_ipv6 = socket_open_ipv6(port, iface);
+		if (llmnrd_sock_ipv6 < 0)
+			goto out;
+	}
+
+	llmnrd_sock_rtnl = socket_open_rtnl(ipv6);
+	if (llmnrd_sock_rtnl < 0)
 		goto out;
 
-	ret = llmnr_run();
+	llmnr_init(hostname, ipv6);
+	iface_init(llmnrd_sock_rtnl, iface, ipv6, &iface_event_handle);
+
+	nfds = max(llmnrd_sock_ipv4, llmnrd_sock_rtnl);
+	if (llmnrd_sock_ipv6 >= 0)
+		nfds = max(nfds, llmnrd_sock_ipv6);
+	nfds += 1;
+
+	while (llmnrd_running) {
+		fd_set rfds;
+
+		FD_ZERO(&rfds);
+		FD_SET(llmnrd_sock_ipv4, &rfds);
+		FD_SET(llmnrd_sock_rtnl, &rfds);
+		if (llmnrd_sock_ipv6 >= 0)
+			FD_SET(llmnrd_sock_ipv6, &rfds);
+
+		ret = select(nfds, &rfds, NULL, NULL, NULL);
+		if (ret < 0) {
+			if (errno != EINTR)
+				log_err("Failed to select() on socket: %s\n", strerror(errno));
+			goto out;
+		} else if (ret) {
+			/* handle RTNL messages first so we can respond with
+			 * up-to-date information.
+			 */
+			if (FD_ISSET(llmnrd_sock_rtnl, &rfds))
+				iface_recv(llmnrd_sock_rtnl);
+			if (FD_ISSET(llmnrd_sock_ipv4, &rfds))
+				llmnr_recv(llmnrd_sock_ipv4);
+			if (llmnrd_sock_ipv6 >= 0 && FD_ISSET(llmnrd_sock_ipv6, &rfds))
+				llmnr_recv(llmnrd_sock_ipv6);
+		}
+	}
+
+	ret = 0;
 out:
+	if (llmnrd_sock_rtnl >= 0)
+		close(llmnrd_sock_rtnl);
+	if (llmnrd_sock_ipv6 >= 0)
+		close(llmnrd_sock_ipv6);
+	if (llmnrd_sock_ipv4 >= 0)
+		close(llmnrd_sock_ipv4);
 	free(hostname);
-	return ret;
+	return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }