Re: Move PRU_SOCKADDR and PRU_PEERADDR requests to corresponding pru_wrappers

2022-09-02 Thread Alexander Bluhm
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

2022-09-02 Thread Vitaliy Makkoveev
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

2022-09-02 Thread Moritz Buhl
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()

2022-09-02 Thread Vitaliy Makkoveev
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

2022-09-02 Thread Vitaliy Makkoveev
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

2022-09-02 Thread Scott Cheloha
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

2022-09-02 Thread Claudio Jeker
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

2022-09-02 Thread Job Snijders
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

2022-09-02 Thread Scott Cheloha
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

2022-09-02 Thread Alexander Bluhm
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()

2022-09-02 Thread Alexander Bluhm
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread Theo Buehler
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread Claudio Jeker
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

2022-09-02 Thread Theo Buehler
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

2022-09-02 Thread Claudio Jeker
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

2022-09-02 Thread Theo Buehler
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

2022-09-02 Thread Claudio Jeker
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

2022-09-02 Thread Alexander Bluhm
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread Claudio Jeker
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

2022-09-02 Thread Mike Larkin
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

2022-09-02 Thread Mike Larkin
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

2022-09-02 Thread Dave Voutila


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

2022-09-02 Thread YASUOKA Masahiko
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

2022-09-02 Thread Scott Cheloha
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

2022-09-02 Thread Alexander Bluhm
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()

2022-09-02 Thread Vitaliy Makkoveev
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

2022-09-02 Thread Vitaliy Makkoveev
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

2022-09-02 Thread Miod Vallat
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?

2022-09-02 Thread Klemens Nanni
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread YASUOKA Masahiko
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

2022-09-02 Thread Alexander Bluhm
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)()

2022-09-02 Thread Alexander Bluhm
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

2022-09-02 Thread Matthieu Herrb
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

2022-09-02 Thread Alexander Bluhm
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

2022-09-02 Thread Stuart Henderson
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

2022-09-02 Thread Matthieu Herrb
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

2022-09-02 Thread Mikolaj Kucharski
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

2022-09-02 Thread Alexander Bluhm
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

2022-09-02 Thread Claudio Jeker
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread YASUOKA Masahiko
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

2022-09-02 Thread Mikolaj Kucharski
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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread Stefan Sperling
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

2022-09-02 Thread Mark Kettenis
> 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

2022-09-02 Thread Sebastian Benoit
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

2022-09-02 Thread Mikolaj Kucharski
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

2022-09-02 Thread Florian Obser
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.