Re: Move PRU_SOCKADDR and PRU_PEERADDR requests to corresponding pru_wrappers
On Sat, Sep 03, 2022 at 02:21:29AM +0300, Vitaliy Makkoveev wrote: > So, let's finish split int two steps, the diff converts PRU_SOCKADDR. OK bluhm@ > Index: sys/kern/uipc_usrreq.c > === > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v > retrieving revision 1.183 > diff -u -p -r1.183 uipc_usrreq.c > --- sys/kern/uipc_usrreq.c2 Sep 2022 13:12:31 - 1.183 > +++ sys/kern/uipc_usrreq.c2 Sep 2022 23:12:06 - > @@ -140,6 +140,7 @@ const struct pr_usrreqs uipc_usrreqs = { > .pru_send = uipc_send, > .pru_abort = uipc_abort, > .pru_sense = uipc_sense, > + .pru_sockaddr = uipc_sockaddr, > .pru_connect2 = uipc_connect2, > }; > > @@ -230,10 +231,6 @@ uipc_usrreq(struct socket *so, int req, > > switch (req) { > > - case PRU_SOCKADDR: > - uipc_setaddr(unp, nam); > - break; > - > case PRU_PEERADDR: > so2 = unp_solock_peer(so); > uipc_setaddr(unp->unp_conn, nam); > @@ -576,6 +573,15 @@ uipc_sense(struct socket *so, struct sta > sb->st_ctim.tv_nsec = unp->unp_ctime.tv_nsec; > sb->st_ino = unp->unp_ino; > > + return (0); > +} > + > +int > +uipc_sockaddr(struct socket *so, struct mbuf *nam) > +{ > + struct unpcb *unp = sotounpcb(so); > + > + uipc_setaddr(unp, nam); > return (0); > } > > Index: sys/net/pfkeyv2.c > === > RCS file: /cvs/src/sys/net/pfkeyv2.c,v > retrieving revision 1.250 > diff -u -p -r1.250 pfkeyv2.c > --- sys/net/pfkeyv2.c 2 Sep 2022 13:12:32 - 1.250 > +++ sys/net/pfkeyv2.c 2 Sep 2022 23:12:06 - > @@ -176,6 +176,7 @@ int pfkeyv2_shutdown(struct socket *); > int pfkeyv2_send(struct socket *, struct mbuf *, struct mbuf *, > struct mbuf *); > int pfkeyv2_abort(struct socket *); > +int pfkeyv2_sockaddr(struct socket *, struct mbuf *); > int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, > struct mbuf *, struct proc *); > int pfkeyv2_output(struct mbuf *, struct socket *); > @@ -211,6 +212,7 @@ const struct pr_usrreqs pfkeyv2_usrreqs > .pru_shutdown = pfkeyv2_shutdown, > .pru_send = pfkeyv2_send, > .pru_abort = pfkeyv2_abort, > + .pru_sockaddr = pfkeyv2_sockaddr, > }; > > const struct protosw pfkeysw[] = { > @@ -389,6 +391,12 @@ pfkeyv2_abort(struct socket *so) > } > > int > +pfkeyv2_sockaddr(struct socket *so, struct mbuf *nam) > +{ > + return (EINVAL); > +} > + > +int > pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m, > struct mbuf *nam, struct mbuf *control, struct proc *p) > { > @@ -410,9 +418,6 @@ pfkeyv2_usrreq(struct socket *so, int re > > switch (req) { > /* minimal support, just implement a fake peer address */ > - case PRU_SOCKADDR: > - error = EINVAL; > - break; > case PRU_PEERADDR: > bcopy(_addr, mtod(nam, caddr_t), pfkey_addr.sa_len); > nam->m_len = pfkey_addr.sa_len; > Index: sys/net/rtsock.c > === > RCS file: /cvs/src/sys/net/rtsock.c,v > retrieving revision 1.351 > diff -u -p -r1.351 rtsock.c > --- sys/net/rtsock.c 2 Sep 2022 13:12:32 - 1.351 > +++ sys/net/rtsock.c 2 Sep 2022 23:12:06 - > @@ -119,6 +119,7 @@ int route_rcvd(struct socket *); > int route_send(struct socket *, struct mbuf *, struct mbuf *, > struct mbuf *); > int route_abort(struct socket *); > +int route_sockaddr(struct socket *, struct mbuf *); > void route_input(struct mbuf *m0, struct socket *, sa_family_t); > int route_arp_conflict(struct rtentry *, struct rt_addrinfo *); > int route_cleargateway(struct rtentry *, void *, unsigned int); > @@ -235,9 +236,6 @@ route_usrreq(struct socket *so, int req, > > switch (req) { > /* minimal support, just implement a fake peer address */ > - case PRU_SOCKADDR: > - error = EINVAL; > - break; > case PRU_PEERADDR: > bcopy(_src, mtod(nam, caddr_t), route_src.sa_len); > nam->m_len = route_src.sa_len; > @@ -394,6 +392,12 @@ route_abort(struct socket *so) > } > > int > +route_sockaddr(struct socket *so, struct mbuf *nam) > +{ > + return (EINVAL); > +} > + > +int > route_ctloutput(int op, struct socket *so, int level, int optname, > struct mbuf *m) > { > @@ -2435,6 +2439,7 @@ const struct pr_usrreqs route_usrreqs = > .pru_rcvd = route_rcvd, > .pru_send = route_send, > .pru_abort = route_abort, > + .pru_sockaddr = route_sockaddr, > }; > > const struct protosw routesw[] = { > Index: sys/netinet/in_pcb.c > === > RCS file: /cvs/src/sys/netinet/in_pcb.c,v > retrieving revision 1.273 > diff -u -p -r1.273
Re: protocol attach wait
On Fri, Sep 02, 2022 at 11:56:02AM +0200, Alexander Bluhm wrote: > On Thu, Sep 01, 2022 at 11:04:19PM +0300, Vitaliy Makkoveev wrote: > > On Thu, Sep 01, 2022 at 10:58:49PM +0300, Vitaliy Makkoveev wrote: > > > On Thu, Sep 01, 2022 at 09:00:50PM +0200, Alexander Bluhm wrote: > > > > On Mon, Aug 15, 2022 at 05:12:22PM +0200, Alexander Bluhm wrote: > > > > > System calls should not fail due to temporary memory shortage in > > > > > malloc(9) or pool_get(9). > > > > > > > > > > Pass down a wait flag to pru_attach(). During syscall socket(2) > > > > > it is ok to wait, this logic was missing for internet pcb. Pfkey > > > > > and route sockets were already waiting. > > > > > > > > > > sonewconn() cannot wait when called during TCP 3-way handshake. > > > > > This logic has been preserved. Unix domain stream socket connect(2) > > > > > can wait until the other side has created the socket to accept. > > > > > > > > rebased to -current. > > > > > > > > Anyone? > > > > > > > > > > At least these ones should have the "pcb == NULL" check as the inet and > > > unix cases. Or the `wait' could be ignored for all cases except tcp. > > Although it should not happen, I made is consistent. Now all > allocators respect the wait and return ENOBUFS in case of failure. > > > But I don't understand what this diff fixes? Userland will check the > > socket(2) return value in any cases, because it's absolutely normal to > > fail here. And nothing stops userland to call socket(2) as times as > > required. > > Userland does not retry socket after memory error. Kernel has to > handle this kind of failure. Otherwise you have to modify all > userland to this: > > #ifdef __OpenBSD__ > while (1) { > fd = socket(...); > if (fd != -1 || error != ENOBUFS) > break; > sleep(1); > } > #else > fd = socket(...); > #edif > Your example is not apposite. The socket(2) error handling depends on logic we implement, but not on the underlying OS. Since we are not the special case where we have no resource allocation in runtime, because all required resources are pre-allocated by design, the memory errors are absolutely normal and should be handled by userland. I'm not blocking this, may be something other has the different opinion. > ok? > > bluhm > > Index: kern/uipc_socket.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v > retrieving revision 1.286 > diff -u -p -r1.286 uipc_socket.c > --- kern/uipc_socket.c28 Aug 2022 18:43:12 - 1.286 > +++ kern/uipc_socket.c1 Sep 2022 18:47:36 - > @@ -138,11 +138,12 @@ soinit(void) > } > > struct socket * > -soalloc(int prflags) > +soalloc(int wait) > { > struct socket *so; > > - so = pool_get(_pool, prflags); > + so = pool_get(_pool, (wait == M_WAIT ? PR_WAITOK : PR_NOWAIT) | > + PR_ZERO); > if (so == NULL) > return (NULL); > rw_init_flags(>so_lock, "solock", RWL_DUPOK); > @@ -174,7 +175,7 @@ socreate(int dom, struct socket **aso, i > return (EPROTONOSUPPORT); > if (prp->pr_type != type) > return (EPROTOTYPE); > - so = soalloc(PR_WAITOK | PR_ZERO); > + so = soalloc(M_WAIT); > klist_init(>so_rcv.sb_sel.si_note, _klistops, so); > klist_init(>so_snd.sb_sel.si_note, _klistops, so); > sigio_init(>so_sigio); > @@ -193,7 +194,7 @@ socreate(int dom, struct socket **aso, i > so->so_rcv.sb_timeo_nsecs = INFSLP; > > solock(so); > - error = pru_attach(so, proto); > + error = pru_attach(so, proto, M_WAIT); > if (error) { > so->so_state |= SS_NOFDREF; > /* sofree() calls sounlock(). */ > Index: kern/uipc_socket2.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket2.c,v > retrieving revision 1.127 > diff -u -p -r1.127 uipc_socket2.c > --- kern/uipc_socket2.c 13 Aug 2022 21:01:46 - 1.127 > +++ kern/uipc_socket2.c 1 Sep 2022 18:47:36 - > @@ -168,7 +168,7 @@ soisdisconnected(struct socket *so) > * Connstatus may be 0 or SS_ISCONNECTED. > */ > struct socket * > -sonewconn(struct socket *head, int connstatus) > +sonewconn(struct socket *head, int connstatus, int wait) > { > struct socket *so; > int persocket = solock_persocket(head); > @@ -185,7 +185,7 @@ sonewconn(struct socket *head, int conns > return (NULL); > if (head->so_qlen + head->so_q0len > head->so_qlimit * 3) > return (NULL); > - so = soalloc(PR_NOWAIT | PR_ZERO); > + so = soalloc(wait); > if (so == NULL) > return (NULL); > so->so_type = head->so_type; > @@ -238,7 +238,7 @@ sonewconn(struct socket *head, int conns > sounlock(head); > } > > - error = pru_attach(so, 0); > + error = pru_attach(so, 0, wait); > >
Re: add sendmmsg and recvmmsg systemcalls
Here is an updated version of the kernel part for sendmmsg. mbuhl Index: sys/kern/syscalls.master === RCS file: /mount/openbsd/cvs/src/sys/kern/syscalls.master,v retrieving revision 1.230 diff -u -p -r1.230 syscalls.master --- sys/kern/syscalls.master2 Sep 2022 13:18:06 - 1.230 +++ sys/kern/syscalls.master2 Sep 2022 20:34:15 - @@ -247,7 +247,9 @@ 116STD NOLOCK { int sys_recvmmsg(int s, struct mmsghdr *mmsg, \ unsigned int vlen, unsigned int flags, \ struct timespec *timeout); } -117UNIMPL sendmmsg +117STD NOLOCK { int sys_sendmmsg(int s, \ + struct mmsghdr *mmsg, unsigned int vlen, \ + unsigned int flags); } 118STD { int sys_getsockopt(int s, int level, int name, \ void *val, socklen_t *avalsize); } 119STD { int sys_thrkill(pid_t tid, int signum, void *tcb); } Index: sys/kern/uipc_syscalls.c === RCS file: /mount/openbsd/cvs/src/sys/kern/uipc_syscalls.c,v retrieving revision 1.202 diff -u -p -r1.202 uipc_syscalls.c --- sys/kern/uipc_syscalls.c2 Sep 2022 13:18:06 - 1.202 +++ sys/kern/uipc_syscalls.c2 Sep 2022 23:26:08 - @@ -606,6 +606,92 @@ done: } int +sys_sendmmsg(struct proc *p, void *v, register_t *retval) +{ + struct sys_sendmmsg_args /* { + syscallarg(int) s; + syscallarg(struct mmsghdr *)mmsg; + syscallarg(unsigned int)vlen; + syscallarg(unsigned int)flags; + } */ *uap = v; + struct mmsghdr mmsg, *mmsgp; + struct iovec aiov[UIO_SMALLIOV], *iov = aiov, *uiov; + size_t iovlen = UIO_SMALLIOV; + register_t retsnd; + unsigned int vlen, dgrams; + int error = 0; + + /* Arbitrarily capped at 1024 datagrams. */ + vlen = SCARG(uap, vlen); + if (vlen > 1024) + vlen = 1024; + + mmsgp = SCARG(uap, mmsg); + for (dgrams = 0; dgrams < vlen; dgrams++) { + error = copyin([dgrams], , sizeof(mmsg)); + if (error) + break; + +#ifdef KTRACE + if (KTRPOINT(p, KTR_STRUCT)) + ktrmmsghdr(p, ); +#endif + + if (mmsg.msg_hdr.msg_iovlen > IOV_MAX) { + error = EMSGSIZE; + break; + } + + if (mmsg.msg_hdr.msg_iovlen > iovlen) { + if (iov != aiov) + free(iov, M_IOV, iovlen * + sizeof(struct iovec)); + + iovlen = mmsg.msg_hdr.msg_iovlen; + iov = mallocarray(iovlen, sizeof(struct iovec), + M_IOV, M_WAITOK); + } + + if (mmsg.msg_hdr.msg_iovlen > 0) { + error = copyin(mmsg.msg_hdr.msg_iov, iov, + mmsg.msg_hdr.msg_iovlen * sizeof(struct iovec)); + if (error) + break; + } + +#ifdef KTRACE + if (mmsg.msg_hdr.msg_iovlen && KTRPOINT(p, KTR_STRUCT)) + ktriovec(p, iov, mmsg.msg_hdr.msg_iovlen); +#endif + + uiov = mmsg.msg_hdr.msg_iov; + mmsg.msg_hdr.msg_iov = iov; + mmsg.msg_hdr.msg_flags = 0; + + error = sendit(p, SCARG(uap, s), _hdr, + SCARG(uap, flags), ); + if (error) + break; + + mmsg.msg_hdr.msg_iov = uiov; + mmsg.msg_len = retsnd; + + error = copyout(, [dgrams], sizeof(mmsg)); + if (error) + break; + } + + if (iov != aiov) + free(iov, M_IOV, sizeof(struct iovec) * iovlen); + + *retval = dgrams; + + if (dgrams) + return 0; + return error; +} + +int sendit(struct proc *p, int s, struct msghdr *mp, int flags, register_t *retsize) { struct file *fp; Index: sys/sys/socket.h === RCS file: /mount/openbsd/cvs/src/sys/sys/socket.h,v retrieving revision 1.103 diff -u -p -r1.103 socket.h --- sys/sys/socket.h2 Sep 2022 13:18:07 - 1.103 +++ sys/sys/socket.h2 Sep 2022 22:31:09 - @@ -579,6 +579,7 @@ ssize_t send(int, const void *, size_t, ssize_tsendto(int, const void *, size_t, int, const struct sockaddr *, socklen_t); ssize_tsendmsg(int, const struct msghdr *, int); +intsendmmsg(int, struct mmsghdr *, unsigned int, unsigned int); intsetsockopt(int, int, int, const void *, socklen_t); intshutdown(int, int); int
Re: Remove "#ifdef INET6" block from in_setpeeraddr()
On Fri, Sep 02, 2022 at 10:46:12PM +0200, Alexander Bluhm wrote: > On Fri, Sep 02, 2022 at 06:01:20PM +0300, Vitaliy Makkoveev wrote: > > We always call in6_setpeeraddr() and never call in_setpeeraddr() fro the > > inet6 case. > > Should we do it the other way around? > > By pushing the inet6 case into the callee allows less switches in > the callers. Instead of adding in6_sockaddr() and in6_peeraddr(), > just call the generic in_...() version. The difference is only in > in_setsockaddr() and in_setpeeraddr(). They can call the specific > in6_...() implementation. This reduces code in upper layers. > > I did somethin like this a while ago and I think the code in the > caller got cleaner. Of course this trick only works as long as > in_...() and in6_...() have the same parameters. > > bluhm This follows the pru_control direction, where we use different in_control() and in6_control(), but they internals are absolutely the same, except mrt{,6}_ioctl() and in{,6}_ioctl(), but they also have the same parameters. Personally I prefer the direction, where we have different handlers, because inet and inet6 are different protocols, and I don't like to mix them. Also I like the guenther@ proposition to split unix(4) sockets dgram handlers, because this simplifies the code by removing not reachable "default:" cases.
Re: Move PRU_SOCKADDR and PRU_PEERADDR requests to corresponding pru_wrappers
On Fri, Sep 02, 2022 at 10:48:56PM +0200, Alexander Bluhm wrote: > On Fri, Sep 02, 2022 at 05:56:33PM +0300, Vitaliy Makkoveev wrote: > > Introduce in{,6}_sockaddr() and in{,6}_peeraddr() functions, and use > > them for all except tcp(4) sockets. Use tcp_sockaddr() and > > tcp_peeraddr() functions to keep debug ability. > > > > The key management and route domain sockets returns EINVAL error for > > PRU_SOCKADDR request, so keep this behaviour for a while instead of make > > pru_sockaddr handler optional and return EOPNOTSUPP. > > > > Within the old *_usrreq() only default panic case left. They are not > > called anymore, so just invoke panic() within. The *_usrreq() will be > > removed with the next diff. > > It does not make sense to keep the two line ..._usrreq(). When I > asked for two diffs, I expected to remove just both PRU_...ADDR > cases. Then the conversion diff stays small. > This leaves some *_usrreq() handlers like below, and don't like to commit this. rip_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, struct mbuf *control, struct proc *p) { struct inpcb *inp; int error = 0; if (req == PRU_CONTROL) return (in_control(so, (u_long)m, (caddr_t)nam, (struct ifnet *)control)); soassertlocked(so); inp = sotoinpcb(so); if (inp == NULL) { error = EINVAL; goto release; } switch (req) { default: panic("rip_usrreq"); } release: if (req != PRU_RCVD && req != PRU_RCVOOB && req != PRU_SENSE) { m_freem(control); m_freem(m); } return (error); } So, let's finish split int two steps, the diff converts PRU_SOCKADDR. Index: sys/kern/uipc_usrreq.c === RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v retrieving revision 1.183 diff -u -p -r1.183 uipc_usrreq.c --- sys/kern/uipc_usrreq.c 2 Sep 2022 13:12:31 - 1.183 +++ sys/kern/uipc_usrreq.c 2 Sep 2022 23:12:06 - @@ -140,6 +140,7 @@ const struct pr_usrreqs uipc_usrreqs = { .pru_send = uipc_send, .pru_abort = uipc_abort, .pru_sense = uipc_sense, + .pru_sockaddr = uipc_sockaddr, .pru_connect2 = uipc_connect2, }; @@ -230,10 +231,6 @@ uipc_usrreq(struct socket *so, int req, switch (req) { - case PRU_SOCKADDR: - uipc_setaddr(unp, nam); - break; - case PRU_PEERADDR: so2 = unp_solock_peer(so); uipc_setaddr(unp->unp_conn, nam); @@ -576,6 +573,15 @@ uipc_sense(struct socket *so, struct sta sb->st_ctim.tv_nsec = unp->unp_ctime.tv_nsec; sb->st_ino = unp->unp_ino; + return (0); +} + +int +uipc_sockaddr(struct socket *so, struct mbuf *nam) +{ + struct unpcb *unp = sotounpcb(so); + + uipc_setaddr(unp, nam); return (0); } Index: sys/net/pfkeyv2.c === RCS file: /cvs/src/sys/net/pfkeyv2.c,v retrieving revision 1.250 diff -u -p -r1.250 pfkeyv2.c --- sys/net/pfkeyv2.c 2 Sep 2022 13:12:32 - 1.250 +++ sys/net/pfkeyv2.c 2 Sep 2022 23:12:06 - @@ -176,6 +176,7 @@ int pfkeyv2_shutdown(struct socket *); int pfkeyv2_send(struct socket *, struct mbuf *, struct mbuf *, struct mbuf *); int pfkeyv2_abort(struct socket *); +int pfkeyv2_sockaddr(struct socket *, struct mbuf *); int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *); int pfkeyv2_output(struct mbuf *, struct socket *); @@ -211,6 +212,7 @@ const struct pr_usrreqs pfkeyv2_usrreqs .pru_shutdown = pfkeyv2_shutdown, .pru_send = pfkeyv2_send, .pru_abort = pfkeyv2_abort, + .pru_sockaddr = pfkeyv2_sockaddr, }; const struct protosw pfkeysw[] = { @@ -389,6 +391,12 @@ pfkeyv2_abort(struct socket *so) } int +pfkeyv2_sockaddr(struct socket *so, struct mbuf *nam) +{ + return (EINVAL); +} + +int pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, struct mbuf *control, struct proc *p) { @@ -410,9 +418,6 @@ pfkeyv2_usrreq(struct socket *so, int re switch (req) { /* minimal support, just implement a fake peer address */ - case PRU_SOCKADDR: - error = EINVAL; - break; case PRU_PEERADDR: bcopy(_addr, mtod(nam, caddr_t), pfkey_addr.sa_len); nam->m_len = pfkey_addr.sa_len; Index: sys/net/rtsock.c === RCS file: /cvs/src/sys/net/rtsock.c,v retrieving revision 1.351 diff -u -p -r1.351 rtsock.c --- sys/net/rtsock.c2 Sep 2022 13:12:32 - 1.351 +++ sys/net/rtsock.c2 Sep 2022 23:12:06 - @@ -119,6 +119,7 @@ int route_rcvd(struct
[please test] pvclock(4): fix several bugs
dv@ suggested coming to the list to request testing for the pvclock(4) driver. Attached is a patch that corrects several bugs. Most of these changes will only matter in the non-TSC_STABLE case on a multiprocessor VM. Ideally, nothing should break. - pvclock yields a 64-bit value. The BSD timecounter layer can only use the lower 32 bits, but internally we need to track the full 64-bit value to allow comparisons with the full value in the non-TSC_STABLE case. So make pvclock_lastcount a 64-bit quantity. - In pvclock_get_timecount(), move rdtsc() up into the lockless read loop to get a more accurate timestamp. - In pvclock_get_timecount(), use rdtsc_lfence(), not rdtsc(). - In pvclock_get_timecount(), check that our TSC value doesn't predate ti->ti_tsc_timestamp, otherwise we will produce an enormous value. - In pvclock_get_timecount(), update pvclock_lastcount in the non-TSC_STABLE case with more care. On amd64 we can do this with an atomic_cas_ulong(9) loop because u_long is 64 bits. On i386 we need to introduce a mutex to protect our comparison and read/write. Index: pvclock.c === RCS file: /cvs/src/sys/dev/pv/pvclock.c,v retrieving revision 1.8 diff -u -p -r1.8 pvclock.c --- pvclock.c 5 Nov 2021 11:38:29 - 1.8 +++ pvclock.c 2 Sep 2022 22:54:08 - @@ -27,6 +27,10 @@ #include #include #include +#include +#if defined(__i386__) +#include +#endif #include #include @@ -35,7 +39,12 @@ #include #include -uint pvclock_lastcount; +#if defined(__amd64__) +volatile u_long pvclock_lastcount; +#elif defined(__i386__) +struct mutex pvclock_mtx = MUTEX_INITIALIZER(IPL_HIGH); +uint64_t pvclock_lastcount; +#endif struct pvclock_softc { struct devicesc_dev; @@ -212,7 +221,7 @@ pvclock_get_timecount(struct timecounter { struct pvclock_softc*sc = tc->tc_priv; struct pvclock_time_info*ti; - uint64_t tsc_timestamp, system_time, delta, ctr; + uint64_t system_time, delta, ctr, tsc; uint32_t version, mul_frac; int8_t shift; uint8_t flags; @@ -220,8 +229,12 @@ pvclock_get_timecount(struct timecounter ti = sc->sc_time; do { version = pvclock_read_begin(ti); + tsc = rdtsc_lfence(); + if (ti->ti_tsc_timestamp < tsc) + delta = tsc - ti->ti_tsc_timestamp; + else + delta = 0; system_time = ti->ti_system_time; - tsc_timestamp = ti->ti_tsc_timestamp; mul_frac = ti->ti_tsc_to_system_mul; shift = ti->ti_tsc_shift; flags = ti->ti_flags; @@ -231,7 +244,6 @@ pvclock_get_timecount(struct timecounter * The algorithm is described in * linux/Documentation/virtual/kvm/msr.txt */ - delta = rdtsc() - tsc_timestamp; if (shift < 0) delta >>= -shift; else @@ -241,10 +253,20 @@ pvclock_get_timecount(struct timecounter if ((flags & PVCLOCK_FLAG_TSC_STABLE) != 0) return (ctr); - if (ctr < pvclock_lastcount) - return (pvclock_lastcount); - - atomic_swap_uint(_lastcount, ctr); - +#if defined(__amd64__) + u_long last; + do { + last = pvclock_lastcount; + if (ctr < last) + return last; + } while (atomic_cas_ulong(_lastcount, last, ctr) != last); +#elif defined(__i386__) + mtx_enter(_mtx); + if (pvclock_lastcount < ctr) + pvclock_lastcount = ctr; + else + ctr = pvclock_lastcount; + mtx_leave(_mtx); +#endif return (ctr); }
Re: rpki-client stop all repo fetching a bit before the timeout
On Fri, Sep 02, 2022 at 09:50:06PM +, Job Snijders wrote: > Hi Claudio, > > This looks mostly OK, just a few nit: > > On Fri, Sep 02, 2022 at 10:02:33PM +0200, Claudio Jeker wrote: > > @@ -1223,8 +1224,26 @@ repo_check_timeout(int timeout) > > { > > struct repo *rp; > > time_t now; > > + int diff; > > > > now = getmonotime(); > > + > > + /* check against our runtime deadline first */ > > + if (deadline != 0) { > > + if (deadline <= now) { > > + warnx("deadline reached, giving up on repository sync"); > > It might be better to avoid executing this code path when 'noop' (-n) is > enabled? > I change main.c to do this instead: + /* give up a bit before the hard timeout and try to finish up */ + if (!noop) + deadline = getmonotime() + timeout - repo_timeout / 2; This should do the same because deadlines remains 0 in noop mode. > Other than that - OK > > Kind regards, > > Job > -- :wq Claudio
Re: rpki-client stop all repo fetching a bit before the timeout
Hi Claudio, This looks mostly OK, just a few nit: On Fri, Sep 02, 2022 at 10:02:33PM +0200, Claudio Jeker wrote: > @@ -1223,8 +1224,26 @@ repo_check_timeout(int timeout) > { > struct repo *rp; > time_t now; > + int diff; > > now = getmonotime(); > + > + /* check against our runtime deadline first */ > + if (deadline != 0) { > + if (deadline <= now) { > + warnx("deadline reached, giving up on repository sync"); It might be better to avoid executing this code path when 'noop' (-n) is enabled? Other than that - OK Kind regards, Job
Re: vmd(8): compute i8254 Read-Back latch from singular timestamp
On Fri, Sep 02, 2022 at 01:13:00PM -0400, Dave Voutila wrote: > > Scott Cheloha writes: > > > The 8254 data sheet [1] says this about the Read-Back command: > > > >> The read-back command may be used to latch multi- > >> ple counter output latches (OL) by setting the > >> COUNT bit D5 = 0 and selecting the desired coun- > >> ter(s). This single command is functionally equiva- > >> lent to several counter latch commands, one for > >> each counter latched. [...] > > > > This is a little ambiguous. But my hunch is that the intent here is > > "you can latch multiple counters all at once". Simultaneously. > > Otherwise the utility of the read-back command is suspect. > > > > To simulate a simultaneous latch, we should only call clock_gettime(2) > > once and use that singular timestamp to compute olatch for each > > counter. > > > > ok? > > > > I'm not an expert on the i825{3,4} but have a question below. I did > quickly test this diff and see no noticeable difference from the point > of view of my local guests. > > > [1] 8254 Programmable Interval Timer, p. 8 > > https://www.scs.stanford.edu/10wi-cs140/pintos/specs/8254.pdf > > > > Index: i8253.c > > === > > RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v > > retrieving revision 1.34 > > diff -u -p -r1.34 i8253.c > > --- i8253.c 16 Jun 2021 16:55:02 - 1.34 > > +++ i8253.c 2 Sep 2022 16:25:02 - > > @@ -128,6 +128,8 @@ i8253_do_readback(uint32_t data) > > int readback_channel[3] = { TIMER_RB_C0, TIMER_RB_C1, TIMER_RB_C2 }; > > int i; > > > > + clock_gettime(CLOCK_MONOTONIC, ); > > + > > Why make this call to clock_gettime here ^ > > > /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */ > > if (data & ~TIMER_RB_STATUS) { > > i8253_channel[0].rbs = (data & TIMER_RB_C0) ? 1 : 0; > > @@ -139,7 +141,6 @@ i8253_do_readback(uint32_t data) > > if (data & ~TIMER_RB_COUNT) { > > ...instead of here where we can save a possible syscall? Yes, that's better, I'll go with that.
Re: Move PRU_SOCKADDR and PRU_PEERADDR requests to corresponding pru_wrappers
On Fri, Sep 02, 2022 at 05:56:33PM +0300, Vitaliy Makkoveev wrote: > Introduce in{,6}_sockaddr() and in{,6}_peeraddr() functions, and use > them for all except tcp(4) sockets. Use tcp_sockaddr() and > tcp_peeraddr() functions to keep debug ability. > > The key management and route domain sockets returns EINVAL error for > PRU_SOCKADDR request, so keep this behaviour for a while instead of make > pru_sockaddr handler optional and return EOPNOTSUPP. > > Within the old *_usrreq() only default panic case left. They are not > called anymore, so just invoke panic() within. The *_usrreq() will be > removed with the next diff. It does not make sense to keep the two line ..._usrreq(). When I asked for two diffs, I expected to remove just both PRU_...ADDR cases. Then the conversion diff stays small. > Index: sys/kern/uipc_usrreq.c > === > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v > retrieving revision 1.183 > diff -u -p -r1.183 uipc_usrreq.c > --- sys/kern/uipc_usrreq.c2 Sep 2022 13:12:31 - 1.183 > +++ sys/kern/uipc_usrreq.c2 Sep 2022 14:44:46 - > @@ -140,6 +140,8 @@ const struct pr_usrreqs uipc_usrreqs = { > .pru_send = uipc_send, > .pru_abort = uipc_abort, > .pru_sense = uipc_sense, > + .pru_sockaddr = uipc_sockaddr, > + .pru_peeraddr = uipc_peeraddr, > .pru_connect2 = uipc_connect2, > }; > > @@ -215,44 +217,8 @@ int > uipc_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, > struct mbuf *control, struct proc *p) > { > - struct unpcb *unp = sotounpcb(so); > - struct socket *so2; > - int error = 0; > - > - if (req != PRU_SEND && control && control->m_len) { > - error = EOPNOTSUPP; > - goto release; > - } > - if (unp == NULL) { > - error = EINVAL; > - goto release; > - } > - > - switch (req) { > - > - case PRU_SOCKADDR: > - uipc_setaddr(unp, nam); > - break; > - > - case PRU_PEERADDR: > - so2 = unp_solock_peer(so); > - uipc_setaddr(unp->unp_conn, nam); > - if (so2 != NULL && so2 != so) > - sounlock(so2); > - break; > - > - case PRU_SLOWTIMO: > - break; > - > - default: > - panic("uipc_usrreq"); > - } > -release: > - if (req != PRU_RCVD && req != PRU_RCVOOB && req != PRU_SENSE) { > - m_freem(control); > - m_freem(m); > - } > - return (error); > + panic("uipc_usrreq"); > + return (EOPNOTSUPP); > } > > /* > @@ -576,6 +542,28 @@ uipc_sense(struct socket *so, struct sta > sb->st_ctim.tv_nsec = unp->unp_ctime.tv_nsec; > sb->st_ino = unp->unp_ino; > > + return (0); > +} > + > +int > +uipc_sockaddr(struct socket *so, struct mbuf *nam) > +{ > + struct unpcb *unp = sotounpcb(so); > + > + uipc_setaddr(unp, nam); > + return (0); > +} > + > +int > +uipc_peeraddr(struct socket *so, struct mbuf *nam) > +{ > + struct unpcb *unp = sotounpcb(so); > + struct socket *so2; > + > + so2 = unp_solock_peer(so); > + uipc_setaddr(unp->unp_conn, nam); > + if (so2 != NULL && so2 != so) > + sounlock(so2); > return (0); > } > > Index: sys/net/pfkeyv2.c > === > RCS file: /cvs/src/sys/net/pfkeyv2.c,v > retrieving revision 1.250 > diff -u -p -r1.250 pfkeyv2.c > --- sys/net/pfkeyv2.c 2 Sep 2022 13:12:32 - 1.250 > +++ sys/net/pfkeyv2.c 2 Sep 2022 14:44:46 - > @@ -176,6 +176,8 @@ int pfkeyv2_shutdown(struct socket *); > int pfkeyv2_send(struct socket *, struct mbuf *, struct mbuf *, > struct mbuf *); > int pfkeyv2_abort(struct socket *); > +int pfkeyv2_sockaddr(struct socket *, struct mbuf *); > +int pfkeyv2_peeraddr(struct socket *, struct mbuf *); > int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, > struct mbuf *, struct proc *); > int pfkeyv2_output(struct mbuf *, struct socket *); > @@ -211,6 +213,8 @@ const struct pr_usrreqs pfkeyv2_usrreqs > .pru_shutdown = pfkeyv2_shutdown, > .pru_send = pfkeyv2_send, > .pru_abort = pfkeyv2_abort, > + .pru_sockaddr = pfkeyv2_sockaddr, > + .pru_peeraddr = pfkeyv2_peeraddr, > }; > > const struct protosw pfkeysw[] = { > @@ -389,45 +393,26 @@ pfkeyv2_abort(struct socket *so) > } > > int > -pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m, > -struct mbuf *nam, struct mbuf *control, struct proc *p) > +pfkeyv2_sockaddr(struct socket *so, struct mbuf *nam) > { > - struct pkpcb *kp; > - int error = 0; > - > - soassertlocked(so); > - > - if (control && control->m_len) { > - error = EOPNOTSUPP; > - goto release; > - } > - > - kp = sotokeycb(so); > - if (kp == NULL) { > - error
Re: Remove "#ifdef INET6" block from in_setpeeraddr()
On Fri, Sep 02, 2022 at 06:01:20PM +0300, Vitaliy Makkoveev wrote: > We always call in6_setpeeraddr() and never call in_setpeeraddr() fro the > inet6 case. Should we do it the other way around? By pushing the inet6 case into the callee allows less switches in the callers. Instead of adding in6_sockaddr() and in6_peeraddr(), just call the generic in_...() version. The difference is only in in_setsockaddr() and in_setpeeraddr(). They can call the specific in6_...() implementation. This reduces code in upper layers. I did somethin like this a while ago and I think the code in the caller got cleaner. Of course this trick only works as long as in_...() and in6_...() have the same parameters. bluhm > Index: sys/netinet/in_pcb.c > === > RCS file: /cvs/src/sys/netinet/in_pcb.c,v > retrieving revision 1.273 > diff -u -p -r1.273 in_pcb.c > --- sys/netinet/in_pcb.c 30 Aug 2022 11:53:04 - 1.273 > +++ sys/netinet/in_pcb.c 2 Sep 2022 14:59:02 - > @@ -649,13 +649,6 @@ in_setpeeraddr(struct inpcb *inp, struct > { > struct sockaddr_in *sin; > > -#ifdef INET6 > - if (sotopf(inp->inp_socket) == PF_INET6) { > - in6_setpeeraddr(inp, nam); > - return; > - } > -#endif /* INET6 */ > - > nam->m_len = sizeof(*sin); > sin = mtod(nam, struct sockaddr_in *); > memset(sin, 0, sizeof(*sin));
Re: rpki-client stop all repo fetching a bit before the timeout
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.09.02 22:02:33 +0200: > Lets try to finish work by stopping all syncs and fall back to what we > have in cache after 7/8 of the timeout (timeout - 1/2 repo_timeout). > This way we still have 1/8 of time to finish the calculation and produce > output. > > Tested this diff by setting the deadline to fire after 60sec. ok > -- > :wq Claudio > > Index: main.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v > retrieving revision 1.217 > diff -u -p -r1.217 main.c > --- main.c2 Sep 2022 19:14:04 - 1.217 > +++ main.c2 Sep 2022 19:58:19 - > @@ -66,6 +66,7 @@ int noop; > int filemode; > int rrdpon = 1; > int repo_timeout; > +time_t deadline; > > struct skiplist skiplist = LIST_HEAD_INITIALIZER(skiplist); > > @@ -1044,6 +1045,9 @@ main(int argc, char *argv[]) >*/ > alarm(timeout); > signal(SIGALRM, suicide); > + > + /* give up a bit before the hard timeout and try to finish up */ > + deadline = getmonotime() + timeout - repo_timeout / 2; > } > > if (pledge("stdio rpath wpath cpath fattr sendfd unveil", NULL) == -1) > Index: repo.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v > retrieving revision 1.38 > diff -u -p -r1.38 repo.c > --- repo.c2 Sep 2022 19:10:37 - 1.38 > +++ repo.c2 Sep 2022 19:15:41 - > @@ -41,6 +41,8 @@ extern struct stats stats; > extern int noop; > extern int rrdpon; > extern int repo_timeout; > +extern time_tdeadline; > +int nofetch; > > enum repo_state { > REPO_LOADING = 0, > @@ -288,7 +290,7 @@ repo_done(const void *vp, int ok) > if (vp == rp->rsync) > entityq_flush(>queue, rp); > if (vp == rp->rrdp) { > - if (!ok) { > + if (!ok && !nofetch) { > /* try to fall back to rsync */ > rp->rrdp = NULL; > rp->rsync = rsync_get(rp->repouri, > @@ -937,8 +939,8 @@ rrdp_finish(unsigned int id, int ok) > stats.rrdp_repos++; > rr->state = REPO_DONE; > } else { > - warnx("%s: load from network failed, fallback to rsync", > - rr->notifyuri); > + warnx("%s: load from network failed, fallback to %s", > + rr->notifyuri, nofetch ? "cache" : "rsync"); > stats.rrdp_fails++; > rr->state = REPO_FAILED; > /* clear the RRDP repo since it failed */ > @@ -1044,7 +1046,6 @@ repo_lookup(int talid, const char *uri, > { > struct repo *rp; > char*repouri; > - int nofetch = 0; > > if ((repouri = rsync_base_uri(uri)) == NULL) > errx(1, "bad caRepository URI: %s", uri); > @@ -1223,8 +1224,26 @@ repo_check_timeout(int timeout) > { > struct repo *rp; > time_t now; > + int diff; > > now = getmonotime(); > + > + /* check against our runtime deadline first */ > + if (deadline != 0) { > + if (deadline <= now) { > + warnx("deadline reached, giving up on repository sync"); > + nofetch = 1; > + /* clear deadline since nofetch is set */ > + deadline = 0; > + /* increase now enough so that all pending repos fail */ > + now += repo_timeout; > + } else { > + diff = deadline - now; > + diff *= 1000; > + if (timeout == INFTIM || diff < timeout) > + timeout = diff; > + } > + } > /* Look up in repository table. (Lookup should actually fail here) */ > SLIST_FOREACH(rp, , entry) { > if (repo_state(rp) == REPO_LOADING) { > @@ -1233,7 +1252,7 @@ repo_check_timeout(int timeout) > rp->repouri); > repo_abort(rp); > } else { > - int diff = rp->alarm - now; > + diff = rp->alarm - now; > diff *= 1000; > if (timeout == INFTIM || diff < timeout) > timeout = diff; >
Re: rpki-client stop all repo fetching a bit before the timeout
On Fri, Sep 02, 2022 at 10:02:33PM +0200, Claudio Jeker wrote: > Lets try to finish work by stopping all syncs and fall back to what we > have in cache after 7/8 of the timeout (timeout - 1/2 repo_timeout). > This way we still have 1/8 of time to finish the calculation and produce > output. > > Tested this diff by setting the deadline to fire after 60sec. ok tb
Re: mld6 remove global variable
that was meant to be an ok :) Sebastian Benoit(be...@openbsd.org) on 2022.09.02 22:04:41 +0200: > Alexander Bluhm(alexander.bl...@gmx.net) on 2022.09.02 20:38:04 +0200: > > Hi, > > > > Due to the KAME scope address hack, the link-local all nodes and > > routers IPv6 addresses cannot be const. So move memory from data > > to stack to make variables MP safe. > > > > ok? > > > > bluhm > > > > Index: netinet6/mld6.c > > === > > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v > > retrieving revision 1.58 > > diff -u -p -r1.58 mld6.c > > --- netinet6/mld6.c 22 Aug 2022 21:02:44 - 1.58 > > +++ netinet6/mld6.c 2 Sep 2022 17:43:06 - > > @@ -85,9 +85,6 @@ > > > > static struct ip6_pktopts ip6_opts; > > intmld6_timers_are_running;/* [N] shortcut for fast timer > > */ > > -/* XXX: These are necessary for KAME's link-local hack */ > > -static struct in6_addr mld_all_nodes_linklocal = > > IN6ADDR_LINKLOCAL_ALLNODES_INIT; > > -static struct in6_addr mld_all_routers_linklocal = > > IN6ADDR_LINKLOCAL_ALLROUTERS_INIT; > > > > void mld6_checktimer(struct ifnet *); > > static void mld6_sendpkt(struct in6_multi *, int, const struct in6_addr *); > > @@ -118,6 +115,9 @@ mld6_init(void) > > void > > mld6_start_listening(struct in6_multi *in6m) > > { > > + /* XXX: These are necessary for KAME's link-local hack */ > > + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; > > + > > /* > > * RFC2710 page 10: > > * The node never sends a Report or Done for the link-scope all-nodes > > @@ -125,9 +125,10 @@ mld6_start_listening(struct in6_multi *i > > * MLD messages are never sent for multicast addresses whose scope is 0 > > * (reserved) or 1 (node-local). > > */ > > - mld_all_nodes_linklocal.s6_addr16[1] = htons(in6m->in6m_ifidx);/* XXX */ > > - if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _all_nodes_linklocal) || > > - __IPV6_ADDR_MC_SCOPE(>in6m_addr) < > > __IPV6_ADDR_SCOPE_LINKLOCAL) { > > + all_nodes.s6_addr16[1] = htons(in6m->in6m_ifidx); > > + if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes) || > > + __IPV6_ADDR_MC_SCOPE(>in6m_addr) < > > + __IPV6_ADDR_SCOPE_LINKLOCAL) { > > in6m->in6m_timer = 0; > > in6m->in6m_state = MLD_OTHERLISTENER; > > } else { > > @@ -143,15 +144,19 @@ mld6_start_listening(struct in6_multi *i > > void > > mld6_stop_listening(struct in6_multi *in6m) > > { > > - mld_all_nodes_linklocal.s6_addr16[1] = htons(in6m->in6m_ifidx);/* XXX */ > > - mld_all_routers_linklocal.s6_addr16[1] = > > - htons(in6m->in6m_ifidx); /* XXX: necessary when mrouting */ > > + /* XXX: These are necessary for KAME's link-local hack */ > > + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; > > + struct in6_addr all_routers = IN6ADDR_LINKLOCAL_ALLROUTERS_INIT; > > + > > + all_nodes.s6_addr16[1] = htons(in6m->in6m_ifidx); > > + /* XXX: necessary when mrouting */ > > + all_routers.s6_addr16[1] = htons(in6m->in6m_ifidx); > > > > if (in6m->in6m_state == MLD_IREPORTEDLAST && > > - (!IN6_ARE_ADDR_EQUAL(>in6m_addr, _all_nodes_linklocal)) && > > - __IPV6_ADDR_MC_SCOPE(>in6m_addr) > > > __IPV6_ADDR_SCOPE_INTFACELOCAL) > > - mld6_sendpkt(in6m, MLD_LISTENER_DONE, > > - _all_routers_linklocal); > > + (!IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes)) && > > + __IPV6_ADDR_MC_SCOPE(>in6m_addr) > > > + __IPV6_ADDR_SCOPE_INTFACELOCAL) > > + mld6_sendpkt(in6m, MLD_LISTENER_DONE, _routers); > > } > > > > void > > @@ -163,6 +168,8 @@ mld6_input(struct mbuf *m, int off) > > struct in6_multi *in6m; > > struct ifmaddr *ifma; > > int timer; /* timer value in the MLD query header */ > > + /* XXX: These are necessary for KAME's link-local hack */ > > + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; > > > > IP6_EXTHDR_GET(mldh, struct mld_hdr *, m, off, sizeof(*mldh)); > > if (mldh == NULL) { > > @@ -239,15 +246,13 @@ mld6_input(struct mbuf *m, int off) > > timer = ntohs(mldh->mld_maxdelay)*PR_FASTHZ/MLD_TIMER_SCALE; > > if (timer == 0 && mldh->mld_maxdelay) > > timer = 1; > > - mld_all_nodes_linklocal.s6_addr16[1] = > > - htons(ifp->if_index); /* XXX */ > > + all_nodes.s6_addr16[1] = htons(ifp->if_index); > > > > TAILQ_FOREACH(ifma, >if_maddrlist, ifma_list) { > > if (ifma->ifma_addr->sa_family != AF_INET6) > > continue; > > in6m = ifmatoin6m(ifma); > > - if (IN6_ARE_ADDR_EQUAL(>in6m_addr, > > - _all_nodes_linklocal) || > > + if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes) || > > __IPV6_ADDR_MC_SCOPE(>in6m_addr) < > >
Re: mld6 remove global variable
Alexander Bluhm(alexander.bl...@gmx.net) on 2022.09.02 20:38:04 +0200: > Hi, > > Due to the KAME scope address hack, the link-local all nodes and > routers IPv6 addresses cannot be const. So move memory from data > to stack to make variables MP safe. > > ok? > > bluhm > > Index: netinet6/mld6.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v > retrieving revision 1.58 > diff -u -p -r1.58 mld6.c > --- netinet6/mld6.c 22 Aug 2022 21:02:44 - 1.58 > +++ netinet6/mld6.c 2 Sep 2022 17:43:06 - > @@ -85,9 +85,6 @@ > > static struct ip6_pktopts ip6_opts; > int mld6_timers_are_running;/* [N] shortcut for fast timer */ > -/* XXX: These are necessary for KAME's link-local hack */ > -static struct in6_addr mld_all_nodes_linklocal = > IN6ADDR_LINKLOCAL_ALLNODES_INIT; > -static struct in6_addr mld_all_routers_linklocal = > IN6ADDR_LINKLOCAL_ALLROUTERS_INIT; > > void mld6_checktimer(struct ifnet *); > static void mld6_sendpkt(struct in6_multi *, int, const struct in6_addr *); > @@ -118,6 +115,9 @@ mld6_init(void) > void > mld6_start_listening(struct in6_multi *in6m) > { > + /* XXX: These are necessary for KAME's link-local hack */ > + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; > + > /* >* RFC2710 page 10: >* The node never sends a Report or Done for the link-scope all-nodes > @@ -125,9 +125,10 @@ mld6_start_listening(struct in6_multi *i >* MLD messages are never sent for multicast addresses whose scope is 0 >* (reserved) or 1 (node-local). >*/ > - mld_all_nodes_linklocal.s6_addr16[1] = htons(in6m->in6m_ifidx);/* XXX */ > - if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _all_nodes_linklocal) || > - __IPV6_ADDR_MC_SCOPE(>in6m_addr) < > __IPV6_ADDR_SCOPE_LINKLOCAL) { > + all_nodes.s6_addr16[1] = htons(in6m->in6m_ifidx); > + if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes) || > + __IPV6_ADDR_MC_SCOPE(>in6m_addr) < > + __IPV6_ADDR_SCOPE_LINKLOCAL) { > in6m->in6m_timer = 0; > in6m->in6m_state = MLD_OTHERLISTENER; > } else { > @@ -143,15 +144,19 @@ mld6_start_listening(struct in6_multi *i > void > mld6_stop_listening(struct in6_multi *in6m) > { > - mld_all_nodes_linklocal.s6_addr16[1] = htons(in6m->in6m_ifidx);/* XXX */ > - mld_all_routers_linklocal.s6_addr16[1] = > - htons(in6m->in6m_ifidx); /* XXX: necessary when mrouting */ > + /* XXX: These are necessary for KAME's link-local hack */ > + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; > + struct in6_addr all_routers = IN6ADDR_LINKLOCAL_ALLROUTERS_INIT; > + > + all_nodes.s6_addr16[1] = htons(in6m->in6m_ifidx); > + /* XXX: necessary when mrouting */ > + all_routers.s6_addr16[1] = htons(in6m->in6m_ifidx); > > if (in6m->in6m_state == MLD_IREPORTEDLAST && > - (!IN6_ARE_ADDR_EQUAL(>in6m_addr, _all_nodes_linklocal)) && > - __IPV6_ADDR_MC_SCOPE(>in6m_addr) > > __IPV6_ADDR_SCOPE_INTFACELOCAL) > - mld6_sendpkt(in6m, MLD_LISTENER_DONE, > - _all_routers_linklocal); > + (!IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes)) && > + __IPV6_ADDR_MC_SCOPE(>in6m_addr) > > + __IPV6_ADDR_SCOPE_INTFACELOCAL) > + mld6_sendpkt(in6m, MLD_LISTENER_DONE, _routers); > } > > void > @@ -163,6 +168,8 @@ mld6_input(struct mbuf *m, int off) > struct in6_multi *in6m; > struct ifmaddr *ifma; > int timer; /* timer value in the MLD query header */ > + /* XXX: These are necessary for KAME's link-local hack */ > + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; > > IP6_EXTHDR_GET(mldh, struct mld_hdr *, m, off, sizeof(*mldh)); > if (mldh == NULL) { > @@ -239,15 +246,13 @@ mld6_input(struct mbuf *m, int off) > timer = ntohs(mldh->mld_maxdelay)*PR_FASTHZ/MLD_TIMER_SCALE; > if (timer == 0 && mldh->mld_maxdelay) > timer = 1; > - mld_all_nodes_linklocal.s6_addr16[1] = > - htons(ifp->if_index); /* XXX */ > + all_nodes.s6_addr16[1] = htons(ifp->if_index); > > TAILQ_FOREACH(ifma, >if_maddrlist, ifma_list) { > if (ifma->ifma_addr->sa_family != AF_INET6) > continue; > in6m = ifmatoin6m(ifma); > - if (IN6_ARE_ADDR_EQUAL(>in6m_addr, > - _all_nodes_linklocal) || > + if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes) || > __IPV6_ADDR_MC_SCOPE(>in6m_addr) < > __IPV6_ADDR_SCOPE_LINKLOCAL) > continue; >
rpki-client stop all repo fetching a bit before the timeout
Lets try to finish work by stopping all syncs and fall back to what we have in cache after 7/8 of the timeout (timeout - 1/2 repo_timeout). This way we still have 1/8 of time to finish the calculation and produce output. Tested this diff by setting the deadline to fire after 60sec. -- :wq Claudio Index: main.c === RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v retrieving revision 1.217 diff -u -p -r1.217 main.c --- main.c 2 Sep 2022 19:14:04 - 1.217 +++ main.c 2 Sep 2022 19:58:19 - @@ -66,6 +66,7 @@ int noop; intfilemode; intrrdpon = 1; intrepo_timeout; +time_t deadline; struct skiplist skiplist = LIST_HEAD_INITIALIZER(skiplist); @@ -1044,6 +1045,9 @@ main(int argc, char *argv[]) */ alarm(timeout); signal(SIGALRM, suicide); + + /* give up a bit before the hard timeout and try to finish up */ + deadline = getmonotime() + timeout - repo_timeout / 2; } if (pledge("stdio rpath wpath cpath fattr sendfd unveil", NULL) == -1) Index: repo.c === RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v retrieving revision 1.38 diff -u -p -r1.38 repo.c --- repo.c 2 Sep 2022 19:10:37 - 1.38 +++ repo.c 2 Sep 2022 19:15:41 - @@ -41,6 +41,8 @@ extern struct stats stats; extern int noop; extern int rrdpon; extern int repo_timeout; +extern time_t deadline; +intnofetch; enum repo_state { REPO_LOADING = 0, @@ -288,7 +290,7 @@ repo_done(const void *vp, int ok) if (vp == rp->rsync) entityq_flush(>queue, rp); if (vp == rp->rrdp) { - if (!ok) { + if (!ok && !nofetch) { /* try to fall back to rsync */ rp->rrdp = NULL; rp->rsync = rsync_get(rp->repouri, @@ -937,8 +939,8 @@ rrdp_finish(unsigned int id, int ok) stats.rrdp_repos++; rr->state = REPO_DONE; } else { - warnx("%s: load from network failed, fallback to rsync", - rr->notifyuri); + warnx("%s: load from network failed, fallback to %s", + rr->notifyuri, nofetch ? "cache" : "rsync"); stats.rrdp_fails++; rr->state = REPO_FAILED; /* clear the RRDP repo since it failed */ @@ -1044,7 +1046,6 @@ repo_lookup(int talid, const char *uri, { struct repo *rp; char*repouri; - int nofetch = 0; if ((repouri = rsync_base_uri(uri)) == NULL) errx(1, "bad caRepository URI: %s", uri); @@ -1223,8 +1224,26 @@ repo_check_timeout(int timeout) { struct repo *rp; time_t now; + int diff; now = getmonotime(); + + /* check against our runtime deadline first */ + if (deadline != 0) { + if (deadline <= now) { + warnx("deadline reached, giving up on repository sync"); + nofetch = 1; + /* clear deadline since nofetch is set */ + deadline = 0; + /* increase now enough so that all pending repos fail */ + now += repo_timeout; + } else { + diff = deadline - now; + diff *= 1000; + if (timeout == INFTIM || diff < timeout) + timeout = diff; + } + } /* Look up in repository table. (Lookup should actually fail here) */ SLIST_FOREACH(rp, , entry) { if (repo_state(rp) == REPO_LOADING) { @@ -1233,7 +1252,7 @@ repo_check_timeout(int timeout) rp->repouri); repo_abort(rp); } else { - int diff = rp->alarm - now; + diff = rp->alarm - now; diff *= 1000; if (timeout == INFTIM || diff < timeout) timeout = diff;
Re: KNF rpki-client
On Fri, Sep 02, 2022 at 09:05:45PM +0200, Claudio Jeker wrote: > Split some overly long lines. ok > > -- > :wq Claudio > > Index: main.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v > retrieving revision 1.215 > diff -u -p -r1.215 main.c > --- main.c30 Aug 2022 22:42:32 - 1.215 > +++ main.c2 Sep 2022 19:04:56 - > @@ -1266,10 +1266,10 @@ main(int argc, char *argv[]) > (long long)stats.user_time.tv_sec, > (long long)stats.system_time.tv_sec); > printf("Skiplist entries: %zu\n", stats.skiplistentries); > - printf("Route Origin Authorizations: %zu (%zu failed parse, %zu > invalid)\n", > - stats.roas, stats.roas_fail, stats.roas_invalid); > - printf("AS Provider Attestations: %zu (%zu failed parse, %zu > invalid)\n", > - stats.aspas, stats.aspas_fail, stats.aspas_invalid); > + printf("Route Origin Authorizations: %zu (%zu failed parse, %zu " > + "invalid)\n", stats.roas, stats.roas_fail, stats.roas_invalid); > + printf("AS Provider Attestations: %zu (%zu failed parse, %zu " > + "invalid)\n", stats.aspas, stats.aspas_fail, stats.aspas_invalid); > printf("BGPsec Router Certificates: %zu\n", stats.brks); > printf("Certificates: %zu (%zu invalid)\n", > stats.certs, stats.certs_fail); >
KNF rpki-client
Split some overly long lines. -- :wq Claudio Index: main.c === RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v retrieving revision 1.215 diff -u -p -r1.215 main.c --- main.c 30 Aug 2022 22:42:32 - 1.215 +++ main.c 2 Sep 2022 19:04:56 - @@ -1266,10 +1266,10 @@ main(int argc, char *argv[]) (long long)stats.user_time.tv_sec, (long long)stats.system_time.tv_sec); printf("Skiplist entries: %zu\n", stats.skiplistentries); - printf("Route Origin Authorizations: %zu (%zu failed parse, %zu invalid)\n", - stats.roas, stats.roas_fail, stats.roas_invalid); - printf("AS Provider Attestations: %zu (%zu failed parse, %zu invalid)\n", - stats.aspas, stats.aspas_fail, stats.aspas_invalid); + printf("Route Origin Authorizations: %zu (%zu failed parse, %zu " + "invalid)\n", stats.roas, stats.roas_fail, stats.roas_invalid); + printf("AS Provider Attestations: %zu (%zu failed parse, %zu " + "invalid)\n", stats.aspas, stats.aspas_fail, stats.aspas_invalid); printf("BGPsec Router Certificates: %zu\n", stats.brks); printf("Certificates: %zu (%zu invalid)\n", stats.certs, stats.certs_fail);
Re: rpki-client abort repos on timeout
On Fri, Sep 02, 2022 at 08:49:12PM +0200, Claudio Jeker wrote: > This diff uses the now available aborts to stop repository > synchronisations once the timeout is hit. > > I played with very short repo_timeouts and it seems to work better then > what we have now. I generally prefer explicit checks against NULL, but it's done this way in the entire file. ok > -- > :wq Claudio > > ? obj > Index: extern.h > === > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v > retrieving revision 1.152 > diff -u -p -r1.152 extern.h > --- extern.h 2 Sep 2022 18:37:17 - 1.152 > +++ extern.h 2 Sep 2022 18:47:16 - > @@ -653,9 +653,11 @@ void rrdp_finish(unsigned int, int); > > void rsync_fetch(unsigned int, const char *, const char *, > const char *); > +void rsync_abort(unsigned int); > void http_fetch(unsigned int, const char *, const char *, int); > void rrdp_fetch(unsigned int, const char *, const char *, > struct rrdp_session *); > +void rrdp_abort(unsigned int); > void rrdp_http_done(unsigned int, enum http_result, const char *); > int repo_check_timeout(int); > > Index: main.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v > retrieving revision 1.215 > diff -u -p -r1.215 main.c > --- main.c30 Aug 2022 22:42:32 - 1.215 > +++ main.c2 Sep 2022 18:47:16 - > @@ -256,6 +256,18 @@ rrdp_fetch(unsigned int id, const char * > io_close_buffer(, b); > } > > +void > +rrdp_abort(unsigned int id) > +{ > + enum rrdp_msg type = RRDP_ABORT; > + struct ibuf *b; > + > + b = io_new_buffer(); > + io_simple_buffer(b, , sizeof(type)); > + io_simple_buffer(b, , sizeof(id)); > + io_close_buffer(, b); > +} > + > /* > * Request a repository sync via rsync URI to directory local. > */ > @@ -270,6 +282,19 @@ rsync_fetch(unsigned int id, const char > io_str_buffer(b, local); > io_str_buffer(b, base); > io_str_buffer(b, uri); > + io_close_buffer(, b); > +} > + > +void > +rsync_abort(unsigned int id) > +{ > + struct ibuf *b; > + > + b = io_new_buffer(); > + io_simple_buffer(b, , sizeof(id)); > + io_str_buffer(b, NULL); > + io_str_buffer(b, NULL); > + io_str_buffer(b, NULL); > io_close_buffer(, b); > } > > Index: repo.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v > retrieving revision 1.37 > diff -u -p -r1.37 repo.c > --- repo.c2 Sep 2022 15:09:19 - 1.37 > +++ repo.c2 Sep 2022 18:47:17 - > @@ -1204,6 +1204,20 @@ repo_fail(struct repo *rp) > errx(1, "%s: bad repo", rp->repouri); > } > > +static void > +repo_abort(struct repo *rp) > +{ > + /* reset the alarm */ > + rp->alarm = getmonotime() + repo_timeout; > + > + if (rp->rsync) > + rsync_abort(rp->rsync->id); > + else if (rp->rrdp) > + rrdp_abort(rp->rrdp->id); > + else > + repo_fail(rp); > +} > + > int > repo_check_timeout(int timeout) > { > @@ -1217,7 +1231,7 @@ repo_check_timeout(int timeout) > if (rp->alarm <= now) { > warnx("%s: synchronisation timeout", > rp->repouri); > - repo_fail(rp); > + repo_abort(rp); > } else { > int diff = rp->alarm - now; > diff *= 1000; >
rpki-client abort repos on timeout
This diff uses the now available aborts to stop repository synchronisations once the timeout is hit. I played with very short repo_timeouts and it seems to work better then what we have now. -- :wq Claudio ? obj Index: extern.h === RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v retrieving revision 1.152 diff -u -p -r1.152 extern.h --- extern.h2 Sep 2022 18:37:17 - 1.152 +++ extern.h2 Sep 2022 18:47:16 - @@ -653,9 +653,11 @@ voidrrdp_finish(unsigned int, int); voidrsync_fetch(unsigned int, const char *, const char *, const char *); +voidrsync_abort(unsigned int); voidhttp_fetch(unsigned int, const char *, const char *, int); voidrrdp_fetch(unsigned int, const char *, const char *, struct rrdp_session *); +voidrrdp_abort(unsigned int); voidrrdp_http_done(unsigned int, enum http_result, const char *); int repo_check_timeout(int); Index: main.c === RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v retrieving revision 1.215 diff -u -p -r1.215 main.c --- main.c 30 Aug 2022 22:42:32 - 1.215 +++ main.c 2 Sep 2022 18:47:16 - @@ -256,6 +256,18 @@ rrdp_fetch(unsigned int id, const char * io_close_buffer(, b); } +void +rrdp_abort(unsigned int id) +{ + enum rrdp_msg type = RRDP_ABORT; + struct ibuf *b; + + b = io_new_buffer(); + io_simple_buffer(b, , sizeof(type)); + io_simple_buffer(b, , sizeof(id)); + io_close_buffer(, b); +} + /* * Request a repository sync via rsync URI to directory local. */ @@ -270,6 +282,19 @@ rsync_fetch(unsigned int id, const char io_str_buffer(b, local); io_str_buffer(b, base); io_str_buffer(b, uri); + io_close_buffer(, b); +} + +void +rsync_abort(unsigned int id) +{ + struct ibuf *b; + + b = io_new_buffer(); + io_simple_buffer(b, , sizeof(id)); + io_str_buffer(b, NULL); + io_str_buffer(b, NULL); + io_str_buffer(b, NULL); io_close_buffer(, b); } Index: repo.c === RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v retrieving revision 1.37 diff -u -p -r1.37 repo.c --- repo.c 2 Sep 2022 15:09:19 - 1.37 +++ repo.c 2 Sep 2022 18:47:17 - @@ -1204,6 +1204,20 @@ repo_fail(struct repo *rp) errx(1, "%s: bad repo", rp->repouri); } +static void +repo_abort(struct repo *rp) +{ + /* reset the alarm */ + rp->alarm = getmonotime() + repo_timeout; + + if (rp->rsync) + rsync_abort(rp->rsync->id); + else if (rp->rrdp) + rrdp_abort(rp->rrdp->id); + else + repo_fail(rp); +} + int repo_check_timeout(int timeout) { @@ -1217,7 +1231,7 @@ repo_check_timeout(int timeout) if (rp->alarm <= now) { warnx("%s: synchronisation timeout", rp->repouri); - repo_fail(rp); + repo_abort(rp); } else { int diff = rp->alarm - now; diff *= 1000;
mld6 remove global variable
Hi, Due to the KAME scope address hack, the link-local all nodes and routers IPv6 addresses cannot be const. So move memory from data to stack to make variables MP safe. ok? bluhm Index: netinet6/mld6.c === RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v retrieving revision 1.58 diff -u -p -r1.58 mld6.c --- netinet6/mld6.c 22 Aug 2022 21:02:44 - 1.58 +++ netinet6/mld6.c 2 Sep 2022 17:43:06 - @@ -85,9 +85,6 @@ static struct ip6_pktopts ip6_opts; intmld6_timers_are_running;/* [N] shortcut for fast timer */ -/* XXX: These are necessary for KAME's link-local hack */ -static struct in6_addr mld_all_nodes_linklocal = IN6ADDR_LINKLOCAL_ALLNODES_INIT; -static struct in6_addr mld_all_routers_linklocal = IN6ADDR_LINKLOCAL_ALLROUTERS_INIT; void mld6_checktimer(struct ifnet *); static void mld6_sendpkt(struct in6_multi *, int, const struct in6_addr *); @@ -118,6 +115,9 @@ mld6_init(void) void mld6_start_listening(struct in6_multi *in6m) { + /* XXX: These are necessary for KAME's link-local hack */ + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; + /* * RFC2710 page 10: * The node never sends a Report or Done for the link-scope all-nodes @@ -125,9 +125,10 @@ mld6_start_listening(struct in6_multi *i * MLD messages are never sent for multicast addresses whose scope is 0 * (reserved) or 1 (node-local). */ - mld_all_nodes_linklocal.s6_addr16[1] = htons(in6m->in6m_ifidx);/* XXX */ - if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _all_nodes_linklocal) || - __IPV6_ADDR_MC_SCOPE(>in6m_addr) < __IPV6_ADDR_SCOPE_LINKLOCAL) { + all_nodes.s6_addr16[1] = htons(in6m->in6m_ifidx); + if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes) || + __IPV6_ADDR_MC_SCOPE(>in6m_addr) < + __IPV6_ADDR_SCOPE_LINKLOCAL) { in6m->in6m_timer = 0; in6m->in6m_state = MLD_OTHERLISTENER; } else { @@ -143,15 +144,19 @@ mld6_start_listening(struct in6_multi *i void mld6_stop_listening(struct in6_multi *in6m) { - mld_all_nodes_linklocal.s6_addr16[1] = htons(in6m->in6m_ifidx);/* XXX */ - mld_all_routers_linklocal.s6_addr16[1] = - htons(in6m->in6m_ifidx); /* XXX: necessary when mrouting */ + /* XXX: These are necessary for KAME's link-local hack */ + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; + struct in6_addr all_routers = IN6ADDR_LINKLOCAL_ALLROUTERS_INIT; + + all_nodes.s6_addr16[1] = htons(in6m->in6m_ifidx); + /* XXX: necessary when mrouting */ + all_routers.s6_addr16[1] = htons(in6m->in6m_ifidx); if (in6m->in6m_state == MLD_IREPORTEDLAST && - (!IN6_ARE_ADDR_EQUAL(>in6m_addr, _all_nodes_linklocal)) && - __IPV6_ADDR_MC_SCOPE(>in6m_addr) > __IPV6_ADDR_SCOPE_INTFACELOCAL) - mld6_sendpkt(in6m, MLD_LISTENER_DONE, - _all_routers_linklocal); + (!IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes)) && + __IPV6_ADDR_MC_SCOPE(>in6m_addr) > + __IPV6_ADDR_SCOPE_INTFACELOCAL) + mld6_sendpkt(in6m, MLD_LISTENER_DONE, _routers); } void @@ -163,6 +168,8 @@ mld6_input(struct mbuf *m, int off) struct in6_multi *in6m; struct ifmaddr *ifma; int timer; /* timer value in the MLD query header */ + /* XXX: These are necessary for KAME's link-local hack */ + struct in6_addr all_nodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT; IP6_EXTHDR_GET(mldh, struct mld_hdr *, m, off, sizeof(*mldh)); if (mldh == NULL) { @@ -239,15 +246,13 @@ mld6_input(struct mbuf *m, int off) timer = ntohs(mldh->mld_maxdelay)*PR_FASTHZ/MLD_TIMER_SCALE; if (timer == 0 && mldh->mld_maxdelay) timer = 1; - mld_all_nodes_linklocal.s6_addr16[1] = - htons(ifp->if_index); /* XXX */ + all_nodes.s6_addr16[1] = htons(ifp->if_index); TAILQ_FOREACH(ifma, >if_maddrlist, ifma_list) { if (ifma->ifma_addr->sa_family != AF_INET6) continue; in6m = ifmatoin6m(ifma); - if (IN6_ARE_ADDR_EQUAL(>in6m_addr, - _all_nodes_linklocal) || + if (IN6_ARE_ADDR_EQUAL(>in6m_addr, _nodes) || __IPV6_ADDR_MC_SCOPE(>in6m_addr) < __IPV6_ADDR_SCOPE_LINKLOCAL) continue;
Re: rpki-client add abort to rrdp
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.09.02 19:55:28 +0200: > We want to be able to abort RRDP syncs. Now the problem is that depending > on the state the abort request is more or less complex. What needs to be > avoided is that a message received after the corresponding RRDP session > was removed. This is mainly the RRDP_FILE and RRDP_HTTP_FIN messages that > cause this. > > So once a RRDP_HTTP_INI message was received the abort code goes through > most states, it just aborts the internal XML parser and closes the input > fd. ok. > -- > :wq Claudio > > Index: extern.h > === > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v > retrieving revision 1.151 > diff -u -p -r1.151 extern.h > --- extern.h 30 Aug 2022 18:56:49 - 1.151 > +++ extern.h 2 Sep 2022 17:41:07 - > @@ -408,6 +408,7 @@ enum rrdp_msg { > RRDP_HTTP_REQ, > RRDP_HTTP_INI, > RRDP_HTTP_FIN, > + RRDP_ABORT, > }; > > /* > Index: rrdp.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/rrdp.c,v > retrieving revision 1.24 > diff -u -p -r1.24 rrdp.c > --- rrdp.c15 May 2022 16:43:35 - 1.24 > +++ rrdp.c2 Sep 2022 17:54:58 - > @@ -57,6 +57,7 @@ struct rrdp { > struct pollfd *pfd; > int infd; > int state; > + int aborted; > unsigned int file_pending; > unsigned int file_failed; > enum http_result res; > @@ -73,7 +74,7 @@ struct rrdp { > struct delta_xml*dxml; > }; > > -TAILQ_HEAD(, rrdp) states = TAILQ_HEAD_INITIALIZER(states); > +static TAILQ_HEAD(, rrdp)states = TAILQ_HEAD_INITIALIZER(states); > > char * > xstrdup(const char *s) > @@ -256,7 +257,7 @@ rrdp_failed(struct rrdp *s) > /* reset file state before retrying */ > s->file_failed = 0; > > - if (s->task == DELTA) { > + if (s->task == DELTA && !s->aborted) { > /* fallback to a snapshot as per RFC8182 */ > free_delta_xml(s->dxml); > s->dxml = NULL; > @@ -289,7 +290,7 @@ rrdp_finished(struct rrdp *s) > if (s->file_pending > 0) > return; > > - if (s->state & RRDP_STATE_PARSE_ERROR) { > + if (s->state & RRDP_STATE_PARSE_ERROR || s->aborted) { > rrdp_failed(s); > return; > } > @@ -372,6 +373,34 @@ rrdp_finished(struct rrdp *s) > } > > static void > +rrdp_abort_req(struct rrdp *s) > +{ > + unsigned int id = s->id; > + > + s->aborted = 1; > + if (s->state == RRDP_STATE_REQ) { > + /* nothing is pending, just abort */ > + rrdp_free(s); > + rrdp_done(id, 1); > + return; > + } > + if (s->state == RRDP_STATE_WAIT) > + /* wait for HTTP_INI which will progress the state */ > + return; > + > + /* > + * RRDP_STATE_PARSE or later, close infd, abort parser but > + * wait for HTTP_FIN and file_pending to drop to 0. > + */ > + if (s->infd != -1) { > + close(s->infd); > + s->infd = -1; > + s->state |= RRDP_STATE_PARSE_DONE | RRDP_STATE_PARSE_ERROR; > + } > + rrdp_finished(s); > +} > + > +static void > rrdp_input_handler(int fd) > { > static struct ibuf *inbuf; > @@ -408,12 +437,15 @@ rrdp_input_handler(int fd) > errx(1, "expected fd not received"); > s = rrdp_get(id); > if (s == NULL) > - errx(1, "rrdp session %u does not exist", id); > + errx(1, "http ini, rrdp session %u does not exist", id); > if (s->state != RRDP_STATE_WAIT) > errx(1, "%s: bad internal state", s->local); > - > s->infd = b->fd; > s->state = RRDP_STATE_PARSE; > + if (s->aborted) { > + rrdp_abort_req(s); > + break; > + } > break; > case RRDP_HTTP_FIN: > io_read_buf(b, , sizeof(res)); > @@ -423,20 +455,19 @@ rrdp_input_handler(int fd) > > s = rrdp_get(id); > if (s == NULL) > - errx(1, "rrdp session %u does not exist", id); > + errx(1, "http fin, rrdp session %u does not exist", id); > if (!(s->state & RRDP_STATE_PARSE)) > errx(1, "%s: bad internal state", s->local); > - > + s->state |= RRDP_STATE_HTTP_DONE; > s->res = res; > free(s->last_mod); > s->last_mod = last_mod; > - s->state |= RRDP_STATE_HTTP_DONE; > rrdp_finished(s); > break; > case RRDP_FILE: > s = rrdp_get(id); > if (s == NULL) > -
rpki-client add abort to rrdp
We want to be able to abort RRDP syncs. Now the problem is that depending on the state the abort request is more or less complex. What needs to be avoided is that a message received after the corresponding RRDP session was removed. This is mainly the RRDP_FILE and RRDP_HTTP_FIN messages that cause this. So once a RRDP_HTTP_INI message was received the abort code goes through most states, it just aborts the internal XML parser and closes the input fd. -- :wq Claudio Index: extern.h === RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v retrieving revision 1.151 diff -u -p -r1.151 extern.h --- extern.h30 Aug 2022 18:56:49 - 1.151 +++ extern.h2 Sep 2022 17:41:07 - @@ -408,6 +408,7 @@ enum rrdp_msg { RRDP_HTTP_REQ, RRDP_HTTP_INI, RRDP_HTTP_FIN, + RRDP_ABORT, }; /* Index: rrdp.c === RCS file: /cvs/src/usr.sbin/rpki-client/rrdp.c,v retrieving revision 1.24 diff -u -p -r1.24 rrdp.c --- rrdp.c 15 May 2022 16:43:35 - 1.24 +++ rrdp.c 2 Sep 2022 17:54:58 - @@ -57,6 +57,7 @@ struct rrdp { struct pollfd *pfd; int infd; int state; + int aborted; unsigned int file_pending; unsigned int file_failed; enum http_result res; @@ -73,7 +74,7 @@ struct rrdp { struct delta_xml*dxml; }; -TAILQ_HEAD(, rrdp) states = TAILQ_HEAD_INITIALIZER(states); +static TAILQ_HEAD(, rrdp) states = TAILQ_HEAD_INITIALIZER(states); char * xstrdup(const char *s) @@ -256,7 +257,7 @@ rrdp_failed(struct rrdp *s) /* reset file state before retrying */ s->file_failed = 0; - if (s->task == DELTA) { + if (s->task == DELTA && !s->aborted) { /* fallback to a snapshot as per RFC8182 */ free_delta_xml(s->dxml); s->dxml = NULL; @@ -289,7 +290,7 @@ rrdp_finished(struct rrdp *s) if (s->file_pending > 0) return; - if (s->state & RRDP_STATE_PARSE_ERROR) { + if (s->state & RRDP_STATE_PARSE_ERROR || s->aborted) { rrdp_failed(s); return; } @@ -372,6 +373,34 @@ rrdp_finished(struct rrdp *s) } static void +rrdp_abort_req(struct rrdp *s) +{ + unsigned int id = s->id; + + s->aborted = 1; + if (s->state == RRDP_STATE_REQ) { + /* nothing is pending, just abort */ + rrdp_free(s); + rrdp_done(id, 1); + return; + } + if (s->state == RRDP_STATE_WAIT) + /* wait for HTTP_INI which will progress the state */ + return; + + /* +* RRDP_STATE_PARSE or later, close infd, abort parser but +* wait for HTTP_FIN and file_pending to drop to 0. +*/ + if (s->infd != -1) { + close(s->infd); + s->infd = -1; + s->state |= RRDP_STATE_PARSE_DONE | RRDP_STATE_PARSE_ERROR; + } + rrdp_finished(s); +} + +static void rrdp_input_handler(int fd) { static struct ibuf *inbuf; @@ -408,12 +437,15 @@ rrdp_input_handler(int fd) errx(1, "expected fd not received"); s = rrdp_get(id); if (s == NULL) - errx(1, "rrdp session %u does not exist", id); + errx(1, "http ini, rrdp session %u does not exist", id); if (s->state != RRDP_STATE_WAIT) errx(1, "%s: bad internal state", s->local); - s->infd = b->fd; s->state = RRDP_STATE_PARSE; + if (s->aborted) { + rrdp_abort_req(s); + break; + } break; case RRDP_HTTP_FIN: io_read_buf(b, , sizeof(res)); @@ -423,20 +455,19 @@ rrdp_input_handler(int fd) s = rrdp_get(id); if (s == NULL) - errx(1, "rrdp session %u does not exist", id); + errx(1, "http fin, rrdp session %u does not exist", id); if (!(s->state & RRDP_STATE_PARSE)) errx(1, "%s: bad internal state", s->local); - + s->state |= RRDP_STATE_HTTP_DONE; s->res = res; free(s->last_mod); s->last_mod = last_mod; - s->state |= RRDP_STATE_HTTP_DONE; rrdp_finished(s); break; case RRDP_FILE: s = rrdp_get(id); if (s == NULL) - errx(1, "rrdp session %u does not exist", id); + errx(1, "file, rrdp session %u does not exist", id);; if (b->fd != -1) errx(1,
Re: vmd(8): compute i8254 Read-Back latch from singular timestamp
On Fri, Sep 02, 2022 at 11:42:03AM -0500, Scott Cheloha wrote: > The 8254 data sheet [1] says this about the Read-Back command: > > > The read-back command may be used to latch multi- > > ple counter output latches (OL) by setting the > > COUNT bit D5 = 0 and selecting the desired coun- > > ter(s). This single command is functionally equiva- > > lent to several counter latch commands, one for > > each counter latched. [...] > > This is a little ambiguous. But my hunch is that the intent here is > "you can latch multiple counters all at once". Simultaneously. > Otherwise the utility of the read-back command is suspect. > > To simulate a simultaneous latch, we should only call clock_gettime(2) > once and use that singular timestamp to compute olatch for each > counter. > > ok? > > [1] 8254 Programmable Interval Timer, p. 8 > https://www.scs.stanford.edu/10wi-cs140/pintos/specs/8254.pdf Didn't see dv's reply earlier; I agree with what he said. > > Index: i8253.c > === > RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v > retrieving revision 1.34 > diff -u -p -r1.34 i8253.c > --- i8253.c 16 Jun 2021 16:55:02 - 1.34 > +++ i8253.c 2 Sep 2022 16:25:02 - > @@ -128,6 +128,8 @@ i8253_do_readback(uint32_t data) > int readback_channel[3] = { TIMER_RB_C0, TIMER_RB_C1, TIMER_RB_C2 }; > int i; > > + clock_gettime(CLOCK_MONOTONIC, ); > + > /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */ > if (data & ~TIMER_RB_STATUS) { > i8253_channel[0].rbs = (data & TIMER_RB_C0) ? 1 : 0; > @@ -139,7 +141,6 @@ i8253_do_readback(uint32_t data) > if (data & ~TIMER_RB_COUNT) { > for (i = 0; i < 3; i++) { > if (data & readback_channel[i]) { > - clock_gettime(CLOCK_MONOTONIC, ); > timespecsub(, _channel[i].ts, ); > ns = delta.tv_sec * 10 + delta.tv_nsec; > ticks = ns / NS_PER_TICK;
Re: vmd(8): compute i8254 Read-Back latch from singular timestamp
On Fri, Sep 02, 2022 at 11:42:03AM -0500, Scott Cheloha wrote: > The 8254 data sheet [1] says this about the Read-Back command: > > > The read-back command may be used to latch multi- > > ple counter output latches (OL) by setting the > > COUNT bit D5 = 0 and selecting the desired coun- > > ter(s). This single command is functionally equiva- > > lent to several counter latch commands, one for > > each counter latched. [...] > > This is a little ambiguous. But my hunch is that the intent here is > "you can latch multiple counters all at once". Simultaneously. > Otherwise the utility of the read-back command is suspect. > > To simulate a simultaneous latch, we should only call clock_gettime(2) > once and use that singular timestamp to compute olatch for each > counter. > > ok? > > [1] 8254 Programmable Interval Timer, p. 8 > https://www.scs.stanford.edu/10wi-cs140/pintos/specs/8254.pdf > Reads ok to me. ok mlarkin > Index: i8253.c > === > RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v > retrieving revision 1.34 > diff -u -p -r1.34 i8253.c > --- i8253.c 16 Jun 2021 16:55:02 - 1.34 > +++ i8253.c 2 Sep 2022 16:25:02 - > @@ -128,6 +128,8 @@ i8253_do_readback(uint32_t data) > int readback_channel[3] = { TIMER_RB_C0, TIMER_RB_C1, TIMER_RB_C2 }; > int i; > > + clock_gettime(CLOCK_MONOTONIC, ); > + > /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */ > if (data & ~TIMER_RB_STATUS) { > i8253_channel[0].rbs = (data & TIMER_RB_C0) ? 1 : 0; > @@ -139,7 +141,6 @@ i8253_do_readback(uint32_t data) > if (data & ~TIMER_RB_COUNT) { > for (i = 0; i < 3; i++) { > if (data & readback_channel[i]) { > - clock_gettime(CLOCK_MONOTONIC, ); > timespecsub(, _channel[i].ts, ); > ns = delta.tv_sec * 10 + delta.tv_nsec; > ticks = ns / NS_PER_TICK;
Re: vmd(8): compute i8254 Read-Back latch from singular timestamp
Scott Cheloha writes: > The 8254 data sheet [1] says this about the Read-Back command: > >> The read-back command may be used to latch multi- >> ple counter output latches (OL) by setting the >> COUNT bit D5 = 0 and selecting the desired coun- >> ter(s). This single command is functionally equiva- >> lent to several counter latch commands, one for >> each counter latched. [...] > > This is a little ambiguous. But my hunch is that the intent here is > "you can latch multiple counters all at once". Simultaneously. > Otherwise the utility of the read-back command is suspect. > > To simulate a simultaneous latch, we should only call clock_gettime(2) > once and use that singular timestamp to compute olatch for each > counter. > > ok? > I'm not an expert on the i825{3,4} but have a question below. I did quickly test this diff and see no noticeable difference from the point of view of my local guests. > [1] 8254 Programmable Interval Timer, p. 8 > https://www.scs.stanford.edu/10wi-cs140/pintos/specs/8254.pdf > > Index: i8253.c > === > RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v > retrieving revision 1.34 > diff -u -p -r1.34 i8253.c > --- i8253.c 16 Jun 2021 16:55:02 - 1.34 > +++ i8253.c 2 Sep 2022 16:25:02 - > @@ -128,6 +128,8 @@ i8253_do_readback(uint32_t data) > int readback_channel[3] = { TIMER_RB_C0, TIMER_RB_C1, TIMER_RB_C2 }; > int i; > > + clock_gettime(CLOCK_MONOTONIC, ); > + Why make this call to clock_gettime here ^ > /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */ > if (data & ~TIMER_RB_STATUS) { > i8253_channel[0].rbs = (data & TIMER_RB_C0) ? 1 : 0; > @@ -139,7 +141,6 @@ i8253_do_readback(uint32_t data) > if (data & ~TIMER_RB_COUNT) { ...instead of here where we can save a possible syscall? I'm ok with this with the mentioned change. > for (i = 0; i < 3; i++) { > if (data & readback_channel[i]) { > - clock_gettime(CLOCK_MONOTONIC, ); > timespecsub(, _channel[i].ts, ); > ns = delta.tv_sec * 10 + delta.tv_nsec; > ticks = ns / NS_PER_TICK;
Re: divert-reply: keep pf state after pcb is dropped
Hi, On Fri, 2 Sep 2022 17:40:13 +0200 Alexander Bluhm wrote: > On Fri, Sep 02, 2022 at 03:04:34PM +0200, YASUOKA Masahiko wrote: >> The diff is to fix a problem in a complex setup. >> >> Normal setup of divert-reply for TCP connection: >> >> client --- relayd --- server >> >> - transparently forward TCP connections >> - divert-reply is configured the outbound connection to the server >> - so that the PF state is removed when the PCB is deleted >> - otherwise if packets from server is comming after the PCB is >> deleted, they are accidentally forwarded directly to the client >> >> In addtion to this, "match out nat-to" is configured for the outbound >> connection instead of dropping "transparent". The purpose of doing >> this is to expand the space of ephemeral ports of NAT. Ephemeral >> ports of PCB is limitted in one 2^16 space, but ephemeral ports of PF >> is limitted in 2^16 for each remote address. >> >> In this case, if the PF state is dropped immediately after the PCB is >> dropped, the port number of NAT might be reused quickly, then a >> problem can happen on the server side since the port is used for the >> old connection. >> >> So the diff is to keep the state until timeout. >> >> comment? > > How does that work together with port reuse? > > One reason I have introduced pf_remove_divert_state() is to behave > correctly in case the client does port reuse. > > When client creates and closes a lot of connections it will reuse > its port before the timeout triggers. My company IIJ is using divert reply in a similar situation. > We have code in pf_state_key_attach(), pf_test_state() and tcp_input() > to remove old states and create new ones in that case. For divert > the old state has to be removed, so that the new packet reaches the > listen state. > > I don't know if this still works with your diff. Have you considered > it? I have hit the same or a similar problem. If the pf state is kept remain and the client reuses the same port, SYN packet of the new connection might be dropped by the old state. (Old version of the diff didn't consider this, the problem actually happened) The diff already considers that situation. If the pf state is kept remain and the client reuses the same port, SYN packet matches the old state in pf_find_state() but it returns PF_DROP. 1151 if (ISSET(s->state_flags, PFSTATE_INP_UNLINKED)) 1152 return (PF_DROP); Then it goes through to pf_test_rule(), then a new state is created and old state is removed in pf_state_key_attach(). The flag of the old state will not inherit to the new state. So the packet is passed and the old state is removed. > Reuse is tested in /usr/src/regress/sys/net/pf_divert/. But I do > not use nat or rdr there. So my test may not cover the code in > your diff as you check "key[PF_SK_WIRE] != si->s->key[PF_SK_STACK]". > > I am running regress test with diff right now, we will see if it > still works. Thanks, > bluhm > >> Index: sys/net/pf.c >> === >> RCS file: /cvs/src/sys/net/pf.c,v >> retrieving revision 1.1138 >> diff -u -p -r1.1138 pf.c >> --- sys/net/pf.c 30 Aug 2022 11:53:03 - 1.1138 >> +++ sys/net/pf.c 2 Sep 2022 12:54:36 - >> @@ -1148,6 +1148,8 @@ pf_find_state(struct pf_pdesc *pd, struc >> >> if (s == NULL) >> return (PF_DROP); >> +if (ISSET(s->state_flags, PFSTATE_INP_UNLINKED)) >> +return (PF_DROP); >> >> if (s->rule.ptr->pktrate.limit && pd->dir == s->direction) { >> pf_add_threshold(>rule.ptr->pktrate); >> @@ -1461,7 +1463,23 @@ pf_remove_divert_state(struct pf_state_k >> if (sk == si->s->key[PF_SK_STACK] && si->s->rule.ptr && >> (si->s->rule.ptr->divert.type == PF_DIVERT_TO || >> si->s->rule.ptr->divert.type == PF_DIVERT_REPLY)) { >> -pf_remove_state(si->s); >> +if (si->s->key[PF_SK_STACK]->proto == IPPROTO_TCP && >> +si->s->key[PF_SK_WIRE] != si->s->key[PF_SK_STACK]) { >> +/* >> + * If the local address is translated, keep >> + * the state for "tcp.closed" seconds to >> + * prevent its source port from being reused. >> + */ >> +if (si->s->src.state < TCPS_FIN_WAIT_2 || >> +si->s->dst.state < TCPS_FIN_WAIT_2) { >> +pf_set_protostate(si->s, PF_PEER_BOTH, >> +TCPS_TIME_WAIT); >> +si->s->timeout = PFTM_TCP_CLOSED; >> +si->s->expire = getuptime(); >> +} >> +si->s->state_flags |= PFSTATE_INP_UNLINKED; >> +} else
vmd(8): compute i8254 Read-Back latch from singular timestamp
The 8254 data sheet [1] says this about the Read-Back command: > The read-back command may be used to latch multi- > ple counter output latches (OL) by setting the > COUNT bit D5 = 0 and selecting the desired coun- > ter(s). This single command is functionally equiva- > lent to several counter latch commands, one for > each counter latched. [...] This is a little ambiguous. But my hunch is that the intent here is "you can latch multiple counters all at once". Simultaneously. Otherwise the utility of the read-back command is suspect. To simulate a simultaneous latch, we should only call clock_gettime(2) once and use that singular timestamp to compute olatch for each counter. ok? [1] 8254 Programmable Interval Timer, p. 8 https://www.scs.stanford.edu/10wi-cs140/pintos/specs/8254.pdf Index: i8253.c === RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v retrieving revision 1.34 diff -u -p -r1.34 i8253.c --- i8253.c 16 Jun 2021 16:55:02 - 1.34 +++ i8253.c 2 Sep 2022 16:25:02 - @@ -128,6 +128,8 @@ i8253_do_readback(uint32_t data) int readback_channel[3] = { TIMER_RB_C0, TIMER_RB_C1, TIMER_RB_C2 }; int i; + clock_gettime(CLOCK_MONOTONIC, ); + /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */ if (data & ~TIMER_RB_STATUS) { i8253_channel[0].rbs = (data & TIMER_RB_C0) ? 1 : 0; @@ -139,7 +141,6 @@ i8253_do_readback(uint32_t data) if (data & ~TIMER_RB_COUNT) { for (i = 0; i < 3; i++) { if (data & readback_channel[i]) { - clock_gettime(CLOCK_MONOTONIC, ); timespecsub(, _channel[i].ts, ); ns = delta.tv_sec * 10 + delta.tv_nsec; ticks = ns / NS_PER_TICK;
Re: divert-reply: keep pf state after pcb is dropped
On Fri, Sep 02, 2022 at 03:04:34PM +0200, YASUOKA Masahiko wrote: > Hi, > > The diff is to fix a problem in a complex setup. > > Normal setup of divert-reply for TCP connection: > > client --- relayd --- server > > - transparently forward TCP connections > - divert-reply is configured the outbound connection to the server > - so that the PF state is removed when the PCB is deleted > - otherwise if packets from server is comming after the PCB is > deleted, they are accidentally forwarded directly to the client > > In addtion to this, "match out nat-to" is configured for the outbound > connection instead of dropping "transparent". The purpose of doing > this is to expand the space of ephemeral ports of NAT. Ephemeral > ports of PCB is limitted in one 2^16 space, but ephemeral ports of PF > is limitted in 2^16 for each remote address. > > In this case, if the PF state is dropped immediately after the PCB is > dropped, the port number of NAT might be reused quickly, then a > problem can happen on the server side since the port is used for the > old connection. > > So the diff is to keep the state until timeout. > > comment? How does that work together with port reuse? One reason I have introduced pf_remove_divert_state() is to behave correctly in case the client does port reuse. When client creates and closes a lot of connections it will reuse its port before the timeout triggers. We have code in pf_state_key_attach(), pf_test_state() and tcp_input() to remove old states and create new ones in that case. For divert the old state has to be removed, so that the new packet reaches the listen state. I don't know if this still works with your diff. Have you considered it? Reuse is tested in /usr/src/regress/sys/net/pf_divert/. But I do not use nat or rdr there. So my test may not cover the code in your diff as you check "key[PF_SK_WIRE] != si->s->key[PF_SK_STACK]". I am running regress test with diff right now, we will see if it still works. bluhm > Index: sys/net/pf.c > === > RCS file: /cvs/src/sys/net/pf.c,v > retrieving revision 1.1138 > diff -u -p -r1.1138 pf.c > --- sys/net/pf.c 30 Aug 2022 11:53:03 - 1.1138 > +++ sys/net/pf.c 2 Sep 2022 12:54:36 - > @@ -1148,6 +1148,8 @@ pf_find_state(struct pf_pdesc *pd, struc > > if (s == NULL) > return (PF_DROP); > + if (ISSET(s->state_flags, PFSTATE_INP_UNLINKED)) > + return (PF_DROP); > > if (s->rule.ptr->pktrate.limit && pd->dir == s->direction) { > pf_add_threshold(>rule.ptr->pktrate); > @@ -1461,7 +1463,23 @@ pf_remove_divert_state(struct pf_state_k > if (sk == si->s->key[PF_SK_STACK] && si->s->rule.ptr && > (si->s->rule.ptr->divert.type == PF_DIVERT_TO || > si->s->rule.ptr->divert.type == PF_DIVERT_REPLY)) { > - pf_remove_state(si->s); > + if (si->s->key[PF_SK_STACK]->proto == IPPROTO_TCP && > + si->s->key[PF_SK_WIRE] != si->s->key[PF_SK_STACK]) { > + /* > + * If the local address is translated, keep > + * the state for "tcp.closed" seconds to > + * prevent its source port from being reused. > + */ > + if (si->s->src.state < TCPS_FIN_WAIT_2 || > + si->s->dst.state < TCPS_FIN_WAIT_2) { > + pf_set_protostate(si->s, PF_PEER_BOTH, > + TCPS_TIME_WAIT); > + si->s->timeout = PFTM_TCP_CLOSED; > + si->s->expire = getuptime(); > + } > + si->s->state_flags |= PFSTATE_INP_UNLINKED; > + } else > + pf_remove_state(si->s); > break; > } > } > Index: sys/net/pfvar.h > === > RCS file: /cvs/src/sys/net/pfvar.h,v > retrieving revision 1.509 > diff -u -p -r1.509 pfvar.h > --- sys/net/pfvar.h 20 Jul 2022 09:33:11 - 1.509 > +++ sys/net/pfvar.h 2 Sep 2022 12:54:37 - > @@ -784,6 +784,7 @@ struct pf_state { > #define PFSTATE_RANDOMID0x0080 > #define PFSTATE_SCRUB_TCP 0x0100 > #define PFSTATE_SETPRIO 0x0200 > +#define PFSTATE_INP_UNLINKED0x0400 > #define PFSTATE_SCRUBMASK > (PFSTATE_NODF|PFSTATE_RANDOMID|PFSTATE_SCRUB_TCP) > #define PFSTATE_SETMASK (PFSTATE_SETTOS|PFSTATE_SETPRIO) > u_int8_t log; > > >
Remove "#ifdef INET6" block from in_setpeeraddr()
We always call in6_setpeeraddr() and never call in_setpeeraddr() fro the inet6 case. Index: sys/netinet/in_pcb.c === RCS file: /cvs/src/sys/netinet/in_pcb.c,v retrieving revision 1.273 diff -u -p -r1.273 in_pcb.c --- sys/netinet/in_pcb.c30 Aug 2022 11:53:04 - 1.273 +++ sys/netinet/in_pcb.c2 Sep 2022 14:59:02 - @@ -649,13 +649,6 @@ in_setpeeraddr(struct inpcb *inp, struct { struct sockaddr_in *sin; -#ifdef INET6 - if (sotopf(inp->inp_socket) == PF_INET6) { - in6_setpeeraddr(inp, nam); - return; - } -#endif /* INET6 */ - nam->m_len = sizeof(*sin); sin = mtod(nam, struct sockaddr_in *); memset(sin, 0, sizeof(*sin));
Move PRU_SOCKADDR and PRU_PEERADDR requests to corresponding pru_wrappers
Introduce in{,6}_sockaddr() and in{,6}_peeraddr() functions, and use them for all except tcp(4) sockets. Use tcp_sockaddr() and tcp_peeraddr() functions to keep debug ability. The key management and route domain sockets returns EINVAL error for PRU_SOCKADDR request, so keep this behaviour for a while instead of make pru_sockaddr handler optional and return EOPNOTSUPP. Within the old *_usrreq() only default panic case left. They are not called anymore, so just invoke panic() within. The *_usrreq() will be removed with the next diff. Index: sys/kern/uipc_usrreq.c === RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v retrieving revision 1.183 diff -u -p -r1.183 uipc_usrreq.c --- sys/kern/uipc_usrreq.c 2 Sep 2022 13:12:31 - 1.183 +++ sys/kern/uipc_usrreq.c 2 Sep 2022 14:44:46 - @@ -140,6 +140,8 @@ const struct pr_usrreqs uipc_usrreqs = { .pru_send = uipc_send, .pru_abort = uipc_abort, .pru_sense = uipc_sense, + .pru_sockaddr = uipc_sockaddr, + .pru_peeraddr = uipc_peeraddr, .pru_connect2 = uipc_connect2, }; @@ -215,44 +217,8 @@ int uipc_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, struct mbuf *control, struct proc *p) { - struct unpcb *unp = sotounpcb(so); - struct socket *so2; - int error = 0; - - if (req != PRU_SEND && control && control->m_len) { - error = EOPNOTSUPP; - goto release; - } - if (unp == NULL) { - error = EINVAL; - goto release; - } - - switch (req) { - - case PRU_SOCKADDR: - uipc_setaddr(unp, nam); - break; - - case PRU_PEERADDR: - so2 = unp_solock_peer(so); - uipc_setaddr(unp->unp_conn, nam); - if (so2 != NULL && so2 != so) - sounlock(so2); - break; - - case PRU_SLOWTIMO: - break; - - default: - panic("uipc_usrreq"); - } -release: - if (req != PRU_RCVD && req != PRU_RCVOOB && req != PRU_SENSE) { - m_freem(control); - m_freem(m); - } - return (error); + panic("uipc_usrreq"); + return (EOPNOTSUPP); } /* @@ -576,6 +542,28 @@ uipc_sense(struct socket *so, struct sta sb->st_ctim.tv_nsec = unp->unp_ctime.tv_nsec; sb->st_ino = unp->unp_ino; + return (0); +} + +int +uipc_sockaddr(struct socket *so, struct mbuf *nam) +{ + struct unpcb *unp = sotounpcb(so); + + uipc_setaddr(unp, nam); + return (0); +} + +int +uipc_peeraddr(struct socket *so, struct mbuf *nam) +{ + struct unpcb *unp = sotounpcb(so); + struct socket *so2; + + so2 = unp_solock_peer(so); + uipc_setaddr(unp->unp_conn, nam); + if (so2 != NULL && so2 != so) + sounlock(so2); return (0); } Index: sys/net/pfkeyv2.c === RCS file: /cvs/src/sys/net/pfkeyv2.c,v retrieving revision 1.250 diff -u -p -r1.250 pfkeyv2.c --- sys/net/pfkeyv2.c 2 Sep 2022 13:12:32 - 1.250 +++ sys/net/pfkeyv2.c 2 Sep 2022 14:44:46 - @@ -176,6 +176,8 @@ int pfkeyv2_shutdown(struct socket *); int pfkeyv2_send(struct socket *, struct mbuf *, struct mbuf *, struct mbuf *); int pfkeyv2_abort(struct socket *); +int pfkeyv2_sockaddr(struct socket *, struct mbuf *); +int pfkeyv2_peeraddr(struct socket *, struct mbuf *); int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *); int pfkeyv2_output(struct mbuf *, struct socket *); @@ -211,6 +213,8 @@ const struct pr_usrreqs pfkeyv2_usrreqs .pru_shutdown = pfkeyv2_shutdown, .pru_send = pfkeyv2_send, .pru_abort = pfkeyv2_abort, + .pru_sockaddr = pfkeyv2_sockaddr, + .pru_peeraddr = pfkeyv2_peeraddr, }; const struct protosw pfkeysw[] = { @@ -389,45 +393,26 @@ pfkeyv2_abort(struct socket *so) } int -pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m, -struct mbuf *nam, struct mbuf *control, struct proc *p) +pfkeyv2_sockaddr(struct socket *so, struct mbuf *nam) { - struct pkpcb *kp; - int error = 0; - - soassertlocked(so); - - if (control && control->m_len) { - error = EOPNOTSUPP; - goto release; - } - - kp = sotokeycb(so); - if (kp == NULL) { - error = EINVAL; - goto release; - } - - switch (req) { - /* minimal support, just implement a fake peer address */ - case PRU_SOCKADDR: - error = EINVAL; - break; - case PRU_PEERADDR: - bcopy(_addr, mtod(nam, caddr_t), pfkey_addr.sa_len); - nam->m_len = pfkey_addr.sa_len; - break; + /*
some more kernel const
Constify nam2blk[] and chrtoblktbl[], these are never modified at runtime. Plus an octeon bonus: devmap[]. Index: sys/arch/alpha/alpha/autoconf.c === RCS file: /OpenBSD/src/sys/arch/alpha/alpha/autoconf.c,v retrieving revision 1.38 diff -u -p -u -p -r1.38 autoconf.c --- sys/arch/alpha/alpha/autoconf.c 27 Jan 2018 22:55:23 - 1.38 +++ sys/arch/alpha/alpha/autoconf.c 2 Sep 2022 14:39:45 - @@ -220,7 +220,7 @@ device_register(dev, aux) (*platform.device_register)(dev, aux); } -struct nam2blk nam2blk[] = { +const struct nam2blk nam2blk[] = { { "wd", 0 }, { "cd", 3 }, { "fd", 4 }, Index: sys/arch/alpha/alpha/conf.c === RCS file: /OpenBSD/src/sys/arch/alpha/alpha/conf.c,v retrieving revision 1.90 diff -u -p -u -p -r1.90 conf.c --- sys/arch/alpha/alpha/conf.c 11 Nov 2021 10:03:08 - 1.90 +++ sys/arch/alpha/alpha/conf.c 2 Sep 2022 14:39:45 - @@ -252,7 +252,7 @@ getnulldev() return makedev(mem_no, 2); } -int chrtoblktbl[] = { +const int chrtoblktbl[] = { /*VCHR*//*VBLK*/ /* 0 */NODEV, /* 1 */NODEV, @@ -293,4 +293,4 @@ int chrtoblktbl[] = { /* 36 */0, /* 37 */4, /* fd */ }; -int nchrtoblktbl = nitems(chrtoblktbl); +const int nchrtoblktbl = nitems(chrtoblktbl); Index: sys/arch/amd64/amd64/autoconf.c === RCS file: /OpenBSD/src/sys/arch/amd64/amd64/autoconf.c,v retrieving revision 1.53 diff -u -p -u -p -r1.53 autoconf.c --- sys/arch/amd64/amd64/autoconf.c 11 Jan 2019 06:25:06 - 1.53 +++ sys/arch/amd64/amd64/autoconf.c 2 Sep 2022 14:39:45 - @@ -223,7 +223,7 @@ diskconf(void) #endif /* HIBERNATE */ } -struct nam2blk nam2blk[] = { +const struct nam2blk nam2blk[] = { { "wd", 0 }, { "fd", 2 }, { "sd", 4 }, Index: sys/arch/amd64/amd64/conf.c === RCS file: /OpenBSD/src/sys/arch/amd64/amd64/conf.c,v retrieving revision 1.75 diff -u -p -u -p -r1.75 conf.c --- sys/arch/amd64/amd64/conf.c 28 Jun 2022 14:43:50 - 1.75 +++ sys/arch/amd64/amd64/conf.c 2 Sep 2022 14:39:45 - @@ -331,7 +331,7 @@ getnulldev(void) return makedev(mem_no, 2); } -int chrtoblktbl[] = { +const int chrtoblktbl[] = { /*VCHR*//*VBLK*/ /* 0 */NODEV, /* 1 */NODEV, @@ -383,7 +383,7 @@ int chrtoblktbl[] = { /* 47 */17, /* rd */ }; -int nchrtoblktbl = nitems(chrtoblktbl); +const int nchrtoblktbl = nitems(chrtoblktbl); /* * In order to map BSD bdev numbers of disks to their BIOS equivalents Index: sys/arch/arm/arm/conf.c === RCS file: /OpenBSD/src/sys/arch/arm/arm/conf.c,v retrieving revision 1.58 diff -u -p -u -p -r1.58 conf.c --- sys/arch/arm/arm/conf.c 11 Nov 2021 10:03:08 - 1.58 +++ sys/arch/arm/arm/conf.c 2 Sep 2022 14:39:45 - @@ -415,7 +415,7 @@ iszerodev(dev_t dev) } -int chrtoblktbl[] = { +const int chrtoblktbl[] = { /*VCHR*//*VBLK*/ /* 0 */NODEV, /* 1 */NODEV, @@ -445,7 +445,7 @@ int chrtoblktbl[] = { /* 25 */NODEV, /* 26 */26,/* cd */ }; -int nchrtoblktbl = nitems(chrtoblktbl); +const int nchrtoblktbl = nitems(chrtoblktbl); dev_t getnulldev(void) Index: sys/arch/arm64/arm64/autoconf.c === RCS file: /OpenBSD/src/sys/arch/arm64/arm64/autoconf.c,v retrieving revision 1.12 diff -u -p -u -p -r1.12 autoconf.c --- sys/arch/arm64/arm64/autoconf.c 21 Feb 2021 14:55:16 - 1.12 +++ sys/arch/arm64/arm64/autoconf.c 2 Sep 2022 14:39:45 - @@ -109,7 +109,7 @@ device_register(struct device *dev, void { } -struct nam2blk nam2blk[] = { +const struct nam2blk nam2blk[] = { { "wd", 0 }, { "sd", 4 }, { "cd", 6 }, Index: sys/arch/arm64/arm64/conf.c === RCS file: /OpenBSD/src/sys/arch/arm64/arm64/conf.c,v retrieving revision 1.19 diff -u -p -u -p -r1.19 conf.c --- sys/arch/arm64/arm64/conf.c 11 Nov 2021 10:03:08 - 1.19 +++ sys/arch/arm64/arm64/conf.c 2 Sep 2022 14:39:45 - @@ -273,7 +273,7 @@ getnulldev(void) return makedev(CMAJ_MM, 2); } -int chrtoblktbl[] = { +const int chrtoblktbl[] = { /*VCHR*//*VBLK*/ /* 0 */NODEV, /* 1 */NODEV, @@ -325,7 +325,7 @@ int chrtoblktbl[] = { /* 47 */17, /* rd */ }; -int nchrtoblktbl = nitems(chrtoblktbl); +const int nchrtoblktbl =
Re: Race in disk_attach_callback?
On Mon, Aug 29, 2022 at 07:51:24PM +, Miod Vallat wrote: > > What's the status on this diff? > > I'm waiting for review from at least one of the softraid suspects prior > to putting this in, in case there are further cinematics to address. Sounds like a good idea. In the meantime, while converting remaining/forgotten architectures from the manual disklabel/newfs dance to 'installboot -p', I also tested this diff wrt. fresh installations on softraid and found no behaviour change. When installing to a fresh softraid volume, the disklabel UID is all zeroes. On macppc for example, the installer still does disklabel $_disk 2>/dev/null | grep -q "^ i:" || disklabel -w -d $_disk newfs -t msdos ${_disk}i which can be replaced with installboot -p $_disk as for example arm64 already does. I only have access to macppc via multi-user SSH, so I can't step through the installer myself. However, reconstructing the installer on softraid on vnd works with manual disklabel/newfs as well as with installboot -p, with and without miod's kernel race fix. With zeroed images/disks, these tests always start out with an all-zeroed disklabel UID on the softraid volume at the time fdisk and newfs (directly or through installboot -p) are run. Again, installation works on softraid with zeroed-DUIDs, with and without miod's diff.
Re: unbound and cannot increase max open fds from 512 to 4152
Stuart Henderson(s...@spacehopper.org) on 2022.09.02 12:16:06 +0100: > On 2022/09/02 11:25, Sebastian Benoit wrote: > > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound > > > > > 1.16.0. > > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: > > > > > validator > > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator > > > > > > > > As far as i understand, the number of fds that unbound is asking for is > > > > based on the num-threads, outgoing-num-tcp, interface and some other > > > > setting > > > > in the unbound config. > > > > > > Those particular settings mentioned I did not modified. However the > > > values are not that important. I expected /etc/login.conf.d/unbound to > > > be present on a fresh install. More on that below. > > > > It would still be interesting why it wants to increase the limit to 4152. > > My list of settings is probably not complete. Show your config if you can. > > The key thing here is that it's for reload not startup. > Easy to reproduce here. Ah yes, thanks. So should the default limit be changed?
divert-reply: keep pf state after pcb is dropped
Hi, The diff is to fix a problem in a complex setup. Normal setup of divert-reply for TCP connection: client --- relayd --- server - transparently forward TCP connections - divert-reply is configured the outbound connection to the server - so that the PF state is removed when the PCB is deleted - otherwise if packets from server is comming after the PCB is deleted, they are accidentally forwarded directly to the client In addtion to this, "match out nat-to" is configured for the outbound connection instead of dropping "transparent". The purpose of doing this is to expand the space of ephemeral ports of NAT. Ephemeral ports of PCB is limitted in one 2^16 space, but ephemeral ports of PF is limitted in 2^16 for each remote address. In this case, if the PF state is dropped immediately after the PCB is dropped, the port number of NAT might be reused quickly, then a problem can happen on the server side since the port is used for the old connection. So the diff is to keep the state until timeout. comment? Index: sys/net/pf.c === RCS file: /cvs/src/sys/net/pf.c,v retrieving revision 1.1138 diff -u -p -r1.1138 pf.c --- sys/net/pf.c30 Aug 2022 11:53:03 - 1.1138 +++ sys/net/pf.c2 Sep 2022 12:54:36 - @@ -1148,6 +1148,8 @@ pf_find_state(struct pf_pdesc *pd, struc if (s == NULL) return (PF_DROP); + if (ISSET(s->state_flags, PFSTATE_INP_UNLINKED)) + return (PF_DROP); if (s->rule.ptr->pktrate.limit && pd->dir == s->direction) { pf_add_threshold(>rule.ptr->pktrate); @@ -1461,7 +1463,23 @@ pf_remove_divert_state(struct pf_state_k if (sk == si->s->key[PF_SK_STACK] && si->s->rule.ptr && (si->s->rule.ptr->divert.type == PF_DIVERT_TO || si->s->rule.ptr->divert.type == PF_DIVERT_REPLY)) { - pf_remove_state(si->s); + if (si->s->key[PF_SK_STACK]->proto == IPPROTO_TCP && + si->s->key[PF_SK_WIRE] != si->s->key[PF_SK_STACK]) { + /* +* If the local address is translated, keep +* the state for "tcp.closed" seconds to +* prevent its source port from being reused. +*/ + if (si->s->src.state < TCPS_FIN_WAIT_2 || + si->s->dst.state < TCPS_FIN_WAIT_2) { + pf_set_protostate(si->s, PF_PEER_BOTH, + TCPS_TIME_WAIT); + si->s->timeout = PFTM_TCP_CLOSED; + si->s->expire = getuptime(); + } + si->s->state_flags |= PFSTATE_INP_UNLINKED; + } else + pf_remove_state(si->s); break; } } Index: sys/net/pfvar.h === RCS file: /cvs/src/sys/net/pfvar.h,v retrieving revision 1.509 diff -u -p -r1.509 pfvar.h --- sys/net/pfvar.h 20 Jul 2022 09:33:11 - 1.509 +++ sys/net/pfvar.h 2 Sep 2022 12:54:37 - @@ -784,6 +784,7 @@ struct pf_state { #definePFSTATE_RANDOMID0x0080 #definePFSTATE_SCRUB_TCP 0x0100 #definePFSTATE_SETPRIO 0x0200 +#definePFSTATE_INP_UNLINKED0x0400 #definePFSTATE_SCRUBMASK (PFSTATE_NODF|PFSTATE_RANDOMID|PFSTATE_SCRUB_TCP) #definePFSTATE_SETMASK (PFSTATE_SETTOS|PFSTATE_SETPRIO) u_int8_t log;
tcp timer mutex
Hi, To remove pressure from the exclusive netlock, I would like to run tcp_slowtimo() with a mutex only. That means that all the consumers have to read tcp_now only once to get consistent time. Same for tcp_iss, update it with mutex, read it once. ok? bluhm Index: net/pf.c === RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v retrieving revision 1.1138 diff -u -p -r1.1138 pf.c --- net/pf.c30 Aug 2022 11:53:03 - 1.1138 +++ net/pf.c1 Sep 2022 14:59:14 - @@ -3562,7 +3562,7 @@ pf_tcp_iss(struct pf_pdesc *pd) } SHA512Final(digest.bytes, ); pf_tcp_iss_off += 4096; - return (digest.words[0] + tcp_iss + pf_tcp_iss_off); + return (digest.words[0] + READ_ONCE(tcp_iss) + pf_tcp_iss_off); } void Index: netinet/tcp_input.c === RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v retrieving revision 1.379 diff -u -p -r1.379 tcp_input.c --- netinet/tcp_input.c 30 Aug 2022 11:53:04 - 1.379 +++ netinet/tcp_input.c 1 Sep 2022 15:40:18 - @@ -190,7 +190,7 @@ void tcp_newreno_partialack(struct tcpc voidsyn_cache_put(struct syn_cache *); voidsyn_cache_rm(struct syn_cache *); -int syn_cache_respond(struct syn_cache *, struct mbuf *); +int syn_cache_respond(struct syn_cache *, struct mbuf *, uint32_t); voidsyn_cache_timer(void *); voidsyn_cache_reaper(void *); voidsyn_cache_insert(struct syn_cache *, struct tcpcb *); @@ -198,10 +198,10 @@ void syn_cache_reset(struct sockaddr *, struct tcphdr *, u_int); int syn_cache_add(struct sockaddr *, struct sockaddr *, struct tcphdr *, unsigned int, struct socket *, struct mbuf *, u_char *, int, - struct tcp_opt_info *, tcp_seq *); + struct tcp_opt_info *, tcp_seq *, uint32_t); struct socket *syn_cache_get(struct sockaddr *, struct sockaddr *, struct tcphdr *, unsigned int, unsigned int, struct socket *, - struct mbuf *); + struct mbuf *, uint32_t); struct syn_cache *syn_cache_lookup(struct sockaddr *, struct sockaddr *, struct syn_cache_head **, u_int); @@ -375,6 +375,7 @@ tcp_input(struct mbuf **mp, int *offp, i short ostate; caddr_t saveti; tcp_seq iss, *reuse = NULL; + uint32_t now; u_long tiwin; struct tcp_opt_info opti; struct tcphdr *th; @@ -389,6 +390,7 @@ tcp_input(struct mbuf **mp, int *offp, i opti.ts_present = 0; opti.maxseg = 0; + now = READ_ONCE(tcp_now); /* * RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN @@ -698,7 +700,7 @@ findpcb: case TH_ACK: so = syn_cache_get(, , - th, iphlen, tlen, so, m); + th, iphlen, tlen, so, m, now); if (so == NULL) { /* * We don't have a SYN for @@ -830,7 +832,8 @@ findpcb: */ if (so->so_qlen > so->so_qlimit || syn_cache_add(, , th, iphlen, - so, m, optp, optlen, , reuse) == -1) { + so, m, optp, optlen, , reuse, now) + == -1) { tcpstat_inc(tcps_dropsyn); goto drop; } @@ -857,7 +860,7 @@ findpcb: * Segment received on connection. * Reset idle time and keep-alive timer. */ - tp->t_rcvtime = tcp_now; + tp->t_rcvtime = now; if (TCPS_HAVEESTABLISHED(tp->t_state)) TCP_TIMER_ARM(tp, TCPT_KEEP, tcp_keepidle); @@ -873,7 +876,7 @@ findpcb: if (optp) #endif if (tcp_dooptions(tp, optp, optlen, th, m, iphlen, , - m->m_pkthdr.ph_rtableid)) + m->m_pkthdr.ph_rtableid, now)) goto drop; if (opti.ts_present && opti.ts_ecr) { @@ -883,7 +886,7 @@ findpcb: opti.ts_ecr -= tp->ts_modulate; /* make sure ts_ecr is sensible */ - rtt_test = tcp_now - opti.ts_ecr; + rtt_test = now - opti.ts_ecr; if (rtt_test < 0 || rtt_test > TCP_RTT_MAX) opti.ts_ecr = 0; } @@ -926,7 +929,7 @@ findpcb: * Fix from Braden, see Stevens p. 870 */ if (opti.ts_present && SEQ_LEQ(th->th_seq, tp->last_ack_sent)) { - tp->ts_recent_age = tcp_now; + tp->ts_recent_age = now;
Re: move PRU_CONTROL request to (*pru_control)()
On Thu, Sep 01, 2022 at 10:52:33PM +0300, Vitaliy Makkoveev wrote: > The 'proc *' is not used for PRU_CONTROL request, so remove it from > pru_control() wrapper. > > I want to use existing in{6,}_control for tcp(4) and udp(4) sockets, so > for inet6 case I introduced `tcp6_usrreqs' and `udp6_usrreqs' > structures. I also want to use them for the following PRU_SOCKADDR and > PRU_PEERADDR. You forgot to remove the "if (req == PRU_CONTROL) {" block from udp_usrreq(). with that OK bluhm@ > Since the PRU_SOCKADDR and PRU_PEERADDR are the last ones, I want to > move the corresponding (*pru_)() handlers and kill (*pru_usrreq)() with > single diff. Is it ok to review? Convert PRU_SOCKADDR and PRU_PEERADDR together in one diff. Then the final diff should only conatin - removing pru_usrreq. We will then check whether we have forgotten anything. > Index: sys/sys/protosw.h > === > RCS file: /cvs/src/sys/sys/protosw.h,v > retrieving revision 1.51 > diff -u -p -r1.51 protosw.h > --- sys/sys/protosw.h 1 Sep 2022 18:21:23 - 1.51 > +++ sys/sys/protosw.h 1 Sep 2022 19:35:08 - > @@ -59,6 +59,7 @@ struct socket; > struct domain; > struct proc; > struct stat; > +struct ifnet; > > struct pr_usrreqs { > /* user request: see list below */ > @@ -77,6 +78,8 @@ struct pr_usrreqs { > int (*pru_send)(struct socket *, struct mbuf *, struct mbuf *, > struct mbuf *); > int (*pru_abort)(struct socket *); > + int (*pru_control)(struct socket *, u_long, caddr_t, > + struct ifnet *); > int (*pru_sense)(struct socket *, struct stat *); > int (*pru_rcvoob)(struct socket *, struct mbuf *, int); > int (*pru_sendoob)(struct socket *, struct mbuf *, struct mbuf *, > @@ -343,12 +346,12 @@ pru_abort(struct socket *so) > } > > static inline int > -pru_control(struct socket *so, u_long cmd, caddr_t data, > -struct ifnet *ifp, struct proc *p) > +pru_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp) > { > - return (*so->so_proto->pr_usrreqs->pru_usrreq)(so, > - PRU_CONTROL, (struct mbuf *)cmd, (struct mbuf *)data, > - (struct mbuf *)ifp, p); > + if (so->so_proto->pr_usrreqs->pru_control) > + return (*so->so_proto->pr_usrreqs->pru_control)(so, > + cmd, data, ifp); > + return (EOPNOTSUPP); > } > > static inline int > Index: sys/kern/sys_socket.c > === > RCS file: /cvs/src/sys/kern/sys_socket.c,v > retrieving revision 1.53 > diff -u -p -r1.53 sys_socket.c > --- sys/kern/sys_socket.c 14 Aug 2022 01:58:28 - 1.53 > +++ sys/kern/sys_socket.c 1 Sep 2022 19:35:07 - > @@ -137,7 +137,7 @@ soo_ioctl(struct file *fp, u_long cmd, c > if (IOCGROUP(cmd) == 'r') > return (EOPNOTSUPP); > KERNEL_LOCK(); > - error = pru_control(so, cmd, data, NULL, p); > + error = pru_control(so, cmd, data, NULL); > KERNEL_UNLOCK(); > break; > } > Index: sys/kern/uipc_usrreq.c > === > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v > retrieving revision 1.182 > diff -u -p -r1.182 uipc_usrreq.c > --- sys/kern/uipc_usrreq.c1 Sep 2022 18:21:22 - 1.182 > +++ sys/kern/uipc_usrreq.c1 Sep 2022 19:35:07 - > @@ -219,8 +219,6 @@ uipc_usrreq(struct socket *so, int req, > struct socket *so2; > int error = 0; > > - if (req == PRU_CONTROL) > - return (EOPNOTSUPP); > if (req != PRU_SEND && control && control->m_len) { > error = EOPNOTSUPP; > goto release; > Index: sys/net/if.c > === > RCS file: /cvs/src/sys/net/if.c,v > retrieving revision 1.663 > diff -u -p -r1.663 if.c > --- sys/net/if.c 13 Aug 2022 21:01:46 - 1.663 > +++ sys/net/if.c 1 Sep 2022 19:35:07 - > @@ -2360,7 +2360,7 @@ forceup: > break; > /* FALLTHROUGH */ > default: > - error = pru_control(so, cmd, data, ifp, p); > + error = pru_control(so, cmd, data, ifp); > if (error != EOPNOTSUPP) > break; > switch (cmd) { > Index: sys/net/pfkeyv2.c > === > RCS file: /cvs/src/sys/net/pfkeyv2.c,v > retrieving revision 1.249 > diff -u -p -r1.249 pfkeyv2.c > --- sys/net/pfkeyv2.c 1 Sep 2022 18:21:23 - 1.249 > +++ sys/net/pfkeyv2.c 1 Sep 2022 19:35:07 - > @@ -395,9 +395,6 @@ pfkeyv2_usrreq(struct socket *so, int re > struct pkpcb *kp; > int error = 0; > > - if (req == PRU_CONTROL) > - return (EOPNOTSUPP); > - >
libX11: disable thread safety constructor in 1.18.1
Hi again, So libX11 1.8.1 introduced a constructor to call XInitThreads(3) unconditionnaly at library load time, to make sure all multithreaded X applications would have correct locks around Xlib calls. Of course this behaviour change also exposes some bugs in existing code. A first one was found and fixed in xfsettingsd, the XFCE settings daemon. There is a 2nd one in x11/fvwm2 and x11/fvwm3 which is more complex to fix. I've sent patches to ports@ but I'm not confident that they are correct. So I propose that for OpenBSD 7.2 this change gets reverted in libX11. This is done with the patch below. Apply in /usr/xenocara/lib/libX11 and rebuild xenocara, following instructions in release(8). Ok, comments ? Index: Makefile.bsd-wrapper === RCS file: /cvs/OpenBSD/xenocara/lib/libX11/Makefile.bsd-wrapper,v retrieving revision 1.28 diff -u -p -u -r1.28 Makefile.bsd-wrapper --- Makefile.bsd-wrapper25 Apr 2022 19:26:17 - 1.28 +++ Makefile.bsd-wrapper26 Aug 2022 05:31:15 - @@ -14,6 +14,7 @@ SHARED_LIBS= X11 18.0 X11_xcb 2.0 CONFIGURE_ARGS= --enable-tcp-transport --enable-unix-transport --enable-ipv6 \ --disable-composecache \ + --disable-thread-safety-constructor \ --without-xmlto --without-fop --without-xsltproc .include -- Matthieu Herrb
Re: add sendmmsg and recvmmsg systemcalls
On Thu, Sep 01, 2022 at 06:06:10PM +0200, Moritz Buhl wrote: > I addressed your concerns as well as these of jca, just the kernel > part (and the new ktrace stuff) below. > > One minor thing: I didn't see any kdump output where one struct was > contained in another one but I am printing it like ddb would so I > guess it should be fine. OK bluhm@ > Index: kern/syscalls.master > === > RCS file: /mount/openbsd/cvs/src/sys/kern/syscalls.master,v > retrieving revision 1.229 > diff -u -p -r1.229 syscalls.master > --- kern/syscalls.master 1 Aug 2022 14:56:59 - 1.229 > +++ kern/syscalls.master 1 Sep 2022 14:52:47 - > @@ -244,8 +244,10 @@ > const char *permissions); } > 115 STD { int sys___realpath(const char *pathname, \ > char *resolved); } > -116 OBSOL t32_gettimeofday > -117 OBSOL t32_getrusage > +116 STD NOLOCK { int sys_recvmmsg(int s, struct mmsghdr *mmsg, \ > + unsigned int vlen, unsigned int flags, \ > + struct timespec *timeout); } > +117 UNIMPL sendmmsg > 118 STD { int sys_getsockopt(int s, int level, int name, \ > void *val, socklen_t *avalsize); } > 119 STD { int sys_thrkill(pid_t tid, int signum, void *tcb); } > Index: kern/uipc_syscalls.c > === > RCS file: /mount/openbsd/cvs/src/sys/kern/uipc_syscalls.c,v > retrieving revision 1.201 > diff -u -p -r1.201 uipc_syscalls.c > --- kern/uipc_syscalls.c 14 Aug 2022 01:58:28 - 1.201 > +++ kern/uipc_syscalls.c 1 Sep 2022 14:37:26 - > @@ -805,6 +805,140 @@ done: > } > > int > +sys_recvmmsg(struct proc *p, void *v, register_t *retval) > +{ > + struct sys_recvmmsg_args /* { > + syscallarg(int) s; > + syscallarg(struct mmsghdr *)mmsg; > + syscallarg(unsigned int)vlen; > + syscallarg(unsigned int)flags; > + syscallarg(struct timespec *) timeout; > + } */ *uap = v; > + struct mmsghdr mmsg, *mmsgp; > + struct timespec ts, now; > + struct iovec aiov[UIO_SMALLIOV], *uiov, *iov = aiov; > + struct file *fp; > + struct socket *so; > + struct timespec *timeout; > + size_t iovlen = UIO_SMALLIOV; > + register_t retrec; > + unsigned int vlen, dgrams; > + int error = 0, flags, s; > + > + s = SCARG(uap, s); > + if ((error = getsock(p, s, ))) > + return (error); > + so = (struct socket *)fp->f_data; > + > + timeout = SCARG(uap, timeout); > + if (timeout != NULL) { > + error = copyin(timeout, , sizeof(ts)); > + if (error) > + return error; > +#ifdef KTRACE > + if (KTRPOINT(p, KTR_STRUCT)) > + ktrreltimespec(p, ); > +#endif > + getnanotime(); > + timespecadd(, , ); > + } > + > + flags = SCARG(uap, flags); > + > + /* Arbitrarily capped at 1024 datagrams. */ > + vlen = SCARG(uap, vlen); > + if (vlen > 1024) > + vlen = 1024; > + > + mmsgp = SCARG(uap, mmsg); > + for (dgrams = 0; dgrams < vlen;) { > + error = copyin([dgrams], , sizeof(mmsg)); > + if (error) > + break; > + > + if (mmsg.msg_hdr.msg_iovlen > IOV_MAX) { > + error = EMSGSIZE; > + break; > + } > + > + if (mmsg.msg_hdr.msg_iovlen > iovlen) { > + if (iov != aiov) > + free(iov, M_IOV, iovlen * > + sizeof(struct iovec)); > + > + iovlen = mmsg.msg_hdr.msg_iovlen; > + iov = mallocarray(iovlen, sizeof(struct iovec), > + M_IOV, M_WAITOK); > + } > + > + if (mmsg.msg_hdr.msg_iovlen > 0) { > + error = copyin(mmsg.msg_hdr.msg_iov, iov, > + mmsg.msg_hdr.msg_iovlen * sizeof(struct iovec)); > + if (error) > + break; > + } > + > + uiov = mmsg.msg_hdr.msg_iov; > + mmsg.msg_hdr.msg_iov = iov; > + mmsg.msg_hdr.msg_flags = flags; > + > + error = recvit(p, s, _hdr, NULL, ); > + if (error) { > + if (error == EAGAIN && dgrams > 0) > + error = 0; > + break; > + } > + > + if (dgrams == 0 && flags & MSG_WAITFORONE) { > + flags &= ~MSG_WAITFORONE; > + flags |= MSG_DONTWAIT; > + } > + > + mmsg.msg_hdr.msg_iov = uiov; > + mmsg.msg_len = retrec; >
Re: unbound and cannot increase max open fds from 512 to 4152
On 2022/09/02 11:25, Sebastian Benoit wrote: > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound > > > > 1.16.0. > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator > > > > > > As far as i understand, the number of fds that unbound is asking for is > > > based on the num-threads, outgoing-num-tcp, interface and some other > > > setting > > > in the unbound config. > > > > Those particular settings mentioned I did not modified. However the > > values are not that important. I expected /etc/login.conf.d/unbound to > > be present on a fresh install. More on that below. > > It would still be interesting why it wants to increase the limit to 4152. > My list of settings is probably not complete. Show your config if you can. The key thing here is that it's for reload not startup. Easy to reproduce here.
xterm at 30bpp with TrueType fonts regression
Hi, if you happen to have a machine where the X server is running with a default depth of 30 bits per pixel and a True Color Visual (10 bit/rgb component), xterm wih TrueType fonts (using faceName X resource or the -fa command-line option) will display as a black window... This happens to be a 2 stage regression from xterm 366, which was ok on these setups, to xterm 369 which started to get faded colors, to xterm 370 which completely fails to display stuff. The patch below backs out the changes between xterm v366 and v372 that cause this regression. As far as I can tell the result is also still ok on other displays. ok ? Index: misc.c === RCS file: /cvs/OpenBSD/xenocara/app/xterm/misc.c,v retrieving revision 1.46 diff -u -r1.46 misc.c --- misc.c 22 May 2022 13:50:19 - 1.46 +++ misc.c 2 Sep 2022 09:58:21 - @@ -2599,7 +2599,7 @@ return result; } -XVisualInfo * +int getVisualInfo(XtermWidget xw) { #define MYFMT "getVisualInfo \ @@ -2640,9 +2640,7 @@ (vi->blue_mask != 0) && ((vi->red_mask & vi->green_mask) == 0) && ((vi->green_mask & vi->blue_mask) == 0) && - ((vi->blue_mask & vi->red_mask) == 0) && - (vi->class == TrueColor - || vi->class == DirectColor)); + ((vi->blue_mask & vi->red_mask) == 0)); if (resource.reportColors) { printf(MYFMT, MYARG); @@ -2652,13 +2650,9 @@ xw->rgb_shifts[0], xw->rgb_shifts[1], xw->rgb_shifts[2])); - TRACE(("...widths %u/%u/%u\n", - xw->rgb_widths[0], - xw->rgb_widths[1], - xw->rgb_widths[2])); } } -return (xw->visInfo != 0) && (xw->numVisuals > 0) ? xw->visInfo : NULL; +return (xw->visInfo != 0) && (xw->numVisuals > 0); #undef MYFMT #undef MYARG } @@ -2671,11 +2665,12 @@ if (AllowColorOps(xw, ecGetAnsiColor)) { XColor color; + Colormap cmap = xw->core.colormap; char buffer[80]; TRACE(("ReportAnsiColorRequest %d\n", colornum)); color.pixel = GET_COLOR_RES(xw, TScreenOf(xw)->Acolors[colornum]); - (void) QueryOneColor(xw, ); + XQueryColor(TScreenOf(xw)->display, cmap, ); sprintf(buffer, "%d;%d;rgb:%04x/%04x/%04x", opcode, (opcode == 5) ? (colornum - NUM_ANSI_COLORS) : colornum, @@ -2742,70 +2737,6 @@ return result; } -/******/ - -/* - * Call this function with def->{red,green,blue} initialized, to obtain a pixel - * value. - */ -Boolean -AllocOneColor(XtermWidget xw, XColor *def) -{ -TScreen *screen = TScreenOf(xw); -Boolean result = True; - -#define MaskIt(name,nn) \ - ((unsigned long) ((def->name >> (16 - xw->rgb_widths[nn])) \ -<< xw->rgb_shifts[nn]) \ -& xw->visInfo->name ##_mask) - -if (getVisualInfo(xw) != NULL && xw->has_rgb) { - def->pixel = MaskIt(red, 0) | MaskIt(green, 1) | MaskIt(blue, 2); -} else { - Display *dpy = screen->display; - if (!XAllocColor(dpy, xw->core.colormap, def)) { - /* -* Decide between foreground and background by a grayscale -* approximation. -*/ - int bright = def->red * 3 + def->green * 10 + def->blue; - int levels = 14 * 0x8000; - def->pixel = ((bright >= levels) - ? xw->dft_background - : xw->dft_foreground); - result = False; - } -} -return result; -} - -/******/ - -/* - * Call this function with def->pixel set to the color that we want to convert - * to separate red/green/blue. - */ -Boolean -QueryOneColor(XtermWidget xw, XColor *def) -{ -Boolean result = True; - -#define UnMaskIt(name,nn) \ - ((unsigned short)((def->pixel & xw->visInfo->name ##_mask) >> xw->rgb_shifts[nn])) -#define UnMaskIt2(name,nn) \ - (unsigned short)UnMaskIt(name,nn) << 8) \ - |UnMaskIt(name,nn))) << (8 - xw->rgb_widths[nn])) - -if (getVisualInfo(xw) != NULL && xw->has_rgb) { - /* *INDENT-EQLS* */ - def->red = UnMaskIt2(red, 0); - def->green = UnMaskIt2(green, 1); - def->blue = UnMaskIt2(blue, 2); -} else if (!XQueryColor(TScreenOf(xw)->display, xw->core.colormap, def)) { - result = False; -} -return result; -} /******/ @@ -2820,7 +2751,7 @@ * Return False if not able to find or allocate a color. */ static Boolean -allocateClosestRGB(XtermWidget xw, XColor *def) +allocateClosestRGB(XtermWidget xw,
Re: unbound and cannot increase max open fds from 512 to 4152
On Fri, Sep 02, 2022 at 11:25:20AM +0200, Sebastian Benoit wrote: > Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 08:07:01 +: > > On Fri, Sep 02, 2022 at 09:53:54AM +0200, Sebastian Benoit wrote: > > > Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 06:47:00 +: > > > > Hi, > > > > > > > > I have a question, could or should unbound in base be delivered with: > > > > > > > > # cat /etc/login.conf.d/unbound > > > > unbound:\ > > > > :openfiles-cur=4096:\ > > > > :openfiles-max=8192:\ > > > > :tc=daemon: > > > > > > > > or the like? Above is taken from dovecot. I was able to trigger via > > > > rcctl reload unbound following warnings in logs: > > > > > > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound > > > > 1.16.0. > > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation > > > > not permitted > > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max > > > > open fds from 512 to 4152 > > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: continuing with less > > > > udp ports: 460 > > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or > > > > decrease threads, ports in config to remove this warning > > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator > > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator > > > > > > > > After placing above login.conf.d login class capability file above > > > > warnings go away: > > > > > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound > > > > 1.16.0. > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator > > > > > > As far as i understand, the number of fds that unbound is asking for is > > > based on the num-threads, outgoing-num-tcp, interface and some other > > > setting > > > in the unbound config. > > > > Those particular settings mentioned I did not modified. However the > > values are not that important. I expected /etc/login.conf.d/unbound to > > be present on a fresh install. More on that below. > > It would still be interesting why it wants to increase the limit to 4152. > My list of settings is probably not complete. Show your config if you can. Sure. Modified a bit to change domain and IP address. # /var/unbound/etc/unbound.conf server: interface: 127.0.0.1 interface: ::1 qname-minimisation: yes access-control: 0.0.0.0/0 refuse access-control: 127.0.0.0/8 allow access-control: ::0/0 refuse access-control: ::1 allow auto-trust-anchor-file: "/var/unbound/db/root.key" val-log-level: 2 aggressive-nsec: yes local-zone: "local." static local-zone: "10.in-addr.arpa." transparent domain-insecure: "10.in-addr.arpa." remote-control: control-enable: yes control-interface: /var/run/unbound.sock forward-zone: name: "example.com." forward-addr: 10.123.123.10 forward-first: no forward-zone: name: "10.in-addr.arpa." forward-addr: 10.123.123.10 forward-first: no forward-zone: name: "." forward-addr: 8.8.4.4 forward-addr: 8.8.8.8 forward-addr: 208.67.222.222 forward-addr: 208.67.220.220 forward-addr: 1.0.0.1 forward-addr: 1.1.1.1 forward-first: yes > Support for login.conf.d was added mostly to support ports that need to make > modifications to drop files in there. > > I dont expect that the base system will ship with files in login.conf.d > anytime soon. Yeah, make sense. > /B. > > > > > > > > > Did you change any? > > > > > > We already ship with this > > > > > > unbound:\ > > > :openfiles=512:\ > > > :tc=daemon: > > > > Ah, in the main file. I didn't expect this. I started this email thread > > as I expected daemon which has rc.d script to have separated login class > > capability file: > > > > # ls -1A /etc/login.conf.d/ | wc -l > >0 > > > > # tar -zvvtf /var/sysmerge/etc.tgz | grep -c login.conf.d > > 0 > > > > > It is expected that you change login.conf yourself when you tune unbound > > > to > > > your needs. > > > > Yes, I'm aware. That's what I did, but were surprised that there is no > > pre-existing /etc/login.conf.d/unbound > > > > However now looking again at this, maybe missing /etc/login.conf.d/ > > from base is by design. > > -- Regards, Mikolaj
Re: protocol attach wait
On Thu, Sep 01, 2022 at 11:04:19PM +0300, Vitaliy Makkoveev wrote: > On Thu, Sep 01, 2022 at 10:58:49PM +0300, Vitaliy Makkoveev wrote: > > On Thu, Sep 01, 2022 at 09:00:50PM +0200, Alexander Bluhm wrote: > > > On Mon, Aug 15, 2022 at 05:12:22PM +0200, Alexander Bluhm wrote: > > > > System calls should not fail due to temporary memory shortage in > > > > malloc(9) or pool_get(9). > > > > > > > > Pass down a wait flag to pru_attach(). During syscall socket(2) > > > > it is ok to wait, this logic was missing for internet pcb. Pfkey > > > > and route sockets were already waiting. > > > > > > > > sonewconn() cannot wait when called during TCP 3-way handshake. > > > > This logic has been preserved. Unix domain stream socket connect(2) > > > > can wait until the other side has created the socket to accept. > > > > > > rebased to -current. > > > > > > Anyone? > > > > > > > At least these ones should have the "pcb == NULL" check as the inet and > > unix cases. Or the `wait' could be ignored for all cases except tcp. Although it should not happen, I made is consistent. Now all allocators respect the wait and return ENOBUFS in case of failure. > But I don't understand what this diff fixes? Userland will check the > socket(2) return value in any cases, because it's absolutely normal to > fail here. And nothing stops userland to call socket(2) as times as > required. Userland does not retry socket after memory error. Kernel has to handle this kind of failure. Otherwise you have to modify all userland to this: #ifdef __OpenBSD__ while (1) { fd = socket(...); if (fd != -1 || error != ENOBUFS) break; sleep(1); } #else fd = socket(...); #edif ok? bluhm Index: kern/uipc_socket.c === RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.286 diff -u -p -r1.286 uipc_socket.c --- kern/uipc_socket.c 28 Aug 2022 18:43:12 - 1.286 +++ kern/uipc_socket.c 1 Sep 2022 18:47:36 - @@ -138,11 +138,12 @@ soinit(void) } struct socket * -soalloc(int prflags) +soalloc(int wait) { struct socket *so; - so = pool_get(_pool, prflags); + so = pool_get(_pool, (wait == M_WAIT ? PR_WAITOK : PR_NOWAIT) | + PR_ZERO); if (so == NULL) return (NULL); rw_init_flags(>so_lock, "solock", RWL_DUPOK); @@ -174,7 +175,7 @@ socreate(int dom, struct socket **aso, i return (EPROTONOSUPPORT); if (prp->pr_type != type) return (EPROTOTYPE); - so = soalloc(PR_WAITOK | PR_ZERO); + so = soalloc(M_WAIT); klist_init(>so_rcv.sb_sel.si_note, _klistops, so); klist_init(>so_snd.sb_sel.si_note, _klistops, so); sigio_init(>so_sigio); @@ -193,7 +194,7 @@ socreate(int dom, struct socket **aso, i so->so_rcv.sb_timeo_nsecs = INFSLP; solock(so); - error = pru_attach(so, proto); + error = pru_attach(so, proto, M_WAIT); if (error) { so->so_state |= SS_NOFDREF; /* sofree() calls sounlock(). */ Index: kern/uipc_socket2.c === RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket2.c,v retrieving revision 1.127 diff -u -p -r1.127 uipc_socket2.c --- kern/uipc_socket2.c 13 Aug 2022 21:01:46 - 1.127 +++ kern/uipc_socket2.c 1 Sep 2022 18:47:36 - @@ -168,7 +168,7 @@ soisdisconnected(struct socket *so) * Connstatus may be 0 or SS_ISCONNECTED. */ struct socket * -sonewconn(struct socket *head, int connstatus) +sonewconn(struct socket *head, int connstatus, int wait) { struct socket *so; int persocket = solock_persocket(head); @@ -185,7 +185,7 @@ sonewconn(struct socket *head, int conns return (NULL); if (head->so_qlen + head->so_q0len > head->so_qlimit * 3) return (NULL); - so = soalloc(PR_NOWAIT | PR_ZERO); + so = soalloc(wait); if (so == NULL) return (NULL); so->so_type = head->so_type; @@ -238,7 +238,7 @@ sonewconn(struct socket *head, int conns sounlock(head); } - error = pru_attach(so, 0); + error = pru_attach(so, 0, wait); if (persocket) { sounlock(so); Index: kern/uipc_usrreq.c === RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_usrreq.c,v retrieving revision 1.181 diff -u -p -r1.181 uipc_usrreq.c --- kern/uipc_usrreq.c 31 Aug 2022 21:23:02 - 1.181 +++ kern/uipc_usrreq.c 1 Sep 2022 18:47:36 - @@ -302,7 +302,7 @@ const struct sysctl_bounded_args unpdgct }; int -uipc_attach(struct socket *so, int proto) +uipc_attach(struct socket *so, int proto, int wait) { struct unpcb *unp; int error; @@ -330,7 +330,8 @@ uipc_attach(struct socket *so, int proto
rpki-client refactor rsync process
The rsync process implements a limit by stopping to read commands from its stdin once too many processes are run. This is all nice and fine but it does not allow to send a abort request to the process reliably. This diff refactors the rsync process and introduces a state queue which can have more items than the max proc limit. With this aborting requests is possible and already implemented (a request that only has the id set will kill the request with that id). -- :wq Claudio Index: rsync.c === RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v retrieving revision 1.41 diff -u -p -r1.41 rsync.c --- rsync.c 9 Aug 2022 09:02:26 - 1.41 +++ rsync.c 2 Sep 2022 09:02:35 - @@ -41,12 +41,17 @@ * We can have multiple of these simultaneously and need to keep track * of which process maps to which request. */ -struct rsyncproc { - char*uri; /* uri of this rsync proc */ - unsigned int id; /* identity of request */ - pid_tpid; /* pid of process or 0 if unassociated */ +struct rsync { + TAILQ_ENTRY(rsync) entry; + char*uri; /* uri of this rsync proc */ + char*dst; /* destination directory */ + char*compdst; /* compare against directory */ + unsigned int id; /* identity of request */ + pid_tpid; /* pid of process or 0 if unassociated */ }; +static TAILQ_HEAD(, rsync) states = TAILQ_HEAD_INITIALIZER(states); + /* * Return the base of a rsync URI (rsync://hostname/module). The * caRepository provided by the RIR CAs point deeper than they should @@ -124,6 +129,83 @@ rsync_fixup_dest(char *destdir, char *co return fn; } +static pid_t +exec_rsync(const char *prog, const char *bind_addr, char *uri, char *dst, +char *compdst) +{ + pid_t pid; + char *args[32]; + char *reldst; + int i; + + + if ((pid = fork()) == -1) + err(1, "fork"); + + if (pid == 0) { + if (pledge("stdio exec", NULL) == -1) + err(1, "pledge"); + i = 0; + args[i++] = (char *)prog; + args[i++] = "-rt"; + args[i++] = "--no-motd"; + args[i++] = "--max-size=" STRINGIFY(MAX_FILE_SIZE); + args[i++] = "--contimeout=" STRINGIFY(MAX_CONN_TIMEOUT); + args[i++] = "--timeout=" STRINGIFY(MAX_IO_TIMEOUT); + args[i++] = "--include=*/"; + args[i++] = "--include=*.cer"; + args[i++] = "--include=*.crl"; + args[i++] = "--include=*.gbr"; + args[i++] = "--include=*.mft"; + args[i++] = "--include=*.roa"; + args[i++] = "--include=*.asa"; + args[i++] = "--exclude=*"; + if (bind_addr != NULL) { + args[i++] = "--address"; + args[i++] = (char *)bind_addr; + } + if (compdst != NULL && + (reldst = rsync_fixup_dest(dst, compdst)) != NULL) { + args[i++] = "--compare-dest"; + args[i++] = reldst; + } + args[i++] = uri; + args[i++] = dst; + args[i] = NULL; + /* XXX args overflow not prevented */ + execvp(args[0], args); + err(1, "%s: execvp", prog); + } + + return pid; +} + +static void +rsync_new(unsigned int id, char *uri, char *dst, char *compdst) +{ + struct rsync *s; + + if ((s = calloc(1, sizeof(*s))) == NULL) + err(1, NULL); + + s->id = id; + s->uri = uri; + s->dst = dst; + s->compdst = compdst; + + TAILQ_INSERT_TAIL(, s, entry); +} + +static void +rsync_free(struct rsync *s) +{ + TAILQ_REMOVE(, s, entry); + free(s->uri); + free(s->dst); + free(s->compdst); + free(s); +} + static void proc_child(int signal) { @@ -141,13 +223,12 @@ proc_child(int signal) void proc_rsync(char *prog, char *bind_addr, int fd) { - size_t i, nprocs = 0; - int rc = 0; + int nprocs = 0, npending = 0, rc = 0; struct pollfdpfd; struct msgbufmsgq; struct ibuf *b, *inbuf = NULL; sigset_t mask, oldmask; - struct rsyncproc ids[MAX_RSYNC_REQUESTS] = { 0 }; + struct rsync*s, *ns; if (pledge("stdio rpath proc exec unveil", NULL) == -1) err(1, "pledge"); @@ -211,11 +292,23 @@ proc_rsync(char *prog, char *bind_addr, int st; pfd.events = 0; - if (nprocs < MAX_RSYNC_REQUESTS) - pfd.events |= POLLIN; +
Re: unbound and cannot increase max open fds from 512 to 4152
Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 08:07:01 +: > On Fri, Sep 02, 2022 at 09:53:54AM +0200, Sebastian Benoit wrote: > > Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 06:47:00 +: > > > Hi, > > > > > > I have a question, could or should unbound in base be delivered with: > > > > > > # cat /etc/login.conf.d/unbound > > > unbound:\ > > > :openfiles-cur=4096:\ > > > :openfiles-max=8192:\ > > > :tc=daemon: > > > > > > or the like? Above is taken from dovecot. I was able to trigger via > > > rcctl reload unbound following warnings in logs: > > > > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 1.16.0. > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation not > > > permitted > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max open > > > fds from 512 to 4152 > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: continuing with less udp > > > ports: 460 > > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or > > > decrease threads, ports in config to remove this warning > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator > > > > > > After placing above login.conf.d login class capability file above > > > warnings go away: > > > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 1.16.0. > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator > > > > As far as i understand, the number of fds that unbound is asking for is > > based on the num-threads, outgoing-num-tcp, interface and some other setting > > in the unbound config. > > Those particular settings mentioned I did not modified. However the > values are not that important. I expected /etc/login.conf.d/unbound to > be present on a fresh install. More on that below. It would still be interesting why it wants to increase the limit to 4152. My list of settings is probably not complete. Show your config if you can. Support for login.conf.d was added mostly to support ports that need to make modifications to drop files in there. I dont expect that the base system will ship with files in login.conf.d anytime soon. /B. > > > > Did you change any? > > > > We already ship with this > > > > unbound:\ > > :openfiles=512:\ > > :tc=daemon: > > Ah, in the main file. I didn't expect this. I started this email thread > as I expected daemon which has rc.d script to have separated login class > capability file: > > # ls -1A /etc/login.conf.d/ | wc -l >0 > > # tar -zvvtf /var/sysmerge/etc.tgz | grep -c login.conf.d > 0 > > > It is expected that you change login.conf yourself when you tune unbound to > > your needs. > > Yes, I'm aware. That's what I did, but were surprised that there is no > pre-existing /etc/login.conf.d/unbound > > However now looking again at this, maybe missing /etc/login.conf.d/ > from base is by design. > > -- > Regards, > Mikolaj >
httpd: fix default request body size
Hello, For HTTP request body, if neither "Content-Encoding: chunked" nor "Content-Length" is specified, it should mean body length is 0. In RFC 9112 Section 6.3, 7.: | 7. If this is a request message and none of the above are true, then | the message body length is zero (no message body is present). The behavior can be tested by requesting POST to a cgi, like this: $ curl -X POST http://127.0.0.1/cgi-bin/test (Ctrl-C is needed without the diff) ok? # first round https://marc.info/?l=openbsd-tech=158173705129829=2 Index: usr.sbin/httpd/server_http.c === RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v retrieving revision 1.151 diff -u -p -r1.151 server_http.c --- usr.sbin/httpd/server_http.c15 Aug 2022 09:36:19 - 1.151 +++ usr.sbin/httpd/server_http.c1 Sep 2022 20:36:10 - @@ -474,12 +474,9 @@ server_read_http(struct bufferevent *bev /* HTTP request payload */ if (clt->clt_toread > 0) bev->readcb = server_read_httpcontent; - - /* Single-pass HTTP body */ - if (clt->clt_toread < 0) { - clt->clt_toread = TOREAD_UNLIMITED; - bev->readcb = server_read; - } + if (clt->clt_toread < 0 && !desc->http_chunked) + /* 7. of RFC 9112 Section 6.3 */ + clt->clt_toread = 0; break; default: server_abort_http(clt, 405, "method not allowed");
Re: unbound and cannot increase max open fds from 512 to 4152
On Fri, Sep 02, 2022 at 09:53:54AM +0200, Sebastian Benoit wrote: > Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 06:47:00 +: > > Hi, > > > > I have a question, could or should unbound in base be delivered with: > > > > # cat /etc/login.conf.d/unbound > > unbound:\ > > :openfiles-cur=4096:\ > > :openfiles-max=8192:\ > > :tc=daemon: > > > > or the like? Above is taken from dovecot. I was able to trigger via > > rcctl reload unbound following warnings in logs: > > > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 1.16.0. > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation not > > permitted > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max open > > fds from 512 to 4152 > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: continuing with less udp > > ports: 460 > > Sep 2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or decrease > > threads, ports in config to remove this warning > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator > > > > After placing above login.conf.d login class capability file above > > warnings go away: > > > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 1.16.0. > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator > > As far as i understand, the number of fds that unbound is asking for is > based on the num-threads, outgoing-num-tcp, interface and some other setting > in the unbound config. Those particular settings mentioned I did not modified. However the values are not that important. I expected /etc/login.conf.d/unbound to be present on a fresh install. More on that below. > Did you change any? > > We already ship with this > > unbound:\ > :openfiles=512:\ > :tc=daemon: Ah, in the main file. I didn't expect this. I started this email thread as I expected daemon which has rc.d script to have separated login class capability file: # ls -1A /etc/login.conf.d/ | wc -l 0 # tar -zvvtf /var/sysmerge/etc.tgz | grep -c login.conf.d 0 > It is expected that you change login.conf yourself when you tune unbound to > your needs. Yes, I'm aware. That's what I did, but were surprised that there is no pre-existing /etc/login.conf.d/unbound However now looking again at this, maybe missing /etc/login.conf.d/ from base is by design. -- Regards, Mikolaj
Re: unbound and cannot increase max open fds from 512 to 4152
Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 06:47:00 +: > Hi, > > I have a question, could or should unbound in base be delivered with: > > # cat /etc/login.conf.d/unbound > unbound:\ > :openfiles-cur=4096:\ > :openfiles-max=8192:\ > :tc=daemon: > > or the like? Above is taken from dovecot. I was able to trigger via > rcctl reload unbound following warnings in logs: > > Sep 2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 1.16.0. > Sep 2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation not > permitted > Sep 2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max open fds > from 512 to 4152 > Sep 2 06:37:21 x1c unbound: [32940:0] warning: continuing with less udp > ports: 460 > Sep 2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or decrease > threads, ports in config to remove this warning > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator > Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator > > After placing above login.conf.d login class capability file above > warnings go away: > > Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 1.16.0. > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator > Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator As far as i understand, the number of fds that unbound is asking for is based on the num-threads, outgoing-num-tcp, interface and some other setting in the unbound config. Did you change any? We already ship with this unbound:\ :openfiles=512:\ :tc=daemon: It is expected that you change login.conf yourself when you tune unbound to your needs.
Re: sparc64: ofwboot: support booting from softraid 1C
On Thu, Sep 01, 2022 at 08:31:04PM +, Klemens Nanni wrote: > This is practically the same diff we already landed for arm64. > > The kernel already supports booting off 1C and with tech@'s > "installboot: sparc64: fix -r on multi-chunk softraid chunk" diff the > install process will no longer fail when installing bootstraps onto any > softraid volume which requires at least two chunks by design. > > Feedback? OK? ok by me, diff looks fine. > Index: boot.c > === > RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/boot.c,v > retrieving revision 1.39 > diff -u -p -r1.39 boot.c > --- boot.c4 Aug 2022 09:16:53 - 1.39 > +++ boot.c1 Sep 2022 20:10:55 - > @@ -366,7 +366,8 @@ srbootdev(const char *bootline) > return ENODEV; > } > > - if (bv->sbv_level == 'C' && bv->sbv_keys == NULL) > + if ((bv->sbv_level == 'C' || bv->sbv_level == 0x1C) && > + bv->sbv_keys == NULL) > if (sr_crypto_unlock_volume(bv) != 0) > return EPERM; > > Index: softraid_sparc64.c > === > RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/softraid_sparc64.c,v > retrieving revision 1.5 > diff -u -p -r1.5 softraid_sparc64.c > --- softraid_sparc64.c9 Dec 2020 18:10:19 - 1.5 > +++ softraid_sparc64.c1 Sep 2022 20:08:03 - > @@ -290,6 +290,7 @@ srprobe(void) > break; > > case 1: > + case 0x1C: > if (bv->sbv_chunk_no == bv->sbv_chunks_found) > bv->sbv_state = BIOC_SVONLINE; > else if (bv->sbv_chunks_found > 0) > @@ -312,7 +313,8 @@ sr_vol_boot_chunk(struct sr_boot_volume > { > struct sr_boot_chunk *bc = NULL; > > - if (bv->sbv_level == 1 || bv->sbv_level == 'C' ) { /* RAID1 or CRYPTO */ > + if (bv->sbv_level == 1 || bv->sbv_level == 'C' || > + bv->sbv_level == 0x1C) { > /* Select first online chunk. */ > SLIST_FOREACH(bc, >sbv_chunks, sbc_link) > if (bc->sbc_state == BIOC_SDONLINE) > @@ -368,7 +370,7 @@ sr_strategy(struct sr_boot_volume *bv, i > err = strategy(, rw, blk, size, buf, rsize); > return err; > > - } else if (bv->sbv_level == 'C') { > + } else if (bv->sbv_level == 'C' || bv->sbv_level == 0x1C) { > /* XXX - select correct key. */ > aes_xts_setkey(, (u_char *)bv->sbv_keys, 64); > > Index: vers.c > === > RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/vers.c,v > retrieving revision 1.24 > diff -u -p -r1.24 vers.c > --- vers.c4 Aug 2022 09:16:53 - 1.24 > +++ vers.c20 Aug 2022 03:59:47 - > @@ -1 +1 @@ > -const char version[] = "1.23"; > +const char version[] = "1.24"; >
Re: sparc64: ofwboot: support booting from softraid 1C
> Date: Thu, 1 Sep 2022 20:31:04 + > From: Klemens Nanni > > This is practically the same diff we already landed for arm64. > > The kernel already supports booting off 1C and with tech@'s > "installboot: sparc64: fix -r on multi-chunk softraid chunk" diff the > install process will no longer fail when installing bootstraps onto any > softraid volume which requires at least two chunks by design. > > Feedback? OK? Looks reasonable in the sense that I convinced myself this can't break booting non-softraid setups. > Index: boot.c > === > RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/boot.c,v > retrieving revision 1.39 > diff -u -p -r1.39 boot.c > --- boot.c4 Aug 2022 09:16:53 - 1.39 > +++ boot.c1 Sep 2022 20:10:55 - > @@ -366,7 +366,8 @@ srbootdev(const char *bootline) > return ENODEV; > } > > - if (bv->sbv_level == 'C' && bv->sbv_keys == NULL) > + if ((bv->sbv_level == 'C' || bv->sbv_level == 0x1C) && > + bv->sbv_keys == NULL) > if (sr_crypto_unlock_volume(bv) != 0) > return EPERM; > > Index: softraid_sparc64.c > === > RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/softraid_sparc64.c,v > retrieving revision 1.5 > diff -u -p -r1.5 softraid_sparc64.c > --- softraid_sparc64.c9 Dec 2020 18:10:19 - 1.5 > +++ softraid_sparc64.c1 Sep 2022 20:08:03 - > @@ -290,6 +290,7 @@ srprobe(void) > break; > > case 1: > + case 0x1C: > if (bv->sbv_chunk_no == bv->sbv_chunks_found) > bv->sbv_state = BIOC_SVONLINE; > else if (bv->sbv_chunks_found > 0) > @@ -312,7 +313,8 @@ sr_vol_boot_chunk(struct sr_boot_volume > { > struct sr_boot_chunk *bc = NULL; > > - if (bv->sbv_level == 1 || bv->sbv_level == 'C' ) { /* RAID1 or CRYPTO */ > + if (bv->sbv_level == 1 || bv->sbv_level == 'C' || > + bv->sbv_level == 0x1C) { > /* Select first online chunk. */ > SLIST_FOREACH(bc, >sbv_chunks, sbc_link) > if (bc->sbc_state == BIOC_SDONLINE) > @@ -368,7 +370,7 @@ sr_strategy(struct sr_boot_volume *bv, i > err = strategy(, rw, blk, size, buf, rsize); > return err; > > - } else if (bv->sbv_level == 'C') { > + } else if (bv->sbv_level == 'C' || bv->sbv_level == 0x1C) { > /* XXX - select correct key. */ > aes_xts_setkey(, (u_char *)bv->sbv_keys, 64); > > Index: vers.c > === > RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/vers.c,v > retrieving revision 1.24 > diff -u -p -r1.24 vers.c > --- vers.c4 Aug 2022 09:16:53 - 1.24 > +++ vers.c20 Aug 2022 03:59:47 - > @@ -1 +1 @@ > -const char version[] = "1.23"; > +const char version[] = "1.24"; > >
Re: httpd: overwrite rather than error for duplicate type entries
thanks, commited! Florian Obser(flor...@openbsd.org) on 2022.09.02 08:08:09 +0200: > This diff is correct and the use-case makes sense to me. > OK florian > > > On 2022-09-01 21:30 +01, Ben Fuller wrote: > > On Thu, Sep 01, 2022 at 21:22:13 +0100, Ben Fuller wrote: > >> On Thu, Sep 01, 2022 at 21:44:34 +0200, Florian Obser wrote: > >> > Pretty sure this doesn't compile. > >> > If it were to compile it would leak memory. > >> > >> It did compile, but you're right. This version should free everything: > > > > Makes more sense to use the existing function (sorry for the spam): > > > > --- > > usr.sbin/httpd/httpd.c | 4 ++-- > > usr.sbin/httpd/httpd.conf.5 | 1 + > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git usr.sbin/httpd/httpd.c usr.sbin/httpd/httpd.c > > index 2acecd1732f..efe199c20a9 100644 > > --- usr.sbin/httpd/httpd.c > > +++ usr.sbin/httpd/httpd.c > > @@ -1080,9 +1080,9 @@ media_add(struct mediatypes *types, struct media_type > > *media) > > struct media_type *entry; > > > > if ((entry = RB_FIND(mediatypes, types, media)) != NULL) { > > - log_debug("%s: duplicated entry for \"%s\"", __func__, > > + log_debug("%s: entry overwritten for \"%s\"", __func__, > > media->media_name); > > - return (NULL); > > + media_delete(types, entry); > > } > > > > if ((entry = malloc(sizeof(*media))) == NULL) > > diff --git usr.sbin/httpd/httpd.conf.5 usr.sbin/httpd/httpd.conf.5 > > index b5f0be465a0..02f240091b0 100644 > > --- usr.sbin/httpd/httpd.conf.5 > > +++ usr.sbin/httpd/httpd.conf.5 > > @@ -753,6 +753,7 @@ to the specified extension > > .Ar name . > > One or more names can be specified per line. > > Each line may end with an optional semicolon. > > +Later lines overwrite earlier lines. > > .It Ic include Ar file > > Include types definitions from an external file, for example > > .Pa /usr/share/misc/mime.types . > > > > -- > I'm not entirely sure you are real. >
unbound and cannot increase max open fds from 512 to 4152
Hi, I have a question, could or should unbound in base be delivered with: # cat /etc/login.conf.d/unbound unbound:\ :openfiles-cur=4096:\ :openfiles-max=8192:\ :tc=daemon: or the like? Above is taken from dovecot. I was able to trigger via rcctl reload unbound following warnings in logs: Sep 2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 1.16.0. Sep 2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation not permitted Sep 2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max open fds from 512 to 4152 Sep 2 06:37:21 x1c unbound: [32940:0] warning: continuing with less udp ports: 460 Sep 2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or decrease threads, ports in config to remove this warning Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator Sep 2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator After placing above login.conf.d login class capability file above warnings go away: Sep 2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 1.16.0. Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator Sep 2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator -- Regards, Mikolaj
Re: httpd: overwrite rather than error for duplicate type entries
This diff is correct and the use-case makes sense to me. OK florian On 2022-09-01 21:30 +01, Ben Fuller wrote: > On Thu, Sep 01, 2022 at 21:22:13 +0100, Ben Fuller wrote: >> On Thu, Sep 01, 2022 at 21:44:34 +0200, Florian Obser wrote: >> > Pretty sure this doesn't compile. >> > If it were to compile it would leak memory. >> >> It did compile, but you're right. This version should free everything: > > Makes more sense to use the existing function (sorry for the spam): > > --- > usr.sbin/httpd/httpd.c | 4 ++-- > usr.sbin/httpd/httpd.conf.5 | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git usr.sbin/httpd/httpd.c usr.sbin/httpd/httpd.c > index 2acecd1732f..efe199c20a9 100644 > --- usr.sbin/httpd/httpd.c > +++ usr.sbin/httpd/httpd.c > @@ -1080,9 +1080,9 @@ media_add(struct mediatypes *types, struct media_type > *media) > struct media_type *entry; > > if ((entry = RB_FIND(mediatypes, types, media)) != NULL) { > - log_debug("%s: duplicated entry for \"%s\"", __func__, > + log_debug("%s: entry overwritten for \"%s\"", __func__, > media->media_name); > - return (NULL); > + media_delete(types, entry); > } > > if ((entry = malloc(sizeof(*media))) == NULL) > diff --git usr.sbin/httpd/httpd.conf.5 usr.sbin/httpd/httpd.conf.5 > index b5f0be465a0..02f240091b0 100644 > --- usr.sbin/httpd/httpd.conf.5 > +++ usr.sbin/httpd/httpd.conf.5 > @@ -753,6 +753,7 @@ to the specified extension > .Ar name . > One or more names can be specified per line. > Each line may end with an optional semicolon. > +Later lines overwrite earlier lines. > .It Ic include Ar file > Include types definitions from an external file, for example > .Pa /usr/share/misc/mime.types . > -- I'm not entirely sure you are real.