From: Alexander Bluhm Subject: Re: deprecate timeout_add_tv To: David Gwynne Cc: tech@openbsd.org, cheloa@openbsd.org Date: Mon, 19 May 2025 16:47:59 +0900 On Mon, May 19, 2025 at 01:19:06PM +1000, David Gwynne wrote: > there's only two things using timeout_add_tv, and i think they'd be fine > going via other timeout_add_foo functions instead. > > those two things are socket splicing and carp. socket splicing > carries timevals around, which can be turned into nsec easily with > TIMEVAL_TO_NSEC. > > carp is a bit weird though. turns advbase and advskew into a timeval, > but scales bits of it based on whether it's in backup mode or not. this > diff changes the semantics slightly so it calculates a time in usec, but > scales the whole time for the backup. > > thoughts? ok? Cleaning up carp makes sense. OK bluhm@ for that part. For socket splicing I don't know. User interface uses struct timeval to make it compatible with select(2). Then kernel carries timeval around. Why not have a timeout_add_tv() interface for that? I don't see benefit of converting timeval to usec and then to ticks. bluhm > Index: netinet/ip_carp.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_carp.c,v > diff -u -p -r1.367 ip_carp.c > --- netinet/ip_carp.c 2 Mar 2025 21:28:32 -0000 1.367 > +++ netinet/ip_carp.c 19 May 2025 03:01:07 -0000 > @@ -1068,7 +1068,7 @@ void > carp_send_ad(struct carp_vhost_entry *vhe) > { > struct carp_header ch; > - struct timeval tv; > + uint64_t usec; > struct carp_softc *sc = vhe->parent_sc; > struct carp_header *ch_ptr; > struct mbuf *m; > @@ -1091,11 +1091,10 @@ carp_send_ad(struct carp_vhost_entry *vh > } else { > advbase = sc->sc_advbase; > advskew = vhe->advskew; > - tv.tv_sec = advbase; > - if (advbase == 0 && advskew == 0) > - tv.tv_usec = 1 * 1000000 / 256; > - else > - tv.tv_usec = advskew * 1000000 / 256; > + usec = (uint64_t)advbase * 1000000; > + usec += advskew * 1000000 / 256; > + if (usec == 0) > + usec = 1000000 / 256; > } > > ch.carp_version = CARP_VERSION; > @@ -1278,7 +1277,7 @@ carp_send_ad(struct carp_vhost_entry *vh > retry_later: > sc->cur_vhe = NULL; > if (advbase != 255 || advskew != 255) > - timeout_add_tv(&vhe->ad_tmo, &tv); > + timeout_add_usec(&vhe->ad_tmo, usec); > if_put(ifp); > } > > @@ -1587,8 +1586,8 @@ void > carp_setrun(struct carp_vhost_entry *vhe, sa_family_t af) > { > struct ifnet *ifp; > - struct timeval tv; > struct carp_softc *sc = vhe->parent_sc; > + uint64_t usec; > > if ((ifp = if_get(sc->sc_carpdevidx)) == NULL) { > sc->sc_if.if_flags &= ~IFF_RUNNING; > @@ -1612,6 +1611,11 @@ carp_setrun(struct carp_vhost_entry *vhe > return; > } > > + usec = (uint64_t)sc->sc_advbase * 1000000; > + usec += vhe->advskew * 1000000 / 256; > + if (usec == 0) > + usec = 1000000 / 256; > + > switch (vhe->state) { > case INIT: > carp_set_state(vhe, BACKUP); > @@ -1619,39 +1623,27 @@ carp_setrun(struct carp_vhost_entry *vhe > break; > case BACKUP: > timeout_del(&vhe->ad_tmo); > - tv.tv_sec = 3 * sc->sc_advbase; > - if (sc->sc_advbase == 0 && vhe->advskew == 0) > - tv.tv_usec = 3 * 1000000 / 256; > - else if (sc->sc_advbase == 0) > - tv.tv_usec = 3 * vhe->advskew * 1000000 / 256; > - else > - tv.tv_usec = vhe->advskew * 1000000 / 256; > if (vhe->vhe_leader) > sc->sc_delayed_arp = -1; > switch (af) { > case AF_INET: > - timeout_add_tv(&vhe->md_tmo, &tv); > + timeout_add_usec(&vhe->md_tmo, 3 * usec); > break; > #ifdef INET6 > case AF_INET6: > - timeout_add_tv(&vhe->md6_tmo, &tv); > + timeout_add_usec(&vhe->md6_tmo, 3 * usec); > break; > #endif /* INET6 */ > default: > if (sc->sc_naddrs) > - timeout_add_tv(&vhe->md_tmo, &tv); > + timeout_add_usec(&vhe->md_tmo, 3 * usec); > if (sc->sc_naddrs6) > - timeout_add_tv(&vhe->md6_tmo, &tv); > + timeout_add_usec(&vhe->md6_tmo, 3 * usec); > break; > } > break; > case MASTER: > - tv.tv_sec = sc->sc_advbase; > - if (sc->sc_advbase == 0 && vhe->advskew == 0) > - tv.tv_usec = 1 * 1000000 / 256; > - else > - tv.tv_usec = vhe->advskew * 1000000 / 256; > - timeout_add_tv(&vhe->ad_tmo, &tv); > + timeout_add_usec(&vhe->ad_tmo, usec); > break; > } > } > Index: kern/uipc_socket.c > =================================================================== > RCS file: /cvs/src/sys/kern/uipc_socket.c,v > diff -u -p -r1.377 uipc_socket.c > --- kern/uipc_socket.c 15 Apr 2025 12:14:06 -0000 1.377 > +++ kern/uipc_socket.c 19 May 2025 03:01:07 -0000 > @@ -1871,8 +1871,10 @@ somove(struct socket *so, int wait) > > return (0); > } > - if (timerisset(&so->so_idletv)) > - timeout_add_tv(&so->so_idleto, &so->so_idletv); > + if (timerisset(&so->so_idletv)) { > + timeout_add_nsec(&so->so_idleto, > + TIMEVAL_TO_NSEC(&so->so_idletv)); > + } > return (1); > } > #endif /* SOCKET_SPLICE */ > Index: kern/kern_timeout.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_timeout.c,v > diff -u -p -r1.103 kern_timeout.c > --- kern/kern_timeout.c 2 May 2025 00:51:09 -0000 1.103 > +++ kern/kern_timeout.c 19 May 2025 03:01:07 -0000 > @@ -369,16 +369,6 @@ timeout_add_ticks(struct timeout *to, ui > } > > int > -timeout_add_tv(struct timeout *to, const struct timeval *tv) > -{ > - uint64_t to_ticks; > - > - to_ticks = (uint64_t)hz * tv->tv_sec + tv->tv_usec / tick; > - > - return timeout_add_ticks(to, to_ticks, tv->tv_usec > 0); > -} > - > -int > timeout_add_sec(struct timeout *to, int secs) > { > uint64_t to_ticks; > Index: sys/timeout.h > =================================================================== > RCS file: /cvs/src/sys/sys/timeout.h,v > diff -u -p -r1.50 timeout.h > --- sys/timeout.h 11 Aug 2024 00:49:34 -0000 1.50 > +++ sys/timeout.h 19 May 2025 03:01:07 -0000 > @@ -106,7 +106,6 @@ void timeout_set_flags(struct timeout *, > void timeout_set_proc(struct timeout *, void (*)(void *), void *); > > int timeout_add(struct timeout *, int); > -int timeout_add_tv(struct timeout *, const struct timeval *); > int timeout_add_sec(struct timeout *, int); > int timeout_add_msec(struct timeout *, uint64_t); > int timeout_add_usec(struct timeout *, uint64_t);