On Mon, Nov 22, 2021 at 08:17:47AM +0100, Peter J. Philipp wrote: > On Mon, Nov 22, 2021 at 12:30:19AM +0000, Klemens Nanni wrote: > > On Sun, Nov 21, 2021 at 11:18:29AM +0100, [email protected] wrote: > > > >Synopsis: session uptime is wrong > > > >Category: system > > > >Environment: > > > System : OpenBSD 7.0 > > > Details : OpenBSD 7.0 (GENERIC.MP) #698: Thu Sep 30 21:07:33 MDT > > > 2021 > > > > > > [email protected]:/usr/src/sys/arch/octeon/compile/GENERIC.MP > > > > > > Architecture: OpenBSD.octeon > > > Machine : octeon > > > >Description: > > > On a router (octeon with no RTC) the uptime looks like so: > > > > > > 11:12AM up 2 days, 15:56, 1 user, load averages: 0.01, 0.03, 0.01 > > > > > > The pppoe(4) interface however displays 51 days uptime for a session: > > > > > > > > > pppoe0: flags=8851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST> mtu 1500 > > > description: Telekom > > > index 7 priority 0 llprio 3 > > > dev: vlan7 state: session > > > sid: 0x3f2f PADI retries: 1 PADR retries: 0 time: 51d 08:03:55 > > > <cut> > > > > Same here on an edgerouter 4; already seen on tech@ in my reply to > > bket's "Print learned DNS from sppp(4) in ifconfig(8)" where the > > freshly rebooted box shows a session of 19 days in ifconfig output. > > > > > I reason that my router rebooted (which it did two days ago) and > > > used microuptime() to fill the session time, and then NTP updated > > > the time and we have this timejump. What should be done is the > > > uptime in seconds should be gotten and the ifconfig code that does > > > the ioctl(2) does the appropriate math. > > > > I can't test/reboot my box at the moment, but this minimal diff should > > fix it. One could also rename the variables and polish further, but > > I focus on the fix alone until I can test myself. > > > > > > Index: sys/net/if_pppoe.c > > =================================================================== > > RCS file: /cvs/src/sys/net/if_pppoe.c,v > > retrieving revision 1.78 > > diff -u -p -r1.78 if_pppoe.c > > --- sys/net/if_pppoe.c 19 Jul 2021 19:00:58 -0000 1.78 > > +++ sys/net/if_pppoe.c 21 Nov 2021 23:50:45 -0000 > > @@ -586,7 +586,7 @@ breakbreak: > > PPPOEDEBUG(("%s: session 0x%x connected\n", > > sc->sc_sppp.pp_if.if_xname, session)); > > sc->sc_state = PPPOE_STATE_SESSION; > > - microtime(&sc->sc_session_time); > > + getmicrouptime(&sc->sc_session_time); > > sc->sc_sppp.pp_up(&sc->sc_sppp); /* notify upper layers > > */ > > > > break; > > Index: sbin/ifconfig/ifconfig.c > > =================================================================== > > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v > > retrieving revision 1.450 > > diff -u -p -r1.450 ifconfig.c > > --- sbin/ifconfig/ifconfig.c 17 Nov 2021 18:00:24 -0000 1.450 > > +++ sbin/ifconfig/ifconfig.c 22 Nov 2021 00:25:04 -0000 > > @@ -5362,12 +5362,13 @@ pppoe_status(void) > > printf(" PADR retries: %d", state.padr_retry_no); > > > > if (state.state == PPPOE_STATE_SESSION) { > > - struct timeval temp_time; > > + struct timespec temp_time; > > time_t diff_time, day = 0; > > unsigned int hour = 0, min = 0, sec = 0; > > > > if (state.session_time.tv_sec != 0) { > > - gettimeofday(&temp_time, NULL); > > + if (clock_gettime(CLOCK_BOOTTIME, &temp_time) == -1) > > + goto notime; > > diff_time = temp_time.tv_sec - > > state.session_time.tv_sec; > > > > @@ -5387,6 +5388,7 @@ pppoe_status(void) > > printf("%lldd ", (long long)day); > > printf("%02u:%02u:%02u", hour, min, sec); > > } > > +notime: > > putchar('\n'); > > } > > > > This looks wrong to me, is microuptime() and clock_gettime(CLOCK_BOOTTIME, > ...) > working on a moving uptime target?
Yes, they're both taking the monotonically increasing time since boot, without accounting for suspend time. > I think what one must do is instead of > absolute timestamps is get the deltas of uptime only and then do a bit of > math with those. What I came up with Scott Cheloha, as pseudo code is this: > > -------------------------------------------> > yes there is a getuptime() taken a value of a time_t I suspect this is seconds > since uptime? if It passes that on ioctl to ifconfig > > > Then userspace will need to subtract that > > value from the current CLOCK_BOOTTIME. That's what I'm doing? At least as per my understanding after having read clock_gettime(2) and microuptime(9). > So you get another value ctime() as a time_t that's timedelta2, > timedelta1 = (timedelta2 - clock_boottime) + ioctltime_from_kernel > and then difftime(timedelta1, timedelta2) afaik gives you the uptime in > seconds. > <------------------------------------------- > > I have an unused router without RTC somewhere at home, I also have another > router where I can put a pppoe server on for testing purposes. I'll set those > up in the next few days and test it. > > Thanks for your patch! I think I can work with "the idea" and see. > > Best Regards, > -peter >
