| Message ID | 20210529233909.1673-1-dobrovolskiy.alexey@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | busybox: sysntpd: add trigger to reload server | expand |
Comments... > On May 29, 2021, at 5:39 PM, Alexey Dobrovolsky <dobrovolskiy.alexey@gmail.com> wrote: > > sysntpd server becomes unavailable if the index of the bound > interface changes. So let's add an interface trigger to reload sysntpd. > > This patch also adds the ability for the sysntpd script to handle > uci interface name from configuration. > > Fixes: 4da60500ebd2 ("busybox: sysntpd: option to bind server to iface") > Signed-off-by: Alexey Dobrovolsky <dobrovolskiy.alexey@gmail.com> > --- > package/utils/busybox/files/sysntpd | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd > index c4c311c242..e1c9e0cdca 100755 > --- a/package/utils/busybox/files/sysntpd > +++ b/package/utils/busybox/files/sysntpd > @@ -56,7 +56,14 @@ start_ntpd_instance() { > procd_set_param command "$PROG" -n -N > if [ "$enable_server" = "1" ]; then > procd_append_param command -l > - [ -n "$interface" ] && procd_append_param command -I $interface > + [ -n "$interface" ] && { > + local ifname > + > + network_get_device ifname "$interface" || \ > + ifname="$interface" > + procd_append_param command -I "$ifname" > + procd_append_param netdev "$ifname" > + } > fi > [ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT" > for peer in $server; do > @@ -79,11 +86,12 @@ start_ntpd_instance() { > } > > start_service() { > + . /lib/functions/network.sh This doesn't look right. It's usually added at the top of the file, unnested. > validate_ntp_section ntp start_ntpd_instance > } > > service_triggers() { > - local script name use_dhcp > + local enable_server interface name script use_dhcp I'd rather leave the existing variables in their original order. > > script=$(readlink -f "$initscript") > name=$(basename ${script:-$initscript}) > @@ -106,5 +114,17 @@ service_triggers() { > fi > } > > + config_get enable_server ntp enable_server > + config_get interface ntp interface > + > + [ "$enable_server" = "1" ] && [ -n "$interface" ] && { > + local ifname > + > + network_get_device ifname "$interface" || \ > + ifname="$interface" > + procd_add_interface_trigger "interface.*" "$ifname" \ > + /etc/init.d/"$name" reload > + } > + > procd_add_validation validate_ntp_section > } > -- > 2.25.1 >
Hi, >> start_service() { >> + . /lib/functions/network.sh > > > This doesn't look right. It's usually added at the top of the file, unnested. Which would be the wrong thing to do here. Since the init script is run on the host system during build (to enable it), it must not source files which are only available on target. Sourcing the library inside the procedure is correct. ~ Jo
Hi, > > service_triggers() { > > - local script name use_dhcp > > + local enable_server interface name script use_dhcp > > > I'd rather leave the existing variables in their original order. > > OpenWrt Submission Guidelines [0] says that entries in lists should be placed in alphabetical order. Or should variables be declared in the order in which they are used? [0] https://openwrt.org/submitting-patches#submission_guidelines
On 2/06/2021 01:10, Alexey Dobrovolskiy wrote: > Hi, > >>> service_triggers() { >>> - local script name use_dhcp >>> + local enable_server interface name script use_dhcp >> >> I'd rather leave the existing variables in their original order. >> >> > OpenWrt Submission Guidelines [0] says that entries in lists should be > placed in alphabetical order. > Or should variables be declared in the order in which they are used? > > [0] https://openwrt.org/submitting-patches#submission_guidelines This is unrelated to the change mentioned in the commit message. Ideally cosmetic changes like this go in an earlier, separate commit. Stijn
> On Jun 2, 2021, at 11:19 AM, Stijn Tintel <stijn@linux-ipv6.be> wrote: > > On 2/06/2021 01:10, Alexey Dobrovolskiy wrote: >> Hi, >> >>>> service_triggers() { >>>> - local script name use_dhcp >>>> + local enable_server interface name script use_dhcp >>> >>> I'd rather leave the existing variables in their original order. >>> >>> >> OpenWrt Submission Guidelines [0] says that entries in lists should be >> placed in alphabetical order. >> Or should variables be declared in the order in which they are used? >> >> [0] https://openwrt.org/submitting-patches#submission_guidelines > > This is unrelated to the change mentioned in the commit message. Ideally > cosmetic changes like this go in an earlier, separate commit. > > Stijn > Agreed. Style and substance shouldn't be mingled.
diff --git a/package/utils/busybox/files/sysntpd b/package/utils/busybox/files/sysntpd index c4c311c242..e1c9e0cdca 100755 --- a/package/utils/busybox/files/sysntpd +++ b/package/utils/busybox/files/sysntpd @@ -56,7 +56,14 @@ start_ntpd_instance() { procd_set_param command "$PROG" -n -N if [ "$enable_server" = "1" ]; then procd_append_param command -l - [ -n "$interface" ] && procd_append_param command -I $interface + [ -n "$interface" ] && { + local ifname + + network_get_device ifname "$interface" || \ + ifname="$interface" + procd_append_param command -I "$ifname" + procd_append_param netdev "$ifname" + } fi [ -x "$HOTPLUG_SCRIPT" ] && procd_append_param command -S "$HOTPLUG_SCRIPT" for peer in $server; do @@ -79,11 +86,12 @@ start_ntpd_instance() { } start_service() { + . /lib/functions/network.sh validate_ntp_section ntp start_ntpd_instance } service_triggers() { - local script name use_dhcp + local enable_server interface name script use_dhcp script=$(readlink -f "$initscript") name=$(basename ${script:-$initscript}) @@ -106,5 +114,17 @@ service_triggers() { fi } + config_get enable_server ntp enable_server + config_get interface ntp interface + + [ "$enable_server" = "1" ] && [ -n "$interface" ] && { + local ifname + + network_get_device ifname "$interface" || \ + ifname="$interface" + procd_add_interface_trigger "interface.*" "$ifname" \ + /etc/init.d/"$name" reload + } + procd_add_validation validate_ntp_section }
sysntpd server becomes unavailable if the index of the bound interface changes. So let's add an interface trigger to reload sysntpd. This patch also adds the ability for the sysntpd script to handle uci interface name from configuration. Fixes: 4da60500ebd2 ("busybox: sysntpd: option to bind server to iface") Signed-off-by: Alexey Dobrovolsky <dobrovolskiy.alexey@gmail.com> --- package/utils/busybox/files/sysntpd | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)