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>

diff --git a/library.c b/library.c
index 13cb6e68e22793ed9cbd2c38260acb3defd87f49..29d4abd665d966a1ff8ca5860f7680b6d031c051 100644 (file)
--- a/library.c
+++ b/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;
 }
 

diff --git a/main.c b/main.c
index 54fe2f76995cb201004231c0f3f09aa161d822c7..48e5bbed19f49f2ec195c03d14f0a6b93447b4e6 100644 (file)
--- a/main.c
+++ b/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);
 }

diff --git a/mainloop.c b/mainloop.c
index b8f401f8a67b9187bc23aa4b50676f222d5dc550..6cafe7ebe810128e0773784f9091620728e5b4e8 100644 (file)
--- a/mainloop.c
+++ b/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) {

diff --git a/openconnect-internal.h b/openconnect-internal.h
index ac692f4c8f36f4de11aa0d0b177537fc0e7bff2b..06f4cec9c260f6885830ff9953badc060bdd55aa 100644 (file)
--- a/openconnect-internal.h
+++ b/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;

diff --git a/ssl.c b/ssl.c
index dbbe223ca82f25176e20607418f2a7d555bac952..a669738693caa3c82122aa8c6a5858083f8caef3 100644 (file)
--- a/ssl.c
+++ b/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);
+               }
        }
 }