[PATCHv3 1/3] base-files: use hwclock --systz

Sun Jan 10 18:03:12 EST 2021

On Sun, Jan 10, 2021 at 10:47 AM Hannu Nyman <hannu.nyman at iki.fi> wrote:
>
> Hannu Nyman kirjoitti 10.1.2021 klo 19.25:
> > Rosen Penev kirjoitti 8.1.2021 klo 5.30:
> >> The date -k patch is non standard and will be removed in the next
> >> commit.
> >>
> >> ...
> >>
> >> --- a/package/base-files/files/etc/init.d/system
> >> +++ b/package/base-files/files/etc/init.d/system
> >> @@ -27,7 +27,7 @@ system_config() {
> >>           ln -sf "/usr/share/zoneinfo/$zonename" /tmp/localtime && rm -f
> >> /tmp/TZ
> >>         # apply timezone to kernel
> >> -    busybox date -k
> >> +    hwclock --systz
> >>   }
> >>     reload_service() {
> >
> >
> > I suspect that this proposed modification causes a noticeable difference to
> > the previous way:
> > After reboot, the early part of the log seems to be in UTC instead of the
> > local timezone, as earlier. (Likely sysfixtime has understood the timestamp
> > of the last file in /etc wrongly)
> >
> > Here is a extract from system log of the router with busybox 1.33 in a
> > simple reboot. I touched a file in /etc just before reboot, so sysfixtime
> > should set roughly correct time early in the boot process. There should be
> > just approx. one minute jump when NTP corrects time (from the
> > most-recent-file based boot time as set by sysfixtime), instead of two
> > hours + 1 minute.
> >
> > Sun Jan 10 16:37:50 2021 daemon.info procd: - init complete -
> > Sun Jan 10 16:37:52 2021 user.notice nlbwmon: Reloading nlbwmon due to ifup
> > of lan6 (br-lan)
> > Sun Jan 10 18:38:56 2021 user.notice ntpd: Time set, stratum=16 interval=32
> > offset=7262.868194
> >
> >
> > Something has prevented the normal timezone handling there.
> >
> >
> > Doing the same reboot with a 2 Jan 2021 build without buysybox 1.33 makes
> > the time shift jump look again normal. There is just the expected one
> > minute jump:
> >
> > Sun Jan 10 19:04:01 2021 daemon.info procd: - init complete -
> > Sun Jan 10 19:05:03 2021 user.notice ntpd: Time set, stratum=16 interval=32
> > offset=60.906955
> > Sun Jan 10 19:05:05 2021 user.notice nlbwmon: Reloading nlbwmon due to ifup
> > of lan6 (br-lan)
> >
> >
> > It is of course possible that this might be due to some change in
> > base-files or kernel last week, but that is hard to believe.
> >
> > I suspect that the "hwclock --systz" does something slightly different than
> > our "date -k", at least on routers without a real RTC.
> >
> > My proposal would be to revert your removal of the date-k patch and
> > continue using it the previous way.
> >
> > (I will try to test with date-k reverted if things then work ok with 1.33.
> > Might be that this is something different.)
> >
> I compiled the same up-to-date master with busybox 1.33 but with the date-k
> removal reveerted and refreshed, and the timestamps in the system log again
> behave normally:
>
>
> After flash of r15469 with busybox 1.33 with date-k changed reverted (and
> refreshed).
> Normal time shift, roughly the difference of build & flashing
>
> Sun Jan 10 19:44:59 2021 user.notice nlbwmon: Reloading nlbwmon due to ifup
> of loopback (lo)
> Sun Jan 10 19:45:02 2021 daemon.info procd: - init complete -
> Sun Jan 10 19:45:02 2021 daemon.info urandom_seed[4597]: Seed saved
> (/etc/urandom.seed)
> Sun Jan 10 20:25:06 2021 user.notice ntpd: Time set, stratum=16 interval=32
> offset=2403.398106
> Sun Jan 10 20:25:06 2021 user.notice nlbwmon: Reloading nlbwmon due to ifup
> of lan6 (br-lan)
>
>
> After a normal boot, again a normal 1 minute leap when NTP kicks in:
>
> Sun Jan 10 20:39:24 2021 daemon.err uhttpd[2934]: luci: accepted login on
> /admin/status/overview for root from 192.168.1.180
> Sun Jan 10 20:39:25 2021 daemon.info procd: - init complete -
> Sun Jan 10 20:40:45 2021 user.notice ntpd: Time set, stratum=16 interval=32
> offset=79.663960
> Sun Jan 10 20:40:46 2021 user.notice ntpd: Stratum change, stratum=4
> interval=32 offset=0.001017
> Sun Jan 10 20:40:48 2021 user.notice nlbwmon: Reloading nlbwmon due to ifup
> of lan6 (br-lan)
>
> I think that the proposed date-k removal and /etc/init.d/system change should
> be removed from the version bump.
>
>
> The patch 250-date-k-flag.patch needs to be refreshed. This is the original
> compared to the refreshed-for-1.33.0
Several problems with this.

The enum change is questionable.

This will fail on musl 1.2.x. busybox with version 1.33 has special
handling for settimeofday. No idea what the glibc story is.

I would prefer that hwclock be patched instead of duplicating code.
date -k is not even a standard option anywhere:

~/> date -k
date: invalid option -- 'k'
>
>
> diff --git a/package/utils/busybox/patches/250-date-k-flag.patch
> b/package/utils/busybox/patches/250-date-k-flag.patch
> index 5aadbb233c..3a666c581d 100644
> --- a/package/utils/busybox/patches/250-date-k-flag.patch
> +++ b/package/utils/busybox/patches/250-date-k-flag.patch
> @@ -1,14 +1,14 @@
>   --- a/coreutils/date.c
>   +++ b/coreutils/date.c
> -@@ -123,6 +123,7 @@
> - //usage:    IF_FEATURE_DATE_ISOFMT(
> - //usage:     "\n    -D FMT        Use FMT (strptime format) for -d TIME
> conversion"
> +@@ -109,6 +109,7 @@
> + //usage:     "\n    -I[SPEC]    Output ISO-8601 date"
> + //usage:     "\n            SPEC=date (default), hours, minutes, seconds or ns"
>    //usage:    )
>   +//usage:     "\n    -k        Set Kernel timezone from localtime and exit"
>    //usage:     "\n"
>    //usage:     "\nRecognized TIME formats:"
> - //usage:     "\n    hh:mm[:ss]"
> -@@ -139,9 +140,8 @@
> + //usage:     "\n    @seconds_since_1970"
> +@@ -126,9 +127,8 @@
>
>    #include "libbb.h"
>    #include "common_bufsiz.h"
> @@ -20,19 +20,19 @@
>
>    enum {
>        OPT_RFC2822   = (1 << 0), /* R */
> -@@ -149,8 +149,9 @@ enum {
> +@@ -136,8 +136,9 @@ enum {
>        OPT_UTC       = (1 << 2), /* u */
>        OPT_DATE      = (1 << 3), /* d */
>        OPT_REFERENCE = (1 << 4), /* r */
> --    OPT_TIMESPEC  = (1 << 5) * ENABLE_FEATURE_DATE_ISOFMT, /* I */
> --    OPT_HINT      = (1 << 6) * ENABLE_FEATURE_DATE_ISOFMT, /* D */
> +-    OPT_ISO8601   = (1 << 5) * ENABLE_FEATURE_DATE_ISOFMT, /* I */
> +-    OPT_STR2DT    = (1 << 6) * ENABLE_FEATURE_DATE_ISOFMT, /* D */
>   +    OPT_KERNELTZ  = (1 << 5), /* k */
> -+    OPT_TIMESPEC  = (1 << 6) * ENABLE_FEATURE_DATE_ISOFMT, /* I */
> -+    OPT_HINT      = (1 << 7) * ENABLE_FEATURE_DATE_ISOFMT, /* D */
> ++    OPT_ISO8601   = (1 << 6) * ENABLE_FEATURE_DATE_ISOFMT, /* I */
> ++    OPT_STR2DT    = (1 << 7) * ENABLE_FEATURE_DATE_ISOFMT, /* D */
>    };
>
>    #if ENABLE_LONG_OPTS
> -@@ -162,6 +163,7 @@ static const char date_longopts[] ALIGN1
> +@@ -149,6 +150,7 @@ static const char date_longopts[] ALIGN1
>        /*    "universal\0" No_argument       "u" */
>            "date\0"      Required_argument "d"
>            "reference\0" Required_argument "r"
> @@ -40,7 +40,7 @@
>            ;
>    #endif
>
> -@@ -181,6 +183,8 @@ static void maybe_set_utc(int opt)
> +@@ -162,6 +164,8 @@ static const char date_longopts[] ALIGN1
>    int date_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>    int date_main(int argc UNUSED_PARAM, char **argv)
>    {
> @@ -49,7 +49,7 @@
>        struct timespec ts;
>        struct tm tm_time;
>        char buf_fmt_dt2str[64];
> -@@ -193,7 +197,7 @@ int date_main(int argc UNUSED_PARAM, cha
> +@@ -174,7 +178,7 @@ int date_main(int argc UNUSED_PARAM, cha
>        char *isofmt_arg = NULL;
>
>        opt = getopt32long(argv, "^"
> @@ -58,7 +58,7 @@
>                IF_FEATURE_DATE_ISOFMT("I::D:")
>                "\0"
>                "d--s:s--d"
> -@@ -256,6 +260,31 @@ int date_main(int argc UNUSED_PARAM, cha
> +@@ -238,6 +242,31 @@ int date_main(int argc UNUSED_PARAM, cha
>        if (*argv)
>            bb_show_usage();
>
>