Skip to content

Commit

Permalink
all: add --ignore-sigint option
Browse files Browse the repository at this point in the history 
This is needed for running keepalived under GDB (see
https://bugzilla.kernel.org/show_bug.cgi?id=9039#c8).

Signed-off-by: Quentin Armitage <[email protected]>
  • Loading branch information
@pqarmitage
pqarmitage committed Jun 13, 2024
1 parent 9309a4b commit 7e4c75f
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 4 deletions.
5 changes: 5 additions & 0 deletions doc/man/man8/keepalived.8.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ keepalived \- load\-balancing and high\-availability service
[\fB\-\-perf\fP[={all|run|end}]]
[\fB\-\-debug\fP[=debug-options]]
[\fB\-\-no-mem-check\fP]
[\fB\-\-ignore-sigint\fP]
[\fB\-v\fP|\fB\-\-version\fP]
[\fB\-h\fP|\fB\-\-help\fP]

Expand Down Expand Up @@ -216,6 +217,10 @@ The data recorded is for use with the perf tool.
\fB --no-mem-check\fP
Disable malloc() etc mem-checks if they have been compiled into keepalived.
.TP
\fB --ignore-sigint\fP
Disable SIGINT handling (defaults to terminating keepalived). This is needed
for running keepalived under GDB.
.TP
\fB --debug\fP[=debug-options]]
Enables debug options if they have been compiled into keepalived.
\fIdebug-options\fP is made up of a sequence of strings of the form Ulll.
Expand Down
5 changes: 4 additions & 1 deletion keepalived/bfd/bfd_daemon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ static void
bfd_signal_init(void)
{
signal_set(SIGHUP, sigreload_bfd, NULL);
signal_set(SIGINT, sigend_bfd, NULL);
if (ignore_sigint)
signal_ignore(SIGINT);
else
signal_set(SIGINT, sigend_bfd, NULL);
signal_set(SIGTERM, sigend_bfd, NULL);
signal_set(SIGUSR1, sigdump_bfd, NULL);
#ifdef THREAD_DUMP
Expand Down
5 changes: 4 additions & 1 deletion keepalived/check/check_daemon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,10 @@ static void
check_signal_init(void)
{
signal_set(SIGHUP, sigreload_check, NULL);
signal_set(SIGINT, sigend_check, NULL);
if (ignore_sigint)
signal_ignore(SIGINT);
else
signal_set(SIGINT, sigend_check, NULL);
signal_set(SIGTERM, sigend_check, NULL);
signal_set(SIGUSR1, sigusr1_check, NULL);
#ifdef THREAD_DUMP
Expand Down
13 changes: 12 additions & 1 deletion keepalived/core/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ static bool create_core_dump = false;
static const char *core_dump_pattern = "core";
static char *orig_core_dump_pattern = NULL;

/* Signal handling */
bool ignore_sigint = false;

/* debug flags */
#if defined _TIMER_CHECK_ || \
defined _SMTP_ALERT_DEBUG_ || \
Expand Down Expand Up @@ -1289,7 +1292,10 @@ signal_init(void)
#ifdef _WITH_JSON_
signal_set(SIGJSON, propagate_signal, NULL);
#endif
signal_set(SIGINT, sigend, NULL);
if (ignore_sigint)
signal_ignore(SIGINT);
else
signal_set(SIGINT, sigend, NULL);
signal_set(SIGTERM, sigend, NULL);
#ifdef THREAD_DUMP
signal_set(SIGTDUMP, thread_dump_signal, NULL);
Expand Down Expand Up @@ -1886,6 +1892,7 @@ usage(const char *prog)
fprintf(stderr, " -i, --config-id id Skip any configuration lines beginning '@' that don't match id\n"
" or any lines beginning @^ that do match.\n"
" The config-id defaults to the node name if option not used\n");
fprintf(stderr, " --ignore-sigint ignore SIGINT (default means terminate) - used for debugging with GDB\n");
fprintf(stderr, " --signum=SIGFUNC Return signal number for STOP, RELOAD, DATA, STATS, STATS_CLEAR"
#ifdef _WITH_JSON_
", JSON"
Expand Down Expand Up @@ -2051,6 +2058,7 @@ parse_cmdline(int argc, char **argv)
{"signum", required_argument, NULL, 4 },
{"config-test", optional_argument, NULL, 't'},
{"config-fd", required_argument, NULL, 8 },
{"ignore-sigint", no_argument, NULL, 9 },
#ifdef _WITH_PERF_
{"perf", optional_argument, NULL, 5 },
#endif
Expand Down Expand Up @@ -2356,6 +2364,9 @@ parse_cmdline(int argc, char **argv)
case 8:
set_config_fd(atoi(optarg));
break;
case 9:
ignore_sigint = true;
break;
case '?':
if (optopt && argv[curind][1] != '-')
fprintf(stderr, "Unknown option -%c\n", optopt);
Expand Down
2 changes: 2 additions & 0 deletions keepalived/include/main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ extern unsigned os_major; /* Kernel version */
extern unsigned os_minor;
extern unsigned os_release;

extern bool ignore_sigint;

extern void free_parent_mallocs_startup(bool);
extern void free_parent_mallocs_exit(void);
extern const char *make_syslog_ident(const char*);
Expand Down
5 changes: 4 additions & 1 deletion keepalived/vrrp/vrrp_daemon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,10 @@ static void
vrrp_signal_init(void)
{
signal_set(SIGHUP, sigreload_vrrp, NULL);
signal_set(SIGINT, sigend_vrrp, NULL);
if (ignore_sigint)
signal_ignore(SIGINT);
else
signal_set(SIGINT, sigend_vrrp, NULL);
signal_set(SIGTERM, sigend_vrrp, NULL);
signal_set(SIGUSR1, sigusr1_vrrp, NULL);
signal_set(SIGUSR2, sigusr2_vrrp, NULL);
Expand Down

5 comments on commit 7e4c75f

@smithAchang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@smithAchang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shuold the commit remove the SIGINT process ?

Reasons:

  • the SIGTERM has the same effect
  • Ctrl+c sends SIGINT is the common practice and keepalived always runs in daemon state. so keepalived should not care SIGINT signal

I Have read the content of https://bugzilla.kernel.org/show_bug.cgi?id=9039#c8).

But the keepalived source codes does not have any sigwait calling

Is it the kernel's bug caused my problem?

@pqarmitage
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot remove the SIGINT handling by default because some users may use SIGINT, albeit they could change to use SIGTERM, but we always attempt to maintain backwards compatibility. Further, you state keepalived always runs in daemon state but keepalived has a --dont-fork option which causes it not to run in daemon state.

The reason I specifically referenced comment 8 at https://bugzilla.kernel.org/show_bug.cgi?id=9039#c8 is that it states "their program is either blocked in sigwait or is accepting SIGINT via signalfd". keepalived receives signals via signalfd.

@smithAchang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thx :)

--

But it will make some trouble when I wants to debug the running keepalived using gdb att $pid or study the runtime logic anytime

so I think the benefit is less for keeping such a compatibility

@pqarmitage
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smithAchang Since you want to be able to attach to a running keepalived with gdb, and also since you know you don't need SIGINT to be handled by keepalived, you can always run keepalived with the --ignore-sigint option, and then you will always be able to attach to keepalvied.

Please sign in to comment.