| Message ID | 1453293244-5797-1-git-send-email-amine.ahd@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 2016-01-20 13:34, amine ahd wrote: > 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 <amine.ahd@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/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 This is all way more complex than it needs to be. There's a simple solution - just replace the sysntpd init script service_triggers function with this: service_triggers() { local script=$(readlink "$initscript") local name=$(basename ${script:-$initscript}) procd_open_trigger procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload procd_close_trigger procd_add_reload_trigger "system" procd_add_validation validate_ntp_section } You can drop the hotplug script entirely. What this will do is it will instruct procd to run the init script, whenever an interface up/down event arrives (waiting for up to 2 seconds before starting the script instead of starting it once for every single event). procd will ensure that the sysntpd daemon is only restarted if the command line actually changed, so you don't have to add any code to compare the old and the new ntp server list. - Felix
Hi, I tried this method the first time but it does nothing. I just tried it again now and the same thing happens (added a log message to logread in case it is restarted). Thats why I went to the hotplug script, add to it the fact that we cant choose specific interfaces to listen to using this method. On Wed, Jan 20, 2016 at 1:51 PM, Felix Fietkau <nbd@openwrt.org> wrote: > On 2016-01-20 13:34, amine ahd wrote: > > 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 <amine.ahd@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/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 > This is all way more complex than it needs to be. There's a simple > solution - > just replace the sysntpd init script service_triggers function with this: > > service_triggers() { > local script=$(readlink "$initscript") > local name=$(basename ${script:-$initscript}) > > procd_open_trigger > procd_add_raw_trigger "interface.*" 2000 /etc/init.d/$name reload > procd_close_trigger > > procd_add_reload_trigger "system" > procd_add_validation validate_ntp_section > } > > You can drop the hotplug script entirely. > What this will do is it will instruct procd to run the init script, > whenever an interface up/down event arrives (waiting for up to 2 seconds > before starting the script instead of starting it once for every single > event). procd will ensure that the sysntpd daemon is only restarted if > the command line actually changed, so you don't have to add any code > to compare the old and the new ntp server list. > > - Felix >
On 2016-01-22 14:12, Amine Aouled Hamed wrote: > Hi, > I tried this method the first time but it does nothing. > I just tried it again now and the same thing happens (added a log > message to logread in case it is restarted). > Thats why I went to the hotplug script, add to it the fact that we cant > choose specific interfaces to listen to using this method. You can also choose specific interfaces via triggers, I just wrote the simple variant that catches all changes. Please show me the exact patch that you tried, so I can check it to see if I can spot the bug. - Felix
Hi Felix, Actually I made my tests on an old version of OpenWrt, maybe procd changed since that version. Will make another tests using the current version and let you know of the results. On Fri, Jan 22, 2016 at 2:39 PM, Felix Fietkau <nbd@openwrt.org> wrote: > On 2016-01-22 14:12, Amine Aouled Hamed wrote: > > Hi, > > I tried this method the first time but it does nothing. > > I just tried it again now and the same thing happens (added a log > > message to logread in case it is restarted). > > Thats why I went to the hotplug script, add to it the fact that we cant > > choose specific interfaces to listen to using this method. > You can also choose specific interfaces via triggers, I just wrote the > simple variant that catches all changes. Please show me the exact patch > that you tried, so I can check it to see if I can spot the bug. > > - Felix >
So using the latest procd version, I added a simple logger message on top start_service() in sysntpd and added the triggers in service_triggers and to test I just unplugged and re-plugget my network cable and checked logread. Nothing shows up. On Mon, Jan 25, 2016 at 10:26 AM, Amine Aouled Hamed <amine.ahd@gmail.com> wrote: > Hi Felix, > Actually I made my tests on an old version of OpenWrt, maybe procd changed > since that version. > Will make another tests using the current version and let you know of the > results. > > On Fri, Jan 22, 2016 at 2:39 PM, Felix Fietkau <nbd@openwrt.org> wrote: > >> On 2016-01-22 14:12, Amine Aouled Hamed wrote: >> > Hi, >> > I tried this method the first time but it does nothing. >> > I just tried it again now and the same thing happens (added a log >> > message to logread in case it is restarted). >> > Thats why I went to the hotplug script, add to it the fact that we cant >> > choose specific interfaces to listen to using this method. >> You can also choose specific interfaces via triggers, I just wrote the >> simple variant that catches all changes. Please show me the exact patch >> that you tried, so I can check it to see if I can spot the bug. >> >> - Felix >> > > > > -- > > Amine Hamed | Software Engineer > > > > Ocedo GmbH | Hirschstrasse 7 | 76133 Karlsruhe | Germany > > Email ahamed@ocedo.com > > > <ahamed@ocedo.com> > > REGISTERED OFFICE: KARLSRUHE | DISTRICT COURT: MANNHEIM | REGISTER NUMBER: > HRB 717873 > MANAGING DIRECTOR: MARKUS HENNIG|JAN HICHERT >
* Amine Aouled Hamed <amine.ahd@gmail.com> [25.01.2016 14:53]: > So using the latest procd version, > I added a simple logger message on top start_service() in sysntpd and added > the triggers in service_triggers and to test I just unplugged and > re-plugget my network cable and checked logread. > Nothing shows up. not every switchport is aware of un- or replug. better wait for DHCP-lease timeout to check if it triggers. (or do 'ifup lan') bye, bastian
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
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 <amine.ahd@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