Close unused ends of BFD pipe, and streamline pipe opening code

Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
acassen · Dec 31, 2017 · 8316ff9 · 8316ff9
1 parent c9d95cd
commit 8316ff9
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 45 deletions.
diff --git a/keepalived/bfd/bfd_daemon.c b/keepalived/bfd/bfd_daemon.c
@@ -46,6 +46,7 @@
 #include "utils.h"
 #include "scheduler.h"
 #include "process.h"
+#include "utils.h"
 
 /* Global variables */
 int bfd_event_pipe[2] = { -1, -1};
@@ -101,13 +102,10 @@ void
 open_bfd_pipe(void)
 {
 	/* Open BFD control pipe */
-	if (pipe(bfd_event_pipe) == -1) {
+	if (open_pipe(bfd_event_pipe) == -1) {
 		log_message(LOG_ERR, "Unable to create BFD event pipe: %m");
 		stop_keepalived();
-		return;
 	}
-	fcntl(bfd_event_pipe[0], F_SETFL, O_NONBLOCK | fcntl(bfd_event_pipe[0], F_GETFL));
-	fcntl(bfd_event_pipe[1], F_SETFL, O_NONBLOCK | fcntl(bfd_event_pipe[1], F_GETFL));
 }
 
 /* Daemon init sequence */
@@ -266,6 +264,9 @@ start_bfd_child(void)
 
 	prog_type = PROG_TYPE_BFD;
 
+	/* Close the read end of the event notification pipe */
+	close(bfd_event_pipe[0]);
+
 	if ((instance_name
 #if HAVE_DECL_CLONE_NEWNET
 			   || network_namespace

diff --git a/keepalived/core/main.c b/keepalived/core/main.c
@@ -385,7 +385,7 @@ static void
 start_keepalived(void)
 {
 #ifdef _WITH_BFD_
-	/* must be opened before vrrp and bfd */
+	/* must be opened before vrrp and bfd start */
 	open_bfd_pipe();
 #endif
 

diff --git a/keepalived/vrrp/vrrp_daemon.c b/keepalived/vrrp/vrrp_daemon.c
@@ -68,6 +68,9 @@
 #ifdef _WITH_JSON_
 #include "vrrp_json.h"
 #endif
+#ifdef _WITH_BFD_
+#include "bfd_daemon.h"
+#endif
 
 /* Global variables */
 bool non_existent_interface_specified;
@@ -564,6 +567,11 @@ start_vrrp_child(void)
 
 	prog_type = PROG_TYPE_VRRP;
 
+#ifdef _WITH_BFD_
+	/* Close the write end of the BFD event notification pipe */
+        close(bfd_event_pipe[1]);
+#endif
+
 	/* Opening local VRRP syslog channel */
 	if ((instance_name
 #if HAVE_DECL_CLONE_NEWNET

diff --git a/keepalived/vrrp/vrrp_dbus.c b/keepalived/vrrp/vrrp_dbus.c
@@ -54,6 +54,7 @@
 #include "global_data.h"
 #include "main.h"
 #include "logger.h"
+#include "utils.h"
 
 typedef enum dbus_action {
 	DBUS_ACTION_NONE,
@@ -859,37 +860,21 @@ dbus_start(void)
 	pthread_t dbus_thread;
 	sigset_t sigset, cursigset;
 
-#ifdef HAVE_PIPE2
-	if (pipe2(dbus_in_pipe, O_CLOEXEC)) {
+	if (open_pipe(dbus_in_pipe)) {
 		log_message(LOG_INFO, "Unable to create inbound dbus pipe - disabling DBus");
 		return false;
 	}
-	if (pipe2(dbus_out_pipe, O_CLOEXEC)) {
+	if (open_pipe(dbus_out_pipe)) {
 		log_message(LOG_INFO, "Unable to create outbound dbus pipe - disabling DBus");
 		close(dbus_in_pipe[0]);
 		close(dbus_in_pipe[1]);
 		return false;
 	}
-#else
-	if (pipe(dbus_in_pipe)) {
-		log_message(LOG_INFO, "Unable to create inbound dbus pipe - disabling DBus");
-		return false;
-	}
-	if (pipe(dbus_out_pipe)) {
-		log_message(LOG_INFO, "Unable to create outbound dbus pipe - disabling DBus");
-		close(dbus_in_pipe[0]);
-		close(dbus_in_pipe[1]);
-		return false;
-	}
-	fcntl (dbus_in_pipe[0], F_SETFD, FD_CLOEXEC | fcntl(dbus_in_pipe[0], F_GETFD));
-	fcntl (dbus_in_pipe[1], F_SETFD, FD_CLOEXEC | fcntl(dbus_in_pipe[1], F_GETFD));
-	fcntl (dbus_out_pipe[0], F_SETFD, FD_CLOEXEC | fcntl(dbus_out_pipe[0], F_GETFD));
-	fcntl (dbus_out_pipe[1], F_SETFD, FD_CLOEXEC | fcntl(dbus_out_pipe[1], F_GETFD));
-#endif
 
-	/* We don't want the main thread to block when using the pipes */
-	fcntl(dbus_in_pipe[0], F_SETFL, O_NONBLOCK | fcntl(dbus_in_pipe[0], F_GETFL));
-	fcntl(dbus_out_pipe[1], F_SETFL, O_NONBLOCK | fcntl(dbus_out_pipe[1], F_GETFL));
+	/* We don't want the main thread to block when using the pipes,
+	 * but the other side of the pipes should block. */
+	fcntl(dbus_in_pipe[1], F_SETFL, fcntl(dbus_in_pipe[1], F_GETFL) & ~O_NONBLOCK);
+	fcntl(dbus_out_pipe[0], F_SETFL, fcntl(dbus_out_pipe[0], F_GETFL) & ~O_NONBLOCK);
 
 	thread_add_read(master, handle_dbus_msg, NULL, dbus_in_pipe[0], TIMER_NEVER);
 

diff --git a/lib/signals.c b/lib/signals.c
@@ -343,25 +343,8 @@ open_signal_fd(void)
 	if (signal_fd == -1)
 		log_message(LOG_INFO, "BUG - signal_fd init failed - %d (%s), please report", errno, strerror(errno));
 #else
-	int n;
-
-#ifdef HAVE_PIPE2
-	n = pipe2(signal_pipe, O_CLOEXEC | O_NONBLOCK);
-#else
-	n = pipe(signal_pipe);
-#endif
-
-	assert(!n);
-	if (n)
+	if (open_pipe(signal_pipe))
 		log_message(LOG_INFO, "BUG - pipe in signal_handler_init failed - %d (%s), please report", errno, strerror(errno));
-
-#ifndef HAVE_PIPE2
-	fcntl(signal_pipe[0], F_SETFL, O_NONBLOCK | fcntl(signal_pipe[0], F_GETFL));
-	fcntl(signal_pipe[1], F_SETFL, O_NONBLOCK | fcntl(signal_pipe[1], F_GETFL));
-
-	fcntl(signal_pipe[0], F_SETFD, FD_CLOEXEC | fcntl(signal_pipe[0], F_GETFD));
-	fcntl(signal_pipe[1], F_SETFD, FD_CLOEXEC | fcntl(signal_pipe[1], F_GETFD));
-#endif
 #endif
 }
 

diff --git a/lib/utils.c b/lib/utils.c
@@ -625,3 +625,27 @@ fork_exec(char **argv)
 	return res;
 }
 #endif
+
+#if defined _WITH_VRRP_ || defined _WITH_BFD_
+int
+open_pipe(int pipe_arr[2])
+{
+	/* Open pipe */
+#ifdef HAVE_PIPE2
+	if (pipe2(pipe_arr, O_CLOEXEC | O_NONBLOCK) == -1)
+#else
+	if (pipe(pipe_arr) == -1)
+#endif
+		return -1;
+
+#ifndef HAVE_PIPE2
+	fcntl(pipe_arr[0], F_SETFL, O_NONBLOCK | fcntl(pipe_arr[0], F_GETFL));
+	fcntl(pipe_arr[1], F_SETFL, O_NONBLOCK | fcntl(pipe_arr[1], F_GETFL));
+
+	fcntl(pipe_arr[0], F_SETFD, FD_CLOEXEC | fcntl(pipe_arr[0], F_GETFD));
+	fcntl(pipe_arr[1], F_SETFD, FD_CLOEXEC | fcntl(pipe_arr[1], F_GETFD));
+#endif
+
+	return 0;
+}
+#endif
diff --git a/lib/utils.h b/lib/utils.h
@@ -131,5 +131,8 @@ extern void close_std_fd(void);
 #if !defined _HAVE_LIBIPTC_ || defined _LIBIPTC_DYNAMIC_
 extern int fork_exec(char **argv);
 #endif
+#if defined _WITH_VRRP_ || defined _WITH_BFD_
+extern int open_pipe(int [2]);
+#endif
 
 #endif