Re: [OE-core] [PATCH] dhcp: Revert "dhcp: enable gentle shutdown" and reset handler

2019-05-07 Thread Richard Purdie
On Tue, 2019-05-07 at 11:06 +0800, Rui Wang wrote:
> This reverts commit 4653fdd4b4cf13543e32fbcf09e50f60c1719a34
> 
> This patch is used to solve the problem in which dhcpd can not
> be killed by SIGINT and SIGTERM. But it seems not good to rely
> on ENABLE_GENTLE_SHUTDOWN, which is an "undesireable feature"
> and will release its address after receive both signal, accoring
> to the comment in "includes/site.h". Then there is no difference
> between "dhclient -x" and "dhclient -r". We can find following
> call trace from source code:
> 
> main() -> dhcp_context_create() -> isc_app_ctxstart()
> -> isc__app_ctxstart()
> 
> The root cause is that exit_action() is registered as the signal
> handler for SIGTERM and SIGINT, which do-nothing as the comment says.
> 
> To fix this problem, we could simply set the default action for
> these two signals if ENABLE_GENTLE_SHUTDOWN is not set.
> 
> Signed-off-by: Rui Wang 

The new patch is missing an Upstream-Status?

Cheers,

Richard

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


[OE-core] [PATCH] dhcp: Revert "dhcp: enable gentle shutdown" and reset handler

2019-05-06 Thread Rui Wang
This reverts commit 4653fdd4b4cf13543e32fbcf09e50f60c1719a34

This patch is used to solve the problem in which dhcpd can not
be killed by SIGINT and SIGTERM. But it seems not good to rely
on ENABLE_GENTLE_SHUTDOWN, which is an "undesireable feature"
and will release its address after receive both signal, accoring
to the comment in "includes/site.h". Then there is no difference
between "dhclient -x" and "dhclient -r". We can find following
call trace from source code:

main() -> dhcp_context_create() -> isc_app_ctxstart()
-> isc__app_ctxstart()

The root cause is that exit_action() is registered as the signal
handler for SIGTERM and SIGINT, which do-nothing as the comment says.

To fix this problem, we could simply set the default action for
these two signals if ENABLE_GENTLE_SHUTDOWN is not set.

Signed-off-by: Rui Wang 
---
 ...et-default-handler-for-SIGTERM-and-SIGINT.patch | 63 ++
 .../dhcp/0006-site.h-enable-gentle-shutdown.patch  | 27 --
 meta/recipes-connectivity/dhcp/dhcp_4.4.1.bb   |  2 +-
 3 files changed, 64 insertions(+), 28 deletions(-)
 create mode 100644 
meta/recipes-connectivity/dhcp/dhcp/0001-dhcp-set-default-handler-for-SIGTERM-and-SIGINT.patch
 delete mode 100644 
meta/recipes-connectivity/dhcp/dhcp/0006-site.h-enable-gentle-shutdown.patch

diff --git 
a/meta/recipes-connectivity/dhcp/dhcp/0001-dhcp-set-default-handler-for-SIGTERM-and-SIGINT.patch
 
b/meta/recipes-connectivity/dhcp/dhcp/0001-dhcp-set-default-handler-for-SIGTERM-and-SIGINT.patch
new file mode 100644
index 000..d5c9e34
--- /dev/null
+++ 
b/meta/recipes-connectivity/dhcp/dhcp/0001-dhcp-set-default-handler-for-SIGTERM-and-SIGINT.patch
@@ -0,0 +1,63 @@
+From eaa74a4fd8ebcc031c63a8a39d20cd1ffe0eaf4f Mon Sep 17 00:00:00 2001
+From: Rui Wang 
+Date: Tue, 7 May 2019 10:41:20 +0800
+Subject: [PATCH] dhcp: set default handler for SIGTERM and SIGINT
+
+dhcp uses function in bind to init it's state, which will block
+signal SIGTERM and SIGINT by default. This causes dhclient not
+able to be killed by those two signals if gentle-shutdown is
+disabled. So reset their signal handler to default in such
+situation.
+
+Signed-off-by: Rui Wang 
+---
+ client/dhclient.c | 3 +++
+ relay/dhcrelay.c  | 3 +++
+ server/dhcpd.c| 3 +++
+ 3 files changed, 9 insertions(+)
+
+diff --git a/client/dhclient.c b/client/dhclient.c
+index 825ab00..159cf39 100644
+--- a/client/dhclient.c
 b/client/dhclient.c
+@@ -971,6 +971,9 @@ main(int argc, char **argv) {
+ /* install signal handlers */
+   signal(SIGINT, dhcp_signal_handler);   /* control-c */
+   signal(SIGTERM, dhcp_signal_handler);  /* kill */
++#else
++signal(SIGINT, SIG_DFL);
++signal(SIGTERM, SIG_DFL);
+ #endif
+ 
+   /* If we're not supposed to wait before getting the address,
+diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c
+index d8caaaf..ec22385 100644
+--- a/relay/dhcrelay.c
 b/relay/dhcrelay.c
+@@ -814,6 +814,9 @@ main(int argc, char **argv) {
+ /* install signal handlers */
+   signal(SIGINT, dhcp_signal_handler);   /* control-c */
+   signal(SIGTERM, dhcp_signal_handler);  /* kill */
++#else
++signal(SIGINT, SIG_DFL);
++signal(SIGTERM, SIG_DFL);
+ #endif
+ 
+   /* Start dispatching packets and timeouts... */
+diff --git a/server/dhcpd.c b/server/dhcpd.c
+index 55ffae7..f5aced3 100644
+--- a/server/dhcpd.c
 b/server/dhcpd.c
+@@ -1052,6 +1052,9 @@ main(int argc, char **argv) {
+ /* install signal handlers */
+   signal(SIGINT, dhcp_signal_handler);   /* control-c */
+   signal(SIGTERM, dhcp_signal_handler);  /* kill */
++#else
++signal(SIGINT, SIG_DFL);
++signal(SIGTERM, SIG_DFL);
+ #endif
+ 
+   /* Log that we are about to start working */
+-- 
+1.8.3.1
+
diff --git 
a/meta/recipes-connectivity/dhcp/dhcp/0006-site.h-enable-gentle-shutdown.patch 
b/meta/recipes-connectivity/dhcp/dhcp/0006-site.h-enable-gentle-shutdown.patch
deleted file mode 100644
index 6ef70cc..000
--- 
a/meta/recipes-connectivity/dhcp/dhcp/0006-site.h-enable-gentle-shutdown.patch
+++ /dev/null
@@ -1,27 +0,0 @@
-From 01641d146e4e6bea954e4a4ee1f6230b822665b4 Mon Sep 17 00:00:00 2001
-From: Chen Qi 
-Date: Tue, 15 Aug 2017 15:37:49 +0800
-Subject: [PATCH 06/11] site.h: enable gentle shutdown
-
-Upstream-Status: Inappropriate [configuration]
-Signed-off-by: Chen Qi 
-
-Rebase to 4.3.6
-Signed-off-by: Hongxu Jia 

- includes/site.h | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-Index: dhcp-4.4.1/includes/site.h
-===
 dhcp-4.4.1.orig/includes/site.h
-+++ dhcp-4.4.1/includes/site.h
-@@ -295,7 +295,7 @@
-situations.  We plan to revisit this feature and may
-make non-backwards compatible changes including the
-removal of this define.  Use at your own risk.  */
--/* #define ENABLE_GENTLE_SHUTDOWN */
-+#define ENABLE_GENTLE_SHUTDOWN
- 
- /* Include old error codes.  This is