[PATCH 0/2] dumpcap,pdump handle cleanup signals

[PATCH 0/2] dumpcap,pdump handle cleanup signals

[Stephen Hemminger]

The dumpcap and pdump commands need to handle more signals.

Stephen Hemminger (2):
  app/dumpcap: handle SIGTERM and SIGHUP
  app/pdump: handle SIGTERM and SIGHUP

 app/dumpcap/main.c | 14 ++++++++++++--
 app/pdump/main.c   | 21 +++++++++++++++------
 2 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH 1/2] app/dumpcap: handle SIGTERM and SIGHUP

[Stephen Hemminger]

If application is killed (SIGTERM), or the console session
ends (SIGHUP) or the write to the output file fails (SIGPIPE)
then do cleanup before exiting.

This also makes DPDK dumpcap behave more like Wireshark dumpcap.

Suggested-by: Isaac Boukris <iboukris@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/dumpcap/main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index d57db0589a3f..76c747511444 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -939,6 +939,11 @@ int main(int argc, char **argv)
 {
 	struct rte_ring *r;
 	struct rte_mempool *mp;
+	struct sigaction action = {
+		.sa_flags = SA_RESTART,
+		.sa_handler = signal_handler,
+	};
+	struct sigaction origaction;
 	dumpcap_out_t out;
 	char *p;
 
@@ -964,8 +969,13 @@ int main(int argc, char **argv)
 
 	compile_filters();
 
-	signal(SIGINT, signal_handler);
-	signal(SIGPIPE, SIG_IGN);
+	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);
 
 	enable_primary_monitor();
 
-- 
2.43.0

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

[Stephen Hemminger]

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(-)

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;
-- 
2.43.0

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

[Pattan, Reshma]

> -----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

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

[Stephen Hemminger]

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.