From 5ede5e4eedf47c856f45849e9f8360fda4e40b92 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 7 May 2018 20:02:39 -0400 Subject: [PATCH 01/10] lib: Add ability to know if we have read anything in When reading the config file add an ability to know if we have properly read in anything. So that a daemon can make fallback plans. Signed-off-by: Donald Sharp --- lib/vty.c | 10 ++++++++-- lib/vty.h | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/vty.c b/lib/vty.c index 280b2ace51..e9d1f2e323 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -2462,12 +2462,13 @@ static FILE *vty_use_backup_config(const char *fullpath) } /* Read up configuration file from file_name. */ -void vty_read_config(const char *config_file, char *config_default_dir) +bool vty_read_config(const char *config_file, char *config_default_dir) { char cwd[MAXPATHLEN]; FILE *confp = NULL; const char *fullpath; char *tmp = NULL; + bool read_success = false; /* If -f flag specified. */ if (config_file != NULL) { @@ -2525,8 +2526,10 @@ void vty_read_config(const char *config_file, char *config_default_dir) if (strstr(config_default_dir, "vtysh") == NULL) { ret = stat(integrate_default, &conf_stat); - if (ret >= 0) + if (ret >= 0) { + read_success = true; goto tmp_free_and_out; + } } #endif /* VTYSH */ confp = fopen(config_default_dir, "r"); @@ -2550,6 +2553,7 @@ void vty_read_config(const char *config_file, char *config_default_dir) } vty_read_file(confp); + read_success = true; fclose(confp); @@ -2558,6 +2562,8 @@ void vty_read_config(const char *config_file, char *config_default_dir) tmp_free_and_out: if (tmp) XFREE(MTYPE_TMP, tmp); + + return read_success; } /* Small utility function which output log to the VTY. */ diff --git a/lib/vty.h b/lib/vty.h index d14ddf5908..b55abf2204 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -242,7 +242,7 @@ extern void vty_frame(struct vty *, const char *, ...) PRINTF_ATTRIBUTE(2, 3); extern void vty_endframe(struct vty *, const char *); bool vty_set_include(struct vty *vty, const char *regexp); -extern void vty_read_config(const char *, char *); +extern bool vty_read_config(const char *, char *); extern void vty_time_print(struct vty *, int); extern void vty_serv_sock(const char *, unsigned short, const char *); extern void vty_close(struct vty *); From 573de11fab9c6ffd5b647ab1b9b0c23ad99fb52e Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 7 May 2018 21:01:15 -0400 Subject: [PATCH 02/10] lib: Add ability to retry if backup is specified If we fail to read in the config file and we have specified a backup of the backup, attempt to read that information. Signed-off-by: Donald Sharp --- lib/libfrr.c | 7 ++++++- lib/libfrr.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/libfrr.c b/lib/libfrr.c index 88203fbeb6..4620cb2586 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -725,7 +725,12 @@ void frr_config_fork(void) { hook_call(frr_late_init, master); - vty_read_config(di->config_file, config_default); + if (!vty_read_config(di->config_file, config_default) && + di->backup_config_file) { + zlog_info("Attempting to read backup config file: %s specified", + di->backup_config_file); + vty_read_config(di->backup_config_file, config_default); + } /* Don't start execution if we are in dry-run mode */ if (di->dryrun) diff --git a/lib/libfrr.h b/lib/libfrr.h index 7ffa780bfb..bd572cce1b 100644 --- a/lib/libfrr.h +++ b/lib/libfrr.h @@ -51,6 +51,7 @@ struct frr_daemon_info { bool daemon_mode; bool terminal; const char *config_file; + const char *backup_config_file; const char *pid_file; const char *vty_path; const char *module_path; From 9e224e60dbffd032d7271ec473366048fb6083d4 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 8 May 2018 08:35:06 -0400 Subject: [PATCH 03/10] lib: Create a thread for reading in the cli The read in of cli was happening prior to thread event handling for non-integrated configs. This is interesting for 2 reasons: 1) Read-in of integrated configs was after thread event loop startup, so we had a difference of behavior 2) Read-in can cause a series of events that cause us to attempt to communicate with zebra. The zebra zapi connection only happens after the thread event loop has been started. This can cause data that is being written down to zebra to be lost and no real way to notice that this has happened and to recover gracefully. Modify the code to create a thread event for read in of client config. Signed-off-by: Donald Sharp --- lib/libfrr.c | 25 +++++++++++++++++++++---- lib/libfrr.h | 2 ++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/libfrr.c b/lib/libfrr.c index 4620cb2586..4f3b8c66d6 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -721,20 +721,37 @@ static void frr_daemonize(void) frr_daemon_wait(fds[0]); } -void frr_config_fork(void) +/* + * Why is this a thread? + * + * The read in of config for integrated config happens *after* + * thread execution starts( because it is passed in via a vtysh -b -n ) + * While if you are not using integrated config we want the ability + * to read the config in after thread execution starts, so that + * we can match this behavior. + */ +static int frr_config_read_in(struct thread *t) { - hook_call(frr_late_init, master); - if (!vty_read_config(di->config_file, config_default) && di->backup_config_file) { zlog_info("Attempting to read backup config file: %s specified", di->backup_config_file); vty_read_config(di->backup_config_file, config_default); } + return 0; +} + +void frr_config_fork(void) +{ + hook_call(frr_late_init, master); /* Don't start execution if we are in dry-run mode */ - if (di->dryrun) + if (di->dryrun) { + frr_config_read_in(NULL); exit(0); + } + + thread_add_event(master, frr_config_read_in, NULL, 0, &di->read_in); if (di->daemon_mode || di->terminal) frr_daemonize(); diff --git a/lib/libfrr.h b/lib/libfrr.h index bd572cce1b..1c744ee1b9 100644 --- a/lib/libfrr.h +++ b/lib/libfrr.h @@ -50,6 +50,8 @@ struct frr_daemon_info { bool dryrun; bool daemon_mode; bool terminal; + + struct thread *read_in; const char *config_file; const char *backup_config_file; const char *pid_file; From 9124048d0e01718899ac4fd591b74d5422780b8f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 14 Jun 2018 22:33:59 -0400 Subject: [PATCH 04/10] ldpd: Schedule application of config till after read-in With commit e94b38d94b5 we are now scheduling the read of vty config until after the startup of main thread processing. It now becomes necessary to move the application of the config until after the read in of the config from a file if we are using a non-integrated config. Signed-off-by: Donald Sharp --- ldpd/ldpd.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c index 255febeb60..1fb9f0fb33 100644 --- a/ldpd/ldpd.c +++ b/ldpd/ldpd.c @@ -187,6 +187,22 @@ FRR_DAEMON_INFO(ldpd, LDP, .privs = &ldpd_privs, ) +static int ldp_config_fork_apply(struct thread *t) +{ + /* + * So the frr_config_fork() function schedules + * the read of the vty config( if there is a + * non-integrated config ) to be after the + * end of startup and we are starting the + * main process loop. We need to schedule + * the application of this if necessary + * after the read in of the config. + */ + ldp_config_apply(NULL, vty_conf); + + return 0; +} + int main(int argc, char *argv[]) { @@ -195,6 +211,7 @@ main(int argc, char *argv[]) int pipe_parent2ldpe[2], pipe_parent2ldpe_sync[2]; int pipe_parent2lde[2], pipe_parent2lde_sync[2]; char *ctl_sock_name; + struct thread *thread = NULL; ldpd_process = PROC_MAIN; log_procname = log_procnames[ldpd_process]; @@ -331,7 +348,7 @@ main(int argc, char *argv[]) frr_config_fork(); /* apply configuration */ - ldp_config_apply(NULL, vty_conf); + thread_add_event(master, ldp_config_fork_apply, NULL, 0, &thread); /* setup pipes to children */ if ((iev_ldpe = calloc(1, sizeof(struct imsgev))) == NULL || From 1f1432364911625d5abd6384b190a2d75c37120f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 15 Jun 2018 12:34:31 -0400 Subject: [PATCH 05/10] vtysh: Fix 'no log syslog ..' to be correct The vtysh version of `no log syslog...` was out of sync with what is actually correct. Fix. Signed-off-by: Donald Sharp --- vtysh/vtysh.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 309493b13e..0c3d84f38f 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2415,10 +2415,11 @@ DEFUNSH(VTYSH_ALL, vtysh_log_syslog, vtysh_log_syslog_cmd, } DEFUNSH(VTYSH_ALL, no_vtysh_log_syslog, no_vtysh_log_syslog_cmd, - "no log syslog [LEVEL]", NO_STR + "no log syslog []", + NO_STR "Logging control\n" "Cancel logging to syslog\n" - "Logging level\n") + LOG_LEVEL_DESC) { return CMD_SUCCESS; } From f8507817cf19d621cefd1d0310925ad07ec4646e Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 15 Jun 2018 13:38:46 -0400 Subject: [PATCH 06/10] lib: Add new cli to specify where to output logs on startup When we are starting a daemon, allow the user to specify: --log This can be used on early startup to put the log files where the end user wants them to show up. Signed-off-by: Donald Sharp --- lib/command.c | 23 ++++++++++++++++++++++- lib/command.h | 1 + lib/libfrr.c | 11 ++++++++++- lib/libfrr.h | 1 + 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/command.c b/lib/command.c index a8e61c6bb4..32b052c36c 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2429,7 +2429,8 @@ static int set_log_file(struct vty *vty, const char *fname, int loglevel) XFREE(MTYPE_TMP, p); if (!ret) { - vty_out(vty, "can't open logfile %s\n", fname); + if (vty) + vty_out(vty, "can't open logfile %s\n", fname); return CMD_WARNING_CONFIG_FAILED; } @@ -2445,6 +2446,26 @@ static int set_log_file(struct vty *vty, const char *fname, int loglevel) return CMD_SUCCESS; } +void command_setup_early_logging(const char *option) +{ + char *token; + + if (strcmp(option, "stdout") == 0) { + zlog_set_level(ZLOG_DEST_STDOUT, zlog_default->default_lvl); + return; + } + + if (strcmp(option, "syslog") == 0) { + zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl); + return; + } + + token = strstr(option, ":"); + token++; + + set_log_file(NULL, token, zlog_default->default_lvl); +} + DEFUN (config_log_file, config_log_file_cmd, "log file FILENAME []", diff --git a/lib/command.h b/lib/command.h index 9bf482f41b..9ccd13c73b 100644 --- a/lib/command.h +++ b/lib/command.h @@ -479,4 +479,5 @@ extern void cmd_variable_handler_register(const struct cmd_variable_handler *cvh); extern char *cmd_variable_comp2str(vector comps, unsigned short cols); +extern void command_setup_early_logging(const char *option); #endif /* _ZEBRA_COMMAND_H */ diff --git a/lib/libfrr.c b/lib/libfrr.c index 4f3b8c66d6..9292688327 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -78,6 +78,7 @@ static void opt_extend(const struct optspec *os) #define OPTION_VTYSOCK 1000 #define OPTION_MODULEDIR 1002 +#define OPTION_LOG 1003 static const struct option lo_always[] = { {"help", no_argument, NULL, 'h'}, @@ -86,6 +87,7 @@ static const struct option lo_always[] = { {"module", no_argument, NULL, 'M'}, {"vty_socket", required_argument, NULL, OPTION_VTYSOCK}, {"moduledir", required_argument, NULL, OPTION_MODULEDIR}, + {"log", required_argument, NULL, OPTION_LOG}, {NULL}}; static const struct optspec os_always = { "hvdM:", @@ -94,7 +96,8 @@ static const struct optspec os_always = { " -d, --daemon Runs in daemon mode\n" " -M, --module Load specified module\n" " --vty_socket Override vty socket path\n" - " --moduledir Override modules directory\n", + " --moduledir Override modules directory\n" + " --log Set Logging to stdout, syslog, or file:\n", lo_always}; @@ -444,6 +447,9 @@ static int frr_opt(int opt) return 1; di->privs->group = optarg; break; + case OPTION_LOG: + di->early_logging = optarg; + break; default: return 1; } @@ -543,6 +549,9 @@ struct thread_master *frr_init(void) openzlog(di->progname, di->logname, di->instance, LOG_CONS | LOG_NDELAY | LOG_PID, LOG_DAEMON); + + if (di->early_logging) + command_setup_early_logging(di->early_logging); #if defined(HAVE_CUMULUS) zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl); #endif diff --git a/lib/libfrr.h b/lib/libfrr.h index 1c744ee1b9..ecef8c616e 100644 --- a/lib/libfrr.h +++ b/lib/libfrr.h @@ -58,6 +58,7 @@ struct frr_daemon_info { const char *vty_path; const char *module_path; const char *pathspace; + const char *early_logging; const char *proghelp; void (*printhelp)(FILE *target); From ed74ddf4033a49774391a3513eac41e0afd107b7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 15 Jun 2018 13:43:13 -0400 Subject: [PATCH 07/10] doc: Add some documentation for cli logging Add some basic documentation for the new cli added to all daemons. Signed-off-by: Donald Sharp --- doc/user/basic.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/user/basic.rst b/doc/user/basic.rst index a75017c442..f6114008f8 100644 --- a/doc/user/basic.rst +++ b/doc/user/basic.rst @@ -405,6 +405,13 @@ These options apply to all |PACKAGE_NAME| daemons. Print program version. +.. option:: --log + + When initializing the daemon, setup the log to go to either stdout, + syslog or to a file. These values will be displayed as part of + a show run. Additionally they can be overridden at runtime if + desired via the normal log commands. + .. _loadable-module-support: Loadable Module Support From 9685abb492c9456c3554dffea948ea2672ada7da Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 15 Jun 2018 13:48:11 -0400 Subject: [PATCH 08/10] lib: Remove special case code to use syslog Remove the special case code to use syslog for Cumulus. They can specify this via startup now instead of having a special compile flag for this option. Signed-off-by: Donald Sharp --- lib/libfrr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/libfrr.c b/lib/libfrr.c index 9292688327..ddc952b2cb 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -552,9 +552,6 @@ struct thread_master *frr_init(void) if (di->early_logging) command_setup_early_logging(di->early_logging); -#if defined(HAVE_CUMULUS) - zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl); -#endif if (!frr_zclient_addr(&zclient_addr, &zclient_addr_len, frr_zclientpath)) { From e9b4e74a7868cd4f8eff60209c1039cacb3421b4 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 19 Jun 2018 09:02:21 -0400 Subject: [PATCH 09/10] lib: Add --log-level to daemons Add the ability to specify the designated log level at startup. --log-level Signed-off-by: Donald Sharp --- lib/command.c | 18 ++++++++++++++---- lib/command.h | 2 +- lib/libfrr.c | 13 +++++++++---- lib/libfrr.h | 1 + 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/command.c b/lib/command.c index 32b052c36c..4ab47e5fc2 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2446,21 +2446,31 @@ static int set_log_file(struct vty *vty, const char *fname, int loglevel) return CMD_SUCCESS; } -void command_setup_early_logging(const char *option) +void command_setup_early_logging(const char *dest, const char *level) { char *token; - if (strcmp(option, "stdout") == 0) { + if (level) { + int nlevel = level_match(level); + + if (nlevel != ZLOG_DISABLED) + zlog_default->default_lvl = nlevel; + } + + if (!dest) + return; + + if (strcmp(dest, "stdout") == 0) { zlog_set_level(ZLOG_DEST_STDOUT, zlog_default->default_lvl); return; } - if (strcmp(option, "syslog") == 0) { + if (strcmp(dest, "syslog") == 0) { zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl); return; } - token = strstr(option, ":"); + token = strstr(dest, ":"); token++; set_log_file(NULL, token, zlog_default->default_lvl); diff --git a/lib/command.h b/lib/command.h index 9ccd13c73b..395c971c55 100644 --- a/lib/command.h +++ b/lib/command.h @@ -479,5 +479,5 @@ extern void cmd_variable_handler_register(const struct cmd_variable_handler *cvh); extern char *cmd_variable_comp2str(vector comps, unsigned short cols); -extern void command_setup_early_logging(const char *option); +extern void command_setup_early_logging(const char *dest, const char *level); #endif /* _ZEBRA_COMMAND_H */ diff --git a/lib/libfrr.c b/lib/libfrr.c index ddc952b2cb..505bea9b18 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -78,7 +78,8 @@ static void opt_extend(const struct optspec *os) #define OPTION_VTYSOCK 1000 #define OPTION_MODULEDIR 1002 -#define OPTION_LOG 1003 +#define OPTION_LOG 1003 +#define OPTION_LOGLEVEL 1004 static const struct option lo_always[] = { {"help", no_argument, NULL, 'h'}, @@ -88,6 +89,7 @@ static const struct option lo_always[] = { {"vty_socket", required_argument, NULL, OPTION_VTYSOCK}, {"moduledir", required_argument, NULL, OPTION_MODULEDIR}, {"log", required_argument, NULL, OPTION_LOG}, + {"log-level", required_argument, NULL, OPTION_LOGLEVEL}, {NULL}}; static const struct optspec os_always = { "hvdM:", @@ -97,7 +99,8 @@ static const struct optspec os_always = { " -M, --module Load specified module\n" " --vty_socket Override vty socket path\n" " --moduledir Override modules directory\n" - " --log Set Logging to stdout, syslog, or file:\n", + " --log Set Logging to stdout, syslog, or file:\n" + " --log-level Set Logging Level to use, debug, info, warn, etc\n", lo_always}; @@ -450,6 +453,9 @@ static int frr_opt(int opt) case OPTION_LOG: di->early_logging = optarg; break; + case OPTION_LOGLEVEL: + di->early_loglevel = optarg; + break; default: return 1; } @@ -550,8 +556,7 @@ struct thread_master *frr_init(void) openzlog(di->progname, di->logname, di->instance, LOG_CONS | LOG_NDELAY | LOG_PID, LOG_DAEMON); - if (di->early_logging) - command_setup_early_logging(di->early_logging); + command_setup_early_logging(di->early_logging, di->early_loglevel); if (!frr_zclient_addr(&zclient_addr, &zclient_addr_len, frr_zclientpath)) { diff --git a/lib/libfrr.h b/lib/libfrr.h index ecef8c616e..d255279906 100644 --- a/lib/libfrr.h +++ b/lib/libfrr.h @@ -59,6 +59,7 @@ struct frr_daemon_info { const char *module_path; const char *pathspace; const char *early_logging; + const char *early_loglevel; const char *proghelp; void (*printhelp)(FILE *target); From 3cf3e018a916254b03b5685bd6da557daa53e4c6 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 19 Jun 2018 09:06:37 -0400 Subject: [PATCH 10/10] doc: Add --log-level documentation Signed-off-by: Donald Sharp --- doc/user/basic.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/user/basic.rst b/doc/user/basic.rst index f6114008f8..cb46080055 100644 --- a/doc/user/basic.rst +++ b/doc/user/basic.rst @@ -412,6 +412,11 @@ These options apply to all |PACKAGE_NAME| daemons. a show run. Additionally they can be overridden at runtime if desired via the normal log commands. +.. option:: --log-level + + When initializing the daemon, allow the specification of a default + log level at startup from one of the specified levels. + .. _loadable-module-support: Loadable Module Support