Discussion:
[OpenWrt-Devel] [PATCH] use NTP server received via DHCP
amine ahd
2015-12-22 16:00:35 UTC
Permalink
The current state of NTP is to load the list of NTP servers
from the static file /etc/config/system.
This patch allows ntpd to get NTP servers from DHCP.
ntpd will restart whenever the list of NTP servers is changed.

Signed-off-by: amine hamed <***@gmail.com>
---
package/utils/busybox/Makefile | 3 +++
package/utils/busybox/files/sysntpd | 15 +++++++++--
.../package/utils/busybox/files/sysntpd.hotplug | 31 ++++++++++++++++++++++
3 files changed, 47 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug

diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 9571d48..3c33246 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -112,6 +112,9 @@ define Package/busybox/install
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/telnet $(1)/etc/init.d/telnet
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..fbe1838 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,12 +1,12 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
START=98

USE_PROCD=1
PROG=/usr/sbin/ntpd
HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug
-
validate_ntp_section() {
uci_validate_section system timeserver "${1}" \
'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0'
@@ -15,6 +15,8 @@ validate_ntp_section() {
start_service() {
local server enabled enable_server peer

+ #get the list of ntp servers from DHCP using ubus.
+ ntpservers=`ubus call network.interface dump | grep "ntpserver" | cut -d":" -f2 | tr -d '"'`
validate_ntp_section ntp || {
echo "validation failed"
return 1
@@ -22,12 +24,20 @@ start_service() {

[ $enabled = 0 ] && return

- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$ntpservers" == "" ] && return

procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ #add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+
for peer in $server; do
procd_append_param command -p $peer
done
@@ -38,5 +48,6 @@ start_service() {
service_triggers()
{
procd_add_reload_trigger "system"
+
procd_add_validation validate_ntp_section
}
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100755
index 0000000..de2946a
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+. /lib/functions.sh
+
+[ "$ACTION" = ifup ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ # append the server to the list
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local proto=`uci get network.$INTERFACE.proto`
+
+#get the list of ntp servers returned from DHCP, remote leading and trailing whitespaces as well as string quotes
+dhcp_ntp_servers=`ubus call network.interface dump | grep "ntpserver" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
+
+new_ntp_servers="$dhcp_ntp_servers"
+
+#get the default list of ntp servers from the config file and append it to the new list
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+current_ntp_servers=`ubus call service get_data '{"name":"sysntpd"}' | grep "ntp_servers" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
+
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$proto" == "dhcp" ] && [ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
\ No newline at end of file
--
2.5.0
Bastian Bittorf
2015-12-22 17:09:20 UTC
Permalink
Post by amine ahd
+ #get the list of ntp servers from DHCP using ubus.
+ ntpservers=`ubus call network.interface dump | grep "ntpserver" | cut -d":" -f2 | tr -d '"'`
remove the comment, it's obvious what you are doing.
when using comment, use a space e.g. # mycomment

when speaking with ubus/parsing json do this:

ubus list network.interface -> interfaces
. /usr/share/libubox/jshn.sh
json_load "$( ubus call network.interface.wan2 status )"
...

@jow: can you say more about that?
Post by amine ahd
validate_ntp_section ntp || {
echo "validation failed"
return 1
@@ -22,12 +24,20 @@ start_service() {
[ $enabled = 0 ] && return
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$ntpservers" == "" ] && return
please do
[ -z "$ntpservers" ]
Post by amine ahd
+handle_default_ntp_servers() {
+ local server="$1"
+ # append the server to the list
+ new_ntp_servers="$new_ntp_servers $server"
+}
this comment also does not help 8-)
Post by amine ahd
+local proto=`uci get network.$INTERFACE.proto`
please use OpenWrt-style: "$(...)"
Post by amine ahd
+#get the list of ntp servers returned from DHCP, remote leading and trailing whitespaces as well as string quotes
+dhcp_ntp_servers=`ubus call network.interface dump | grep "ntpserver" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
same as on top, dont parse JSON like this.
Post by amine ahd
+#get the current list of ntp servers in the running instance
+current_ntp_servers=`ubus call service get_data '{"name":"sysntpd"}' | grep "ntp_servers" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
same as on top

bye, bastian
Yousong Zhou
2015-12-24 13:35:18 UTC
Permalink
Hi, amine
Post by amine ahd
The current state of NTP is to load the list of NTP servers
from the static file /etc/config/system.
This patch allows ntpd to get NTP servers from DHCP.
ntpd will restart whenever the list of NTP servers is changed.
---
package/utils/busybox/Makefile | 3 +++
package/utils/busybox/files/sysntpd | 15 +++++++++--
.../package/utils/busybox/files/sysntpd.hotplug | 31 ++++++++++++++++++++++
3 files changed, 47 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug
diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 9571d48..3c33246 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -112,6 +112,9 @@ define Package/busybox/install
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/telnet $(1)/etc/init.d/telnet
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..fbe1838 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,12 +1,12 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org
+. /lib/functions.sh
START=98
USE_PROCD=1
PROG=/usr/sbin/ntpd
HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug
-
validate_ntp_section() {
uci_validate_section system timeserver "${1}" \
'server:list(host)' 'enabled:bool:1' 'enable_server:bool:0'
@@ -15,6 +15,8 @@ validate_ntp_section() {
start_service() {
local server enabled enable_server peer
+ #get the list of ntp servers from DHCP using ubus.
+ ntpservers=`ubus call network.interface dump | grep "ntpserver" | cut -d":" -f2 | tr -d '"'`
This can be done with help from jsonfilter

ubus call network.interface dump | jsonfilter -e
'$["interface"][*]["data"]["ntpserver"]'

It should also be possible for users to specify sources of timeserver
settings, whether it be static list of servers in uci configs or dhcp
settings from some network interfaces.
Post by amine ahd
validate_ntp_section ntp || {
echo "validation failed"
return 1
@@ -22,12 +24,20 @@ start_service() {
[ $enabled = 0 ] && return
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$ntpservers" == "" ] && return
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ #add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+
Not quite sure about this, but are we going to replace uci_set_state
with procd data param? Personally I prefer uci state for such
"dynamic" data store

Regards,

yousong
Post by amine ahd
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+
for peer in $server; do
procd_append_param command -p $peer
done
@@ -38,5 +48,6 @@ start_service() {
service_triggers()
{
procd_add_reload_trigger "system"
+
procd_add_validation validate_ntp_section
}
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100755
index 0000000..de2946a
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+. /lib/functions.sh
+
+[ "$ACTION" = ifup ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ # append the server to the list
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local proto=`uci get network.$INTERFACE.proto`
+
+#get the list of ntp servers returned from DHCP, remote leading and trailing whitespaces as well as string quotes
+dhcp_ntp_servers=`ubus call network.interface dump | grep "ntpserver" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
+
+new_ntp_servers="$dhcp_ntp_servers"
+
+#get the default list of ntp servers from the config file and append it to the new list
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+current_ntp_servers=`ubus call service get_data '{"name":"sysntpd"}' | grep "ntp_servers" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
+
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$proto" == "dhcp" ] && [ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
\ No newline at end of file
--
2.5.0
_______________________________________________
openwrt-devel mailing list
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Amine Aouled Hamed
2015-12-30 12:12:22 UTC
Permalink
Hi,
Can you elaborate more on why you prefer uci state? I am just starting with
OpenWRT and the first thing I found was procd.

Regards,
Amine.
Post by Yousong Zhou
Hi, amine
Post by amine ahd
The current state of NTP is to load the list of NTP servers
from the static file /etc/config/system.
This patch allows ntpd to get NTP servers from DHCP.
ntpd will restart whenever the list of NTP servers is changed.
---
package/utils/busybox/Makefile | 3 +++
package/utils/busybox/files/sysntpd | 15 +++++++++--
.../package/utils/busybox/files/sysntpd.hotplug | 31
++++++++++++++++++++++
Post by amine ahd
3 files changed, 47 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug
diff --git a/package/utils/busybox/Makefile
b/package/utils/busybox/Makefile
Post by amine ahd
index 9571d48..3c33246 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -112,6 +112,9 @@ define Package/busybox/install
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/telnet $(1)/etc/init.d/telnet
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug
$(1)/etc/hotplug.d/iface/30-sysntpd
Post by amine ahd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd
b/package/utils/busybox/files/sysntpd
Post by amine ahd
index f73bb83..fbe1838 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,12 +1,12 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org
+. /lib/functions.sh
START=98
USE_PROCD=1
PROG=/usr/sbin/ntpd
HOTPLUG_SCRIPT=/usr/sbin/ntpd-hotplug
-
validate_ntp_section() {
uci_validate_section system timeserver "${1}" \
'server:list(host)' 'enabled:bool:1'
'enable_server:bool:0'
Post by amine ahd
@@ -15,6 +15,8 @@ validate_ntp_section() {
start_service() {
local server enabled enable_server peer
+ #get the list of ntp servers from DHCP using ubus.
+ ntpservers=`ubus call network.interface dump | grep "ntpserver"
| cut -d":" -f2 | tr -d '"'`
This can be done with help from jsonfilter
ubus call network.interface dump | jsonfilter -e
'$["interface"][*]["data"]["ntpserver"]'
It should also be possible for users to specify sources of timeserver
settings, whether it be static list of servers in uci configs or dhcp
settings from some network interfaces.
Post by amine ahd
validate_ntp_section ntp || {
echo "validation failed"
return 1
@@ -22,12 +24,20 @@ start_service() {
[ $enabled = 0 ] && return
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$ntpservers" == "" ] && return
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S
"$HOTPLUG_SCRIPT"
Post by amine ahd
+
+ #add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+
Not quite sure about this, but are we going to replace uci_set_state
with procd data param? Personally I prefer uci state for such
"dynamic" data store
Regards,
yousong
Post by amine ahd
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+
for peer in $server; do
procd_append_param command -p $peer
done
@@ -38,5 +48,6 @@ start_service() {
service_triggers()
{
procd_add_reload_trigger "system"
+
procd_add_validation validate_ntp_section
}
diff --git a/package/utils/busybox/files/sysntpd.hotplug
b/package/utils/busybox/files/sysntpd.hotplug
Post by amine ahd
new file mode 100755
index 0000000..de2946a
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+. /lib/functions.sh
+
+[ "$ACTION" = ifup ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ # append the server to the list
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local proto=`uci get network.$INTERFACE.proto`
+
+#get the list of ntp servers returned from DHCP, remote leading and
trailing whitespaces as well as string quotes
Post by amine ahd
+dhcp_ntp_servers=`ubus call network.interface dump | grep "ntpserver" |
cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
Post by amine ahd
+
+new_ntp_servers="$dhcp_ntp_servers"
+
+#get the default list of ntp servers from the config file and append it
to the new list
Post by amine ahd
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+current_ntp_servers=`ubus call service get_data '{"name":"sysntpd"}' |
grep "ntp_servers" | cut -d":" -f2 | sed 's/\"//g;s/^[ \t]*//;s/[ \t]*$//'`
Post by amine ahd
+
+#if its an up action, the iface uses DHCP and the new list of ntp
servers is different from the old, restart sysntpd
Post by amine ahd
+[ "$proto" == "dhcp" ] && [ "$current_ntp_servers" !=
"$new_ntp_servers" ] || exit 0
Post by amine ahd
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface
$INTERFACE and a change of NTP servers"
Post by amine ahd
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
\ No newline at end of file
--
2.5.0
_______________________________________________
openwrt-devel mailing list
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
--
Amine Hamed | Software Engineer



Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany

Email ***@ocedo.com


<***@ocedo.com>

REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER:
HRB 717873
MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT
Yousong Zhou
2015-12-30 13:14:59 UTC
Permalink
Hi,
Hi,
Can you elaborate more on why you prefer uci state? I am just starting with OpenWRT and the first thing I found was procd.
Most of it is really just personal preference. I prefer uci because
it can provide more structured access/manipulation of the state, e.g.

- disable those ntpservers on interface down
- when it comes to ntpservers from multiple dhcp interfaces
- the state can be easily checked with uci command if something went wrong

These can of course also be done with procd data, but uci is already there...

Regards,
yousong
Amine Aouled Hamed
2016-01-05 08:34:37 UTC
Permalink
Happy new year!
Thanks for the explanation, I will be sending a new patch today.
Regards,
Amine.
Hi,
Post by Amine Aouled Hamed
Hi,
Can you elaborate more on why you prefer uci state? I am just starting
with OpenWRT and the first thing I found was procd.
Most of it is really just personal preference. I prefer uci because
it can provide more structured access/manipulation of the state, e.g.
- disable those ntpservers on interface down
- when it comes to ntpservers from multiple dhcp interfaces
- the state can be easily checked with uci command if something went wrong
These can of course also be done with procd data, but uci is already there...
Regards,
yousong
--
Amine Hamed | Software Engineer



Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany

Email ***@ocedo.com


<***@ocedo.com>

REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER:
HRB 717873
MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT
Jo-Philipp Wich
2016-01-05 09:23:19 UTC
Permalink
Hi,

please do not use uci state for new features, we consider uci state
variables to be deprecated.

~ Jow
Felix Fietkau
2016-01-07 18:19:13 UTC
Permalink
Post by Yousong Zhou
Hi,
Hi,
Can you elaborate more on why you prefer uci state? I am just starting with OpenWRT and the first thing I found was procd.
Most of it is really just personal preference. I prefer uci because
it can provide more structured access/manipulation of the state, e.g.
- disable those ntpservers on interface down
- when it comes to ntpservers from multiple dhcp interfaces
- the state can be easily checked with uci command if something went wrong
These can of course also be done with procd data, but uci is already there...
Here's some context for why we stopped using uci state:

Mixing configuration with a state overlay can easily get very messy,
especially when the config changes - it's just too easy for namespace
collisions to creep in.
Cleanup is also difficult when there's no clear separation of which
script is responsible for which part of the state overlay.

Using procd data has the advantange that the state is tied to the
running state of the service. If the service dies or is restarted, its
state is automatically cleaned up too.

- Felix
amine ahd
2016-01-07 09:15:31 UTC
Permalink
---
openwrt/package/utils/busybox/Makefile | 3 ++
openwrt/package/utils/busybox/files/sysntpd | 28 ++++++++++-
.../package/utils/busybox/files/sysntpd.hotplug | 54 ++++++++++++++++++++++
3 files changed, 83 insertions(+), 2 deletions(-)
create mode 100755 openwrt/package/utils/busybox/files/sysntpd.hotplug

diff --git a/openwrt/package/utils/busybox/Makefile b/openwrt/package/utils/busybox/Makefile
index 9571d48..3c33246 100644
--- a/openwrt/package/utils/busybox/Makefile
+++ b/openwrt/package/utils/busybox/Makefile
@@ -112,6 +112,9 @@ define Package/busybox/install
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/telnet $(1)/etc/init.d/telnet
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/openwrt/package/utils/busybox/files/sysntpd b/openwrt/package/utils/busybox/files/sysntpd
index f73bb83..dec474c 100755
--- a/openwrt/package/utils/busybox/files/sysntpd
+++ b/openwrt/package/utils/busybox/files/sysntpd
@@ -1,6 +1,8 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
START=98

USE_PROCD=1
@@ -22,12 +24,32 @@ start_service() {

[ $enabled = 0 ] && return

- [ -z "$server" ] && return
-
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ if [ "$use_dhcp" = 1 ]; then
+ if [ -z "$dhcp_ifaces" ]; then
+ dump=$(ubus call network.interface dump)
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for int in $dhcp_ifaces; do
+ status=$(ubus call network.interface.$int status)
+ ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] &&
+ ntpservers="$ntpservers $ntpserver"
+ done
+ fi
+ # add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+ fi
+
for peer in $server; do
procd_append_param command -p $peer
done
@@ -38,5 +60,7 @@ start_service() {
service_triggers()
{
procd_add_reload_trigger "system"
+
procd_add_validation validate_ntp_section
+
}
diff --git a/openwrt/package/utils/busybox/files/sysntpd.hotplug b/openwrt/package/utils/busybox/files/sysntpd.hotplug
new file mode 100755
index 0000000..a9fae1b
--- /dev/null
+++ b/openwrt/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+check_int() {
+ list=$(uci get system.ntp.dhcp_ifaces)
+ if [ -z $list ];
+ then
+ return 0
+ fi
+
+ if [ "${list#*$INTERFACE}" != "$list" ]
+ then
+ return 0
+ else
+ return 1
+ fi
+}
+
+local proto="$(uci get network.$INTERFACE.proto)"
+local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+[ "$use_dhcp" = 1 ] && [ "$ACTION" = ifup ] && check_int && [ "$proto" = dhcp -o "$proto" = dhcp6 ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+if [ -z "$dhcp_ifaces" ]; then
+ dump=$(ubus call network.interface dump)
+ dhcp_ntp_servers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+else
+ for int in $dhcp_ifaces; do
+ status=$(ubus call network.interface.$int status)
+ ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] &&
+ dhcp_ntp_servers="dhcp_ntp_servers $ntpserver"
+ done
+fi
+
+new_ntp_servers="$dhcp_ntp_servers"
+#get the default list of ntp servers from the config file and append it to the new list
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+current_ntp_servers=$(ubus call service list '{"name":"sysntpd", "verbose":true}' | jsonfilter -e '$["sysntpd"]["instances"][*]["data"]["ntp_servers"]')
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
--
2.5.0
Bastian Bittorf
2016-01-07 09:58:50 UTC
Permalink
* amine ahd <***@gmail.com> [07.01.2016 10:34]:

the patch is from wrong dir.
please do a 'git format-patch' inside the OpenWrt-dir,
so the modified files are:

package/utils/busybox/Makefile
package/utils/busybox/files/sysntpd
package/utils/busybox/files/sysntpd.hotplug

for the subject: what means "[DEV-1329]"?
Post by amine ahd
+. /usr/share/libubox/jshn.sh
START=98
USE_PROCD=1
@@ -22,12 +24,32 @@ start_service() {
[ $enabled = 0 ] && return
- [ -z "$server" ] && return
please check if any interface is in DHCP-mode
and has a chance to get an NTP, otherwise return.
Post by amine ahd
+ if [ "$use_dhcp" = 1 ]; then
minor: use OpenWrt-style:
when there is no 'else', just do:

[ "$use_dhcp" = 1 ] && {
...
}
Post by amine ahd
+ if [ -z "$dhcp_ifaces" ]; then
+ dump=$(ubus call network.interface dump)
make 'dump' also 'local'
Post by amine ahd
+check_int() {
minor: choose better function name.
Post by amine ahd
+ list=$(uci get system.ntp.dhcp_ifaces)
+ if [ -z $list ];
+ then
+ return 0
+ fi
it's shorter:
[ -z "$list" ] && return
Post by amine ahd
+ if [ "${list#*$INTERFACE}" != "$list" ]
this looks strange to me and will IMHO not work
for similar names, e.g. eth0 eth0.1 eth0.2

you want to test, if the upcoming $INTERFACE is part
of allowed interfaces ("system.ntp.dhcp_ifaces"), aren't you?

is_valid_interface()
{
local list="$(uci get system.ntp.dhcp_ifaces)"

case " $list " in
*" $INTERFACE "*)
;;
*)
return 1
;;
esac
}
Post by amine ahd
+ for int in $dhcp_ifaces; do
please you 'iface' or 'interface' not 'int'
but thanks for the patch for now!

bye, bastian
Amine Aouled Hamed
2016-01-07 13:37:18 UTC
Permalink
Post by Bastian Bittorf
the patch is from wrong dir.
please do a 'git format-patch' inside the OpenWrt-dir,
package/utils/busybox/Makefile
package/utils/busybox/files/sysntpd
package/utils/busybox/files/sysntpd.hotplug
for the subject: what means "[DEV-1329]"?
please check if any interface is in DHCP-mode
and has a chance to get an NTP, otherwise return.
My mistake I forgot to change the directory when creating the patch!
[DEV-1329] is used internally by our company :p
question: Why would we need to return if there aren't interface in DHCP
mode? the static list is always used and for DHCP it is left to the user to
add that option in etc/config/system. I think it is safe to assume if the
user chooses to use the DHCP option then there will be at least one
interface in DHCP mode?

Regards,
Amine.
Bastian Bittorf
2016-01-07 13:42:05 UTC
Permalink
Post by Amine Aouled Hamed
Post by Bastian Bittorf
please check if any interface is in DHCP-mode
and has a chance to get an NTP, otherwise return.
question: Why would we need to return if there aren't interface in DHCP
mode? the static list is always used and for DHCP it is left to the user to
add that option in etc/config/system. I think it is safe to assume if the
user chooses to use the DHCP option then there will be at least one
interface in DHCP mode?
when looking at the present code, it
returns when the static list is empty.

maybe it's a rare condition, but your code
changes the behavior. for now ignore this
and polish the rest, lets see what the others say.

bye, bastian
Amine Aouled Hamed
2016-01-07 15:06:20 UTC
Permalink
Post by Bastian Bittorf
when looking at the present code, it
returns when the static list is empty.
maybe it's a rare condition, but your code
changes the behavior. for now ignore this
and polish the rest, lets see what the others say.
bye, bastian
AFAIK, this was the default behaviour. I removed it because IMO it is not
the case anymore to exit when we have an empty static list.
If this needs to be changed, please let me know.

Regards,
Amine.
amine ahd
2016-01-07 15:04:02 UTC
Permalink
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 28 +++++++++++-
.../package/utils/busybox/files/sysntpd.hotplug | 53 ++++++++++++++++++++++
3 files changed, 82 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug

diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 9571d48..3c33246 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -112,6 +112,9 @@ define Package/busybox/install
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/telnet $(1)/etc/init.d/telnet
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..b36afdb 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,6 +1,8 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
START=98

USE_PROCD=1
@@ -22,12 +24,32 @@ start_service() {

[ $enabled = 0 ] && return

- [ -z "$server" ] && return
-
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump=$(ubus call network.interface dump)
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
+ local status=$(ubus call network.interface.$int status)
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] &&
+ ntpservers="$ntpservers $ntpserver"
+ done
+ fi
+ # add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+ }
+
for peer in $server; do
procd_append_param command -p $peer
done
@@ -38,5 +60,7 @@ start_service() {
service_triggers()
{
procd_add_reload_trigger "system"
+
procd_add_validation validate_ntp_section
+
}
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100755
index 0000000..e0ba6cf
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+is_valid_interface() {
+ local list=$(uci get system.ntp.dhcp_ifaces)
+ [ -z "$list" ] && return 0;
+
+ case "$list" in
+ *"$INTERFACE"*)
+ return 0
+ ;;
+ *)
+ return 1
+ ;;
+ esac
+}
+
+local proto="$(uci get network.$INTERFACE.proto)"
+local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+[ "$use_dhcp" = 1 ] && [ "$ACTION" = ifup ] && is_valid_interface && [ "$proto" = dhcp -o "$proto" = dhcp6 ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+if [ -z "$dhcp_ifaces" ]; then
+ local dump=$(ubus call network.interface dump)
+ local dhcp_ntp_servers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+else
+ for iface in $dhcp_ifaces; do
+ local status=$(ubus call network.interface.$int status)
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] &&
+ dhcp_ntp_servers="dhcp_ntp_servers $ntpserver"
+ done
+fi
+
+new_ntp_servers="$dhcp_ntp_servers"
+#get the default list of ntp servers from the config file and append it to the new list
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+local current_ntp_servers=$(ubus call service list '{"name":"sysntpd", "verbose":true}' | jsonfilter -e '$["sysntpd"]["instances"][*]["data"]["ntp_servers"]')
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
--
2.5.0
Bastian Bittorf
2016-01-07 18:49:04 UTC
Permalink
Post by amine ahd
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 28 +++++++++++-
.../package/utils/busybox/files/sysntpd.hotplug | 53 ++++++++++++++++++++++
the path for "sysntpd.hotplug" is wrong, it should be:
package/utils/busybox/files/sysntpd.hotplug
Post by amine ahd
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump=$(ubus call network.interface dump)
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
+ local status=$(ubus call network.interface.$int status)
please test if your code really runs.
you have renamed "int" to "iface", but not everywhere
Post by amine ahd
+ [ -n "$ntpserver" ] &&
+ ntpservers="$ntpservers $ntpserver"
here is a '\' at line end missing, is'nt it?
Post by amine ahd
service_triggers()
{
procd_add_reload_trigger "system"
+
procd_add_validation validate_ntp_section
+
}
are these newlines really needed?
Post by amine ahd
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+is_valid_interface() {
+ local list=$(uci get system.ntp.dhcp_ifaces)
+ [ -z "$list" ] && return 0;
the ";" is unneeded
Post by amine ahd
+
+ case "$list" in
+ *"$INTERFACE"*)
+ return 0
please use the spaces like i wrote:
" $list "
and
*" $INTERFACE "*
Post by amine ahd
+ [ -n "$ntpserver" ] &&
+ dhcp_ntp_servers="dhcp_ntp_servers $ntpserver"
here is also a "\" missing

please: before sending this patch to the mailinglist, try
to manually apply it to a fresh git-checkout of openwrt.
after applying, test the resulting files with "shellcheck.net".

bye, bastian.
amine ahd
2016-01-12 08:44:53 UTC
Permalink
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 26 ++++++++++-
package/utils/busybox/files/sysntpd.hotplug | 53 ++++++++++++++++++++++
3 files changed, 80 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug

diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 9571d48..3c33246 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -112,6 +112,9 @@ define Package/busybox/install
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/telnet $(1)/etc/init.d/telnet
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..cc75cf9 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,6 +1,8 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
START=98

USE_PROCD=1
@@ -22,12 +24,32 @@ start_service() {

[ $enabled = 0 ] && return

- [ -z "$server" ] && return
-
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump=$(ubus call network.interface dump)
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
+ local status=$(ubus call network.interface.$iface status)
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \\
+ ntpservers="$ntpservers $ntpserver"
+ done
+ fi
+ # add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+ }
+
for peer in $server; do
procd_append_param command -p $peer
done
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100755
index 0000000..c0d0f37
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+is_valid_interface() {
+ local list=$(uci get system.ntp.dhcp_ifaces)
+ [ -z "$list" ] && return 0
+
+ case " $list " in
+ *" $INTERFACE "*)
+ return 0
+ ;;
+ *)
+ return 1
+ ;;
+ esac
+}
+
+local proto="$(uci get network.$INTERFACE.proto)"
+local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+[ "$use_dhcp" = 1 ] && [ "$ACTION" = ifup ] && is_valid_interface && [ "$proto" = dhcp -o "$proto" = dhcp6 ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+if [ -z "$dhcp_ifaces" ]; then
+ local dump=$(ubus call network.interface dump)
+ local dhcp_ntp_servers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+else
+ for iface in $dhcp_ifaces; do
+ local status=$(ubus call network.interface.$iface status)
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \\
+ dhcp_ntp_servers="dhcp_ntp_servers $ntpserver"
+ done
+fi
+
+new_ntp_servers="$dhcp_ntp_servers"
+#get the default list of ntp servers from the config file and append it to the new list
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+local current_ntp_servers=$(ubus call service list '{"name":"sysntpd", "verbose":true}' | jsonfilter -e '$["sysntpd"]["instances"][*]["data"]["ntp_servers"]')
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
--
2.5.0
Bastian Bittorf
2016-01-12 09:13:59 UTC
Permalink
Post by amine ahd
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump=$(ubus call network.interface dump)
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
please make 'ntpservers' once local in the function head
Post by amine ahd
+ else
+ for iface in $dhcp_ifaces; do
+ local status=$(ubus call network.interface.$iface status)
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \\
this is one '\' to much
Post by amine ahd
+is_valid_interface() {
+ local list=$(uci get system.ntp.dhcp_ifaces)
just for me: use list="$( ... )"

bye,bastian
Bastian Bittorf
2016-01-12 09:20:45 UTC
Permalink
Post by amine ahd
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 26 ++++++++++-
package/utils/busybox/files/sysntpd.hotplug | 53 ++++++++++++++++++++++
3 files changed, 80 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug
also it does not apply:

***@X301:~/software/openwrt$ curl -s https://patchwork.ozlabs.org/patch/566411/mbox/ | git apply --check
error: patch failed: package/utils/busybox/Makefile:112
error: package/utils/busybox/Makefile: patch does not apply

bye, bastian
Amine Aouled Hamed
2016-01-13 08:47:48 UTC
Permalink
Apologies, I was working with an old fork of Openwrt.
another version (hopefully the last) will be sent soon.

Regards,
Amine.
Post by amine ahd
Post by amine ahd
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 26 ++++++++++-
package/utils/busybox/files/sysntpd.hotplug | 53
++++++++++++++++++++++
Post by amine ahd
3 files changed, 80 insertions(+), 2 deletions(-)
create mode 100755 package/utils/busybox/files/sysntpd.hotplug
https://patchwork.ozlabs.org/patch/566411/mbox/ | git apply --check
error: patch failed: package/utils/busybox/Makefile:112
error: package/utils/busybox/Makefile: patch does not apply
bye, bastian
--
Amine Hamed | Software Engineer



Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany

Email ***@ocedo.com


<***@ocedo.com>

REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER:
HRB 717873
MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT
amine ahd
2016-01-14 09:13:34 UTC
Permalink
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 30 ++++++++++++++--
package/utils/busybox/files/sysntpd.hotplug | 53 +++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 3 deletions(-)
create mode 100644 package/utils/busybox/files/sysntpd.hotplug

diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 5ca4363..3066a85 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -111,6 +111,9 @@ define Package/busybox/install
$(CP) $(PKG_INSTALL_DIR)/* $(1)/
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..86c1036 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,6 +1,9 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
START=98

USE_PROCD=1
@@ -13,7 +16,8 @@ validate_ntp_section() {
}

start_service() {
- local server enabled enable_server peer
+ local server enabled enable_server peer ntpservers
+ local use_dhcp="$(uci -q get system.ntp.use_dhcp)"

validate_ntp_section ntp || {
echo "validation failed"
@@ -21,13 +25,33 @@ start_service() {
}

[ $enabled = 0 ] && return
-
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return

procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump="$(ubus call network.interface dump)"
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
+ local status="$(ubus call network.interface.$iface status)"
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \
+ ntpservers="$ntpservers $ntpserver"
+ done
+ fi
+ # add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+ }
+
for peer in $server; do
procd_append_param command -p $peer
done
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100644
index 0000000..c690304
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+is_valid_interface() {
+ local list="$(uci get system.ntp.dhcp_ifaces)"
+ [ -z "$list" ] && return 0
+
+ case " $list " in
+ *" $INTERFACE "*)
+ return 0
+ ;;
+ *)
+ return 1
+ ;;
+ esac
+}
+
+local proto="$(uci get network.$INTERFACE.proto)"
+local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
+[ "$use_dhcp" = 1 ] && [ "$ACTION" = ifup ] && is_valid_interface && [ "$proto" = dhcp -o "$proto" = dhcp6 ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+if [ -z "$dhcp_ifaces" ]; then
+ local dump="$(ubus call network.interface dump)"
+ local dhcp_ntp_servers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+else
+ for iface in $dhcp_ifaces; do
+ local status="$(ubus call network.interface.$iface status)"
+ local ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \
+ dhcp_ntp_servers="$dhcp_ntp_servers $ntpserver"
+ done
+fi
+
+new_ntp_servers="$dhcp_ntp_servers"
+#get the default list of ntp servers from the config file and append it to the new list
+config_load system
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+local current_ntp_servers=$(ubus call service list '{"name":"sysntpd", "verbose":true}' | jsonfilter -e '$["sysntpd"]["instances"][*]["data"]["ntp_servers"]')
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
\ No newline at end of file
--
2.5.0
Bastian Bittorf
2016-01-14 09:44:55 UTC
Permalink
* amine ahd <***@gmail.com> [14.01.2016 10:29]:

thank you, patch applies...
Post by amine ahd
start_service() {
- local server enabled enable_server peer
+ local server enabled enable_server peer ntpservers
+ local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
validate_ntp_section ntp || {
echo "validation failed"
@@ -21,13 +25,33 @@ start_service() {
}
[ $enabled = 0 ] && return
-
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return
i'am ok with this, if you like you can reuse
'config_get_bool()' from /lib/functions.sh
Post by amine ahd
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ [ "$use_dhcp" = 1 ] && {
this should also be 'bool'
Post by amine ahd
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump="$(ubus call network.interface dump)"
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
please make var 'iface' local and while you are there, move
all the 'local' declaration to the head of the function.

the rest looks OK to me. - thank you

bye, bastian
Amine Aouled Hamed
2016-01-15 08:48:13 UTC
Permalink
Post by Bastian Bittorf
please make var 'iface' local and while you are there, move
all the 'local' declaration to the head of the function.
Wouldn't be better if the local vars inside of the if and for statements
stay inside?
I mean what is the point of declaring ifaces if I will be using all the
ifaces? in term of performance(even if it is negligible and clarity of the
code).

Thanks for following the patch!
Regards,
Amine
Post by Bastian Bittorf
thank you, patch applies...
Post by amine ahd
start_service() {
- local server enabled enable_server peer
+ local server enabled enable_server peer ntpservers
+ local use_dhcp="$(uci -q get system.ntp.use_dhcp)"
validate_ntp_section ntp || {
echo "validation failed"
@@ -21,13 +25,33 @@ start_service() {
}
[ $enabled = 0 ] && return
-
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return
i'am ok with this, if you like you can reuse
'config_get_bool()' from /lib/functions.sh
Post by amine ahd
procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S
"$HOTPLUG_SCRIPT"
Post by amine ahd
+
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+ [ "$use_dhcp" = 1 ] && {
this should also be 'bool'
Post by amine ahd
+ if [ -z "$dhcp_ifaces" ]; then
+ local dump="$(ubus call network.interface dump)"
+ ntpservers=$(jsonfilter -s "$dump" -e
'$["interface"][*]["data"]["ntpserver"]')
Post by amine ahd
+ else
+ for iface in $dhcp_ifaces; do
please make var 'iface' local and while you are there, move
all the 'local' declaration to the head of the function.
the rest looks OK to me. - thank you
bye, bastian
--
Amine Hamed | Software Engineer



Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany

Email ***@ocedo.com


<***@ocedo.com>

REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER:
HRB 717873
MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT
Bastian Bittorf
2016-01-15 10:19:55 UTC
Permalink
Post by Amine Aouled Hamed
Post by Bastian Bittorf
please make var 'iface' local and while you are there, move
all the 'local' declaration to the head of the function.
Wouldn't be better if the local vars inside of the if and for statements
stay inside?
maybe thats personal preference, i like it outside a loop.
Post by Amine Aouled Hamed
I mean what is the point of declaring ifaces if I will be using all the
ifaces? in term of performance(even if it is negligible and clarity of the
code).
you are right, performance is not an issue here.
i just spotted this in memory if a former double-local-crash-bug
in busybox (which is what happens in a for-loop)

also: declaring iface 'local' means, that it is thrown away
when the functions returns, otherwise it will pollute your
env-space.

bye, bastian
amine ahd
2016-01-19 08:02:27 UTC
Permalink
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 33 +++++++++++++++---
package/utils/busybox/files/sysntpd.hotplug | 54 +++++++++++++++++++++++++++++
3 files changed, 86 insertions(+), 4 deletions(-)
create mode 100644 package/utils/busybox/files/sysntpd.hotplug

diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 5ca4363..3066a85 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -111,6 +111,9 @@ define Package/busybox/install
$(CP) $(PKG_INSTALL_DIR)/* $(1)/
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..118c3e3 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,6 +1,9 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
START=98

USE_PROCD=1
@@ -13,21 +16,43 @@ validate_ntp_section() {
}

start_service() {
- local server enabled enable_server peer
-
+ local server enabled enable_server peer ntpservers iface status ntpserver dump
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+
+ config_load system
+ config_get_bool "use_dhcp" "ntp" "use_dhcp"
validate_ntp_section ntp || {
echo "validation failed"
return 1
}

[ $enabled = 0 ] && return
-
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return

procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ dump="$(ubus call network.interface dump)"
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
+ status="$(ubus call network.interface.$iface status)"
+ ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \
+ ntpservers="$ntpservers $ntpserver"
+ done
+ fi
+ # add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+ }
+
for peer in $server; do
procd_append_param command -p $peer
done
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100644
index 0000000..34a2f7a
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+is_valid_interface() {
+ local list="$(uci get system.ntp.dhcp_ifaces)"
+ [ -z "$list" ] && return 0
+
+ case " $list " in
+ *" $INTERFACE "*)
+ return 0
+ ;;
+ *)
+ return 1
+ ;;
+ esac
+}
+
+config_load system
+local proto="$(uci get network.$INTERFACE.proto)"
+config_get_bool "use_dhcp" "ntp" "use_dhcp"
+[ "$use_dhcp" = 1 ] && [ "$ACTION" = ifup ] && is_valid_interface && [ "$proto" = dhcp -o "$proto" = dhcp6 ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local dhcp_ntp_servers iface status ntpserver dump
+local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+if [ -z "$dhcp_ifaces" ]; then
+ dump="$(ubus call network.interface dump)"
+ dhcp_ntp_servers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+else
+ for iface in $dhcp_ifaces; do
+ status="$(ubus call network.interface.$iface status)"
+ ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \
+ dhcp_ntp_servers="$dhcp_ntp_servers $ntpserver"
+ done
+fi
+
+new_ntp_servers="$dhcp_ntp_servers"
+#get the default list of ntp servers from the config file and append it to the new list
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+local current_ntp_servers=$(ubus call service list '{"name":"sysntpd", "verbose":true}' | jsonfilter -e '$["sysntpd"]["instances"][*]["data"]["ntp_servers"]')
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
\ No newline at end of file
--
2.5.0
Bastian Bittorf
2016-01-19 09:21:16 UTC
Permalink
Post by amine ahd
start_service() {
- local server enabled enable_server peer
-
+ local server enabled enable_server peer ntpservers iface status ntpserver dump
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+
please remove whitespaces in the line above,
otherwise it looks fine to me.

bye, bastian
John Crispin
2016-01-19 09:35:50 UTC
Permalink
Post by Bastian Bittorf
Post by amine ahd
start_service() {
- local server enabled enable_server peer
-
+ local server enabled enable_server peer ntpservers iface status ntpserver dump
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+
please remove whitespaces in the line above,
otherwise it looks fine to me.
bye, bastian
the subject prefix is also missing

John
David Lang
2016-01-27 08:04:45 UTC
Permalink
Just a side note while you are working in this area.

all the documentation for udhcpc (including it's default config settings) says
that it uses one default file path, but when it's run that path gets overridden
on the command line and a different file is used instead.

The default in the config should be changed to what's actually used and the
extra copy should be eliminated.

David Lang

amine ahd
2016-01-20 09:04:17 UTC
Permalink
The current state of NTP is to load the list of NTP servers
from the static file /etc/config/system.
This patch allows ntpd to get NTP servers from DHCP.


Changes from V1:
-Users could choose not to use DHCP by setting "use_dhcp" to 0 in /etc/config/system under the ntp section.
-Users could specify which interfaces to use to get NTP servers from.
-Sysntpd will exit if no servers are specified in the static list and the DHCP option is disabled.
-Sysntpd will restart only if all of the following conditions are met:
*The user allowed DHCP to be used for NTP.
*The iface action is UP.
*The iface is specified in the list.
*The protocol in use is either DHCP or DHCPv6.
-Code improvements.

Signed-off-by: amine hamed <***@gmail.com>
---
package/utils/busybox/Makefile | 3 ++
package/utils/busybox/files/sysntpd | 31 +++++++++++++++--
package/utils/busybox/files/sysntpd.hotplug | 54 +++++++++++++++++++++++++++++
3 files changed, 85 insertions(+), 3 deletions(-)
create mode 100644 package/utils/busybox/files/sysntpd.hotplug

diff --git a/package/utils/busybox/Makefile b/package/utils/busybox/Makefile
index 5ca4363..3066a85 100644
--- a/package/utils/busybox/Makefile
+++ b/package/utils/busybox/Makefile
@@ -111,6 +111,9 @@ define Package/busybox/install
$(CP) $(PKG_INSTALL_DIR)/* $(1)/
$(INSTALL_BIN) ./files/cron $(1)/etc/init.d/cron
$(INSTALL_BIN) ./files/sysntpd $(1)/etc/init.d/sysntpd
+ $(INSTALL_DIR) $(1)/etc/hotplug.d
+ $(INSTALL_DIR) $(1)/etc/hotplug.d/iface
+ $(INSTALL_BIN) ./files/sysntpd.hotplug $(1)/etc/hotplug.d/iface/30-sysntpd
$(INSTALL_BIN) ./files/ntpd-hotplug $(1)/usr/sbin/ntpd-hotplug
-rm -rf $(1)/lib64
endef
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd
index f73bb83..c5dac8b 100755
--- a/package/utils/busybox/files/sysntpd
+++ b/package/utils/busybox/files/sysntpd
@@ -1,6 +1,9 @@
#!/bin/sh /etc/rc.common
# Copyright (C) 2011 OpenWrt.org

+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
START=98

USE_PROCD=1
@@ -13,21 +16,43 @@ validate_ntp_section() {
}

start_service() {
- local server enabled enable_server peer
+ local server enabled enable_server peer ntpservers iface status ntpserver dump
+ local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"

+ config_load system
+ config_get_bool "use_dhcp" "ntp" "use_dhcp"
validate_ntp_section ntp || {
echo "validation failed"
return 1
}

[ $enabled = 0 ] && return
-
- [ -z "$server" ] && return
+ [ -z "$server" ] && [ "$use_dhcp" = 0 ] && return

procd_open_instance
procd_set_param command "$PROG" -n
[ "$enable_server" = "1" ] && procd_append_param command -l
[ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT"
+
+ [ "$use_dhcp" = 1 ] && {
+ if [ -z "$dhcp_ifaces" ]; then
+ dump="$(ubus call network.interface dump)"
+ ntpservers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+ else
+ for iface in $dhcp_ifaces; do
+ status="$(ubus call network.interface.$iface status)"
+ ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \
+ ntpservers="$ntpservers $ntpserver"
+ done
+ fi
+ # add this data so we can use it in the sysntpd hotplug script.
+ procd_set_param data ntp_servers="$ntpservers $server"
+ for ntpserver in $ntpservers; do
+ procd_append_param command -p $ntpserver
+ done
+ }
+
for peer in $server; do
procd_append_param command -p $peer
done
diff --git a/package/utils/busybox/files/sysntpd.hotplug b/package/utils/busybox/files/sysntpd.hotplug
new file mode 100644
index 0000000..34a2f7a
--- /dev/null
+++ b/package/utils/busybox/files/sysntpd.hotplug
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /usr/share/libubox/jshn.sh
+
+is_valid_interface() {
+ local list="$(uci get system.ntp.dhcp_ifaces)"
+ [ -z "$list" ] && return 0
+
+ case " $list " in
+ *" $INTERFACE "*)
+ return 0
+ ;;
+ *)
+ return 1
+ ;;
+ esac
+}
+
+config_load system
+local proto="$(uci get network.$INTERFACE.proto)"
+config_get_bool "use_dhcp" "ntp" "use_dhcp"
+[ "$use_dhcp" = 1 ] && [ "$ACTION" = ifup ] && is_valid_interface && [ "$proto" = dhcp -o "$proto" = dhcp6 ] || exit 0
+
+handle_default_ntp_servers() {
+ local server="$1"
+ new_ntp_servers="$new_ntp_servers $server"
+}
+
+local dhcp_ntp_servers iface status ntpserver dump
+local dhcp_ifaces="$(uci -q get system.ntp.dhcp_ifaces)"
+if [ -z "$dhcp_ifaces" ]; then
+ dump="$(ubus call network.interface dump)"
+ dhcp_ntp_servers=$(jsonfilter -s "$dump" -e '$["interface"][*]["data"]["ntpserver"]')
+else
+ for iface in $dhcp_ifaces; do
+ status="$(ubus call network.interface.$iface status)"
+ ntpserver=$(jsonfilter -s "$status" -e '$["data"]["ntpserver"]')
+ [ -n "$ntpserver" ] && \
+ dhcp_ntp_servers="$dhcp_ntp_servers $ntpserver"
+ done
+fi
+
+new_ntp_servers="$dhcp_ntp_servers"
+#get the default list of ntp servers from the config file and append it to the new list
+config_list_foreach "ntp" "server" handle_default_ntp_servers
+
+#get the current list of ntp servers in the running instance
+local current_ntp_servers=$(ubus call service list '{"name":"sysntpd", "verbose":true}' | jsonfilter -e '$["sysntpd"]["instances"][*]["data"]["ntp_servers"]')
+#if its an up action, the iface uses DHCP and the new list of ntp servers is different from the old, restart sysntpd
+[ "$current_ntp_servers" != "$new_ntp_servers" ] || exit 0
+
+logger -t sysntpd "Reloading sysntpd due to $ACTION of interface $INTERFACE and a change of NTP servers"
+/etc/init.d/sysntpd enabled && /etc/init.d/sysntpd reload
\ No newline at end of file
--
2.5.0
Bastian Bittorf
2016-01-20 09:14:07 UTC
Permalink
* amine ahd <***@gmail.com> [20.01.2016 10:09]:

thank you!
Post by amine ahd
The current state of NTP is to load the list of NTP servers
from the static file /etc/config/system.
This patch allows ntpd to get NTP servers from DHCP.
like john wrote already:
if you edit the first line of the commit-message, please write:

busybox: ntp - use server received via DHCP

the rest looks good to me. - bye, bastian
Amine Aouled Hamed
2016-01-20 09:16:38 UTC
Permalink
Ah sorry, I thought he meant the description of the patch.
Will change it now
Post by Bastian Bittorf
thank you!
Post by amine ahd
The current state of NTP is to load the list of NTP servers
from the static file /etc/config/system.
This patch allows ntpd to get NTP servers from DHCP.
busybox: ntp - use server received via DHCP
the rest looks good to me. - bye, bastian
--
Amine Hamed | Software Engineer



Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany

Email ***@ocedo.com


<***@ocedo.com>

REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER:
HRB 717873
MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT
Loading...