diff mbox series

[2/2] app/pdump: handle SIGTERM and SIGHUP

Message ID 20240226205143.66702-3-stephen@networkplumber.org (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series dumpcap,pdump handle cleanup signals |

Checks

Context Check Description
ci/loongarch-compilation success Compilation OK
ci/checkpatch success coding style OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Stephen Hemminger Feb. 26, 2024, 8:49 p.m. UTC 
  Like dumpcap, the pdump process should cleanup if process
terminates due to being killed or hangup.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/pdump/main.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Comments

Pattan, Reshma Feb. 27, 2024, 9:59 a.m. UTC | #1 
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> +	sigaction(SIGHUP, NULL, &origaction);
 
Why do we need this ?  Can't be this direct sigaction(SIGHUP, &action, NULL)  without a below if code? 
Can you please explain how this works. 

> +	if (origaction.sa_handler == SIG_DFL)
> +		sigaction(SIGHUP, &action, NULL);
> 


Other changes looks ok to me.

Regards,
Reshma
Stephen Hemminger Feb. 27, 2024, 6:09 p.m. UTC | #2 
On Tue, 27 Feb 2024 09:59:37 +0000
"Pattan, Reshma" <reshma.pattan@intel.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > +	sigaction(SIGHUP, NULL, &origaction);  
>  
> Why do we need this ?  Can't be this direct sigaction(SIGHUP, &action, NULL)  without a below if code? 
> Can you please explain how this works. 
> 
> > +	if (origaction.sa_handler == SIG_DFL)
> > +		sigaction(SIGHUP, &action, NULL);
> >   
> 


If sighup is being ignored already (for example being run by nohup)
then the program should keep ignoring SIGHUP.

This is the method used in tshark and dumpcap utilities in wireshark
and it made sense.
diff mbox series

Patch

diff --git a/app/pdump/main.c b/app/pdump/main.c
index a9205e130bb1..3592f8a865ad 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -571,11 +571,9 @@  disable_primary_monitor(void)
 }
 
 static void
-signal_handler(int sig_num)
+signal_handler(int sig_num __rte_unused)
 {
-	if (sig_num == SIGINT) {
-		quit_signal = 1;
-	}
+	quit_signal = 1;
 }
 
 static inline int
@@ -975,6 +973,11 @@  enable_primary_monitor(void)
 int
 main(int argc, char **argv)
 {
+	struct sigaction action = {
+		.sa_flags = SA_RESTART,
+		.sa_handler = signal_handler,
+	};
+	struct sigaction origaction;
 	int diag;
 	int ret;
 	int i;
@@ -983,8 +986,14 @@  main(int argc, char **argv)
 	char mp_flag[] = "--proc-type=secondary";
 	char *argp[argc + 2];
 
-	/* catch ctrl-c so we can print on exit */
-	signal(SIGINT, signal_handler);
+	/* catch ctrl-c so we can cleanup on exit */
+	sigemptyset(&action.sa_mask);
+	sigaction(SIGTERM, &action, NULL);
+	sigaction(SIGINT, &action, NULL);
+	sigaction(SIGPIPE, &action, NULL);
+	sigaction(SIGHUP, NULL, &origaction);
+	if (origaction.sa_handler == SIG_DFL)
+		sigaction(SIGHUP, &action, NULL);
 
 	argp[0] = argv[0];
 	argp[1] = n_flag;