From e5440e73a650c1023969641d3da29df0b642774e Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Sat, 5 Oct 2024 13:13:36 +0100 Subject: [PATCH 01/19] core: remove diagnostic message accidently added in commit 7cb09b2 Commit 7cb09b2 - all: Ensure pid file exists when respawning child process Signed-off-by: Quentin Armitage --- keepalived/core/pidfile.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/keepalived/core/pidfile.c b/keepalived/core/pidfile.c index 7696e8912..0a2faba97 100644 --- a/keepalived/core/pidfile.c +++ b/keepalived/core/pidfile.c @@ -251,8 +251,6 @@ keepalived_running(unsigned long mode) bool pidfile_write(pidfile_t *pidf) { - int ret; - /* If keepalived originally started with no configuration for this process, * the process won't have originally been started, and the parent process * will not have created and opened a pid file. This means that pidf->fd @@ -294,12 +292,7 @@ pidfile_write(pidfile_t *pidf) } } - ret = dprintf(pidf->fd, "%d\n", getpid()); - - if (ret < 0) - log_message(LOG_INFO, "pidfile_write returned %d, errno %d - %m", ret, errno); - else - log_message(LOG_INFO, "pidfile_write returned %d", ret); + dprintf(pidf->fd, "%d\n", getpid()); return true; } From c01a2d286912eab92c5caf7358172f9145f654f0 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Sat, 5 Oct 2024 14:54:44 +0100 Subject: [PATCH 02/19] test: Make tcp_server use of SO_LINGER optional Signed-off-by: Quentin Armitage --- test/tcp_server.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/test/tcp_server.c b/test/tcp_server.c index d59558835..18dd63b33 100644 --- a/test/tcp_server.c +++ b/test/tcp_server.c @@ -103,14 +103,14 @@ find_resp(int in_fd, int out_fd, const char *cmd) if (!e) e = s + strlen(s); len = e - s; -printf("(%d) Looking at %p, len 0x%x (%d)\n", getpid(), s, len, len); +// printf("(%d) Looking at %p, len 0x%x (%d)\n", getpid(), s, len, len); if (debug) printf("(%d) Looking at '%.*s'\n", getpid(), (int)len, s); resp = s; for (p = cmd_resp_list; p; p = p->next) { -printf("(%d) Comparing '%s' len %zu\n", getpid(), p->cmd, strlen(p->cmd)); +// printf("(%d) Comparing '%s' len %zu\n", getpid(), p->cmd, strlen(p->cmd)); if (len == strlen(p->cmd) && !strncmp(p->cmd, s, len)) { if (debug) @@ -284,6 +284,7 @@ print_usage(FILE *fp, const char *name) fprintf(fp, "\t-x url\t\tsend a pre-build HTTP response for url /\n"); fprintf(fp, "\t-M[server_name]\tbe an email server\n"); fprintf(fp, "\t-l val\t\tASCII value to use for EOL char\n"); + fprintf(fp, "\t-L val\t\tSO_LINGER timeout (default none)\n"); fprintf(fp, "\t-d delay\tdelay in ms before replying\n"); fprintf(fp, "\t-r\t\tuse random delay\n"); fprintf(fp, "\t-m mod\t\tOnly report every mod'th connection\n"); @@ -329,8 +330,9 @@ int main(int argc, char **argv) bool immediate_data_malloc = false; char client_addr_buf[40]; unsigned client_port; + unsigned linger_timeout = 0; - while ((opt = getopt(argc, argv, ":h46a:p:suPeb:c:l:d:rm:v:WM::w:x:ZDgGi:S" + while ((opt = getopt(argc, argv, ":h46a:p:suPeb:c:l:L:d:rm:v:WM::w:x:ZDgGi:S" #ifdef TCP_FASTOPEN "f:" #endif @@ -413,6 +415,9 @@ int main(int argc, char **argv) case 'l': EOL = strtoul(optarg, &endptr, 10); break; + case 'L': + linger_timeout = strtoul(optarg, &endptr, 10); + break; case 'd': rep_delay.tv_nsec = strtoul(optarg, &endptr, 10); rep_delay.tv_sec = rep_delay.tv_nsec / 1000; @@ -494,13 +499,16 @@ int main(int argc, char **argv) exit(1); } - struct linger li = { .l_onoff = 1, .l_linger = 1 }; - if (setsockopt(listenfd, SOL_SOCKET, SO_LINGER, (char *)&li, sizeof (struct linger))) { - printf("(%d) Set SO_LINGER failed, errno %d (%m)\n", getpid(), errno); - exit(1); + if (linger_timeout) { + struct linger li = { .l_onoff = 1, .l_linger = linger_timeout }; + if (setsockopt(listenfd, SOL_SOCKET, SO_LINGER, (char *)&li, sizeof (struct linger))) { + printf("(%d) Set SO_LINGER failed, errno %d (%m)\n", getpid(), errno); + exit(1); + } } - if (setsockopt(listenfd, SOL_SOCKET, SO_REUSEADDR, &li.l_onoff, sizeof (li.l_onoff))) { + int reuse = 1; + if (setsockopt(listenfd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof (reuse))) { printf("(%d) Set SO_REUSEADDR failed, errno %d (%m)\n", getpid(), errno); exit(1); } From 96fef063918c545eafd136ca94ef700477994f0c Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Sat, 5 Oct 2024 15:08:57 +0100 Subject: [PATCH 03/19] itest: Warn if close after send not set in tcp_server for http Signed-off-by: Quentin Armitage --- test/tcp_server.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/tcp_server.c b/test/tcp_server.c index 18dd63b33..c9d150262 100644 --- a/test/tcp_server.c +++ b/test/tcp_server.c @@ -194,6 +194,9 @@ new_html_cr(const char *url, const char *resp, const char *html_version, bool cl cr = new_cr(cmd, resp, close_after_send, TYPE_HTML); cr->html_version = strdup(html_version); + + if (!close_after_send) + fprintf(stderr, "Warning close after send (-Z) should be set\n"); } static void From 876ea3093eb50ed61dcaa9c992f76ebd73e69ebc Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:26:52 +0100 Subject: [PATCH 04/19] configure: explicitly set language to C for configure Signed-off-by: Quentin Armitage --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 5d3db6004..557449a7c 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,7 @@ AC_PREREQ([2.63]) AC_INIT([Keepalived], [2.3.1], [keepalived-users@groups.io], [], [http://www.keepalived.org/]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_MACRO_DIR([m4]) +AC_LANG([C]) AM_INIT_AUTOMAKE([-Wall -Werror -Woverride foreign]) AC_CONFIG_SRCDIR([keepalived/core/main.c]) From de37c3acb2c9f379d195fb76ac1233a4695f791c Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:29:06 +0100 Subject: [PATCH 05/19] all: make min_auto_priority delay variable an unsigned It was a long which was excessive. It cannot be negative, and doesn't need more than 32 bits. Signed-off-by: Quentin Armitage --- keepalived/core/global_data.c | 2 +- keepalived/core/global_parser.c | 4 ++-- keepalived/include/global_data.h | 2 +- lib/process.c | 4 ++-- lib/process.h | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/keepalived/core/global_data.c b/keepalived/core/global_data.c index b082ffc08..e8d7c5951 100644 --- a/keepalived/core/global_data.c +++ b/keepalived/core/global_data.c @@ -814,7 +814,7 @@ dump_global_data(FILE *fp, data_t * data) conf_write(fp, " Max auto priority = Disabled"); else conf_write(fp, " Max auto priority = %d", data->max_auto_priority); - conf_write(fp, " Min auto priority delay = %ld usecs", data->min_auto_priority_delay); + conf_write(fp, " Min auto priority delay = %u usecs", data->min_auto_priority_delay); conf_write(fp, " VRRP process priority = %d", data->vrrp_process_priority); conf_write(fp, " VRRP don't swap = %s", data->vrrp_no_swap ? "true" : "false"); conf_write(fp, " VRRP realtime priority = %u", data->vrrp_realtime_priority); diff --git a/keepalived/core/global_parser.c b/keepalived/core/global_parser.c index c08f77c33..38f13807d 100644 --- a/keepalived/core/global_parser.c +++ b/keepalived/core/global_parser.c @@ -403,13 +403,13 @@ max_auto_priority_handler(const vector_t *strvec) static void min_auto_priority_delay_handler(const vector_t *strvec) { - int delay; + unsigned delay; if (vector_size(strvec) < 2) { report_config_error(CONFIG_GENERAL_ERROR, "min_auto_priority_delay requires delay time"); return; } - if (!read_int_strvec(strvec, 1, &delay, 1, 10000000, true)) { + if (!read_unsigned_strvec(strvec, 1, &delay, 1U, 10000000U, true)) { report_config_error(CONFIG_GENERAL_ERROR, "min_auto_priority_delay '%s' must be in [1, 10000000] - ignoring", strvec_slot(strvec, 1)); return; } diff --git a/keepalived/include/global_data.h b/keepalived/include/global_data.h index f4533389f..3440a6163 100644 --- a/keepalived/include/global_data.h +++ b/keepalived/include/global_data.h @@ -150,7 +150,7 @@ typedef struct _data { lvs_flush_t lvs_flush_on_stop; /* flush any LVS config at shutdown */ #endif int max_auto_priority; - long min_auto_priority_delay; + unsigned min_auto_priority_delay; #ifdef _WITH_VRRP_ struct sockaddr_in6 vrrp_mcast_group6 __attribute__((aligned(__alignof__(sockaddr_t)))); struct sockaddr_in vrrp_mcast_group4 __attribute__((aligned(__alignof__(sockaddr_t)))); diff --git a/lib/process.c b/lib/process.c index 387165fd8..84c3372d7 100644 --- a/lib/process.c +++ b/lib/process.c @@ -42,7 +42,7 @@ static unsigned cur_rt_priority; static unsigned max_rt_priority; -long min_auto_priority_delay; +unsigned min_auto_priority_delay; static unsigned cur_rlimit_rttime; static unsigned default_rlimit_rttime; @@ -120,7 +120,7 @@ reset_process_priority(void) variable length buffer" warning */ RELAX_STACK_PROTECTOR_START void -set_process_priorities(int realtime_priority, int max_realtime_priority, long min_delay, +set_process_priorities(int realtime_priority, int max_realtime_priority, unsigned min_delay, int rlimit_rt, int process_priority, int no_swap_stack_size) { if (max_realtime_priority != -1) diff --git a/lib/process.h b/lib/process.h index aab1d11e2..e753bc469 100644 --- a/lib/process.h +++ b/lib/process.h @@ -32,10 +32,10 @@ /* The maximum pid is 2^22 - see definition of PID_MAX_LIMIT in kernel source include/linux/threads.h */ #define PID_MAX_DIGITS 7 -extern long min_auto_priority_delay; +extern unsigned min_auto_priority_delay; extern pid_t main_pid; -extern void set_process_priorities(int, int, long, int, int, int); +extern void set_process_priorities(int, int, unsigned, int, int, int); extern void reset_process_priorities(void); extern void increment_process_priority(void); extern unsigned get_cur_priority(void) __attribute__((pure)); From e50dcef8976f060d7215fd880bfb575161da562e Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:30:55 +0100 Subject: [PATCH 06/19] bfd: make alloc_bfd() return NULL rather than false on error alloc_bfd() returns a bfd_t *, but in the case of errors it was returning false, which clearly should have been NULL. This issues was identified by compiling with -std=c23. Signed-off-by: Quentin Armitage --- keepalived/bfd/bfd_data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keepalived/bfd/bfd_data.c b/keepalived/bfd/bfd_data.c index bd717d50f..b739421a0 100644 --- a/keepalived/bfd/bfd_data.c +++ b/keepalived/bfd/bfd_data.c @@ -54,14 +54,14 @@ alloc_bfd(const char *name) " name too long (maximum length is %zu" " characters) - ignoring", name, sizeof(bfd->iname) - 1); - return false; + return NULL; } if (find_bfd_by_name(name)) { report_config_error(CONFIG_GENERAL_ERROR, "Configuration error: BFD instance %s" " already configured - ignoring", name); - return false; + return NULL; } PMALLOC(bfd); From 4bb488e3afc2a4a70849ff545c659cc6dc561379 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:33:24 +0100 Subject: [PATCH 07/19] all: use correct format specifier for time fields 32 bit Debian uses a 32 bit TIMESIZE, whereas 32 bit Ubuntu uses a 64 bit TIMESIZE. This means that on 32 bit Ubuntu some time types need to be printed using "%lld", whereas on 32 bit Debian, and on 64 bit systems "%ld" is what is needed. Using the wrong format specifier was causing compilation warnings on 32 bit Debian. The issue impacts printing time_t, struct timeval tv_sec and tv_usec and struct timespec tv_sec fields. Peversely, on a 32 bit system when TIMESIZE is 64, struct timeval tv_usec is 64 bits, whereas struct timesec tv_nsec is 32 bits. The commit adds configure time checking of the right format specifiers to use, and adds definitions PRI_time_t, PRI_tv_sec, PRI_ts_sec etc. Signed-off-by: Quentin Armitage --- configure.ac | 55 ++++++++++++++++++++++++++++ keepalived/bfd/bfd_data.c | 4 +- keepalived/check/check_http.c | 2 +- keepalived/check/check_misc.c | 2 +- keepalived/core/global_data.c | 2 +- keepalived/core/keepalived_netlink.c | 2 +- keepalived/core/main.c | 3 +- keepalived/vrrp/vrrp_daemon.c | 4 +- keepalived/vrrp/vrrp_data.c | 12 +++--- keepalived/vrrp/vrrp_if.c | 14 +++---- keepalived/vrrp/vrrp_scheduler.c | 10 ++--- lib/logger.c | 2 +- lib/scheduler.c | 6 +-- lib/timer.c | 2 +- lib/utils.c | 7 ++-- 15 files changed, 92 insertions(+), 35 deletions(-) diff --git a/configure.ac b/configure.ac index 557449a7c..d8e6b1dfc 100644 --- a/configure.ac +++ b/configure.ac @@ -656,6 +656,61 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ ]) CFLAGS=$SAV_CFLAGS +dnl -- printing time types (32 bit Ubuntu uses long long int for time_t timeval tv_sec/tv_usec and timespec tv_sec - but timespec tv_nsec is long int!) +dnl -- normally all the fields are long int +AC_MSG_CHECKING([time print types]) +AH_TEMPLATE([PRI_time_t], [Define for print format for time_t]) +AH_TEMPLATE([PRI_tv_sec], [Define for print format for struct timeval tv_sec]) +AH_TEMPLATE([PRI_tv_usec], [Define for print format for struct timeval tv_usec]) +AH_TEMPLATE([PRI_ts_sec], [Define for print format for struct timespec tv_sec]) +AH_TEMPLATE([PRI_ts_nsec], [Define for print format for struct timespec tv_nsec]) +SAV_CFLAGS=$CFLAGS +CFLAGS="$CFLAGS -Wformat -Wformat-signedness" +AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ + int main(void) + { + } + ]])], + [ + signs="d u" + WARN_SIGN="-Werror=format-signedness" + ], + [ + signs="d" + WARN_SIGN="" + ]) +CFLAGS="$CFLAGS -Werror=format $WARN_SIGN" +for field in t tv.tv_sec tv.tv_usec ts.tv_sec ts.tv_nsec; do + for sign in $signs; do + for len in "" l ll; do + AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ + #include + #include + #include + + int main(void) + { + time_t t = 1; + struct timeval tv = { .tv_sec = 2 }; + struct timespec ts = { .tv_sec = 3 }; + + printf("%$len$sign", $field); + } + ]])], + [ + if [[ $field = t ]]; then + name=time_t + else + name=$(<<<$field $SED -e "s/\.tv//") + fi + AC_DEFINE_UNQUOTED([PRI_$name], [ "$len$sign" ]) + ], []) + done + done +done +CFLAGS=$SAV_CFLAGS +AC_MSG_RESULT([done]) + dnl -- Check for diagnostic pragmas in functions - GCC 4.6.0 AC_MSG_CHECKING([diagnostic pragmas in functions]) AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ diff --git a/keepalived/bfd/bfd_data.c b/keepalived/bfd/bfd_data.c index b739421a0..abd758fba 100644 --- a/keepalived/bfd/bfd_data.c +++ b/keepalived/bfd/bfd_data.c @@ -120,7 +120,7 @@ conf_write_sands(FILE *fp, const char *text, unsigned long sands) secs = sands / TIMER_HZ; if (!ctime_r(&secs, time_str)) strcpy(time_str, "invalid time "); - conf_write(fp, " %s = %ld.%6.6lu (%.19s.%6.6lu)", text, secs, sands % TIMER_HZ, time_str, sands % TIMER_HZ); + conf_write(fp, " %s = %" PRI_time_t ".%6.6lu (%.19s.%6.6lu)", text, secs, sands % TIMER_HZ, time_str, sands % TIMER_HZ); } /* Dump BFD instance configuration parameters */ @@ -186,7 +186,7 @@ dump_bfd(FILE *fp, const bfd_t *bfd) conf_write(fp, " last_seen = [never]"); else { ctime_r(&bfd->last_seen.tv_sec, time_str); - conf_write(fp, " last seen = %ld.%6.6ld (%.24s.%6.6ld)", bfd->last_seen.tv_sec, bfd->last_seen.tv_usec, time_str, bfd->last_seen.tv_usec); + conf_write(fp, " last seen = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.24s.%6.6" PRI_tv_usec ")", bfd->last_seen.tv_sec, bfd->last_seen.tv_usec, time_str, bfd->last_seen.tv_usec); } } } diff --git a/keepalived/check/check_http.c b/keepalived/check/check_http.c index 9517a27e6..9d3dbb8e8 100644 --- a/keepalived/check/check_http.c +++ b/keepalived/check/check_http.c @@ -196,7 +196,7 @@ free_url(url_t *url) #ifdef _WITH_REGEX_TIMERS_ if (do_regex_timers) - log_message(LOG_INFO, "Total regex time %ld.%9.9ld, num match calls %u, num url checks %u", total_regex_times.tv_sec, total_regex_times.tv_nsec, total_num_matches, total_regex_urls); + log_message(LOG_INFO, "Total regex time %" PRI_ts_sec ".%9.9" PRI_ts_nsec ", num match calls %u, num url checks %u", total_regex_times.tv_sec, total_regex_times.tv_nsec, total_num_matches, total_regex_urls); #endif } } diff --git a/keepalived/check/check_misc.c b/keepalived/check/check_misc.c index f85ec6671..a9df6883d 100644 --- a/keepalived/check/check_misc.c +++ b/keepalived/check/check_misc.c @@ -76,7 +76,7 @@ dump_misc_check(FILE *fp, const checker_t *checker) conf_write(fp, " dynamic = %s", misck_checker->dynamic ? "YES" : "NO"); conf_write(fp, " uid:gid = %u:%u", misck_checker->script.uid, misck_checker->script.gid); ctime_r(&misck_checker->last_ran.tv_sec, time_str); - conf_write(fp, " Last ran = %ld.%6.6ld (%.24s.%6.6ld)", misck_checker->last_ran.tv_sec, misck_checker->last_ran.tv_usec, time_str, misck_checker->last_ran.tv_usec); + conf_write(fp, " Last ran = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.24s.%6.6" PRI_tv_usec ")", misck_checker->last_ran.tv_sec, misck_checker->last_ran.tv_usec, time_str, misck_checker->last_ran.tv_usec); conf_write(fp, " Last status = %u", misck_checker->last_exit_code); } diff --git a/keepalived/core/global_data.c b/keepalived/core/global_data.c index e8d7c5951..96a94cafd 100644 --- a/keepalived/core/global_data.c +++ b/keepalived/core/global_data.c @@ -746,7 +746,7 @@ dump_global_data(FILE *fp, data_t * data) conf_write(fp, " Gratuitous ARP delay = %u", data->vrrp_garp_delay/TIMER_HZ); conf_write(fp, " Gratuitous ARP repeat = %u", data->vrrp_garp_rep); - conf_write(fp, " Gratuitous ARP refresh timer = %ld", data->vrrp_garp_refresh.tv_sec); + conf_write(fp, " Gratuitous ARP refresh timer = %" PRI_tv_sec, data->vrrp_garp_refresh.tv_sec); conf_write(fp, " Gratuitous ARP refresh repeat = %u", data->vrrp_garp_refresh_rep); conf_write(fp, " Gratuitous ARP lower priority delay = %u", data->vrrp_garp_lower_prio_delay == PARAMETER_UNSET ? PARAMETER_UNSET : data->vrrp_garp_lower_prio_delay / TIMER_HZ); conf_write(fp, " Gratuitous ARP lower priority repeat = %u", data->vrrp_garp_lower_prio_rep); diff --git a/keepalived/core/keepalived_netlink.c b/keepalived/core/keepalived_netlink.c index da3596fdb..10f591236 100644 --- a/keepalived/core/keepalived_netlink.c +++ b/keepalived/core/keepalived_netlink.c @@ -121,7 +121,7 @@ report_and_clear_netlink_timers(const char * str) log_message(LOG_INFO, "Netlink timers - %s", str); for (i = 0; i <= MAX_NETLINK_TIMER; i++) { if (netlink_count[i]) { - log_message(LOG_INFO, " netlink cmd %d (%u calls), time %ld.%6.6ld", i, netlink_count[i], netlink_times[i].tv_sec, netlink_times[i].tv_usec); + log_message(LOG_INFO, " netlink cmd %d (%u calls), time %" PRI_tv_sec ".%6.6" PRI_tv_usec, i, netlink_count[i], netlink_times[i].tv_sec, netlink_times[i].tv_usec); netlink_times[i].tv_sec = netlink_times[i].tv_usec = netlink_count[i] = 0; } } diff --git a/keepalived/core/main.c b/keepalived/core/main.c index 9f34d9999..f596ad8e4 100644 --- a/keepalived/core/main.c +++ b/keepalived/core/main.c @@ -2885,7 +2885,8 @@ keepalived_main(int argc, char **argv) getrusage(RUSAGE_SELF, &usage); getrusage(RUSAGE_CHILDREN, &child_usage); - log_message(LOG_INFO, "CPU usage (self/children) user: %ld.%6.6ld/%ld.%6.6ld system: %ld.%6.6ld/%ld.%6.6ld", + log_message(LOG_INFO, "CPU usage (self/children) user: %" PRI_tv_sec ".%6.6" PRI_tv_usec "/%" PRI_tv_sec ".%6.6" PRI_tv_usec + " system: %" PRI_tv_sec ".%6.6" PRI_tv_usec "/%" PRI_tv_sec ".%6.6" PRI_tv_usec, usage.ru_utime.tv_sec, usage.ru_utime.tv_usec, child_usage.ru_utime.tv_sec, child_usage.ru_utime.tv_usec, usage.ru_stime.tv_sec, usage.ru_stime.tv_usec, child_usage.ru_stime.tv_sec, child_usage.ru_stime.tv_usec); } diff --git a/keepalived/vrrp/vrrp_daemon.c b/keepalived/vrrp/vrrp_daemon.c index c339ceca3..6201aa7e8 100644 --- a/keepalived/vrrp/vrrp_daemon.c +++ b/keepalived/vrrp/vrrp_daemon.c @@ -145,9 +145,9 @@ dump_vrrp_fd(void) else { timersub(&vrrp->sands, &time_now, &time_diff); if (time_diff.tv_sec >= 0) - log_message(LOG_INFO, " %s: sands %ld.%6.6ld", vrrp->iname, time_diff.tv_sec, time_diff.tv_usec); + log_message(LOG_INFO, " %s: sands %" PRI_tv_sec ".%6.6" PRI_tv_usec, vrrp->iname, time_diff.tv_sec, time_diff.tv_usec); else - log_message(LOG_INFO, " %s: sands -%ld.%6.6ld", vrrp->iname, -time_diff.tv_sec - (time_diff.tv_usec ? 1 : 0), time_diff.tv_usec ? 1000000 - time_diff.tv_usec : 0); + log_message(LOG_INFO, " %s: sands -%" PRI_tv_sec ".%6.6" PRI_tv_usec, vrrp->iname, -time_diff.tv_sec - (time_diff.tv_usec ? 1 : 0), time_diff.tv_usec ? 1000000 - time_diff.tv_usec : 0); } } diff --git a/keepalived/vrrp/vrrp_data.c b/keepalived/vrrp/vrrp_data.c index 87ffa4cac..61d98da0f 100644 --- a/keepalived/vrrp/vrrp_data.c +++ b/keepalived/vrrp/vrrp_data.c @@ -634,13 +634,13 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp) conf_write(fp, " Duplicate VRID"); #endif conf_write(fp, " Number of track scripts init = %u", vrrp->num_script_init); - conf_write(fp, " Last transition = %ld.%6.6ld (%s)", vrrp->last_transition.tv_sec, vrrp->last_transition.tv_usec, ctime_us_r(&vrrp->last_transition, time_str)); + conf_write(fp, " Last transition = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%s)", vrrp->last_transition.tv_sec, vrrp->last_transition.tv_usec, ctime_us_r(&vrrp->last_transition, time_str)); if (!ctime_r(&vrrp->sands.tv_sec, time_str)) strcpy(time_str, "invalid time "); if (vrrp->sands.tv_sec == TIMER_DISABLED) conf_write(fp, " Read timeout = DISABLED"); else - conf_write(fp, " Read timeout = %ld.%6.6ld (%s)", vrrp->sands.tv_sec, vrrp->sands.tv_usec, ctime_us_r(&vrrp->sands, time_str)); + conf_write(fp, " Read timeout = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%s)", vrrp->sands.tv_sec, vrrp->sands.tv_usec, ctime_us_r(&vrrp->sands, time_str)); conf_write(fp, " Master down timer = %u usecs", vrrp->ms_down_timer); } #ifdef _HAVE_VRRP_VMAC_ @@ -719,7 +719,7 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp) conf_write(fp, " Gratuitous ARP delay = %u", vrrp->garp_delay/TIMER_HZ); conf_write(fp, " Gratuitous ARP repeat = %u", vrrp->garp_rep); - conf_write(fp, " Gratuitous ARP refresh = %ld", + conf_write(fp, " Gratuitous ARP refresh = %" PRI_tv_sec, vrrp->garp_refresh.tv_sec); conf_write(fp, " Gratuitous ARP refresh repeat = %u", vrrp->garp_refresh_rep); conf_write(fp, " Gratuitous ARP lower priority delay = %u", vrrp->garp_lower_prio_delay / TIMER_HZ); @@ -727,9 +727,9 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp) conf_write(fp, " Down timer adverts = %u", vrrp->down_timer_adverts); #ifdef _HAVE_VRRP_VMAC_ if (vrrp->vmac_garp_intvl.tv_sec) { - conf_write(fp, " Gratuitous ARP for each secondary %s = %ld", __test_bit(VRRP_FLAG_VMAC_GARP_ALL_IF, &vrrp->flags) ? "i/f" : "VMAC", vrrp->vmac_garp_intvl.tv_sec); + conf_write(fp, " Gratuitous ARP for each secondary %s = %" PRI_time_t, __test_bit(VRRP_FLAG_VMAC_GARP_ALL_IF, &vrrp->flags) ? "i/f" : "VMAC", vrrp->vmac_garp_intvl.tv_sec); ctime_r(&vrrp->vmac_garp_timer.tv_sec, time_str); - conf_write(fp, " Next gratuitous ARP for such secondary = %ld.%6.6ld (%.24s.%6.6ld)", vrrp->vmac_garp_timer.tv_sec, vrrp->vmac_garp_timer.tv_usec, time_str, vrrp->vmac_garp_timer.tv_usec); + conf_write(fp, " Next gratuitous ARP for such secondary = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.24s.%6.6" PRI_tv_usec ")", vrrp->vmac_garp_timer.tv_sec, vrrp->vmac_garp_timer.tv_usec, time_str, vrrp->vmac_garp_timer.tv_usec); } #endif conf_write(fp, " Send advert after receive lower priority advert = %s", vrrp->lower_prio_no_advert ? "false" : "true"); @@ -746,7 +746,7 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp) (vrrp->version == VRRP_VERSION_2) ? (vrrp->adver_int / TIMER_HZ) : (vrrp->adver_int / (TIMER_HZ / 1000)), (vrrp->version == VRRP_VERSION_2) ? "sec" : "milli-sec"); - conf_write(fp, " Last advert sent = %ld.%6.6ld", vrrp->last_advert_sent.tv_sec, vrrp->last_advert_sent.tv_usec); + conf_write(fp, " Last advert sent = %" PRI_tv_sec ".%6.6" PRI_tv_usec, vrrp->last_advert_sent.tv_sec, vrrp->last_advert_sent.tv_usec); #ifdef _WITH_FIREWALL_ conf_write(fp, " Accept = %s", vrrp->accept ? "enabled" : "disabled"); #endif diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c index cb74d700c..7834355dd 100644 --- a/keepalived/vrrp/vrrp_if.c +++ b/keepalived/vrrp/vrrp_if.c @@ -419,17 +419,17 @@ dump_garp_delay(FILE *fp, const garp_delay_t *gd) conf_write(fp, "------< GARP delay group %d >------", gd->aggregation_group); if (gd->have_garp_interval) { - conf_write(fp, " GARP interval = %g", gd->garp_interval.tv_sec + ((double)gd->garp_interval.tv_usec) / 1000000); + conf_write(fp, " GARP interval = %" PRI_tv_sec "%6.6" PRI_tv_usec, gd->garp_interval.tv_sec, gd->garp_interval.tv_usec); if (!ctime_r(&gd->garp_next_time.tv_sec, time_str)) strcpy(time_str, "invalid time "); - conf_write(fp, " GARP next time %ld.%6.6ld (%.19s.%6.6ld)", gd->garp_next_time.tv_sec, gd->garp_next_time.tv_usec, time_str, gd->garp_next_time.tv_usec); + conf_write(fp, " GARP next time %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.19s.%6.6" PRI_tv_usec ")", gd->garp_next_time.tv_sec, gd->garp_next_time.tv_usec, time_str, gd->garp_next_time.tv_usec); } if (gd->have_gna_interval) { - conf_write(fp, " GNA interval = %g", gd->gna_interval.tv_sec + ((double)gd->gna_interval.tv_usec) / 1000000); + conf_write(fp, " GNA interval = %" PRI_tv_sec ".%6.6" PRI_tv_usec, gd->gna_interval.tv_sec, gd->gna_interval.tv_usec); if (!ctime_r(&gd->gna_next_time.tv_sec, time_str)) strcpy(time_str, "invalid time "); - conf_write(fp, " GNA next time %ld.%6.6ld (%.19s.%6.6ld)", gd->gna_next_time.tv_sec, gd->gna_next_time.tv_usec, time_str, gd->gna_next_time.tv_usec); + conf_write(fp, " GNA next time %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.19s.%6.6" PRI_tv_usec ")", gd->gna_next_time.tv_sec, gd->gna_next_time.tv_usec, time_str, gd->gna_next_time.tv_usec); } else if (!gd->have_garp_interval) conf_write(fp, " No configuration"); @@ -689,12 +689,12 @@ dump_if(FILE *fp, const interface_t *ifp) if (ifp->garp_delay) { if (ifp->garp_delay->have_garp_interval) - conf_write(fp, " Gratuitous ARP interval %ldms", + conf_write(fp, " Gratuitous ARP interval %" PRI_tv_sec "ms", ifp->garp_delay->garp_interval.tv_sec * 1000 + ifp->garp_delay->garp_interval.tv_usec / (TIMER_HZ / 1000)); if (ifp->garp_delay->have_gna_interval) - conf_write(fp, " Gratuitous NA interval %ldms", + conf_write(fp, " Gratuitous NA interval %" PRI_time_t "ms", ifp->garp_delay->gna_interval.tv_sec * 1000 + ifp->garp_delay->gna_interval.tv_usec / (TIMER_HZ / 1000)); if (ifp->garp_delay->aggregation_group) @@ -712,7 +712,7 @@ dump_if(FILE *fp, const interface_t *ifp) conf_write(fp, " Reset promote_secondaries counter %" PRIu32, ifp->reset_promote_secondaries); if (timerisset(&ifp->last_gna_router_check)) { ctime_r(&ifp->last_gna_router_check.tv_sec, time_str); - conf_write(fp, " %sIPv6 forwarding. Last checked %ld.%6.6ld (%.24s.%6.6ld)", ifp->gna_router ? "" : "Not ", ifp->last_gna_router_check.tv_sec, ifp->last_gna_router_check.tv_usec, time_str, ifp->last_gna_router_check.tv_usec); + conf_write(fp, " %sIPv6 forwarding. Last checked %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.24s.%6.6" PRI_tv_usec ")", ifp->gna_router ? "" : "Not ", ifp->last_gna_router_check.tv_sec, ifp->last_gna_router_check.tv_usec, time_str, ifp->last_gna_router_check.tv_usec); } diff --git a/keepalived/vrrp/vrrp_scheduler.c b/keepalived/vrrp/vrrp_scheduler.c index 8c1747c6e..e10f0c81e 100644 --- a/keepalived/vrrp/vrrp_scheduler.c +++ b/keepalived/vrrp/vrrp_scheduler.c @@ -1095,18 +1095,18 @@ vrrp_dispatcher_read(sock_t *sock) expected_cmsg = true; if (cmsg->cmsg_type == SO_TIMESTAMPNS) { strftime(time_buf, sizeof time_buf, "%T", localtime(&ts->tv_sec)); - log_message(LOG_INFO, "TIMESTAMPNS (socket %d - VRID %u) %s.%9.9ld" + log_message(LOG_INFO, "TIMESTAMPNS (socket %d - VRID %u) %s.%9.9" PRI_ts_nsec , sock->fd_in, hd->vrid, time_buf, ts->tv_nsec); } #if 0 if (cmsg->cmsg_type == SO_TIMESTAMP) { struct timeval *tv = (void *)CMSG_DATA(cmsg); - log_message(LOG_INFO, "TIMESTAMP message (%d - %u) %ld.%9.9ld" + log_message(LOG_INFO, "TIMESTAMP message (%d - %u) %" PRI_tv_sec ".%6.6" PRI_tv_usec , sock->fd_in, hd->vrid, tv->tv_sec, tv->tv_usec); } else if (cmsg->cmsg_type == SO_TIMESTAMPING) { struct timespec *ts = (void *)CMSG_DATA(cmsg); - log_message(LOG_INFO, "TIMESTAMPING message (%d - %u) %ld.%9.9ld, raw %ld.%9.9ld" + log_message(LOG_INFO, "TIMESTAMPING message (%d - %u) %" PRI_ts_sec ".%9.9" PRI_ts_nsec ", raw %" PRI_ts_sec ".%9.9" PRI_ts_nsec , sock->fd_in, hd->vrid, ts->tv_sec, ts->tv_nsec, (ts+2)->tv_sec, (ts+2)->tv_nsec); } #endif @@ -1484,7 +1484,7 @@ dump_threads(void) set_time_now(); ctime_r(&time_now.tv_sec, time_buf); - fprintf(fp, "\n%.19s.%6.6ld: Thread dump\n", time_buf, time_now.tv_usec); + fprintf(fp, "\n%.19s.%6.6" PRI_tv_usec ": Thread dump\n", time_buf, time_now.tv_usec); dump_thread_data(master, fp); @@ -1493,7 +1493,7 @@ dump_threads(void) fprintf(fp, "\n"); list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list) { ctime_r(&vrrp->sands.tv_sec, time_buf); - fprintf(fp, "VRRP instance %s, sands %.19s.%6.6ld, status %s\n", vrrp->iname, time_buf, vrrp->sands.tv_usec, + fprintf(fp, "VRRP instance %s, sands %.19s.%6.6" PRI_tv_usec ", status %s\n", vrrp->iname, time_buf, vrrp->sands.tv_usec, vrrp->state == VRRP_STATE_INIT ? "INIT" : vrrp->state == VRRP_STATE_BACK ? "BACKUP" : vrrp->state == VRRP_STATE_MAST ? "MASTER" : diff --git a/lib/logger.c b/lib/logger.c index 0e9d59787..9e62ca9ce 100644 --- a/lib/logger.c +++ b/lib/logger.c @@ -171,7 +171,7 @@ vlog_message(const int facility, const char* format, va_list args) if (log_file) { p = timestamp; p += strftime(timestamp, sizeof(timestamp), "%a %b %d %T", &tm); - p += snprintf(p, timestamp + sizeof(timestamp) - p, ".%9.9ld", ts.tv_nsec); + p += snprintf(p, timestamp + sizeof(timestamp) - p, ".%9.9" PRI_ts_nsec, ts.tv_nsec); strftime(p, timestamp + sizeof(timestamp) - p, " %Y", &tm); fprintf(log_file, "%s: %s\n", timestamp, buf); if (always_flush_log_file) diff --git a/lib/scheduler.c b/lib/scheduler.c index 4ff001f01..fd153416b 100644 --- a/lib/scheduler.c +++ b/lib/scheduler.c @@ -350,7 +350,7 @@ thread_set_timer(thread_master_t *m) #ifdef _EPOLL_DEBUG_ if (do_epoll_debug) - log_message(LOG_INFO, "Setting timer_fd %ld.%9.9ld", its.it_value.tv_sec, its.it_value.tv_nsec); + log_message(LOG_INFO, "Setting timer_fd %" PRI_ts_sec ".%9.9" PRI_ts_nsec, its.it_value.tv_sec, its.it_value.tv_nsec); #endif return timer_wait_time; @@ -830,10 +830,10 @@ timer_delay(timeval_t sands) if (timercmp(&sands, &time_now, >=)) { sands = timer_sub_now(sands); - snprintf(str, sizeof str, "%ld.%6.6ld", sands.tv_sec, sands.tv_usec); + snprintf(str, sizeof str, "%" PRI_tv_sec ".%6.6" PRI_tv_usec, sands.tv_sec, sands.tv_usec); } else { timersub(&time_now, &sands, &sands); - snprintf(str, sizeof str, "-%ld.%6.6ld", sands.tv_sec, sands.tv_usec); + snprintf(str, sizeof str, "-%" PRI_tv_sec ".%6.6" PRI_tv_usec, sands.tv_sec, sands.tv_usec); } return str; diff --git a/lib/timer.c b/lib/timer.c index 5bd51fe75..1bfec6dbb 100644 --- a/lib/timer.c +++ b/lib/timer.c @@ -196,7 +196,7 @@ set_time_now(void) #ifdef _TIMER_CHECK_ if (do_timer_check) { unsigned long timediff = (time_now.tv_sec - last_time.tv_sec) * 1000000 + time_now.tv_usec - last_time.tv_usec; - log_message(LOG_INFO, "set_time_now called from %s %s:%d, time %ld.%6.6ld difference %lu usec", file, function, line_no, time_now.tv_sec, time_now.tv_usec, timediff); + log_message(LOG_INFO, "set_time_now called from %s %s:%d, time %" PRI_tv_sec ".%6.6" PRI_tv_usec " difference %lu usec", file, function, line_no, time_now.tv_sec, time_now.tv_usec, timediff); last_time = time_now; } #endif diff --git a/lib/utils.c b/lib/utils.c index 73a5c1b6b..aeea91d08 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -1018,7 +1018,7 @@ ctime_us_r(const timeval_t *timep, char *buf) localtime_r(&timep->tv_sec, &tm); asctime_r(&tm, buf); - snprintf(buf + 19, 8, ".%6.6ld", timep->tv_usec); + snprintf(buf + 19, 8, ".%6.6" PRI_tv_usec, timep->tv_usec); strftime(buf + 26, 6, " %Y", &tm); return buf; @@ -1389,11 +1389,12 @@ log_stopping(void) getrusage(RUSAGE_CHILDREN, &child_usage); if (child_usage.ru_utime.tv_sec || child_usage.ru_utime.tv_usec) - log_message(LOG_INFO, "Stopped - used (self/children) %ld.%6.6ld/%ld.%6.6ld user time, %ld.%6.6ld/%ld.%6.6ld system time", + log_message(LOG_INFO, "Stopped - used (self/children) %" PRI_tv_sec ".%6.6" PRI_tv_usec "/%" PRI_tv_sec ".%6.6" PRI_tv_usec " user time," + " %" PRI_tv_sec ".%6.6" PRI_tv_usec "/%" PRI_tv_sec ".%6.6" PRI_tv_usec " system time", usage.ru_utime.tv_sec, usage.ru_utime.tv_usec, child_usage.ru_utime.tv_sec, child_usage.ru_utime.tv_usec, usage.ru_stime.tv_sec, usage.ru_stime.tv_usec, child_usage.ru_stime.tv_sec, child_usage.ru_stime.tv_usec); else - log_message(LOG_INFO, "Stopped - used %ld.%6.6ld user time, %ld.%6.6ld system time", + log_message(LOG_INFO, "Stopped - used %" PRI_tv_sec ".%6.6" PRI_tv_usec " user time, %" PRI_tv_sec ".%6.6" PRI_tv_usec " system time", usage.ru_utime.tv_sec, usage.ru_utime.tv_usec, usage.ru_stime.tv_sec, usage.ru_stime.tv_usec); } else log_message(LOG_INFO, "Stopped"); From 6a9224faa0014a3fafb81dc8c2a744418fe85c87 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:49:47 +0100 Subject: [PATCH 08/19] vrrp ipvs: fix warnings related to signedness of statfs() f_flags On 32 bit systems, struct statfs field f_type is unsigned on 32 bit systems, but signed on 64 bit systems. This caused warnings on 32 bit systems about comparing signed and unsigned when comparing against defined values with the most significat bit was set. This commit copies the f_type field into a long long type which resolves the warnings. Signed-off-by: Quentin Armitage --- keepalived/trackers/track_file.c | 69 +++++++++++++++++++------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/keepalived/trackers/track_file.c b/keepalived/trackers/track_file.c index 72cdb2410..62bbca770 100644 --- a/keepalived/trackers/track_file.c +++ b/keepalived/trackers/track_file.c @@ -435,36 +435,49 @@ track_file_end_handler(void) } log_message(LOG_INFO, "statfs(%s) returned errno %d (%m)", track_file->fname, errno); - } else if (fs_buf.f_flags & ST_RDONLY || - fs_buf.f_type == BPF_FS_MAGIC || - fs_buf.f_type == CGROUP_SUPER_MAGIC || - fs_buf.f_type == CGROUP2_SUPER_MAGIC || - fs_buf.f_type == DEBUGFS_MAGIC || - fs_buf.f_type == DEVPTS_SUPER_MAGIC || - fs_buf.f_type == EFIVARFS_MAGIC || - fs_buf.f_type == FUSE_SUPER_MAGIC || - fs_buf.f_type == MQUEUE_MAGIC || - fs_buf.f_type == NFS_SUPER_MAGIC || - fs_buf.f_type == PIPEFS_MAGIC || - fs_buf.f_type == PROC_SUPER_MAGIC || - fs_buf.f_type == ROMFS_MAGIC || - fs_buf.f_type == SELINUX_MAGIC || - fs_buf.f_type == SMB_SUPER_MAGIC || - fs_buf.f_type == SMB2_MAGIC_NUMBER || - fs_buf.f_type == SOCKFS_MAGIC || - fs_buf.f_type == SYSFS_MAGIC || - fs_buf.f_type == SYSV2_SUPER_MAGIC || - fs_buf.f_type == SYSV4_SUPER_MAGIC || - fs_buf.f_type == TRACEFS_MAGIC) { - /* The specified file is on a type of filesystem that inotify cannot monitor, - * or the filesystem is read-only. */ - report_config_error(CONFIG_GENERAL_ERROR, - "The filesystem of %s is read only or cannot be monitored - ignoring", track_file->file_path); + } else { + /* This is a workaround to the problem of fs_buf.f_type being unsigned int (32 bits) + * on most 32 bit platforms, but signed long (64 bits) on 64 bit systems. + * Without this workaround, comparison of fs_buf.f_type against, for example, + * BPF_FS_MAGIC causes a signed/unsigned warning on 32 bit systems. + * This also applies with EFIVARS_MAGIC, SELINUX_MAGIC and SMB2_MAGIC_NUMBER, + * i.e. when bit 31 (0x80000000) is set. + * + * See /usr/include/asm-generic/statfs.h for more details. + */ + long long f_type = fs_buf.f_type; + + if (fs_buf.f_flags & ST_RDONLY || + f_type == BPF_FS_MAGIC || + f_type == CGROUP_SUPER_MAGIC || + f_type == CGROUP2_SUPER_MAGIC || + f_type == DEBUGFS_MAGIC || + f_type == DEVPTS_SUPER_MAGIC || + f_type == EFIVARFS_MAGIC || + f_type == FUSE_SUPER_MAGIC || + f_type == MQUEUE_MAGIC || + f_type == NFS_SUPER_MAGIC || + f_type == PIPEFS_MAGIC || + f_type == PROC_SUPER_MAGIC || + f_type == ROMFS_MAGIC || + f_type == SELINUX_MAGIC || + f_type == SMB_SUPER_MAGIC || + f_type == SMB2_MAGIC_NUMBER || + f_type == SOCKFS_MAGIC || + f_type == SYSFS_MAGIC || + f_type == SYSV2_SUPER_MAGIC || + f_type == SYSV4_SUPER_MAGIC || + f_type == TRACEFS_MAGIC) { + /* The specified file is on a type of filesystem that inotify cannot monitor, + * or the filesystem is read-only. */ + report_config_error(CONFIG_GENERAL_ERROR, + "The filesystem of %s is read only or cannot be monitored - ignoring", track_file->file_path); - FREE_CONST(track_file->fname); - FREE(track_file); + FREE_CONST(track_file->fname); + FREE(track_file); - return; + return; + } } if (track_file_init != TRACK_FILE_NO_INIT) { From 55eb6457a5d6915201a4257eaac111055d30ffcd Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:56:04 +0100 Subject: [PATCH 09/19] vrrp ipvs: remove unused definition of XENFS_SUPER_MAGIC Signed-off-by: Quentin Armitage --- keepalived/trackers/track_file.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/keepalived/trackers/track_file.c b/keepalived/trackers/track_file.c index 62bbca770..384fdf4fe 100644 --- a/keepalived/trackers/track_file.c +++ b/keepalived/trackers/track_file.c @@ -117,9 +117,6 @@ #ifndef TRACEFS_MAGIC #define TRACEFS_MAGIC 0x74726163 #endif -#ifndef XENFS_SUPER_MAGIC -#define XENFS_SUPER_MAGIC 0xabba1974 -#endif /* Used for initialising track files */ From d04635a19fc2258b861fdc2ac515e61d770d8805 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:56:40 +0100 Subject: [PATCH 10/19] build: don't redefine FORTIFY_SOURCE if host environment defines it Signed-off-by: Quentin Armitage --- configure.ac | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index d8e6b1dfc..ff762482d 100644 --- a/configure.ac +++ b/configure.ac @@ -425,6 +425,15 @@ ENABLE_LOG_FILE_APPEND=No # AC_PROG_LIBTOOL +# Ensure we don't override FORTIFY_SOURCE +ADD_FORTIFY_SOURCE=1 +grep -q -- "-D *_FORTIFY_SOURCE=" <<<$CPPFLAGS +AS_IF([test $? -eq 0], + [ + FORTIFY_SOURCE=$(<<<$CPPFLAGS $SED -e "s/.*-D *_FORTIFY_SOURCE=//" -e "s/ .*//") + ADD_FORTIFY_SOURCE=0 + ],[FORTIFY_SOURCE=2]) + # # save the configure arguments # @@ -761,7 +770,8 @@ if test "$enable_conversion_checks" = yes; then # Check if we can sensibly enable -Wconversion AC_MSG_CHECKING([for usable -Wconversion]) SAV_CFLAGS="$CFLAGS" - CFLAGS="$CFLAGS -Wconversion -O2 -Wp,-D_FORTIFY_SOURCE=2 -Werror" + CFLAGS="$CFLAGS -Wconversion -O2 -Werror" + AS_IF([test $ADD_FORTIFY_SOURCE -eq 1], [CFLAGS="$CFLAGS -Wp,-D_FORTIFY_SOURCE=$FORTIFY_SOURCE"]) AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ #include #include @@ -995,7 +1005,7 @@ if test "$enable_hardening" != no; then for FLAG in \ "-Wformat -Werror=format-security" \ - "-Wp,-D_FORTIFY_SOURCE=2" \ + "-Wp,-D_FORTIFY_SOURCE=$FORTIFY_SOURCE" \ "-fexceptions" \ "-fstack-protector-strong" \ "--param=ssp-buffer-size=4" \ From 8bead9cfedcc981f7cdd7ec4665cc010aba1283f Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:58:02 +0100 Subject: [PATCH 11/19] ipvs: resolve a "cast increases required alignment" warning Signed-off-by: Quentin Armitage --- keepalived/check/check_parser.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/keepalived/check/check_parser.c b/keepalived/check/check_parser.c index 037d8f7b2..ce15fe49f 100644 --- a/keepalived/check/check_parser.c +++ b/keepalived/check/check_parser.c @@ -545,12 +545,18 @@ pgr_handler(const vector_t *strvec) } #endif - /* Cast via void * to stop -Wcast-align warning. - * Since alignment of struct addrinfo >= alignment of struct sockaddr_in and res->ai_addr is - * aligned to a struct addrinfo, it is not a problem. - * e.g. vs->persistence_granularity = ((struct sockaddr_in *)((void *)res->ai_addr))->sin_addr.s_addr; + /* On 32 bit systems, gcc produces a warning : cast increases required alignment of target type [-Wcast-align] + * for + * current_vs->persistence_granularity = ((struct sockaddr_in *)res->ai_addr)->sin_addr.s_addr; */ - current_vs->persistence_granularity = ((struct sockaddr_in *)res->ai_addr)->sin_addr.s_addr; + union { + struct sockaddr sa; + struct sockaddr_in sa_in; + } sa; + + sa.sa = *res->ai_addr; + current_vs->persistence_granularity = sa.sa_in.sin_addr.s_addr; + freeaddrinfo(res); } From 835d8c63f59e394613b2c97d87364118638df8e5 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 20:59:10 +0100 Subject: [PATCH 12/19] core: use /* FALLTHROUGH */ rather than __fallthrough; It would be good to be able to use the standardized "[[fallthrough]];" which is added in C23, but we are a long way from being able to specify C23 was the base standard. Signed-off-by: Quentin Armitage --- keepalived/core/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepalived/core/main.c b/keepalived/core/main.c index f596ad8e4..a9c7f2e23 100644 --- a/keepalived/core/main.c +++ b/keepalived/core/main.c @@ -2271,7 +2271,7 @@ parse_cmdline(int argc, char **argv) set_core_dump_pattern = true; if (optarg && optarg[0]) core_dump_pattern = optarg; - __fallthrough; + /* FALLTHROUGH */ case 'm': create_core_dump = true; break; From a2aa422733c27837050c52f2ff67a16f9432e841 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:02:10 +0100 Subject: [PATCH 13/19] all: suppress an increases alignment warning Signed-off-by: Quentin Armitage --- keepalived/core/keepalived_netlink.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/keepalived/core/keepalived_netlink.c b/keepalived/core/keepalived_netlink.c index 10f591236..af15eeead 100644 --- a/keepalived/core/keepalived_netlink.c +++ b/keepalived/core/keepalived_netlink.c @@ -1545,8 +1545,12 @@ netlink_request(nl_handle_t *nl, req.nlh.nlmsg_flags |= NLM_F_DUMP; #if HAVE_DECL_RTEXT_FILTER_SKIP_STATS /* The following produces a -Wstringop-overflow warning due to writing - * 4 bytes into a region of size 0. This is, however, safe. */ + * 4 bytes into a region of size 0. This is, however, safe. + * By GCC 14 the warning is -Warray-bounds= + */ +RELAX_ARRAY_BOUNDS_START addattr32(&req.nlh, sizeof req, IFLA_EXT_MASK, RTEXT_FILTER_SKIP_STATS); +RELAX_ARRAY_BOUNDS_END #endif status = sendto(nl->fd, (void *) &req, sizeof (req) From 5f58b5341f236b69bca5502179ebad25979314fb Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:04:34 +0100 Subject: [PATCH 14/19] lib: add file missing from previous commit Signed-off-by: Quentin Armitage --- lib/warnings.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/warnings.h b/lib/warnings.h index b1afdcf3b..a9a2b15c1 100644 --- a/lib/warnings.h +++ b/lib/warnings.h @@ -17,7 +17,7 @@ * as published by the Free Software Foundation; either version * 2 of the License, or (at your option) any later version. * - * Copyright (C) 2019-2019 Alexandre Cassen, + * Copyright (C) 2019-2024 Alexandre Cassen, */ #ifndef _WARNINGS_H @@ -139,4 +139,14 @@ _Pragma("GCC diagnostic ignored \"-Winline\"") #define RELAX_INLINE_END #endif +#if defined _HAVE_DIAGNOSTIC_PUSH_POP_PRAGMAS_ && defined _HAVE_WARNING_ARRAY_BOUNDS_ +#define RELAX_ARRAY_BOUNDS_START \ +_Pragma("GCC diagnostic push") \ +_Pragma("GCC diagnostic ignored \"-Warray-bounds\"") +#define RELAX_ARRAY_BOUNDS_END RELAX_END +#else +#define RELAX_ARRAY_BOUNDS_START +#define RELAX_ARRAY_BOUNDS_END +#endif + #endif From 4a1c5050ca8d32833c382b780484adf263b9293e Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:05:09 +0100 Subject: [PATCH 15/19] core: update addattr_l to match current iproute2 code - almost The alignment calculations were not coerect, so this commit updates addattr_l to match the iproute2 version, EXCEPT there appears to be 1 issue in the iproute2 code when NLMSG_ALIGN is used when RTA_ALIGN should be used. The difference is entirely cosmetic (at the moment) since the functionality of the 2 macros is currently identical. Signed-off-by: Quentin Armitage --- keepalived/core/keepalived_netlink.c | 72 +++++++++++++++------------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/keepalived/core/keepalived_netlink.c b/keepalived/core/keepalived_netlink.c index af15eeead..418732fc8 100644 --- a/keepalived/core/keepalived_netlink.c +++ b/keepalived/core/keepalived_netlink.c @@ -680,20 +680,21 @@ netlink_close(nl_handle_t *nl) int GCC_LTO_NOINLINE addattr_l(struct nlmsghdr *n, size_t maxlen, unsigned short type, const void *data, size_t alen) { - size_t len = RTA_LENGTH(alen); - size_t align_len = NLMSG_ALIGN(len); + unsigned short len = RTA_LENGTH(alen); + uint32_t align_len = RTA_SPACE(alen); struct rtattr *rta; - if (n->nlmsg_len + align_len > maxlen) + if (NLMSG_ALIGN(n->nlmsg_len) + align_len > maxlen) return -1; - rta = PTR_CAST(struct rtattr, (((char *)n) + n->nlmsg_len)); + rta = PTR_CAST(struct rtattr, NLMSG_TAIL(n)); rta->rta_type = type; - rta->rta_len = (unsigned short)len; + rta->rta_len = len; RELAX_STRINGOP_OVERFLOW - memcpy(RTA_DATA(rta), data, alen); + if (alen) + memcpy(RTA_DATA(rta), data, alen); RELAX_STRINGOP_OVERFLOW_END - n->nlmsg_len += (uint32_t)align_len; + n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + align_len; return 0; } @@ -702,19 +703,21 @@ RELAX_STRINGOP_OVERFLOW_END int addattr_l2(struct nlmsghdr *n, size_t maxlen, unsigned short type, const void *data, size_t alen, const void *data2, size_t alen2) { - size_t len = RTA_LENGTH(alen + alen2); - size_t align_len = NLMSG_ALIGN(len); + unsigned short len = RTA_LENGTH(alen); + uint32_t align_len = RTA_SPACE(alen + alen2); struct rtattr *rta; - if (n->nlmsg_len + align_len > maxlen) + if (NLMSG_ALIGN(n->nlmsg_len) + align_len > maxlen) return -1; - rta = PTR_CAST(struct rtattr, (((char *)n) + n->nlmsg_len)); + rta = PTR_CAST(struct rtattr, NLMSG_TAIL(n)); rta->rta_type = type; - rta->rta_len = (unsigned short)len; - memcpy(RTA_DATA(rta), data, alen); - memcpy((char *)RTA_DATA(rta) + alen, data2, alen2); - n->nlmsg_len += (uint32_t)align_len; + rta->rta_len = len; + if (alen) + memcpy(RTA_DATA(rta), data, alen); + if (alen2) + memcpy((char *)RTA_DATA(rta) + alen, data2, alen2); + n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + align_len; return 0; } @@ -722,15 +725,15 @@ addattr_l2(struct nlmsghdr *n, size_t maxlen, unsigned short type, const void *d int addraw_l(struct nlmsghdr *n, size_t maxlen, const void *data, size_t len) { - size_t align_len = NLMSG_ALIGN(len); + uint32_t align_len = NLMSG_ALIGN(len); - if (n->nlmsg_len + align_len > maxlen) + if (NLMSG_ALIGN(n->nlmsg_len) + align_len > maxlen) return -1; memcpy(NLMSG_TAIL(n), data, len); if (align_len > len) memset(PTR_CAST(char, NLMSG_TAIL(n)) + len, 0, align_len - len); - n->nlmsg_len += (uint32_t)align_len; + n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + align_len; return 0; } @@ -739,17 +742,18 @@ rta_addattr_l(struct rtattr *rta, size_t maxlen, unsigned short type, const void *data, size_t alen) { struct rtattr *subrta; - size_t len = RTA_LENGTH(alen); - size_t align_len = RTA_ALIGN(len); + unsigned short len = RTA_LENGTH(alen); + unsigned short align_len = RTA_SPACE(alen); - if (rta->rta_len + align_len > maxlen) + if (RTA_ALIGN(rta->rta_len) + align_len > maxlen) return 0; - subrta = PTR_CAST(struct rtattr, (char *)rta + rta->rta_len); + subrta = PTR_CAST(struct rtattr, (char *)rta + RTA_ALIGN(rta->rta_len)); subrta->rta_type = type; - subrta->rta_len = (unsigned short)len; - memcpy(RTA_DATA(subrta), data, alen); - rta->rta_len = (unsigned short)(rta->rta_len + align_len); + subrta->rta_len = len; + if (alen) + memcpy(RTA_DATA(subrta), data, alen); + rta->rta_len = RTA_ALIGN(rta->rta_len) + align_len; return align_len; } @@ -759,18 +763,20 @@ rta_addattr_l2(struct rtattr *rta, size_t maxlen, unsigned short type, const void *data2, size_t alen2) { struct rtattr *subrta; - size_t len = RTA_LENGTH(alen + alen2); - size_t align_len = RTA_ALIGN(len); + unsigned short len = RTA_LENGTH(alen + alen2); + unsigned short align_len = RTA_ALIGN(len); - if (rta->rta_len + align_len > maxlen) + if (RTA_ALIGN(rta->rta_len) + align_len > maxlen) return 0; - subrta = PTR_CAST(struct rtattr, (((char*)rta) + rta->rta_len)); + subrta = PTR_CAST(struct rtattr, (char*)rta + RTA_ALIGN(rta->rta_len)); subrta->rta_type = type; - subrta->rta_len = (unsigned short)len; - memcpy(RTA_DATA(subrta), data, alen); - memcpy((char *)RTA_DATA(subrta) + alen, data2, alen2); - rta->rta_len = (unsigned short)(rta->rta_len + align_len); + subrta->rta_len = len; + if (alen) + memcpy(RTA_DATA(subrta), data, alen); + if (alen2) + memcpy((char *)RTA_DATA(subrta) + alen, data2, alen2); + rta->rta_len = RTA_ALIGN(rta->rta_len) + align_len; return align_len; } From d5873b6531bf32ee78358353b798963be011efff Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:09:04 +0100 Subject: [PATCH 16/19] lib: add micro-second timers to memory allocation debugging Previously the time was logged for memory allocation/freeing operations in seconds. When comparing when memory was allocated/freed to debugging logging via a log file, it was helpful, in terms of being able to identify the sequence of events, to have the time of memory allocations etc logged in micro-seconds. Signed-off-by: Quentin Armitage --- lib/memory.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/memory.c b/lib/memory.c index 0d547eb9c..b5687b17f 100644 --- a/lib/memory.c +++ b/lib/memory.c @@ -169,7 +169,7 @@ enum slot_type { ALLOCATED, } ; -#define TIME_STR_LEN 9 +#define TIME_STR_LEN 16 /* "00:00:00.000000 " */ #if ULONG_MAX == 0xffffffffffffffffUL #define CHECK_VAL 0xa5a55a5aa5a55a5aUL @@ -213,8 +213,10 @@ static const char * format_time(void) { static char time_buf[TIME_STR_LEN+1]; + size_t len; - strftime(time_buf, sizeof time_buf, "%T ", localtime(&time_now.tv_sec)); + len = strftime(time_buf, sizeof time_buf, "%T", localtime(&time_now.tv_sec)); + snprintf(time_buf + len, sizeof(time_buf) - len, ".%6.6" PRI_tv_usec " ", time_now.tv_usec); return time_buf; } From 7e04261d551ea2902af92f99d8b2e423d05fbc99 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:12:44 +0100 Subject: [PATCH 17/19] lib: fix corruption of master-child_pid red black tree Child process thread_t structures use two red-black trees, one for the timeout, and the other for pids. It is important to ensure that threads are removed from the child_pid RB tree at the correct time. This was not happening when reloads were occurring and there was a THREAD_CHILD_TIMEOUT thread on the ready list. A few other instances of the thread not being removed from the child_pid RB tree correctly, which are also resolved by this commit. Signed-off-by: Quentin Armitage --- lib/scheduler.c | 57 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/lib/scheduler.c b/lib/scheduler.c index fd153416b..f096391b5 100644 --- a/lib/scheduler.c +++ b/lib/scheduler.c @@ -944,7 +944,7 @@ thread_add_unuse(thread_master_t *m, thread_t *thread) /* Move list element to unuse queue */ static void -thread_destroy_list(thread_master_t *m, list_head_t *l) +thread_destroy_list(thread_master_t *m, list_head_t *l, bool keep_children) { thread_t *thread, *thread_tmp; @@ -969,7 +969,8 @@ thread_destroy_list(thread_master_t *m, list_head_t *l) /* Do we need to free arg? */ if (thread->u.f.flags & THREAD_DESTROY_FREE_ARG) FREE(thread->arg); - } + } else if (thread->type == THREAD_CHILD_TIMEOUT && keep_children) + rb_erase(&thread->rb_data, &m->child_pid); list_del_init(&thread->e_list); thread_add_unuse(m, thread); @@ -982,6 +983,11 @@ thread_destroy_rb(thread_master_t *m, rb_root_cached_t *root) thread_t *thread; thread_t *thread_sav; + /* We use rbtree_postorder_for_each_entry_safe() so that we + * can simply invalidate the thread structures as they are + * removed, and not have to call rb_erase() for each entry, + * possibly causing a tree rebalance each time. + */ rbtree_postorder_for_each_entry_safe(thread, thread_sav, &root->rb_root, n) { /* The following are relevant for the read and write rb lists */ if (thread->type == THREAD_READ || @@ -1018,11 +1024,15 @@ thread_cleanup_master(thread_master_t * m, bool keep_children) thread_destroy_rb(m, &m->timer); if (!keep_children) thread_destroy_rb(m, &m->child); - thread_destroy_list(m, &m->event); + thread_destroy_list(m, &m->event, false); #ifdef USE_SIGNAL_THREADS - thread_destroy_list(m, &m->signal); + thread_destroy_list(m, &m->signal, false); #endif - thread_destroy_list(m, &m->ready); + thread_destroy_list(m, &m->ready, keep_children); + + /* This is an optimisation to avoid having to call + * rb_erase() for each thread that is removed from + * the master->child_pid RB tree. */ if (!keep_children) m->child_pid = RB_ROOT; @@ -1411,7 +1421,32 @@ thread_add_timer_shutdown(thread_master_t *m, thread_func_t func, void *arg, uns return thread.cp; } -/* Add a child thread. */ +/* Add a child thread. + * + * NOTE: + * A child thread is added with thread type THREAD_CHILD, and the thread is on the + * master->child and master->child_pid RB trees. + * After a child terminates, the thread type is changed to THREAD_CHILD_TERMINATED + * and it is removed from both RB trees, and added to the READY list. + * If a child times out, the thread type is changed to THREAD_CHILD_TIMEOUT, it + * is removed from the master->child RB tree, and added to the READY list. + * It is LEFT ON the master->child_pid RB tree, in case it subsequently terminates, + * when the thread type is changed from THREAD_CHILD_TIMEOUT to THREAD_CHILD_TERMINATED. + * When the thread type is changed, the thread must be removed from the + * master->child_pid RB tree. + * Further, if a thread of type THREAD_CHILD or THREAD_CHILD_TIMEOUT is removed from + * a queue, e.g. in thread_cancel() or thread_destroy_list(), it MUST be removed + * from the master->child_pid RB tree. + * + * If thread_destroy_list() is called and some child threads are being kept (i.e. at a + * reload), then any child threads on the ready list that are still on the child_pid + * RB tree (i.e. thread type THREAD_CHILD_TIMEOUT), the threads must be explicitly + * removed from the child_pid RB tree. + * + * Failure to remove the thread from the master->child_pid RB tree can cause segfaults + * to occur, and these are difficult to track down since the error occurred some + * time before the segfault. + */ thread_ref_t thread_add_child(thread_master_t * m, thread_func_t func, void * arg, pid_t pid, unsigned long timer) { @@ -1453,6 +1488,7 @@ thread_children_reschedule(thread_master_t *m, thread_func_t func, unsigned long rb_for_each_entry_cached(thread, &m->child, n) { thread->func = func; thread->sands = timer_add_long(time_now, timer); + thread->arg = NULL; // The object being referenced will have been freed } } @@ -1588,14 +1624,16 @@ thread_cancel(thread_ref_t thread_cp) thread_event_del(thread, THREAD_FL_EPOLL_WRITE_BIT); list_del_init(&thread->e_list); break; + case THREAD_CHILD_TIMEOUT: + rb_erase(&thread->rb_data, &master->child_pid); + /* FALLTHROUGH */ + case THREAD_CHILD_TERMINATED: case THREAD_EVENT: case THREAD_READY: case THREAD_READY_TIMER: #ifdef USE_SIGNAL_THREADS case THREAD_SIGNAL: #endif - case THREAD_CHILD_TIMEOUT: - case THREAD_CHILD_TERMINATED: list_del_init(&thread->e_list); break; case THREAD_TIMER_SHUTDOWN: @@ -2079,7 +2117,7 @@ process_threads(thread_master_t *m) thread_type = thread->type; if (thread->type == THREAD_CHILD_TIMEOUT) { - /* We remove the thread from the child_pid queue here so that + /* We remove the thread from the child_pid RB tree here so that * if the termination arrives before we processed the timeout * we can still handle the termination. */ rb_erase(&thread->rb_data, &master->child_pid); @@ -2172,6 +2210,7 @@ process_child_termination(pid_t pid, int status) * and it is still on the thread->ready queue. Since we have now got * the termination, just handle the termination instead. */ thread->type = THREAD_CHILD_TERMINATED; + rb_erase(&thread->rb_data, &master->child_pid); } else thread_move_ready(m, &m->child, thread, THREAD_CHILD_TERMINATED); From 99d73a3217b58377d0dccae6aeb5ca0132eb9be5 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:17:16 +0100 Subject: [PATCH 18/19] vrrp: on reload only configured track_script name was checked On a reload, only the configured name of a track_script was being checked to see if the new config track_script matched the old config track_script. If the script to be executed were changed, but the configured named of the script were kept the same, then the status of the old script would be transferred to the new script, despite the scripts being completely different. This commit now checks that the script really is the same, in terms of the path, parameters and user executing the script. Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c index cfa4220d6..152659882 100644 --- a/keepalived/vrrp/vrrp.c +++ b/keepalived/vrrp/vrrp.c @@ -5144,10 +5144,30 @@ void clear_diff_script(void) { vrrp_script_t *vscript, *nvscript; + bool different; + int i; list_for_each_entry(vscript, &old_vrrp_data->vrrp_script, e_list) { nvscript = find_script_by_name(vscript->sname); if (nvscript) { + /* Check if the scripts are the same */ + if (vscript->script.num_args != nvscript->script.num_args || + vscript->script.uid != nvscript->script.uid || + vscript->script.gid != nvscript->script.gid || + !vscript->script.path != !nvscript->script.path || + (vscript->script.path && + strcmp(vscript->script.path, nvscript->script.path))) + continue; + for (i = 0, different = false; i < vscript->script.num_args; i++) { + if (strcmp(vscript->script.args[i], nvscript->script.args[i])) { + different = true; + break; + } + } + + if (different) + continue; + if (vscript->init_state == SCRIPT_INIT_STATE_INIT) { /* We need to undo the startup assumptions and apply new startup assumptions */ nvscript->init_state = SCRIPT_INIT_STATE_INIT; From 6f9ace3c1033d38fe282e6959e78ce58e02135ab Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Tue, 8 Oct 2024 21:21:21 +0100 Subject: [PATCH 19/19] vrrp: allow specifing interval amd timeout to milli-second resolution Although running track_scripts too rapidly can have use cause heavy system load, there are use cases for being able to run scripts more frequently than 1 second, and also at intervals not in whole seconds. This commit adds the option to be able to specify the interval and timeout timers to a resolution in milli-seconds. Signed-off-by: Quentin Armitage --- doc/KEEPALIVED-MIB.txt | 30 +++++++++++++++++++++++++++--- doc/man/man5/keepalived.conf.5.in | 20 +++++++++++++------- keepalived/vrrp/vrrp_data.c | 10 ++++++++-- keepalived/vrrp/vrrp_parser.c | 16 ++++++++-------- keepalived/vrrp/vrrp_snmp.c | 10 ++++++++++ 5 files changed, 66 insertions(+), 20 deletions(-) diff --git a/doc/KEEPALIVED-MIB.txt b/doc/KEEPALIVED-MIB.txt index 3e138fee5..f86f491cb 100644 --- a/doc/KEEPALIVED-MIB.txt +++ b/doc/KEEPALIVED-MIB.txt @@ -22,12 +22,14 @@ IMPORTS FROM SNMPv2-TC; keepalived MODULE-IDENTITY - LAST-UPDATED "202404050001Z" + LAST-UPDATED "202410040001Z" ORGANIZATION "Keepalived" CONTACT-INFO "http://www.keepalived.org" DESCRIPTION "This MIB describes objects used by keepalived, both for VRRP and health checker." + REVISION "202410040001Z" + DESCRIPTION "add VrrpSciptIntervalUsec and VrrpScriptTimeoutUsec for higher resolution timers" REVISION "202404050001Z" DESCRIPTION "add missing 64 bit counters for 64 bit stats" REVISION "202403180001Z" @@ -2592,7 +2594,9 @@ VrrpScriptEntry ::= SEQUENCE { vrrpScriptRise Unsigned32, vrrpScriptFall Unsigned32, vrrpScriptWgtRev Integer32, - vrrpScriptPath DisplayString + vrrpScriptPath DisplayString, + vrrpScriptIntervalUsec Unsigned32, + vrrpScriptTimeoutUsec Unsigned32 } vrrpScriptIndex OBJECT-TYPE @@ -2681,6 +2685,24 @@ vrrpScriptPath OBJECT-TYPE "Path to file to be executed when running the script." ::= { vrrpScriptEntry 10 } +vrrpScriptIntervalUsec OBJECT-TYPE + SYNTAX Unsigned32 + UNITS "micro-seconds" + MAX-ACCESS read-only + STATUS current + DESCRIPTION + "Interval in micro-seconds between two runs of the script." + ::= { vrrpScriptEntry 11 } + +vrrpScriptTimeoutUsec OBJECT-TYPE + SYNTAX Unsigned32 + UNITS "micro-seconds" + MAX-ACCESS read-only + STATUS current + DESCRIPTION + "Timeout in micro-seconds for a run of the script." + ::= { vrrpScriptEntry 12 } + -- VRRP files -- see vrrp_track.h @@ -5225,7 +5247,9 @@ vrrpScriptGroup OBJECT-GROUP vrrpScriptRise, vrrpScriptFall, vrrpScriptWgtRev, - vrrpScriptPath + vrrpScriptPath, + vrrpScriptIntervalUsec, + vrrpScriptTimeoutUsec } STATUS current DESCRIPTION diff --git a/doc/man/man5/keepalived.conf.5.in b/doc/man/man5/keepalived.conf.5.in index 86aa04a92..2bb729bdf 100644 --- a/doc/man/man5/keepalived.conf.5.in +++ b/doc/man/man5/keepalived.conf.5.in @@ -1326,9 +1326,15 @@ The syntax for bfd instance is : # Default tracking weight # Normally, positive weights are added to the vrrp instance priority when # the bfd instance is up, negative weights reduce the priority when it is down. - # However, if reverse is specified, the priority is decreased when up and - # increased when down. 'weight 0 reverse' will cause the vrrp instance to be down - # when the bfd instance is up, and vice versa. + # However, if reverse is specified, a positive weight decreases the priority the + # script is up, and a negative weight increases the priority when the script is + # 'weight 0 reverse' will cause the vrrp instance to be down when the bfd + # instance is up, and vice versa. + # Weight Reverse Script up Script down + # +ve No prio + - + # -ve No - prio - + # +ve Yes prio - - + # -ve Yes - prio + \fBweight\fR <-253:253> [reverse] # Normally bfd event notifications are sent to both the VRRP and checker processes. @@ -1373,11 +1379,11 @@ The syntax for the vrrp script is: # path of the script to execute \fBscript \fR| - # seconds between script invocations, (default: 1 second) - \fBinterval \fR + # seconds between script invocations, (default: 1 second, resolution milliseconds) + \fBinterval \fR - # seconds after which script is considered to have failed - \fBtimeout \fR + # seconds after which script is considered to have failed (default = interval, resolution milliseconds) + \fBtimeout \fR # adjust priority by this weight, (default: 0) # For description of reverse, see track_script. diff --git a/keepalived/vrrp/vrrp_data.c b/keepalived/vrrp/vrrp_data.c index 61d98da0f..9c7a0b9c8 100644 --- a/keepalived/vrrp/vrrp_data.c +++ b/keepalived/vrrp/vrrp_data.c @@ -299,8 +299,14 @@ dump_vscript(FILE *fp, const vrrp_script_t *vscript) conf_write(fp, " Command = %s", cmd_str(&vscript->script)); if (vscript->script.path) conf_write(fp, " Path = %s", vscript->script.path); - conf_write(fp, " Interval = %lu sec", vscript->interval / TIMER_HZ); - conf_write(fp, " Timeout = %lu sec", vscript->timeout / TIMER_HZ); + if (vscript->interval % TIMER_HZ) + conf_write(fp, " Interval = %lu.%3.3lu sec", vscript->interval / TIMER_HZ, (vscript->interval % TIMER_HZ) / 1000); + else + conf_write(fp, " Interval = %lu sec", vscript->interval / TIMER_HZ); + if (vscript->timeout % TIMER_HZ) + conf_write(fp, " Timeout = %lu.%3.3lu sec", vscript->timeout / TIMER_HZ, (vscript->timeout % TIMER_HZ) / 1000); + else + conf_write(fp, " Timeout = %lu sec", vscript->timeout / TIMER_HZ); conf_write(fp, " Weight = %d%s", vscript->weight, vscript->weight_reverse ? " reverse" : ""); conf_write(fp, " Rise = %d", vscript->rise); conf_write(fp, " Fall = %d", vscript->fall); diff --git a/keepalived/vrrp/vrrp_parser.c b/keepalived/vrrp/vrrp_parser.c index 523e8517a..751d7dbba 100644 --- a/keepalived/vrrp/vrrp_parser.c +++ b/keepalived/vrrp/vrrp_parser.c @@ -1548,19 +1548,19 @@ vrrp_vscript_interval_handler(const vector_t *strvec) { unsigned interval; - /* The min value should be 1, but allow 0 to maintain backward compatibility + /* The min value should be 0.001, but allow 0 to maintain backward compatibility * with pre v2.0.7 */ - if (!read_unsigned_strvec(strvec, 1, &interval, 0, UINT_MAX / TIMER_HZ, true)) { - report_config_error(CONFIG_GENERAL_ERROR, "(%s): vrrp script interval '%s' must be between 1 and %u - ignoring", current_vscr->sname, strvec_slot(strvec, 1), UINT_MAX / TIMER_HZ); + if (!read_decimal_unsigned_strvec(strvec, 1, &interval, 0, (UINT_MAX / TIMER_HZ) * 1000, 3, true)) { + report_config_error(CONFIG_GENERAL_ERROR, "(%s): vrrp script interval '%s' must be between 0.001 and %u - ignoring", current_vscr->sname, strvec_slot(strvec, 1), UINT_MAX / TIMER_HZ); return; } if (interval == 0) { report_config_error(CONFIG_GENERAL_ERROR, "(%s): vrrp script interval must be greater than 0, setting to 1", current_vscr->sname); - interval = 1; + interval = 1000; } - current_vscr->interval = interval * TIMER_HZ; + current_vscr->interval = interval * (TIMER_HZ / 1000); } static void vrrp_vscript_timeout_handler(const vector_t *strvec) @@ -1569,17 +1569,17 @@ vrrp_vscript_timeout_handler(const vector_t *strvec) /* The min value should be 1, but allow 0 to maintain backward compatibility * with pre v2.0.7 */ - if (!read_unsigned_strvec(strvec, 1, &timeout, 0, UINT_MAX / TIMER_HZ, true)) { + if (!read_decimal_unsigned_strvec(strvec, 1, &timeout, 0, (UINT_MAX / TIMER_HZ) * 1000, 3, true)) { report_config_error(CONFIG_GENERAL_ERROR, "(%s): vrrp script timeout '%s' invalid - ignoring", current_vscr->sname, strvec_slot(strvec, 1)); return; } if (timeout == 0) { report_config_error(CONFIG_GENERAL_ERROR, "(%s): vrrp script timeout must be greater than 0, setting to 1", current_vscr->sname); - timeout = 1; + timeout = 1000; } - current_vscr->timeout = timeout * TIMER_HZ; + current_vscr->timeout = timeout * (TIMER_HZ / 1000); } static void vrrp_vscript_weight_handler(const vector_t *strvec) diff --git a/keepalived/vrrp/vrrp_snmp.c b/keepalived/vrrp/vrrp_snmp.c index e18272ae7..d8f859168 100644 --- a/keepalived/vrrp/vrrp_snmp.c +++ b/keepalived/vrrp/vrrp_snmp.c @@ -144,6 +144,8 @@ enum snmp_vrrp_magic { VRRP_SNMP_SCRIPT_RISE, VRRP_SNMP_SCRIPT_FALL, VRRP_SNMP_SCRIPT_PATH, + VRRP_SNMP_SCRIPT_INTERVAL_USEC, + VRRP_SNMP_SCRIPT_TIMEOUT_USEC, VRRP_SNMP_FILE_NAME, VRRP_SNMP_FILE_PATH, VRRP_SNMP_FILE_RESULT, @@ -599,6 +601,12 @@ vrrp_snmp_script(struct variable *vp, oid *name, size_t *length, ret.cp = scr->script.path ? scr->script.path : scr->script.args[0]; *var_len = strlen(ret.cp); return ret.p; + case VRRP_SNMP_SCRIPT_INTERVAL_USEC: + long_ret.u = scr->interval; + return PTR_CAST(u_char, &long_ret); + case VRRP_SNMP_SCRIPT_TIMEOUT_USEC: + long_ret.u = scr->timeout; + return PTR_CAST(u_char, &long_ret); default: break; } @@ -2909,6 +2917,8 @@ static struct variable3 vrrp_vars[] = { {VRRP_SNMP_SCRIPT_FALL, ASN_UNSIGNED, RONLY, vrrp_snmp_script, 3, {9, 1, 8}}, {VRRP_SNMP_SCRIPT_WEIGHT_REVERSE, ASN_INTEGER, RONLY, vrrp_snmp_script, 3, {9, 1, 9}}, {VRRP_SNMP_SCRIPT_PATH, ASN_OCTET_STR, RONLY, vrrp_snmp_script, 3, {9, 1, 10}}, + {VRRP_SNMP_SCRIPT_INTERVAL_USEC, ASN_UNSIGNED, RONLY, vrrp_snmp_script, 3, {9, 1, 11}}, + {VRRP_SNMP_SCRIPT_TIMEOUT_USEC, ASN_UNSIGNED, RONLY, vrrp_snmp_script, 3, {9, 1, 12}}, /* vrrpRouteNextHopTable */ {VRRP_SNMP_ROUTE_NEXT_HOP_ADDRESS_TYPE, ASN_INTEGER, RONLY,