Re: [PATCH] busybox: sysntpd: add trigger to reload server

2021-06-02 Thread Philip Prindeville 



> On Jun 2, 2021, at 11:19 AM, Stijn Tintel  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.


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] busybox: sysntpd: add trigger to reload server

2021-06-02 Thread Stijn Tintel 
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


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] busybox: sysntpd: add trigger to reload server

2021-06-01 Thread Alexey Dobrovolskiy 
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
-- 
Best regards,
Alexey

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] busybox: sysntpd: add trigger to reload server

2021-06-01 Thread Jo-Philipp Wich 
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

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] busybox: sysntpd: add trigger to reload server

2021-06-01 Thread Philip Prindeville 
Comments...


> On May 29, 2021, at 5:39 PM, Alexey Dobrovolsky 
>  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 
> ---
> 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
> 


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[PATCH] busybox: sysntpd: add trigger to reload server

2021-05-29 Thread Alexey Dobrovolsky 
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 
---
 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
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
 }
-- 
2.25.1


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel