vtysh: fork() on boot

When using -b flag to apply config to all running daemons, fork a copy
of vtysh for each daemon we need to configure instead of doing them one
at a time. This is about N times faster when you have N daemons.

Signed-off-by: Quentin Young <qlyoung@nvidia.com>
mergeme

diff --git a/doc/manpages/vtysh.rst b/doc/manpages/vtysh.rst
index af527bea407ccb19bf40c5245abb5a43670634d5..94ba3baebd295e5bcaa28ecc193ff0109b385328 100644 (file)
--- a/doc/manpages/vtysh.rst
+++ b/doc/manpages/vtysh.rst
@@ -67,6 +67,11 @@ OPTIONS available for the vtysh command:
 
    Display a usage message on standard output and exit.
 
+.. option:: --no-fork
+
+   When used in conjunction with ``-b``, prevents vtysh from forking children to handle configuring each target daemon.
+
+
 ENVIRONMENT VARIABLES
 =====================
 VTYSH_PAGER

diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
index 0a164ec16d5b3cbf6515ab63740a1d1cda39480d..93c3e9ae1e19dd32e24a14bc88b7245ca946abd2 100644 (file)
--- a/vtysh/vtysh.c
+++ b/vtysh/vtysh.c
@@ -49,19 +49,6 @@ char *vtysh_pager_name = NULL;
 /* VTY should add timestamp */
 bool vtysh_add_timestamp;
 
-/* VTY shell client structure */
-struct vtysh_client {
-       int fd;
-       const char *name;
-       int flag;
-       char path[MAXPATHLEN];
-       struct vtysh_client *next;
-
-       struct event *log_reader;
-       int log_fd;
-       uint32_t lost_msgs;
-};
-
 static bool stderr_tty;
 static bool stderr_stdout_same;
 
@@ -119,6 +106,10 @@ static void vtysh_pager_envdef(bool fallback)
 
 /* --- */
 
+/*
+ * When updating this array, remember to change the array size here and in
+ * vtysh.h
+ */
 struct vtysh_client vtysh_client[] = {
        {.name = "mgmtd", .flag = VTYSH_MGMTD},
        {.name = "zebra", .flag = VTYSH_ZEBRA},
@@ -144,6 +135,8 @@ struct vtysh_client vtysh_client[] = {
        {.name = "pim6d", .flag = VTYSH_PIM6D},
 };
 
+char my_client[64];
+
 /* Searches for client by name, returns index */
 static int vtysh_client_lookup(const char *name)
 {

diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h
index cb6357f5e6525f734b8c8d0f58c38ad0daa22c07..e43dd69350701e80782e89203c97713e6e2daf21 100644 (file)
--- a/vtysh/vtysh.h
+++ b/vtysh/vtysh.h
@@ -119,4 +119,19 @@ extern int user_mode;
 
 extern bool vtysh_add_timestamp;
 
+struct vtysh_client {
+       int fd;
+       const char *name;
+       int flag;
+       char path[MAXPATHLEN];
+       struct vtysh_client *next;
+
+       struct event *log_reader;
+       int log_fd;
+       uint32_t lost_msgs;
+};
+
+extern struct vtysh_client vtysh_client[22];
+extern char my_client[64];
+
 #endif /* VTYSH_H */

diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c
index c22889a4f6cd785963019c67f4cb523e3bbe7a48..1721a8370b1528faf5f18a34eae00f46623fce0c 100644 (file)
--- a/vtysh/vtysh_main.c
+++ b/vtysh/vtysh_main.c
@@ -178,6 +178,7 @@ static void usage(int status)
                       "-u  --user               Run as an unprivileged user\n"
                       "-w, --writeconfig        Write integrated config (frr.conf) and exit\n"
                       "-H, --histfile           Override history file\n"
+                      "    --no-fork            Don't fork clients to handle daemons (slower for large configs)\n"
                       "-h, --help               Display this help and exit\n\n"
                       "Note that multiple commands may be executed from the command\n"
                       "line by passing multiple -c args, or by embedding linefeed\n"
@@ -191,6 +192,7 @@ static void usage(int status)
 /* VTY shell options, we use GNU getopt library. */
 #define OPTION_VTYSOCK 1000
 #define OPTION_CONFDIR 1001
+#define OPTION_NOFORK 1002
 struct option longopts[] = {
        {"boot", no_argument, NULL, 'b'},
        /* For compatibility with older zebra/quagga versions */
@@ -210,6 +212,7 @@ struct option longopts[] = {
        {"pathspace", required_argument, NULL, 'N'},
        {"user", no_argument, NULL, 'u'},
        {"timestamp", no_argument, NULL, 't'},
+       {"no-fork", no_argument, NULL, OPTION_NOFORK},
        {0}};
 
 bool vtysh_loop_exited;
@@ -321,6 +324,7 @@ int main(int argc, char **argv, char **env)
        int dryrun = 0;
        int boot_flag = 0;
        bool ts_flag = false;
+       bool no_fork = false;
        const char *daemon_name = NULL;
        const char *inputfile = NULL;
        struct cmd_rec {
@@ -392,6 +396,9 @@ int main(int argc, char **argv, char **env)
                        ditch_suid = 1; /* option disables SUID */
                        snprintf(sysconfdir, sizeof(sysconfdir), "%s/", optarg);
                        break;
+               case OPTION_NOFORK:
+                       no_fork = true;
+                       break;
                case 'N':
                        if (strchr(optarg, '/') || strchr(optarg, '.')) {
                                fprintf(stderr,
@@ -440,6 +447,10 @@ int main(int argc, char **argv, char **env)
                }
        }
 
+       /* No need for forks if we're talking to 1 daemon */
+       if (daemon_name)
+               no_fork = true;
+
        if (ditch_suid) {
                elevuid = realuid;
                elevgid = realgid;
@@ -705,19 +716,95 @@ int main(int argc, char **argv, char **env)
 
        /* Boot startup configuration file. */
        if (boot_flag) {
+               /*
+                * flock config file before fork. After fork, each child will
+                * hold the same lock. The lock can be released by any one of
+                * them but they will exit without releasing the lock - the
+                * parent (us) will release it when they are done
+                */
                vtysh_flock_config(frr_config);
+
+               /*
+                * In boot mode, we need to apply the whole config file to all
+                * daemons. Instead of having one client talk to N daemons, we
+                * fork N times and let each child handle one daemon.
+                */
+               pid_t fork_pid = getpid();
+               int status = 0;
+
+               if (!no_fork) {
+                       for (unsigned int i = 0; i < array_size(vtysh_client);
+                            i++) {
+                               /* Store name of client this fork will handle */
+                               strlcpy(my_client, vtysh_client[i].name,
+                                       sizeof(my_client));
+                               fork_pid = fork();
+
+                               /* If child, break */
+                               if (fork_pid == 0)
+                                       break;
+                       }
+
+                       /* parent, wait for children */
+                       if (fork_pid != 0) {
+                               fprintf(stdout,
+                                       "Waiting for children to finish applying config...\n");
+                               while (wait(&status) > 0)
+                                       ;
+                               ret = 0;
+                               goto boot_done;
+                       }
+
+                       /*
+                        * children, grow up to be cowboys
+                        */
+                       for (unsigned int i = 0; i < array_size(vtysh_client);
+                            i++) {
+                               if (strcmp(my_client, vtysh_client[i].name)) {
+                                       /*
+                                        * If this is a client we aren't
+                                        * responsible for, disconnect
+                                        */
+                                       if (vtysh_client[i].fd >= 0)
+                                               close(vtysh_client[i].fd);
+                                       vtysh_client[i].fd = -1;
+                               } else if (vtysh_client[i].fd == -1) {
+                                       /*
+                                        * If this is the client we are
+                                        * responsible for, but we aren't
+                                        * already connected to that client,
+                                        * that means the client isn't up in the
+                                        * first place and we can exit early
+                                        */
+                                       ret = 0;
+                                       goto boot_done;
+                               }
+                       }
+
+                       fprintf(stdout, "[%d|%s] sending configuration\n",
+                               getpid(), my_client);
+               }
+
                ret = vtysh_read_config(frr_config, dryrun);
-               vtysh_unflock_config();
                if (ret) {
-                       fprintf(stderr,
-                               "Configuration file[%s] processing failure: %d\n",
-                               frr_config, ret);
-                       if (no_error)
-                               exit(0);
+                       if (!no_fork)
+                               fprintf(stderr,
+                                       "[%d|%s] Configuration file[%s] processing failure: %d\n",
+                                       getpid(), my_client, frr_config, ret);
                        else
-                               exit(ret);
-               } else
-                       exit(0);
+                               fprintf(stderr,
+                                       "Configuration file[%s] processing failure: %d\n",
+                                       frr_config, ret);
+                       if (no_error)
+                               ret = 0;
+               } else if (!no_fork) {
+                       fprintf(stderr, "[%d|%s] done\n", getpid(), my_client);
+               }
+
+       boot_done:
+               if (fork_pid != 0)
+                       vtysh_unflock_config();
+               exit(ret);
        }
 
        vtysh_readline_init();