Stop polling cmd_fd while busy

We have an explicit select() call on the cmd_fd even when we're busy
shovelling packets and never hit the main select() in the mainloop.
This is *just* to ensure that we react to a cancel command quickly.

In the *common* case that we're running in openconnect(8), there's no
need for that since the *only* thing that will write to the cmd_fd is
openconnect itself, and *that* can set a flag in memory to tell us to
look.

So implement that optimisation — don't check it each time around the
mainloop unless the vpninfo->need_poll_cmd_fd flag is set. That flag
is set whenever we have written to cmd_fd and there's something to be
read. And cleared by poll_cmd_fd() when it runs and finds nothing there.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

library.c

@@ -1046,6 +1046,7 @@ OPENCONNECT_CMD_SOCKET openconnect_setup_cmd_pipe(struct openconnect_info *vpnin
 	}
 	vpninfo->cmd_fd = pipefd[0];
 	vpninfo->cmd_fd_write = pipefd[1];
+	vpninfo->need_poll_cmd_fd = 1;
 	return vpninfo->cmd_fd_write;
 }
 

main.c

@@ -109,6 +109,7 @@ static int authgroup_set;
 static int last_form_empty;
 
 static int sig_cmd_fd;
+static struct openconnect_info *sig_vpninfo;
 
 static void add_form_field(char *field);
 
@@ -799,6 +800,8 @@ static void handle_signal(int sig)
 	if (write(sig_cmd_fd, &cmd, 1) < 0) {
 	/* suppress warn_unused_result */
 	}
+	if (sig_vpninfo)
+		sig_vpninfo->need_poll_cmd_fd = 1;
 }
 #else /* _WIN32 */
 static const char *default_vpncscript;
@@ -1520,6 +1523,7 @@ static int background_self(struct openconnect_info *vpninfo, char *pidfile) {
 		if (!fp) {
 			fprintf(stderr, _("Failed to open '%s' for write: %s\n"),
 				pidfile, strerror(errno));
+			sig_vpninfo = NULL;
 			openconnect_vpninfo_free(vpninfo);
 			exit(1);
 		}
@@ -1536,6 +1540,7 @@ static int background_self(struct openconnect_info *vpninfo, char *pidfile) {
 		vpn_progress(vpninfo, PRG_INFO,
 			     _("Continuing in background; pid %d\n"),
 			     pid);
+		sig_vpninfo = NULL;
 		openconnect_vpninfo_free(vpninfo);
 		exit(0);
 	}
@@ -2084,11 +2089,13 @@ int main(int argc, char **argv)
 	sigaction(SIGUSR2, &sa, NULL);
 #endif /* !_WIN32 */
 
+	sig_vpninfo = vpninfo;
 	sig_cmd_fd = openconnect_setup_cmd_pipe(vpninfo);
 	if (sig_cmd_fd < 0) {
 		fprintf(stderr, _("Error opening cmd pipe\n"));
 		exit(1);
 	}
+	vpninfo->cmd_fd_internal = 1;
 
 	if (vpninfo->certinfo[0].key && do_passphrase_from_fsid)
 		openconnect_passphrase_from_fsid(vpninfo);
@@ -2142,12 +2149,14 @@ int main(int argc, char **argv)
 			printf("RESOLVE='%s:%.*s'\n", vpninfo->hostname, l, p);
 		} else
 			printf("RESOLVE=");
+		sig_vpninfo = NULL;
 		openconnect_vpninfo_free(vpninfo);
 		exit(0);
 	} else if (cookieonly) {
 		printf("%s\n", vpninfo->cookie);
 		if (cookieonly == 1) {
 			/* We use cookieonly=2 for 'print it and continue' */
+			sig_vpninfo = NULL;
 			openconnect_vpninfo_free(vpninfo);
 			exit(0);
 		}
@@ -2240,6 +2249,7 @@ int main(int argc, char **argv)
 		break;
 	}
 
+	sig_vpninfo = NULL;
 	openconnect_vpninfo_free(vpninfo);
 	exit(ret);
 }

mainloop.c

@@ -238,7 +238,9 @@ int openconnect_mainloop(struct openconnect_info *vpninfo,
 		if (vpninfo->quit_reason)
 			break;
 
-		poll_cmd_fd(vpninfo, 0);
+		if (vpninfo->need_poll_cmd_fd)
+			poll_cmd_fd(vpninfo, 0);
+
 		if (vpninfo->got_cancel_cmd) {
 			if (vpninfo->delay_close != NO_DELAY_CLOSE) {
 				if (vpninfo->delay_close == DELAY_CLOSE_IMMEDIATE_CALLBACK) {

openconnect-internal.h

@@ -730,6 +730,12 @@ struct openconnect_info {
 	int dtls_pass_tos;
 	int dtls_tos_proto, dtls_tos_optname;
 
+	/* An optimisation for the case where our own code is the only
+	 * thing that *could* write to the cmd_fd, to avoid constantly
+	 * polling on it while we're busy shovelling packets. */
+	int need_poll_cmd_fd;
+	int cmd_fd_internal;
+
 	int cmd_fd;
 	int cmd_fd_write;
 	int got_cancel_cmd;

ssl.c

@@ -860,14 +860,20 @@ void poll_cmd_fd(struct openconnect_info *vpninfo, int timeout)
 		tv.tv_sec = now >= expiration ? 0 : expiration - now;
 		tv.tv_usec = 0;
 
+		/* If the cmd_fd is internal and we've been told to poll it,
+		 * don't *keep* doing so afterwards. */
+		vpninfo->need_poll_cmd_fd = !vpninfo->cmd_fd_internal;
+
 		FD_ZERO(&rd_set);
 		cmd_fd_set(vpninfo, &rd_set, &maxfd);
 		if (select(maxfd + 1, &rd_set, NULL, NULL, &tv) < 0 &&
 		    errno != EINTR) {
 			vpn_perror(vpninfo, _("Failed select() for command socket"));
 		}
-
-		check_cmd_fd(vpninfo, &rd_set);
+		if (FD_ISSET(vpninfo->cmd_fd, &rd_set)) {
+			vpninfo->need_poll_cmd_fd = 1; /* Until it's *empty */
+			check_cmd_fd(vpninfo, &rd_set);
+		}
 	}
 }