Message ID | 20210108033032.1827942-1-rosenp@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [PATCHv3,1/3] base-files: use hwclock --systz | expand |
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.) Ps. Let's just focus to the Busybox version upgrade, without doing these "let's remove OpenWrt-specific patches" cleanup at the same time. This clock handling change is quite separate from the 1.31 --> 1.33 version bump (unless 1.33 really prevents the previous way).
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 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();
On Sun, Jan 10, 2021 at 10:47 AM Hannu Nyman <hannu.nyman@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(); > >
Hi Rosen, Are you planning to send a new version soon? Hauke
On Sun, Jan 31, 2021 at 3:46 PM Hauke Mehrtens <hauke@hauke-m.de> wrote: > > Hi Rosen, > > Are you planning to send a new version soon? Sent. > > Hauke
diff --git a/package/base-files/files/etc/init.d/system b/package/base-files/files/etc/init.d/system index 0e33c522b4..585eeeef7e 100755 --- 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() {
The date -k patch is non standard and will be removed in the next commit. Tested behavior to be identical with a simple C program: #define _GNU_SOURCE #include <unistd.h> #include <stdio.h> #include <sys/time.h> #include <sys/syscall.h> int main() { struct timezone tt; struct timezone tz; int a = syscall(SYS_gettimeofday, NULL, &tt); int b = gettimeofday(NULL, &tz); printf("%d - %d, %d\n", a, tt.tz_minuteswest, tt.tz_dsttime); printf("%d - %d, %d\n", b, tz.tz_minuteswest, tz.tz_dsttime); } Signed-off-by: Rosen Penev <rosenp@gmail.com> --- v2: moved patch to the front for easier git bisect package/base-files/files/etc/init.d/system | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)