use splraise in pccbb

2018-07-09 Thread David Gwynne
i don't have the hardware to test, but im pretty confident this is
ok.

ok?

Index: pccbb.c
===
RCS file: /cvs/src/sys/dev/pci/pccbb.c,v
retrieving revision 1.97
diff -u -p -r1.97 pccbb.c
--- pccbb.c 8 Sep 2017 05:36:52 -   1.97
+++ pccbb.c 10 Jul 2018 04:09:38 -
@@ -959,50 +959,16 @@ pccbbintr_function(struct pccbb_softc *s
 {
int retval = 0, val;
struct pccbb_intrhand_list *pil;
-   int s, splchanged;
+   int s;
 
for (pil = sc->sc_pil; pil != NULL; pil = pil->pil_next) {
-   /*
-* XXX priority change.  gross.  I use if-else
-* sentences instead of switch-case sentences in order
-* to avoid duplicate case value error.  More than one
-* IPL_XXX may use the same value.  It depends on the
-* implementation.
-*/
-   splchanged = 1;
-#if 0
-   if (pil->pil_level == IPL_SERIAL) {
-   s = splserial();
-   } else if (pil->pil_level == IPL_HIGH) {
-#endif
-   if (pil->pil_level == IPL_HIGH) {
-   s = splhigh();
-   } else if (pil->pil_level == IPL_CLOCK) {
-   s = splclock();
-   } else if (pil->pil_level == IPL_AUDIO) {
-   s = splaudio();
-   } else if (pil->pil_level == IPL_VM) {
-   s = splvm();
-   } else if (pil->pil_level == IPL_TTY) {
-   s = spltty();
-#if 0
-   } else if (pil->pil_level == IPL_SOFTSERIAL) {
-   s = splsoftserial();
-#endif
-   } else if (pil->pil_level == IPL_NET) {
-   s = splnet();
-   } else {
-   splchanged = 0;
-   /* XXX: ih lower than IPL_BIO runs w/ IPL_BIO. */
-   }
+   s = splraise(pil->pil_level);
 
val = (*pil->pil_func)(pil->pil_arg);
if (val != 0)
pil->pil_count.ec_count++;
 
-   if (splchanged != 0) {
-   splx(s);
-   }
+   splx(s);
 
if (retval == 0 || val != 0)
retval = val;



Re: bgpd: replacing the rib is needed when the flag changes

2018-07-09 Thread Claudio Jeker
On Tue, Jul 10, 2018 at 12:07:04AM +0200, Sebastian Benoit wrote:
> Sebastian Benoit(be...@openbsd.org) on 2018.07.10 00:06:06 +0200:
> > Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.07.10 00:05:08 +0200:
> > > On Mon, Jul 09, 2018 at 11:58:05PM +0200, Sebastian Benoit wrote:
> > > > 
> > > > compare the right things here: we want to know if the flag has changed.
> > > > Found with claudios help and patience.
> > > > 
> > > > ok?
> > > 
> > > This is fucked up. :)
> > 
> fixed

OK beer@
 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.386
> diff -u -p -u -1 -2 -r1.386 rde.c
> --- rde.c 9 Jul 2018 14:44:02 -   1.386
> +++ rde.c 9 Jul 2018 22:05:31 -
> @@ -758,25 +758,25 @@ rde_dispatch_imsg_parent(struct imsgbuf 
>   }
>   break;
>   case IMSG_RECONF_RIB:
>   if (imsg.hdr.len - IMSG_HEADER_SIZE !=
>   sizeof(struct rde_rib))
>   fatalx("IMSG_RECONF_RIB bad len");
>   memcpy(, imsg.data, sizeof(rn));
>   rib = rib_find(rn.name);
>   if (rib == NULL)
>   rib = rib_new(rn.name, rn.rtableid, rn.flags);
>   else if (rib->rtableid != rn.rtableid ||
>   (rib->flags & F_RIB_HASNOFIB) !=
> - (rib->flags & F_RIB_HASNOFIB)) {
> + (rn.flags & F_RIB_HASNOFIB)) {
>   struct filter_head  *in_rules;
>   struct rib_desc *ribd = rib_desc(rib);
>   /*
>* Big hammer in the F_RIB_HASNOFIB case but
>* not often enough used to optimise it more.
>* Need to save the filters so that they're not
>* lost.
>*/
>   in_rules = ribd->in_rules;
>   ribd->in_rules = NULL;
>   rde_rib_free(ribd);
>   rib = rib_new(rn.name, rn.rtableid, rn.flags);
> 

-- 
:wq Claudio



Re: bgpd: replacing the rib is needed when the flag changes

2018-07-09 Thread Sebastian Benoit
Sebastian Benoit(be...@openbsd.org) on 2018.07.10 00:06:06 +0200:
> Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.07.10 00:05:08 +0200:
> > On Mon, Jul 09, 2018 at 11:58:05PM +0200, Sebastian Benoit wrote:
> > > 
> > > compare the right things here: we want to know if the flag has changed.
> > > Found with claudios help and patience.
> > > 
> > > ok?
> > 
> > This is fucked up. :)
> 
fixed

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.386
diff -u -p -u -1 -2 -r1.386 rde.c
--- rde.c   9 Jul 2018 14:44:02 -   1.386
+++ rde.c   9 Jul 2018 22:05:31 -
@@ -758,25 +758,25 @@ rde_dispatch_imsg_parent(struct imsgbuf 
}
break;
case IMSG_RECONF_RIB:
if (imsg.hdr.len - IMSG_HEADER_SIZE !=
sizeof(struct rde_rib))
fatalx("IMSG_RECONF_RIB bad len");
memcpy(, imsg.data, sizeof(rn));
rib = rib_find(rn.name);
if (rib == NULL)
rib = rib_new(rn.name, rn.rtableid, rn.flags);
else if (rib->rtableid != rn.rtableid ||
(rib->flags & F_RIB_HASNOFIB) !=
-   (rib->flags & F_RIB_HASNOFIB)) {
+   (rn.flags & F_RIB_HASNOFIB)) {
struct filter_head  *in_rules;
struct rib_desc *ribd = rib_desc(rib);
/*
 * Big hammer in the F_RIB_HASNOFIB case but
 * not often enough used to optimise it more.
 * Need to save the filters so that they're not
 * lost.
 */
in_rules = ribd->in_rules;
ribd->in_rules = NULL;
rde_rib_free(ribd);
rib = rib_new(rn.name, rn.rtableid, rn.flags);



Re: bgpd: replacing the rib is needed when the flag changes

2018-07-09 Thread Claudio Jeker
On Mon, Jul 09, 2018 at 11:58:05PM +0200, Sebastian Benoit wrote:
> 
> compare the right things here: we want to know if the flag has changed.
> Found with claudios help and patience.
> 
> ok?

This is fucked up. :)
 
> (benno_claudio_rde_reconf_F_RIB_HASNOFIB.diff)
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.386
> diff -u -p -u -1 -2 -r1.386 rde.c
> --- rde.c 9 Jul 2018 14:44:02 -   1.386
> +++ rde.c 9 Jul 2018 21:56:40 -
> @@ -758,25 +758,25 @@ rde_dispatch_imsg_parent(struct imsgbuf 
>   }
>   break;
>   case IMSG_RECONF_RIB:
>   if (imsg.hdr.len - IMSG_HEADER_SIZE !=
>   sizeof(struct rde_rib))
>   fatalx("IMSG_RECONF_RIB bad len");
>   memcpy(, imsg.data, sizeof(rn));
>   rib = rib_find(rn.name);
>   if (rib == NULL)
>   rib = rib_new(rn.name, rn.rtableid, rn.flags);
>   else if (rib->rtableid != rn.rtableid ||
>   (rib->flags & F_RIB_HASNOFIB) !=
> - (rib->flags & F_RIB_HASNOFIB)) {
> + (rn->flags & F_RIB_HASNOFIB)) {
>   struct filter_head  *in_rules;
>   struct rib_desc *ribd = rib_desc(rib);
>   /*
>* Big hammer in the F_RIB_HASNOFIB case but
>* not often enough used to optimise it more.
>* Need to save the filters so that they're not
>* lost.
>*/
>   in_rules = ribd->in_rules;
>   ribd->in_rules = NULL;
>   rde_rib_free(ribd);
>   rib = rib_new(rn.name, rn.rtableid, rn.flags);
> 

-- 
:wq Claudio



Re: bgpd: free the right thing in rib_free

2018-07-09 Thread Claudio Jeker
On Mon, Jul 09, 2018 at 11:45:41PM +0200, Sebastian Benoit wrote:
> Actually free the right thing in rib_free()
> Found by and with claudio.
> 
> (benno_claudio_rde_rib_rib_free.diff)

OK claudio@
 
> Index: rde_rib.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.167
> diff -u -p -r1.167 rde_rib.c
> --- rde_rib.c 9 Jul 2018 15:35:59 -   1.167
> +++ rde_rib.c 9 Jul 2018 21:43:51 -
> @@ -171,7 +171,7 @@ rib_free(struct rib *rib)
>   rd = [rib->id];
>   filterlist_free(rd->in_rules_tmp);
>   filterlist_free(rd->in_rules);
> - bzero(rib, sizeof(struct rib_desc));
> + bzero(rd, sizeof(struct rib_desc));
>  }
>  
>  int
> 

-- 
:wq Claudio



bgpd: replacing the rib is needed when the flag changes

2018-07-09 Thread Sebastian Benoit


compare the right things here: we want to know if the flag has changed.
Found with claudios help and patience.

ok?

(benno_claudio_rde_reconf_F_RIB_HASNOFIB.diff)

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.386
diff -u -p -u -1 -2 -r1.386 rde.c
--- rde.c   9 Jul 2018 14:44:02 -   1.386
+++ rde.c   9 Jul 2018 21:56:40 -
@@ -758,25 +758,25 @@ rde_dispatch_imsg_parent(struct imsgbuf 
}
break;
case IMSG_RECONF_RIB:
if (imsg.hdr.len - IMSG_HEADER_SIZE !=
sizeof(struct rde_rib))
fatalx("IMSG_RECONF_RIB bad len");
memcpy(, imsg.data, sizeof(rn));
rib = rib_find(rn.name);
if (rib == NULL)
rib = rib_new(rn.name, rn.rtableid, rn.flags);
else if (rib->rtableid != rn.rtableid ||
(rib->flags & F_RIB_HASNOFIB) !=
-   (rib->flags & F_RIB_HASNOFIB)) {
+   (rn->flags & F_RIB_HASNOFIB)) {
struct filter_head  *in_rules;
struct rib_desc *ribd = rib_desc(rib);
/*
 * Big hammer in the F_RIB_HASNOFIB case but
 * not often enough used to optimise it more.
 * Need to save the filters so that they're not
 * lost.
 */
in_rules = ribd->in_rules;
ribd->in_rules = NULL;
rde_rib_free(ribd);
rib = rib_new(rn.name, rn.rtableid, rn.flags);



Re: kill raw_cb.h and raw_usrreq.c

2018-07-09 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.07.09 19:20:35 +0200:
> raw_usrreq() is no longer used so remove it from the tree.
> The only thing form the headerfile that was still used was RAWSNDQ and
> RAWRCVQ. I replaced them with per protocol defines.
> 
> OK?
> -- 
> :wq Claudio
> 
> Index: conf/files
> ===
> RCS file: /cvs/src/sys/conf/files,v
> retrieving revision 1.662
> diff -u -p -r1.662 files
> --- conf/files2 Jul 2018 12:46:20 -   1.662
> +++ conf/files9 Jul 2018 17:18:11 -
> @@ -798,7 +798,6 @@ file net/switchctl.c  switch
>  file net/switchofp.c switch
>  file net/pipex.c pipex
>  file net/radix.c pf | ipsec | pipex | nfsserver
> -file net/raw_usrreq.c
>  file net/rtable.c
>  file net/route.c
>  file net/rtsock.c
> Index: net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.187
> diff -u -p -r1.187 pfkeyv2.c
> --- net/pfkeyv2.c 9 Jul 2018 16:51:29 -   1.187
> +++ net/pfkeyv2.c 9 Jul 2018 17:18:12 -
> @@ -86,7 +86,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -97,6 +96,8 @@
>  #include 
>  #endif
>  
> +#define  PFKEYSNDQ   8192
> +#define  PFKEYRCVQ   8192
>  
>  static const struct sadb_alg ealgs[] = {
>   { SADB_EALG_NULL, 0, 0, 0 },
> @@ -269,7 +270,7 @@ pfkeyv2_attach(struct socket *so, int pr
>   so->so_pcb = kp;
>   refcnt_init(>kcb_refcnt);
>  
> - error = soreserve(so, RAWSNDQ, RAWRCVQ);
> + error = soreserve(so, PFKEYSNDQ, PFKEYRCVQ);
>   if (error) {
>   free(kp, M_PCB, sizeof(struct pkpcb));
>   return (error);
> Index: net/raw_cb.h
> ===
> RCS file: net/raw_cb.h
> diff -N net/raw_cb.h
> --- net/raw_cb.h  4 Nov 2017 16:48:09 -   1.16
> +++ /dev/null 1 Jan 1970 00:00:00 -
> @@ -1,62 +0,0 @@
> -/*   $OpenBSD: raw_cb.h,v 1.16 2017/11/04 16:48:09 mpi Exp $ */
> -/*   $NetBSD: raw_cb.h,v 1.9 1996/02/13 22:00:41 christos Exp $  */
> -
> -/*
> - * Copyright (c) 1980, 1986, 1993
> - *   The Regents of the University of California.  All rights reserved.
> - *
> - * Redistribution and use in source and binary forms, with or without
> - * modification, are permitted provided that the following conditions
> - * are met:
> - * 1. Redistributions of source code must retain the above copyright
> - *notice, this list of conditions and the following disclaimer.
> - * 2. Redistributions in binary form must reproduce the above copyright
> - *notice, this list of conditions and the following disclaimer in the
> - *documentation and/or other materials provided with the distribution.
> - * 3. Neither the name of the University nor the names of its contributors
> - *may be used to endorse or promote products derived from this software
> - *without specific prior written permission.
> - *
> - * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
> - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> - * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
> - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> - * SUCH DAMAGE.
> - *
> - *   @(#)raw_cb.h8.1 (Berkeley) 6/10/93
> - */
> -
> -#ifndef _NET_RAW_CB_H_
> -#define _NET_RAW_CB_H_
> -
> -/*
> - * Raw protocol interface control block.  Used
> - * to tie a socket to the generic raw interface.
> - */
> -struct rawcb {
> - struct  socket *rcb_socket; /* back pointer to socket */
> - struct  sockaddr *rcb_faddr;/* destination address */
> - struct  sockaddr *rcb_laddr;/* socket's address */
> - struct  sockproto rcb_proto;/* protocol family, protocol */
> -};
> -
> -/*
> - * Nominal space allocated to a raw socket.
> - */
> -#define  RAWSNDQ 8192
> -#define  RAWRCVQ 8192
> -
> -#ifdef _KERNEL
> -
> -#define  sotorawcb(so)   ((struct rawcb *)(so)->so_pcb)
> -int   raw_usrreq(struct socket *,
> - int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
> -#endif /* _KERNEL */
> -
> -#endif /* _NET_RAW_CB_H_ */
> Index: net/raw_usrreq.c
> ===
> RCS 

bgpd: free the right thing in rib_free

2018-07-09 Thread Sebastian Benoit
Actually free the right thing in rib_free()
Found by and with claudio.

(benno_claudio_rde_rib_rib_free.diff)

Index: rde_rib.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
retrieving revision 1.167
diff -u -p -r1.167 rde_rib.c
--- rde_rib.c   9 Jul 2018 15:35:59 -   1.167
+++ rde_rib.c   9 Jul 2018 21:43:51 -
@@ -171,7 +171,7 @@ rib_free(struct rib *rib)
rd = [rib->id];
filterlist_free(rd->in_rules_tmp);
filterlist_free(rd->in_rules);
-   bzero(rib, sizeof(struct rib_desc));
+   bzero(rd, sizeof(struct rib_desc));
 }
 
 int



Re: signal to process or posix thread

2018-07-09 Thread Philip Guenther
On Sun, Jul 1, 2018 at 9:47 AM Joerg Sonnenberger  wrote:

> On Fri, Jun 29, 2018 at 04:21:17PM +0200, Alexander Bluhm wrote:
> > The problem is that POSIX has signals that are sent to processes
> > and signals sent to individual threads.  Our kernel does not support
> > this properly.
>
> Well, not exactly. POSIX has synchronous and asynchronous signals. I.e.
> SIGFPE after a division by zero or SIGBUS/SIGSEGV are typically
> synchronous traps thrown by the processing of the instructions of the
> current thread. This signals are delivered to the current thread. All
> other signals, i.e. those created as side effect of kill(2) are sent to
> the process at large.


I think Bluhm's wording is more true to POSIX, as there are other methods
for a signal to be delivered to a specific thread.  To quote POSIX XSH
2.4.1p2:
At the time of generation, a determination shall be made whether the
signal
has been generated for the process or for a specific thread within the
process.
Signals which are generated by some action attributable to a particular
thread,
such as a hardware fault, shall be generated for the thread that caused
the
signal to be generated. Signals that are generated in association with
a process
ID or process group ID or an asynchronous event, such as terminal
activity,
shall be generated for the process.

Later, it specifies that pthread_kill() generates the signal for the
specific thread:
The pthread_kill() function shall request that a signal be delivered to
the specified thread.


Those signals are handled by the first thread that
> doesn't have them masked. In that case, it should be put on the pending
> list of the process and any unmasking operation should check the pending
> list on whether a signal should be delivered delayed.
>

Yep.


Philip Guenther


Re: Patch for column number in doas error messages

2018-07-09 Thread Laurence Tratt
On Mon, Jul 09, 2018 at 10:04:35AM +0200, Otto Moerbeek wrote:

Hello Otto,

>> I still don't see the point.  In 30 years, I've gotten by with parsers
>> that say "syntax error", and had very bad experiences with programs that
>> do a poor job anticipating where the parse error is.  Of course there are
>> programs which do a good job of detecting the error, but generally those
>> don't use yacc...
> I'm not trying to guess anything. yacc uses one-token lookahead. If it
> cannot continue, the lexical value of the token that could not be processed
> gives a pretty clear indication of the error spot.

We can state something even a little stronger than that. An LR parser (such
as Yacc) will always stop at the first possible point an error can be
guaranteed to have occurred. Often that point is the same as where the error
was made (e.g. "x = 1 + ;" will report an error at the semi-colon), although
sometimes it isn't (a simple example is "fi (x) { ... }": the error will be
reported at the open curly, even though a human would consider the error to
have occurred at "fi"). In my opinion, when the error location is correct
it's a genuine help (and, in practise, most errors seem to fall into this
category); when it's wrong, it tends to be very obvious, and you're in no
worse a situation than you were if it just says "syntax error".

[My personal opinion is that a lot of the ill feeling towards error recovery
is to do with when it gets it wrong, tries to keep on parsing, and fills the
terminal up with incorrect errors -- the cascading error problem. Anyone with
too much time on their hands can read my attempted solutions to that at
https://arxiv.org/abs/1804.07133]


Laurie
-- 
Personal http://tratt.net/laurie/
Software Development Teamhttp://soft-dev.org/
   https://github.com/ltratt  http://twitter.com/laurencetratt



kill raw_cb.h and raw_usrreq.c

2018-07-09 Thread Claudio Jeker
raw_usrreq() is no longer used so remove it from the tree.
The only thing form the headerfile that was still used was RAWSNDQ and
RAWRCVQ. I replaced them with per protocol defines.

OK?
-- 
:wq Claudio

Index: conf/files
===
RCS file: /cvs/src/sys/conf/files,v
retrieving revision 1.662
diff -u -p -r1.662 files
--- conf/files  2 Jul 2018 12:46:20 -   1.662
+++ conf/files  9 Jul 2018 17:18:11 -
@@ -798,7 +798,6 @@ file net/switchctl.cswitch
 file net/switchofp.c   switch
 file net/pipex.c   pipex
 file net/radix.c   pf | ipsec | pipex | nfsserver
-file net/raw_usrreq.c
 file net/rtable.c
 file net/route.c
 file net/rtsock.c
Index: net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.187
diff -u -p -r1.187 pfkeyv2.c
--- net/pfkeyv2.c   9 Jul 2018 16:51:29 -   1.187
+++ net/pfkeyv2.c   9 Jul 2018 17:18:12 -
@@ -86,7 +86,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -97,6 +96,8 @@
 #include 
 #endif
 
+#definePFKEYSNDQ   8192
+#definePFKEYRCVQ   8192
 
 static const struct sadb_alg ealgs[] = {
{ SADB_EALG_NULL, 0, 0, 0 },
@@ -269,7 +270,7 @@ pfkeyv2_attach(struct socket *so, int pr
so->so_pcb = kp;
refcnt_init(>kcb_refcnt);
 
-   error = soreserve(so, RAWSNDQ, RAWRCVQ);
+   error = soreserve(so, PFKEYSNDQ, PFKEYRCVQ);
if (error) {
free(kp, M_PCB, sizeof(struct pkpcb));
return (error);
Index: net/raw_cb.h
===
RCS file: net/raw_cb.h
diff -N net/raw_cb.h
--- net/raw_cb.h4 Nov 2017 16:48:09 -   1.16
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,62 +0,0 @@
-/* $OpenBSD: raw_cb.h,v 1.16 2017/11/04 16:48:09 mpi Exp $ */
-/* $NetBSD: raw_cb.h,v 1.9 1996/02/13 22:00:41 christos Exp $  */
-
-/*
- * Copyright (c) 1980, 1986, 1993
- * The Regents of the University of California.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *may be used to endorse or promote products derived from this software
- *without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- * @(#)raw_cb.h8.1 (Berkeley) 6/10/93
- */
-
-#ifndef _NET_RAW_CB_H_
-#define _NET_RAW_CB_H_
-
-/*
- * Raw protocol interface control block.  Used
- * to tie a socket to the generic raw interface.
- */
-struct rawcb {
-   struct  socket *rcb_socket; /* back pointer to socket */
-   struct  sockaddr *rcb_faddr;/* destination address */
-   struct  sockaddr *rcb_laddr;/* socket's address */
-   struct  sockproto rcb_proto;/* protocol family, protocol */
-};
-
-/*
- * Nominal space allocated to a raw socket.
- */
-#defineRAWSNDQ 8192
-#defineRAWRCVQ 8192
-
-#ifdef _KERNEL
-
-#definesotorawcb(so)   ((struct rawcb *)(so)->so_pcb)
-int raw_usrreq(struct socket *,
-   int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
-#endif /* _KERNEL */
-
-#endif /* _NET_RAW_CB_H_ */
Index: net/raw_usrreq.c
===
RCS file: net/raw_usrreq.c
diff -N net/raw_usrreq.c
--- net/raw_usrreq.c24 Apr 2018 15:40:55 -  1.35
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,161 +0,0 @@
-/* $OpenBSD: raw_usrreq.c,v 1.35 2018/04/24 15:40:55 pirofti Exp $ */
-/* $NetBSD: raw_usrreq.c,v 1.11 

Re: bgpd, don't allocate initial aspath for update parsing

2018-07-09 Thread Denis Fondras
On Mon, Jul 09, 2018 at 05:47:11PM +0200, Claudio Jeker wrote:
> Similar to the rde_filter code there is no need to path_get() the aspath
> used in rde_update_dispatch(). Also makes the code a bit easier since the
> cleanup can be done all the time.
> 

looks good, compiles fine, runs well, OK denis@

> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.386
> diff -u -p -r1.386 rde.c
> --- rde.c 9 Jul 2018 14:44:02 -   1.386
> +++ rde.c 9 Jul 2018 14:50:21 -
> @@ -938,10 +938,10 @@ rde_dispatch_imsg_parent(struct imsgbuf 
>  int
>  rde_update_dispatch(struct imsg *imsg)
>  {
> + struct rde_aspathasp;
>   struct bgpd_addr prefix;
>   struct mpattrmpa;
>   struct rde_peer *peer;
> - struct rde_aspath   *asp = NULL;
>   u_char  *p, *mpp = NULL;
>   int  error = -1, pos = 0;
>   u_int16_tafi, len, mplen;
> @@ -986,11 +986,11 @@ rde_update_dispatch(struct imsg *imsg)
>   imsg->hdr.len - IMSG_HEADER_SIZE - 4 - withdrawn_len - attrpath_len;
>   bzero(, sizeof(mpa));
>  
> + path_prep();
>   if (attrpath_len != 0) { /* 0 = no NLRI information in this message */
>   /* parse path attributes */
> - asp = path_get();
>   while (len > 0) {
> - if ((pos = rde_attr_parse(p, len, peer, asp,
> + if ((pos = rde_attr_parse(p, len, peer, ,
>   )) < 0)
>   goto done;
>   p += pos;
> @@ -998,19 +998,19 @@ rde_update_dispatch(struct imsg *imsg)
>   }
>  
>   /* check for missing but necessary attributes */
> - if ((subtype = rde_attr_missing(asp, peer->conf.ebgp,
> + if ((subtype = rde_attr_missing(, peer->conf.ebgp,
>   nlri_len))) {
>   rde_update_err(peer, ERR_UPDATE, ERR_UPD_MISSNG_WK_ATTR,
>   , sizeof(u_int8_t));
>   goto done;
>   }
>  
> - rde_as4byte_fixup(peer, asp);
> + rde_as4byte_fixup(peer, );
>  
>   /* enforce remote AS if requested */
> - if (asp->flags & F_ATTR_ASPATH &&
> + if (asp.flags & F_ATTR_ASPATH &&
>   peer->conf.enforce_as == ENFORCE_AS_ON) {
> - fas = aspath_neighbor(asp->aspath);
> + fas = aspath_neighbor(asp.aspath);
>   if (peer->conf.remote_as != fas) {
>   log_peer_warnx(>conf, "bad path, "
>   "starting with %s, "
> @@ -1021,7 +1021,7 @@ rde_update_dispatch(struct imsg *imsg)
>   }
>   }
>  
> - rde_reflector(peer, asp);
> + rde_reflector(peer, );
>   }
>  
>   p = imsg->data;
> @@ -1103,7 +1103,7 @@ rde_update_dispatch(struct imsg *imsg)
>   goto done;
>   }
>  
> - if ((asp->flags & ~F_ATTR_MP_UNREACH) == 0 && mplen == 0) {
> + if ((asp.flags & ~F_ATTR_MP_UNREACH) == 0 && mplen == 0) {
>   /* EoR marker */
>   peer_recv_eor(peer, aid);
>   }
> @@ -1166,7 +1166,7 @@ rde_update_dispatch(struct imsg *imsg)
>   break;
>   }
>  
> - if ((asp->flags & ~F_ATTR_MP_UNREACH) == 0) {
> + if ((asp.flags & ~F_ATTR_MP_UNREACH) == 0) {
>   error = 0;
>   goto done;
>   }
> @@ -1178,8 +1178,8 @@ rde_update_dispatch(struct imsg *imsg)
>   /* aspath needs to be loop free nota bene this is not a hard error */
>   if (peer->conf.ebgp &&
>   peer->conf.enforce_local_as == ENFORCE_AS_ON &&
> - !aspath_loopfree(asp->aspath, peer->conf.local_as))
> - asp->flags |= F_ATTR_LOOP;
> + !aspath_loopfree(asp.aspath, peer->conf.local_as))
> + asp.flags |= F_ATTR_LOOP;
>  
>   /* parse nlri prefix */
>   while (nlri_len > 0) {
> @@ -1208,7 +1208,7 @@ rde_update_dispatch(struct imsg *imsg)
>   goto done;
>   }
>  
> - if (rde_update_update(peer, asp, , prefixlen) == -1)
> + if (rde_update_update(peer, , , prefixlen) == -1)
>   goto done;
>  
>   }
> @@ -1244,11 +1244,11 @@ rde_update_dispatch(struct imsg *imsg)
>* this works because asp is not linked.
>* But first unlock the previously locked nexthop.
>*/
> - if (asp->nexthop) {
> - (void)nexthop_put(asp->nexthop);
> - asp->nexthop = NULL;
> + if (asp.nexthop) {
> + (void)nexthop_put(asp.nexthop);
> +

Re: bgpd: check max prefix just once

2018-07-09 Thread Claudio Jeker
On Mon, Jul 09, 2018 at 05:50:07PM +0200, Denis Fondras wrote:
> I am late for a comment because it has already been commited but...
> 
> > @@ -1373,6 +1339,14 @@ rde_update_update(struct rde_peer *peer,
> > if (path_update([RIB_ADJ_IN].rib, peer, asp, prefix, prefixlen, 0))
> > peer->prefix_cnt++;
> >  
> > +   /* max prefix checker */
> > +   if (peer->conf.max_prefix && peer->prefix_cnt > peer->conf.max_prefix) {
> > +   log_peer_warnx(>conf, "prefix limit reached (>%u/%u)",
> 
> Is it useful to display peer->prefix_cnt here ? I think it will always be
> peer->conf.max_prefix+1
> 
> > +   peer->prefix_cnt, peer->conf.max_prefix);
> > +   rde_update_err(peer, ERR_CEASE, ERR_CEASE_MAX_PREFIX, NULL, 0);
> > +   return (-1);
> > +   }
> > +
> > p = prefix_get([RIB_ADJ_IN].rib, peer, prefix, prefixlen, 0);
> > if (p == NULL)
> > fatalx("rde_update_update: no prefix in Adj-RIB-In");
> 

If you change max-prefix and reload then it is possible to be different.
I agree that the message could be better. Lucky us we only need to adjust
it once now :)

-- 
:wq Claudio



Re: bgpd: check max prefix just once

2018-07-09 Thread Denis Fondras
I am late for a comment because it has already been commited but...

> @@ -1373,6 +1339,14 @@ rde_update_update(struct rde_peer *peer,
>   if (path_update([RIB_ADJ_IN].rib, peer, asp, prefix, prefixlen, 0))
>   peer->prefix_cnt++;
>  
> + /* max prefix checker */
> + if (peer->conf.max_prefix && peer->prefix_cnt > peer->conf.max_prefix) {
> + log_peer_warnx(>conf, "prefix limit reached (>%u/%u)",

Is it useful to display peer->prefix_cnt here ? I think it will always be
peer->conf.max_prefix+1

> + peer->prefix_cnt, peer->conf.max_prefix);
> + rde_update_err(peer, ERR_CEASE, ERR_CEASE_MAX_PREFIX, NULL, 0);
> + return (-1);
> + }
> +
>   p = prefix_get([RIB_ADJ_IN].rib, peer, prefix, prefixlen, 0);
>   if (p == NULL)
>   fatalx("rde_update_update: no prefix in Adj-RIB-In");



bgpd, don't allocate initial aspath for update parsing

2018-07-09 Thread Claudio Jeker
Similar to the rde_filter code there is no need to path_get() the aspath
used in rde_update_dispatch(). Also makes the code a bit easier since the
cleanup can be done all the time.

-- 
:wq Claudio


Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.386
diff -u -p -r1.386 rde.c
--- rde.c   9 Jul 2018 14:44:02 -   1.386
+++ rde.c   9 Jul 2018 14:50:21 -
@@ -938,10 +938,10 @@ rde_dispatch_imsg_parent(struct imsgbuf 
 int
 rde_update_dispatch(struct imsg *imsg)
 {
+   struct rde_aspathasp;
struct bgpd_addr prefix;
struct mpattrmpa;
struct rde_peer *peer;
-   struct rde_aspath   *asp = NULL;
u_char  *p, *mpp = NULL;
int  error = -1, pos = 0;
u_int16_tafi, len, mplen;
@@ -986,11 +986,11 @@ rde_update_dispatch(struct imsg *imsg)
imsg->hdr.len - IMSG_HEADER_SIZE - 4 - withdrawn_len - attrpath_len;
bzero(, sizeof(mpa));
 
+   path_prep();
if (attrpath_len != 0) { /* 0 = no NLRI information in this message */
/* parse path attributes */
-   asp = path_get();
while (len > 0) {
-   if ((pos = rde_attr_parse(p, len, peer, asp,
+   if ((pos = rde_attr_parse(p, len, peer, ,
)) < 0)
goto done;
p += pos;
@@ -998,19 +998,19 @@ rde_update_dispatch(struct imsg *imsg)
}
 
/* check for missing but necessary attributes */
-   if ((subtype = rde_attr_missing(asp, peer->conf.ebgp,
+   if ((subtype = rde_attr_missing(, peer->conf.ebgp,
nlri_len))) {
rde_update_err(peer, ERR_UPDATE, ERR_UPD_MISSNG_WK_ATTR,
, sizeof(u_int8_t));
goto done;
}
 
-   rde_as4byte_fixup(peer, asp);
+   rde_as4byte_fixup(peer, );
 
/* enforce remote AS if requested */
-   if (asp->flags & F_ATTR_ASPATH &&
+   if (asp.flags & F_ATTR_ASPATH &&
peer->conf.enforce_as == ENFORCE_AS_ON) {
-   fas = aspath_neighbor(asp->aspath);
+   fas = aspath_neighbor(asp.aspath);
if (peer->conf.remote_as != fas) {
log_peer_warnx(>conf, "bad path, "
"starting with %s, "
@@ -1021,7 +1021,7 @@ rde_update_dispatch(struct imsg *imsg)
}
}
 
-   rde_reflector(peer, asp);
+   rde_reflector(peer, );
}
 
p = imsg->data;
@@ -1103,7 +1103,7 @@ rde_update_dispatch(struct imsg *imsg)
goto done;
}
 
-   if ((asp->flags & ~F_ATTR_MP_UNREACH) == 0 && mplen == 0) {
+   if ((asp.flags & ~F_ATTR_MP_UNREACH) == 0 && mplen == 0) {
/* EoR marker */
peer_recv_eor(peer, aid);
}
@@ -1166,7 +1166,7 @@ rde_update_dispatch(struct imsg *imsg)
break;
}
 
-   if ((asp->flags & ~F_ATTR_MP_UNREACH) == 0) {
+   if ((asp.flags & ~F_ATTR_MP_UNREACH) == 0) {
error = 0;
goto done;
}
@@ -1178,8 +1178,8 @@ rde_update_dispatch(struct imsg *imsg)
/* aspath needs to be loop free nota bene this is not a hard error */
if (peer->conf.ebgp &&
peer->conf.enforce_local_as == ENFORCE_AS_ON &&
-   !aspath_loopfree(asp->aspath, peer->conf.local_as))
-   asp->flags |= F_ATTR_LOOP;
+   !aspath_loopfree(asp.aspath, peer->conf.local_as))
+   asp.flags |= F_ATTR_LOOP;
 
/* parse nlri prefix */
while (nlri_len > 0) {
@@ -1208,7 +1208,7 @@ rde_update_dispatch(struct imsg *imsg)
goto done;
}
 
-   if (rde_update_update(peer, asp, , prefixlen) == -1)
+   if (rde_update_update(peer, , , prefixlen) == -1)
goto done;
 
}
@@ -1244,11 +1244,11 @@ rde_update_dispatch(struct imsg *imsg)
 * this works because asp is not linked.
 * But first unlock the previously locked nexthop.
 */
-   if (asp->nexthop) {
-   (void)nexthop_put(asp->nexthop);
-   asp->nexthop = NULL;
+   if (asp.nexthop) {
+   (void)nexthop_put(asp.nexthop);
+   asp.nexthop = NULL;
}
-   if ((pos = rde_get_mp_nexthop(mpp, mplen, aid, asp)) == -1) {
+   if ((pos 

Re: disable seemingly unsupported MSI for AMD 17h/Raven Ridge HD Audio in azalia.c

2018-07-09 Thread Bryan Steele
On Mon, Jul 09, 2018 at 11:38:13AM +1000, Jonathan Gray wrote:
> On Sun, Jul 08, 2018 at 12:15:39PM -0700, Thomas Frohwein wrote:
> > Hi,
> > 
> > It appears that HD Audio from AMD's generation Ryzen can't handle MSI.
> > This leads to the bug that I reported here:
> > 
> > https://marc.info/?l=openbsd-bugs=151648196215922=2
> > 
> > Disabling MSI resolves the problem on my current system which is a Raven
> > Ridge APU. I don't have the Summit Ridge hardware anymore to test that
> > it is also resolved there, but the line is included in the diff
> > (PCI_PRODUCT_AMD_AMD64_17_HDA). It seems likely that this diff will also
> > fix HD Audio on Summit Ridge. However, testing would be welcome by
> > anyone who has a first-gen Ryzen.
> > 
> > I was slightly confused by the fact that so far it seems I've been the
> > only one who reported this. Even searching online for such an issue in
> > other OS didn't yield anything. However, the issue was there between 2
> > different Ryzen CPUs, 3 different motherboards, and at least 2 separate
> > OpenBSD -current installations. And it was never there on any
> > Intel-based setup, with otherwise same hardware and OpenBSD install.
> > While there have been several reports of people using Ryzen with
> > OpenBSD, they may not have used audio (that's my explanation for this
> > at the moment).
> > 
> > This diff was collaborative work with brynet@.
> > 
> > ok?
> 
> Does following the HUDSON2_HDA case and enabling pcie snooping
> instead change anything?

I had suggested this to Thomas and he said it had no effect for him,
unfortunately.

> > 
> > Index: azalia.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> > retrieving revision 1.244
> > diff -u -p -r1.244 azalia.c
> > --- azalia.c22 Apr 2018 10:02:13 -  1.244
> > +++ azalia.c7 Jul 2018 18:26:20 -
> > @@ -517,6 +517,15 @@ azalia_pci_attach(struct device *parent,
> > azalia_pci_write(sc->pc, sc->tag, ICH_PCI_MMC, reg);
> > }
> >  
> > +   /* disable MSI for AMD Summit Ridge/Raven Ridge HD Audio */
> > +   if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_AMD) {
> > +   switch (PCI_PRODUCT(sc->pciid)) {
> > +   case PCI_PRODUCT_AMD_AMD64_17_HDA:
> > +   case PCI_PRODUCT_AMD_RAVENRIDGE_HDA:
> > +   pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
> > +   }
> > +   }
> > +
> > /* interrupt */
> > if (pci_intr_map_msi(pa, ) && pci_intr_map(pa, )) {
> > printf(": can't map interrupt\n");
> > 
> 
> 



Re: pfkeyv2 without raw_usrreq

2018-07-09 Thread Alexander Bluhm
On Mon, Jul 09, 2018 at 01:51:59PM +0200, Claudio Jeker wrote:
> Same kind of diff done for rtsock applied to pfkeyv2.
> This is using a simplified version of route_usrreq() mainly because
> PRU_RCVD is unused.
> 
> Quick testing with iked done, more checking welcome

passes ipsec regress; OK bluhm@

> Index: net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.186
> diff -u -p -r1.186 pfkeyv2.c
> --- net/pfkeyv2.c 25 Jun 2018 09:48:17 -  1.186
> +++ net/pfkeyv2.c 9 Jul 2018 11:36:11 -
> @@ -142,10 +142,7 @@ struct domain pfkeydomain;
>   *   s   socket lock
>   */
>  struct pkpcb {
> - struct rawcbpkp_rcb;
> -#define kcb_socket   pkp_rcb.rcb_socket  /* [I] associated socket */
> -#define kcb_faddrpkp_rcb.rcb_faddr   /* [I] */
> -#define kcb_protopkp_rcb.rcb_proto   /* [I] */
> + struct socket   *kcb_socket;/* [I] associated socket */
>  
>   SRPL_ENTRY(pkpcb)   kcb_list;   /* [l] */
>   struct refcnt   kcb_refcnt; /* [a] */
> @@ -279,13 +276,10 @@ pfkeyv2_attach(struct socket *so, int pr
>   }
>  
>   kp->kcb_socket = so;
> - kp->kcb_proto.sp_family = so->so_proto->pr_domain->dom_family;
> - kp->kcb_proto.sp_protocol = proto;
>  
>   so->so_options |= SO_USELOOPBACK;
>   soisconnected(so);
>  
> - kp->kcb_faddr = _addr;
>   kp->kcb_pid = curproc->p_p->ps_pid;
>   kp->kcb_rdomain = rtable_l2(curproc->p_p->ps_rtableid);
>  
> @@ -337,10 +331,81 @@ pfkeyv2_detach(struct socket *so)
>  }
>  
>  int
> -pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *mbuf,
> +pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m,
>  struct mbuf *nam, struct mbuf *control, struct proc *p)
>  {
> - return (raw_usrreq(so, req, mbuf, nam, control, p));
> + struct pkpcb *kp;
> + int error = 0;
> +
> + if (req == PRU_CONTROL)
> + return (EOPNOTSUPP);
> +
> + soassertlocked(so);
> +
> + if (control && control->m_len) {
> + error = EOPNOTSUPP;
> + goto release;
> + }
> +
> + kp = sotokeycb(so);
> + if (kp == NULL) {
> + error = EINVAL;
> + goto release;
> + }
> +
> + switch (req) {
> + /* no connect, bind, accept. Socket is connected from the start */
> + case PRU_CONNECT:
> + case PRU_BIND:
> + case PRU_CONNECT2:
> + case PRU_LISTEN:
> + case PRU_ACCEPT:
> + error = EOPNOTSUPP;
> + break;
> +
> + case PRU_DISCONNECT:
> + case PRU_ABORT:
> + soisdisconnected(so);
> + break;
> + case PRU_SHUTDOWN:
> + socantsendmore(so);
> + break;
> + case PRU_SENSE:
> + /* stat: don't bother with a blocksize. */
> + return (0);
> +
> + /* 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;
> +
> + case PRU_RCVOOB:
> + case PRU_RCVD:
> + return (EOPNOTSUPP);
> +
> + case PRU_SENDOOB:
> + error = EOPNOTSUPP;
> + break;
> + case PRU_SEND:
> + if (nam) {
> + error = EISCONN;
> + break;
> + }
> + error = (*so->so_proto->pr_output)(m, so, NULL, NULL);
> + m = NULL;
> + break;
> + default:
> + panic("pfkeyv2_usrreq");
> + }
> +
> + release:
> + m_freem(control);
> + m_freem(m);
> + return (error);
>  }
>  
>  int



Re: adding __func__ identifier to panic() calls in vmm.c

2018-07-09 Thread Mike Larkin
On Mon, Jul 09, 2018 at 07:08:14AM -0600, nayden wrote:
> 
> Hi,
> 
> Would this change make it simpler to diagnose a vmm crash?
> 
> Nayden
> 

sure, go ahead.

> Index: arch/amd64/amd64/vmm.c
> ===
> RCS file: /home/nayden/cvsync/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 vmm.c
> --- arch/amd64/amd64/vmm.c5 Jul 2018 04:36:14 -   1.207
> +++ arch/amd64/amd64/vmm.c9 Jul 2018 12:30:48 -
> @@ -700,7 +700,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp,
>   vcpu_readregs_svm(vcpu, vrwp->vrwp_mask, vrs) :
>   vcpu_writeregs_svm(vcpu, vrwp->vrwp_mask, vrs);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -1284,7 +1284,7 @@ vm_impl_init(struct vm *vm, struct proc 
>vmm_softc->mode == VMM_MODE_RVI)
>   return vm_impl_init_svm(vm, p);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -1333,7 +1333,7 @@ vm_impl_deinit(struct vm *vm)
>vmm_softc->mode == VMM_MODE_RVI)
>   vm_impl_deinit_svm(vm);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -3020,7 +3020,7 @@ vcpu_reset_regs(struct vcpu *vcpu, struc
>vmm_softc->mode == VMM_MODE_RVI)
>   ret = vcpu_reset_regs_svm(vcpu, vrs);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  
>   return (ret);
>  }
> @@ -3166,7 +3166,7 @@ vcpu_init(struct vcpu *vcpu)
>vmm_softc->mode == VMM_MODE_RVI)
>   ret = vcpu_init_svm(vcpu);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  
>   return (ret);
>  }
> @@ -3244,7 +3244,7 @@ vcpu_deinit(struct vcpu *vcpu)
>vmm_softc->mode == VMM_MODE_RVI)
>   vcpu_deinit_svm(vcpu);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -4725,7 +4725,7 @@ vmm_get_guest_faulttype(void)
>   else if (vmm_softc->mode == VMM_MODE_RVI)
>   return vmx_get_guest_faulttype();
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> Index: arch/i386/i386/vmm.c
> ===
> RCS file: /home/nayden/cvsync/src/sys/arch/i386/i386/vmm.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 vmm.c
> --- arch/i386/i386/vmm.c  24 May 2018 07:27:41 -  1.39
> +++ arch/i386/i386/vmm.c  9 Jul 2018 12:40:36 -
> @@ -669,7 +669,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp,
>   vcpu_readregs_svm(vcpu, vrwp->vrwp_mask, vrs) :
>   vcpu_writeregs_svm(vcpu, vrwp->vrwp_mask, vrs);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -1236,7 +1236,7 @@ vm_impl_init(struct vm *vm, struct proc 
>vmm_softc->mode == VMM_MODE_RVI)
>   return vm_impl_init_svm(vm, p);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -1276,7 +1276,7 @@ vm_impl_deinit(struct vm *vm)
>vmm_softc->mode == VMM_MODE_RVI)
>   vm_impl_deinit_svm(vm);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -2592,7 +2592,7 @@ vcpu_reset_regs(struct vcpu *vcpu, struc
>vmm_softc->mode == VMM_MODE_RVI)
>   ret = vcpu_reset_regs_svm(vcpu, vrs);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  
>   return (ret);
>  }
> @@ -2737,7 +2737,7 @@ vcpu_init(struct vcpu *vcpu)
>vmm_softc->mode == VMM_MODE_RVI)
>   ret = vcpu_init_svm(vcpu);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  
>   return (ret);
>  }
> @@ -2810,7 +2810,7 @@ vcpu_deinit(struct vcpu *vcpu)
>vmm_softc->mode == VMM_MODE_RVI)
>   vcpu_deinit_svm(vcpu);
>   else
> - panic("unknown vmm mode");
> + panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
>  }
>  
>  /*
> @@ -3885,7 +3885,7 @@ vmm_get_guest_faulttype(void)
>

adding __func__ identifier to panic() calls in vmm.c

2018-07-09 Thread nayden


Hi,

Would this change make it simpler to diagnose a vmm crash?

Nayden

Index: arch/amd64/amd64/vmm.c
===
RCS file: /home/nayden/cvsync/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.207
diff -u -p -r1.207 vmm.c
--- arch/amd64/amd64/vmm.c  5 Jul 2018 04:36:14 -   1.207
+++ arch/amd64/amd64/vmm.c  9 Jul 2018 12:30:48 -
@@ -700,7 +700,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp,
vcpu_readregs_svm(vcpu, vrwp->vrwp_mask, vrs) :
vcpu_writeregs_svm(vcpu, vrwp->vrwp_mask, vrs);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -1284,7 +1284,7 @@ vm_impl_init(struct vm *vm, struct proc 
 vmm_softc->mode == VMM_MODE_RVI)
return vm_impl_init_svm(vm, p);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -1333,7 +1333,7 @@ vm_impl_deinit(struct vm *vm)
 vmm_softc->mode == VMM_MODE_RVI)
vm_impl_deinit_svm(vm);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -3020,7 +3020,7 @@ vcpu_reset_regs(struct vcpu *vcpu, struc
 vmm_softc->mode == VMM_MODE_RVI)
ret = vcpu_reset_regs_svm(vcpu, vrs);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 
return (ret);
 }
@@ -3166,7 +3166,7 @@ vcpu_init(struct vcpu *vcpu)
 vmm_softc->mode == VMM_MODE_RVI)
ret = vcpu_init_svm(vcpu);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 
return (ret);
 }
@@ -3244,7 +3244,7 @@ vcpu_deinit(struct vcpu *vcpu)
 vmm_softc->mode == VMM_MODE_RVI)
vcpu_deinit_svm(vcpu);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -4725,7 +4725,7 @@ vmm_get_guest_faulttype(void)
else if (vmm_softc->mode == VMM_MODE_RVI)
return vmx_get_guest_faulttype();
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
Index: arch/i386/i386/vmm.c
===
RCS file: /home/nayden/cvsync/src/sys/arch/i386/i386/vmm.c,v
retrieving revision 1.39
diff -u -p -r1.39 vmm.c
--- arch/i386/i386/vmm.c24 May 2018 07:27:41 -  1.39
+++ arch/i386/i386/vmm.c9 Jul 2018 12:40:36 -
@@ -669,7 +669,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp,
vcpu_readregs_svm(vcpu, vrwp->vrwp_mask, vrs) :
vcpu_writeregs_svm(vcpu, vrwp->vrwp_mask, vrs);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -1236,7 +1236,7 @@ vm_impl_init(struct vm *vm, struct proc 
 vmm_softc->mode == VMM_MODE_RVI)
return vm_impl_init_svm(vm, p);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -1276,7 +1276,7 @@ vm_impl_deinit(struct vm *vm)
 vmm_softc->mode == VMM_MODE_RVI)
vm_impl_deinit_svm(vm);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -2592,7 +2592,7 @@ vcpu_reset_regs(struct vcpu *vcpu, struc
 vmm_softc->mode == VMM_MODE_RVI)
ret = vcpu_reset_regs_svm(vcpu, vrs);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 
return (ret);
 }
@@ -2737,7 +2737,7 @@ vcpu_init(struct vcpu *vcpu)
 vmm_softc->mode == VMM_MODE_RVI)
ret = vcpu_init_svm(vcpu);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 
return (ret);
 }
@@ -2810,7 +2810,7 @@ vcpu_deinit(struct vcpu *vcpu)
 vmm_softc->mode == VMM_MODE_RVI)
vcpu_deinit_svm(vcpu);
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode);
 }
 
 /*
@@ -3885,7 +3885,7 @@ vmm_get_guest_faulttype(void)
else if (vmm_softc->mode == VMM_MODE_RVI)
return vmx_get_guest_faulttype();
else
-   panic("unknown vmm mode");
+   panic("%s: unknown vmm mode: %d", 

Re: remove raw_usrreq from rtsock.c

2018-07-09 Thread Alexander Bluhm
On Mon, Jul 09, 2018 at 01:19:52PM +0200, Claudio Jeker wrote:
> raw_usrreq() is just unneeded abstraction. It is easier to just handle
> all the PRU cases in route_usrreq() instead. Simplifies a few things
> and will hopefully make further MP work easier.
> 
> This is a mix of raw_usrreq() and uipc_usrreq() (I think that raw_usrreq
> is leaking mbufs in some cases). route regress, bgpd and route(8) are
> happy with this.
> -- 
> :wq Claudio

OK bluhm@

> Index: net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.276
> diff -u -p -r1.276 rtsock.c
> --- net/rtsock.c  5 Jul 2018 21:48:32 -   1.276
> +++ net/rtsock.c  9 Jul 2018 10:52:18 -
> @@ -136,11 +136,7 @@ int   sysctl_ifnames(struct walkarg *);
>  int   sysctl_rtable_rtstat(void *, size_t *, void *);
>  
>  struct rtpcb {
> - struct rawcbrop_rcb;
> -#define rop_socket   rop_rcb.rcb_socket
> -#define rop_faddrrop_rcb.rcb_faddr
> -#define rop_laddrrop_rcb.rcb_laddr
> -#define rop_protorop_rcb.rcb_proto
> + struct socket   *rop_socket;
>  
>   SRPL_ENTRY(rtpcb)   rop_list;
>   struct refcnt   rop_refcnt;
> @@ -148,6 +144,7 @@ struct rtpcb {
>   unsigned introp_msgfilter;
>   unsigned introp_flags;
>   u_int   rop_rtableid;
> + unsigned short  rop_proto;
>   u_char  rop_priority;
>  };
>  #define  sotortpcb(so)   ((struct rtpcb *)(so)->so_pcb)
> @@ -203,15 +200,54 @@ route_usrreq(struct socket *so, int req,
>   struct rtpcb*rop;
>   int  error = 0;
>  
> + if (req == PRU_CONTROL)
> + return (EOPNOTSUPP);
> +
>   soassertlocked(so);
>  
> + if (control && control->m_len) {
> + error = EOPNOTSUPP;
> + goto release;
> + }
> +
>   rop = sotortpcb(so);
>   if (rop == NULL) {
> - m_freem(m);
> - return (EINVAL);
> + error = EINVAL;
> + goto release;
>   }
>  
>   switch (req) {
> + /* no connect, bind, accept. Socket is connected from the start */
> + case PRU_CONNECT:
> + case PRU_BIND:
> + case PRU_CONNECT2:
> + case PRU_LISTEN:
> + case PRU_ACCEPT:
> + error = EOPNOTSUPP;
> + break;
> +
> + case PRU_DISCONNECT:
> + case PRU_ABORT:
> + soisdisconnected(so);
> + break;
> + case PRU_SHUTDOWN:
> + socantsendmore(so);
> + break;
> + case PRU_SENSE:
> + /* stat: don't bother with a blocksize. */
> + return (0);
> +
> + /* 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;
> + break;
> +
> + case PRU_RCVOOB:
> + return (EOPNOTSUPP);
>   case PRU_RCVD:
>   /*
>* If we are in a FLUSH state, check if the buffer is
> @@ -221,12 +257,26 @@ route_usrreq(struct socket *so, int req,
>   ((sbspace(rop->rop_socket, >rop_socket->so_rcv) ==
>   rop->rop_socket->so_rcv.sb_hiwat)))
>   rop->rop_flags &= ~ROUTECB_FLAG_FLUSH;
> - break;
> + return (0);
>  
> + case PRU_SENDOOB:
> + error = EOPNOTSUPP;
> + break;
> + case PRU_SEND:
> + if (nam) {
> + error = EISCONN;
> + break;
> + }
> + error = (*so->so_proto->pr_output)(m, so, NULL, NULL);
> + m = NULL;
> + break;
>   default:
> - error = raw_usrreq(so, req, m, nam, control, p);
> + panic("route_usrreq");
>   }
>  
> + release:
> + m_freem(control);
> + m_freem(m);
>   return (error);
>  }
>  
> @@ -257,16 +307,13 @@ route_attach(struct socket *so, int prot
>   }
>  
>   rop->rop_socket = so;
> - rop->rop_proto.sp_family = so->so_proto->pr_domain->dom_family;
> - rop->rop_proto.sp_protocol = proto;
> + rop->rop_proto = proto;
>  
>   rop->rop_rtableid = curproc->p_p->ps_rtableid;
>  
>   soisconnected(so);
>   so->so_options |= SO_USELOOPBACK;
>  
> - rop->rop_faddr = _src;
> -
>   rw_enter(_lk, RW_WRITE);
>   SRPL_INSERT_HEAD_LOCKED(_rc, _list, rop, 
> rop_list);
>   rtptable.rtp_count++;
> @@ -438,9 +485,8 @@ route_input(struct mbuf *m0, struct sock
>* messages that match the address family. Address family
>* agnostic messages are always sent.
>*/
> - if (sa_family != AF_UNSPEC &&
> - rop->rop_proto.sp_protocol != AF_UNSPEC 

bgpd: check max prefix just once

2018-07-09 Thread Claudio Jeker
No need to duplicate the same block over and over again.
Just check it when we increment the counter and return failure if we hit
the limit.

OK?
-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.384
diff -u -p -r1.384 rde.c
--- rde.c   5 Jul 2018 10:25:26 -   1.384
+++ rde.c   9 Jul 2018 12:47:16 -
@@ -51,7 +51,7 @@ void   rde_sighdlr(int);
 voidrde_dispatch_imsg_session(struct imsgbuf *);
 voidrde_dispatch_imsg_parent(struct imsgbuf *);
 int rde_update_dispatch(struct imsg *);
-voidrde_update_update(struct rde_peer *, struct rde_aspath *,
+int rde_update_update(struct rde_peer *, struct rde_aspath *,
 struct bgpd_addr *, u_int8_t);
 voidrde_update_withdraw(struct rde_peer *, struct bgpd_addr *,
 u_int8_t);
@@ -1208,18 +1208,8 @@ rde_update_dispatch(struct imsg *imsg)
goto done;
}
 
-   rde_update_update(peer, asp, , prefixlen);
-
-   /* max prefix checker */
-   if (peer->conf.max_prefix &&
-   peer->prefix_cnt > peer->conf.max_prefix) {
-   log_peer_warnx(>conf, "prefix limit reached"
-   " (>%u/%u)", peer->prefix_cnt,
-   peer->conf.max_prefix);
-   rde_update_err(peer, ERR_CEASE, ERR_CEASE_MAX_PREFIX,
-   NULL, 0);
+   if (rde_update_update(peer, asp, , prefixlen) == -1)
goto done;
-   }
 
}
 
@@ -1289,21 +1279,9 @@ rde_update_dispatch(struct imsg *imsg)
mpp += pos;
mplen -= pos;
 
-   rde_update_update(peer, asp, ,
-   prefixlen);
-
-   /* max prefix checker */
-   if (peer->conf.max_prefix &&
-   peer->prefix_cnt > peer->conf.max_prefix) {
-   log_peer_warnx(>conf,
-   "prefix limit reached"
-   " (>%u/%u)", peer->prefix_cnt,
-   peer->conf.max_prefix);
-   rde_update_err(peer, ERR_CEASE,
-   ERR_CEASE_MAX_PREFIX, NULL, 0);
+   if (rde_update_update(peer, asp, ,
+   prefixlen) == -1)
goto done;
-   }
-
}
break;
case AID_VPN_IPv4:
@@ -1327,21 +1305,9 @@ rde_update_dispatch(struct imsg *imsg)
mpp += pos;
mplen -= pos;
 
-   rde_update_update(peer, asp, ,
-   prefixlen);
-
-   /* max prefix checker */
-   if (peer->conf.max_prefix &&
-   peer->prefix_cnt > peer->conf.max_prefix) {
-   log_peer_warnx(>conf,
-   "prefix limit reached"
-   " (>%u/%u)", peer->prefix_cnt,
-   peer->conf.max_prefix);
-   rde_update_err(peer, ERR_CEASE,
-   ERR_CEASE_MAX_PREFIX, NULL, 0);
+   if (rde_update_update(peer, asp, ,
+   prefixlen) == -1)
goto done;
-   }
-
}
break;
default:
@@ -1359,7 +1325,7 @@ done:
return (error);
 }
 
-void
+int
 rde_update_update(struct rde_peer *peer, struct rde_aspath *asp,
 struct bgpd_addr *prefix, u_int8_t prefixlen)
 {
@@ -1373,6 +1339,14 @@ rde_update_update(struct rde_peer *peer,
if (path_update([RIB_ADJ_IN].rib, peer, asp, prefix, prefixlen, 0))
peer->prefix_cnt++;
 
+   /* max prefix checker */
+   if (peer->conf.max_prefix && peer->prefix_cnt > peer->conf.max_prefix) {
+   log_peer_warnx(>conf, "prefix limit reached (>%u/%u)",
+   peer->prefix_cnt, peer->conf.max_prefix);
+   rde_update_err(peer, ERR_CEASE, ERR_CEASE_MAX_PREFIX, NULL, 0);
+   return (-1);
+   }
+
p = prefix_get([RIB_ADJ_IN].rib, peer, prefix, prefixlen, 0);
if (p == NULL)
fatalx("rde_update_update: no prefix in Adj-RIB-In");
@@ -1401,6 

Re: Patch for column number in doas error messages

2018-07-09 Thread Phil Eaton
Thanks for the comments. Happy to take a stab at going the bc/bc.y route if
that seems acceptable. That yyerror handler looks much more helpful.

On Mon, Jul 9, 2018 at 4:04 AM Otto Moerbeek  wrote:

> On Mon, Jul 09, 2018 at 01:52:29AM -0600, Theo de Raadt wrote:
>
> > Otto Moerbeek  wrote:
> >
> > > On Mon, Jul 09, 2018 at 01:21:25AM -0600, Theo de Raadt wrote:
> > >
> > > > Phil Eaton  wrote:
> > > >
> > > > > Hey,
> > > > >
> > > > > Doas currently tells you the line but not the column for syntax
> errors. In
> > > > > the case of a missing newline at the end of a line I was confused.
> So I
> > > > > added the column number to the message as well.
> > > > >
> > > > > Also, is there any interest in relaxing the grammar so a trailing
> rule
> > > > > without a newline is ok?
> > > > >
> > > > > Let me know what you think.
> > > > >
> > > > > diff --git parse.y parse.y
> > > > > index fde406bcf5a..f98deb81706 100644
> > > > > --- parse.y
> > > > > +++ parse.y
> > > > > @@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
> > > > >   va_start(va, fmt);
> > > > >   vfprintf(stderr, fmt, va);
> > > > >   va_end(va);
> > > > > - fprintf(stderr, " at line %d\n", yylval.lineno + 1);
> > > > > + fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
> > > > > yylval.colno);
> > > > >   parse_errors++;
> > > > >  }
> > > >
> > > > I don't see the point of this verboseness, and doubt our yacc and
> lexer
> > > > cooperate well enough to provide a correct colno.
> > >
> > > Agreed. I prefer to have the parser print the unexpected token. This
> > > is on my to-do list, but give it a shot if you want.  You can use
> > > bc/bc.y as an exmaple of a possible approach.
> >
> > I still don't see the point.  In 30 years, I've gotten by with parsers
> > that say "syntax error", and had very bad experiences with programs
> > that do a poor job anticipating where the parse error is.  Of course
> > there are programs which do a good job of detecting the error, but
> > generally those don't use yacc...
>
> I'm not trying to guess anything. yacc uses one-token lookahead. If it
> cannot continue, the lexical value of the token that could not be
> processed gives a pretty clear indication of the error spot.
>
> -Otto
>


-- 
Phil Eaton


pfkeyv2 without raw_usrreq

2018-07-09 Thread Claudio Jeker
Same kind of diff done for rtsock applied to pfkeyv2.
This is using a simplified version of route_usrreq() mainly because
PRU_RCVD is unused.

Quick testing with iked done, more checking welcome
-- 
:wq Claudio

Index: net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.186
diff -u -p -r1.186 pfkeyv2.c
--- net/pfkeyv2.c   25 Jun 2018 09:48:17 -  1.186
+++ net/pfkeyv2.c   9 Jul 2018 11:36:11 -
@@ -142,10 +142,7 @@ struct domain pfkeydomain;
  * s   socket lock
  */
 struct pkpcb {
-   struct rawcbpkp_rcb;
-#define kcb_socket pkp_rcb.rcb_socket  /* [I] associated socket */
-#define kcb_faddr  pkp_rcb.rcb_faddr   /* [I] */
-#define kcb_proto  pkp_rcb.rcb_proto   /* [I] */
+   struct socket   *kcb_socket;/* [I] associated socket */
 
SRPL_ENTRY(pkpcb)   kcb_list;   /* [l] */
struct refcnt   kcb_refcnt; /* [a] */
@@ -279,13 +276,10 @@ pfkeyv2_attach(struct socket *so, int pr
}
 
kp->kcb_socket = so;
-   kp->kcb_proto.sp_family = so->so_proto->pr_domain->dom_family;
-   kp->kcb_proto.sp_protocol = proto;
 
so->so_options |= SO_USELOOPBACK;
soisconnected(so);
 
-   kp->kcb_faddr = _addr;
kp->kcb_pid = curproc->p_p->ps_pid;
kp->kcb_rdomain = rtable_l2(curproc->p_p->ps_rtableid);
 
@@ -337,10 +331,81 @@ pfkeyv2_detach(struct socket *so)
 }
 
 int
-pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *mbuf,
+pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m,
 struct mbuf *nam, struct mbuf *control, struct proc *p)
 {
-   return (raw_usrreq(so, req, mbuf, nam, control, p));
+   struct pkpcb *kp;
+   int error = 0;
+
+   if (req == PRU_CONTROL)
+   return (EOPNOTSUPP);
+
+   soassertlocked(so);
+
+   if (control && control->m_len) {
+   error = EOPNOTSUPP;
+   goto release;
+   }
+
+   kp = sotokeycb(so);
+   if (kp == NULL) {
+   error = EINVAL;
+   goto release;
+   }
+
+   switch (req) {
+   /* no connect, bind, accept. Socket is connected from the start */
+   case PRU_CONNECT:
+   case PRU_BIND:
+   case PRU_CONNECT2:
+   case PRU_LISTEN:
+   case PRU_ACCEPT:
+   error = EOPNOTSUPP;
+   break;
+
+   case PRU_DISCONNECT:
+   case PRU_ABORT:
+   soisdisconnected(so);
+   break;
+   case PRU_SHUTDOWN:
+   socantsendmore(so);
+   break;
+   case PRU_SENSE:
+   /* stat: don't bother with a blocksize. */
+   return (0);
+
+   /* 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;
+
+   case PRU_RCVOOB:
+   case PRU_RCVD:
+   return (EOPNOTSUPP);
+
+   case PRU_SENDOOB:
+   error = EOPNOTSUPP;
+   break;
+   case PRU_SEND:
+   if (nam) {
+   error = EISCONN;
+   break;
+   }
+   error = (*so->so_proto->pr_output)(m, so, NULL, NULL);
+   m = NULL;
+   break;
+   default:
+   panic("pfkeyv2_usrreq");
+   }
+
+ release:
+   m_freem(control);
+   m_freem(m);
+   return (error);
 }
 
 int



remove raw_usrreq from rtsock.c

2018-07-09 Thread Claudio Jeker
raw_usrreq() is just unneeded abstraction. It is easier to just handle
all the PRU cases in route_usrreq() instead. Simplifies a few things
and will hopefully make further MP work easier.

This is a mix of raw_usrreq() and uipc_usrreq() (I think that raw_usrreq
is leaking mbufs in some cases). route regress, bgpd and route(8) are
happy with this.
-- 
:wq Claudio


Index: net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.276
diff -u -p -r1.276 rtsock.c
--- net/rtsock.c5 Jul 2018 21:48:32 -   1.276
+++ net/rtsock.c9 Jul 2018 10:52:18 -
@@ -136,11 +136,7 @@ int sysctl_ifnames(struct walkarg *);
 int sysctl_rtable_rtstat(void *, size_t *, void *);
 
 struct rtpcb {
-   struct rawcbrop_rcb;
-#define rop_socket rop_rcb.rcb_socket
-#define rop_faddr  rop_rcb.rcb_faddr
-#define rop_laddr  rop_rcb.rcb_laddr
-#define rop_proto  rop_rcb.rcb_proto
+   struct socket   *rop_socket;
 
SRPL_ENTRY(rtpcb)   rop_list;
struct refcnt   rop_refcnt;
@@ -148,6 +144,7 @@ struct rtpcb {
unsigned introp_msgfilter;
unsigned introp_flags;
u_int   rop_rtableid;
+   unsigned short  rop_proto;
u_char  rop_priority;
 };
 #definesotortpcb(so)   ((struct rtpcb *)(so)->so_pcb)
@@ -203,15 +200,54 @@ route_usrreq(struct socket *so, int req,
struct rtpcb*rop;
int  error = 0;
 
+   if (req == PRU_CONTROL)
+   return (EOPNOTSUPP);
+
soassertlocked(so);
 
+   if (control && control->m_len) {
+   error = EOPNOTSUPP;
+   goto release;
+   }
+
rop = sotortpcb(so);
if (rop == NULL) {
-   m_freem(m);
-   return (EINVAL);
+   error = EINVAL;
+   goto release;
}
 
switch (req) {
+   /* no connect, bind, accept. Socket is connected from the start */
+   case PRU_CONNECT:
+   case PRU_BIND:
+   case PRU_CONNECT2:
+   case PRU_LISTEN:
+   case PRU_ACCEPT:
+   error = EOPNOTSUPP;
+   break;
+
+   case PRU_DISCONNECT:
+   case PRU_ABORT:
+   soisdisconnected(so);
+   break;
+   case PRU_SHUTDOWN:
+   socantsendmore(so);
+   break;
+   case PRU_SENSE:
+   /* stat: don't bother with a blocksize. */
+   return (0);
+
+   /* 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;
+   break;
+
+   case PRU_RCVOOB:
+   return (EOPNOTSUPP);
case PRU_RCVD:
/*
 * If we are in a FLUSH state, check if the buffer is
@@ -221,12 +257,26 @@ route_usrreq(struct socket *so, int req,
((sbspace(rop->rop_socket, >rop_socket->so_rcv) ==
rop->rop_socket->so_rcv.sb_hiwat)))
rop->rop_flags &= ~ROUTECB_FLAG_FLUSH;
-   break;
+   return (0);
 
+   case PRU_SENDOOB:
+   error = EOPNOTSUPP;
+   break;
+   case PRU_SEND:
+   if (nam) {
+   error = EISCONN;
+   break;
+   }
+   error = (*so->so_proto->pr_output)(m, so, NULL, NULL);
+   m = NULL;
+   break;
default:
-   error = raw_usrreq(so, req, m, nam, control, p);
+   panic("route_usrreq");
}
 
+ release:
+   m_freem(control);
+   m_freem(m);
return (error);
 }
 
@@ -257,16 +307,13 @@ route_attach(struct socket *so, int prot
}
 
rop->rop_socket = so;
-   rop->rop_proto.sp_family = so->so_proto->pr_domain->dom_family;
-   rop->rop_proto.sp_protocol = proto;
+   rop->rop_proto = proto;
 
rop->rop_rtableid = curproc->p_p->ps_rtableid;
 
soisconnected(so);
so->so_options |= SO_USELOOPBACK;
 
-   rop->rop_faddr = _src;
-
rw_enter(_lk, RW_WRITE);
SRPL_INSERT_HEAD_LOCKED(_rc, _list, rop, 
rop_list);
rtptable.rtp_count++;
@@ -438,9 +485,8 @@ route_input(struct mbuf *m0, struct sock
 * messages that match the address family. Address family
 * agnostic messages are always sent.
 */
-   if (sa_family != AF_UNSPEC &&
-   rop->rop_proto.sp_protocol != AF_UNSPEC &&
-   rop->rop_proto.sp_protocol != sa_family)
+   if (sa_family != AF_UNSPEC && rop->rop_proto != AF_UNSPEC &&
+ 

Re: ospf6d: remove unneded log_setverbose()

2018-07-09 Thread Claudio Jeker
On Mon, Jul 09, 2018 at 11:33:05AM +0200, Remi Locherer wrote:
> On Mon, Jul 09, 2018 at 10:42:16AM +0200, Claudio Jeker wrote:
> > On Mon, Jul 09, 2018 at 10:31:15AM +0200, Remi Locherer wrote:
> > > later on it is set with:
> > > log_setverbose(ospfd_conf->opts & OSPFD_OPT_VERBOSE);
> > > 
> > > OK?
> > 
> > Shouldn't we instead remove the log_setverbose(1) above the getopt call?
> > I think the idea is that we want to be able to print debug messages
> > between the early log_init() and the later one.
> > In general that will allow debugging in parse_config() if -v is used.
> 
> Yes, this makes sense to me. Also adapt ospfd.
> 
> OK?

OK claudio@
 
> 
> Index: ospf6d/ospf6d.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 ospf6d.c
> --- ospf6d/ospf6d.c   5 Nov 2017 16:56:02 -   1.35
> +++ ospf6d/ospf6d.c   9 Jul 2018 09:22:54 -
> @@ -121,7 +121,6 @@ main(int argc, char *argv[])
>  
>   log_init(1, LOG_DAEMON);/* log to stderr until daemonized */
>   log_procinit(log_procnames[ospfd_process]);
> - log_setverbose(1);
>  
>   while ((ch = getopt(argc, argv, "cdD:f:s:nv")) != -1) {
>   switch (ch) {
> Index: ospfd/ospfd.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 ospfd.c
> --- ospfd/ospfd.c 11 Feb 2018 02:27:33 -  1.97
> +++ ospfd/ospfd.c 9 Jul 2018 09:26:54 -
> @@ -123,7 +123,6 @@ main(int argc, char *argv[])
>  
>   log_init(1, LOG_DAEMON);/* log to stderr until daemonized */
>   log_procinit(log_procnames[ospfd_process]);
> - log_setverbose(1);
>  
>   while ((ch = getopt(argc, argv, "cdD:f:ns:v")) != -1) {
>   switch (ch) {
> @@ -151,6 +150,7 @@ main(int argc, char *argv[])
>   if (opts & OSPFD_OPT_VERBOSE)
>   opts |= OSPFD_OPT_VERBOSE2;
>   opts |= OSPFD_OPT_VERBOSE;
> + log_setverbose(1);
>   break;
>   default:
>   usage();
> 

-- 
:wq Claudio



Re: ospf6d: remove unneded log_setverbose()

2018-07-09 Thread Jeremie Courreges-Anglas
On Mon, Jul 09 2018, Remi Locherer  wrote:
> On Mon, Jul 09, 2018 at 10:42:16AM +0200, Claudio Jeker wrote:
>> On Mon, Jul 09, 2018 at 10:31:15AM +0200, Remi Locherer wrote:
>> > later on it is set with:
>> > log_setverbose(ospfd_conf->opts & OSPFD_OPT_VERBOSE);
>> > 
>> > OK?
>> 
>> Shouldn't we instead remove the log_setverbose(1) above the getopt call?
>> I think the idea is that we want to be able to print debug messages
>> between the early log_init() and the later one.
>> In general that will allow debugging in parse_config() if -v is used.
>
> Yes, this makes sense to me. Also adapt ospfd.
>
> OK?

ok jca@

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: ospf6d: remove unneded log_setverbose()

2018-07-09 Thread Remi Locherer
On Mon, Jul 09, 2018 at 10:42:16AM +0200, Claudio Jeker wrote:
> On Mon, Jul 09, 2018 at 10:31:15AM +0200, Remi Locherer wrote:
> > later on it is set with:
> > log_setverbose(ospfd_conf->opts & OSPFD_OPT_VERBOSE);
> > 
> > OK?
> 
> Shouldn't we instead remove the log_setverbose(1) above the getopt call?
> I think the idea is that we want to be able to print debug messages
> between the early log_init() and the later one.
> In general that will allow debugging in parse_config() if -v is used.

Yes, this makes sense to me. Also adapt ospfd.

OK?


Index: ospf6d/ospf6d.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.35
diff -u -p -r1.35 ospf6d.c
--- ospf6d/ospf6d.c 5 Nov 2017 16:56:02 -   1.35
+++ ospf6d/ospf6d.c 9 Jul 2018 09:22:54 -
@@ -121,7 +121,6 @@ main(int argc, char *argv[])
 
log_init(1, LOG_DAEMON);/* log to stderr until daemonized */
log_procinit(log_procnames[ospfd_process]);
-   log_setverbose(1);
 
while ((ch = getopt(argc, argv, "cdD:f:s:nv")) != -1) {
switch (ch) {
Index: ospfd/ospfd.c
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.97
diff -u -p -r1.97 ospfd.c
--- ospfd/ospfd.c   11 Feb 2018 02:27:33 -  1.97
+++ ospfd/ospfd.c   9 Jul 2018 09:26:54 -
@@ -123,7 +123,6 @@ main(int argc, char *argv[])
 
log_init(1, LOG_DAEMON);/* log to stderr until daemonized */
log_procinit(log_procnames[ospfd_process]);
-   log_setverbose(1);
 
while ((ch = getopt(argc, argv, "cdD:f:ns:v")) != -1) {
switch (ch) {
@@ -151,6 +150,7 @@ main(int argc, char *argv[])
if (opts & OSPFD_OPT_VERBOSE)
opts |= OSPFD_OPT_VERBOSE2;
opts |= OSPFD_OPT_VERBOSE;
+   log_setverbose(1);
break;
default:
usage();



Re: [diff] httpd: pass through basic tls peer info to fcgi

2018-07-09 Thread Jack Burton
On Sat, 26 May 2018 22:44:56 +0930
Jack Burton  wrote:
> This diff adds three new TLS_PEER_* fastcgi variables: ISSUER, SERIAL
> & SUBJECT.
> 
> That seems the logical next step in making tls client certs usable for
> authorisation & accounting. Last week's commit (thanks Joel!) took
> care of authentication.
> 
> The cert subject on its own should suffice for the simplest cases.
> 
> For anything more involved, {issuer,serial} is the bare minimum
> necessary to identify any certificate uniquely (at least within the
> realm of trusted CAs, which is all that's relevant after
> authentication).
> 
> That should be enough for any fastcgi responder to record tls client
> cert accounting data.
> 
> It should also be enough for any fastcgi responder which has
> out-of-band access to relevant CA metadata to complete tls client
> cert authorisation tasks of any complexity.
> 
> That requirement for out-of-band access to CA metadata is why this
> approach is not as flexible as passing through the raw client cert to
> fastcgi (although admittedly passing the whole chain, as I proposed
> last year, was probably overkill). It's probably still worth coming
> back to revisit passing the client cert itself once the issues
> currently blocking that approach have been resolved.
> 
> But in the meantime this approach should cover the majority of use
> cases (and has the benefit of not requiring fastcgi responders to link
> to libcrypto just for cert authorisation, as would be the case when
> passing through the raw cert).

Ping...

Here's a slightly updated diff that applies cleanly against today's
-current and deals with the one compiler warning the last one generated.

Am I on the right track with this more minimal approach?

Or would it be better to add a tls_peer_cert_serial() to libtls, then
call that from httpd's server_fcgi(), given that the unique
{issuer,serial} pair seems like a fairly common thing to need?

Or would we be better off reworking the way libtls presents client cert
chains first, then returning to the earlier proposed approach of having
httpd pass the client cert chain through to fastcgi responders instead?


Index: usr.sbin/httpd/httpd.conf.5
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.101
diff -u -p -r1.101 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 20 Jun 2018 16:43:05 -  1.101
+++ usr.sbin/httpd/httpd.conf.5 9 Jul 2018 07:42:48 -
@@ -344,11 +344,27 @@ The revision of the HTTP specification u
 .It Ic SERVER_SOFTWARE
 The server software name of
 .Xr httpd 8 .
+.It Ic TLS_PEER_ISSUER
+The issuer of the TLS client certificate.
+.It Ic TLS_PEER_SERIAL
+The serial number of the TLS client certificate, expressed in hexadecimal.
+.It Ic TLS_PEER_SUBJECT
+The subject of the TLS client certificate.
 .It Ic TLS_PEER_VERIFY
 A variable that is set to a comma separated list of TLS client verification
-features in use
-.Pq omitted when TLS client verification is not in use .
+features in use.
 .El
+.Pp
+All
+.Ic TLS_PEER_*
+variables are omitted when TLS client certificate verification is not in use.
+All except
+.Ic TLS_PEER_VERIFY
+are also omitted if no TLS client certificate is offered when the
+.Ic optional
+argument to the
+.Ic tls client ca
+directive is in use.
 .It Ic hsts Oo Ar option Oc
 Enable HTTP Strict Transport Security.
 Valid options are:
Index: usr.sbin/httpd/server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.76
diff -u -p -r1.76 server_fcgi.c
--- usr.sbin/httpd/server_fcgi.c19 May 2018 13:56:56 -  1.76
+++ usr.sbin/httpd/server_fcgi.c9 Jul 2018 07:42:48 -
@@ -34,6 +34,13 @@
 #include 
 #include 
 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
 #include "httpd.h"
 #include "http.h"
 
@@ -87,17 +94,22 @@ int fcgi_add_param(struct server_fcgi_pa
 int
 server_fcgi(struct httpd *env, struct client *clt)
 {
+   ASN1_INTEGER*sn;
+   BIGNUM  *bn = NULL;
+   BIO *bio = NULL;
+   X509*cert = NULL;
struct server_fcgi_param param;
struct server_config*srv_conf = clt->clt_srv_conf;
struct http_descriptor  *desc = clt->clt_descreq;
struct fcgi_record_header   *h;
struct fcgi_begin_request_body  *begin;
char hbuf[HOST_NAME_MAX+1];
-   size_t   scriptlen;
+   size_t   chainlen, scriptlen;
int  pathlen;
int  fd = -1, ret;
const char  *stripped, *p, *alias, *errstr = NULL;
-   char*str, *script = NULL;
+   char

Re: ioctl(2) & fcntl(2) tweaks

2018-07-09 Thread Martin Pieuchot
On 03/07/18(Tue) 12:38, Martin Pieuchot wrote:
> The next important step towards removing the KERNEL_LOCK() from the
> kernel is to be able to execute ioctl(2)s without it.
> 
> The first area that can benefit from this is obviously the Network
> Stack.  tb@ and kn@ are working on this area so they'll soon need a
> way to test really test their diffs. 
> 
> But some pseudo-devices could also benefit from this.  I'm thinking
> at vmm(4), bpf(4) and pf(4) in a first place...
> 
> So here's a diff that remove some duplicate logic from the generics
> sys_ioctl() and sys_fcntl().  Once this is in we can start pushing
> the KERNEL_LOCK() down, inside every `fo_ioctl'.

Updated diff on top of recent changes.  This one doesn't include the
M_IOCTLOPS change.

It also keeps the previous semantic allowing a negative value for pgid
for sockets.  I'm also adding this ability for the FIOSETOWN ioctl(2)
issued on pipes to be coherent with fcntl(2)'s F_SETOWN.

Ok?

Index: kern/kern_descrip.c
===
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.176
diff -u -p -r1.176 kern_descrip.c
--- kern/kern_descrip.c 7 Jul 2018 16:14:40 -   1.176
+++ kern/kern_descrip.c 9 Jul 2018 08:44:22 -
@@ -484,14 +484,6 @@ restart:
break;
 
case F_GETOWN:
-   if (fp->f_type == DTYPE_SOCKET) {
-   *retval = ((struct socket *)fp->f_data)->so_pgid;
-   break;
-   }
-   if (fp->f_type == DTYPE_PIPE) {
-   *retval = ((struct pipe *)fp->f_data)->pipe_pgid;
-   break;
-   }
tmp = 0;
error = (*fp->f_ops->fo_ioctl)
(fp, TIOCGPGRP, (caddr_t), p);
@@ -500,21 +492,9 @@ restart:
 
case F_SETOWN:
tmp = (long)SCARG(uap, arg);
-   if (fp->f_type == DTYPE_SOCKET) {
-   struct socket *so = fp->f_data;
-
-   so->so_pgid = tmp;
-   so->so_siguid = p->p_ucred->cr_ruid;
-   so->so_sigeuid = p->p_ucred->cr_uid;
-   break;
-   }
-   if (fp->f_type == DTYPE_PIPE) {
-   struct pipe *mpipe = fp->f_data;
-
-   mpipe->pipe_pgid = tmp;
-   break;
-   }
-   if (tmp <= 0) {
+   if (fp->f_type == DTYPE_SOCKET || fp->f_type == DTYPE_PIPE) {
+   /* nothing */
+   } else if (tmp <= 0) {
tmp = -tmp;
} else {
struct process *pr1 = prfind(tmp);
Index: kern/sys_generic.c
===
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.119
diff -u -p -r1.119 sys_generic.c
--- kern/sys_generic.c  8 May 2018 08:53:41 -   1.119
+++ kern/sys_generic.c  9 Jul 2018 08:44:18 -
@@ -389,7 +389,7 @@ sys_ioctl(struct proc *p, void *v, regis
syscallarg(void *) data;
} */ *uap = v;
struct file *fp;
-   struct filedesc *fdp;
+   struct filedesc *fdp = p->p_fd;
u_long com = SCARG(uap, com);
int error = 0;
u_int size;
@@ -398,7 +398,6 @@ sys_ioctl(struct proc *p, void *v, regis
 #define STK_PARAMS 128
long long stkbuf[STK_PARAMS / sizeof(long long)];
 
-   fdp = p->p_fd;
if ((fp = fd_getfile_mode(fdp, SCARG(uap, fd), FREAD|FWRITE)) == NULL)
return (EBADF);
 
@@ -478,16 +477,10 @@ sys_ioctl(struct proc *p, void *v, regis
 
case FIOSETOWN:
tmp = *(int *)data;
-   if (fp->f_type == DTYPE_SOCKET) {
-   struct socket *so = fp->f_data;
 
-   so->so_pgid = tmp;
-   so->so_siguid = p->p_ucred->cr_ruid;
-   so->so_sigeuid = p->p_ucred->cr_uid;
-   error = 0;
-   break;
-   }
-   if (tmp <= 0) {
+   if (fp->f_type == DTYPE_SOCKET || fp->f_type == DTYPE_PIPE) {
+   /* nothing */
+   } else if (tmp <= 0) {
tmp = -tmp;
} else {
struct process *pr = prfind(tmp);
@@ -502,11 +495,6 @@ sys_ioctl(struct proc *p, void *v, regis
break;
 
case FIOGETOWN:
-   if (fp->f_type == DTYPE_SOCKET) {
-   error = 0;
-   *(int *)data = ((struct socket *)fp->f_data)->so_pgid;
-   break;
-   }
error = (*fp->f_ops->fo_ioctl)(fp, TIOCGPGRP, data, p);
*(int *)data = -*(int *)data;
break;
Index: kern/sys_pipe.c
===

Re: ospf6d: remove unneded log_setverbose()

2018-07-09 Thread Claudio Jeker
On Mon, Jul 09, 2018 at 10:31:15AM +0200, Remi Locherer wrote:
> later on it is set with:
> log_setverbose(ospfd_conf->opts & OSPFD_OPT_VERBOSE);
> 
> OK?

Shouldn't we instead remove the log_setverbose(1) above the getopt call?
I think the idea is that we want to be able to print debug messages
between the early log_init() and the later one.
In general that will allow debugging in parse_config() if -v is used.
 
> 
> Index: ospf6d.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 ospf6d.c
> --- ospf6d.c  5 Nov 2017 16:56:02 -   1.35
> +++ ospf6d.c  8 Jul 2018 12:59:38 -
> @@ -149,7 +149,6 @@ main(int argc, char *argv[])
>   if (opts & OSPFD_OPT_VERBOSE)
>   opts |= OSPFD_OPT_VERBOSE2;
>   opts |= OSPFD_OPT_VERBOSE;
> - log_setverbose(1);
>   break;
>   default:
>   usage();
> 
> 
> 

-- 
:wq Claudio



ospf6d: remove unneded log_setverbose()

2018-07-09 Thread Remi Locherer
later on it is set with:
log_setverbose(ospfd_conf->opts & OSPFD_OPT_VERBOSE);

OK?


Index: ospf6d.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.35
diff -u -p -r1.35 ospf6d.c
--- ospf6d.c5 Nov 2017 16:56:02 -   1.35
+++ ospf6d.c8 Jul 2018 12:59:38 -
@@ -149,7 +149,6 @@ main(int argc, char *argv[])
if (opts & OSPFD_OPT_VERBOSE)
opts |= OSPFD_OPT_VERBOSE2;
opts |= OSPFD_OPT_VERBOSE;
-   log_setverbose(1);
break;
default:
usage();





Re: Patch for column number in doas error messages

2018-07-09 Thread Otto Moerbeek
On Mon, Jul 09, 2018 at 01:52:29AM -0600, Theo de Raadt wrote:

> Otto Moerbeek  wrote:
> 
> > On Mon, Jul 09, 2018 at 01:21:25AM -0600, Theo de Raadt wrote:
> > 
> > > Phil Eaton  wrote:
> > > 
> > > > Hey,
> > > > 
> > > > Doas currently tells you the line but not the column for syntax errors. 
> > > > In
> > > > the case of a missing newline at the end of a line I was confused. So I
> > > > added the column number to the message as well.
> > > > 
> > > > Also, is there any interest in relaxing the grammar so a trailing rule
> > > > without a newline is ok?
> > > > 
> > > > Let me know what you think.
> > > > 
> > > > diff --git parse.y parse.y
> > > > index fde406bcf5a..f98deb81706 100644
> > > > --- parse.y
> > > > +++ parse.y
> > > > @@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
> > > >   va_start(va, fmt);
> > > >   vfprintf(stderr, fmt, va);
> > > >   va_end(va);
> > > > - fprintf(stderr, " at line %d\n", yylval.lineno + 1);
> > > > + fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
> > > > yylval.colno);
> > > >   parse_errors++;
> > > >  }
> > > 
> > > I don't see the point of this verboseness, and doubt our yacc and lexer
> > > cooperate well enough to provide a correct colno.
> > 
> > Agreed. I prefer to have the parser print the unexpected token. This
> > is on my to-do list, but give it a shot if you want.  You can use
> > bc/bc.y as an exmaple of a possible approach.
> 
> I still don't see the point.  In 30 years, I've gotten by with parsers
> that say "syntax error", and had very bad experiences with programs
> that do a poor job anticipating where the parse error is.  Of course
> there are programs which do a good job of detecting the error, but
> generally those don't use yacc...

I'm not trying to guess anything. yacc uses one-token lookahead. If it
cannot continue, the lexical value of the token that could not be
processed gives a pretty clear indication of the error spot.

-Otto



Re: Patch for column number in doas error messages

2018-07-09 Thread Theo de Raadt
Otto Moerbeek  wrote:

> On Mon, Jul 09, 2018 at 01:21:25AM -0600, Theo de Raadt wrote:
> 
> > Phil Eaton  wrote:
> > 
> > > Hey,
> > > 
> > > Doas currently tells you the line but not the column for syntax errors. In
> > > the case of a missing newline at the end of a line I was confused. So I
> > > added the column number to the message as well.
> > > 
> > > Also, is there any interest in relaxing the grammar so a trailing rule
> > > without a newline is ok?
> > > 
> > > Let me know what you think.
> > > 
> > > diff --git parse.y parse.y
> > > index fde406bcf5a..f98deb81706 100644
> > > --- parse.y
> > > +++ parse.y
> > > @@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
> > >   va_start(va, fmt);
> > >   vfprintf(stderr, fmt, va);
> > >   va_end(va);
> > > - fprintf(stderr, " at line %d\n", yylval.lineno + 1);
> > > + fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
> > > yylval.colno);
> > >   parse_errors++;
> > >  }
> > 
> > I don't see the point of this verboseness, and doubt our yacc and lexer
> > cooperate well enough to provide a correct colno.
> 
> Agreed. I prefer to have the parser print the unexpected token. This
> is on my to-do list, but give it a shot if you want.  You can use
> bc/bc.y as an exmaple of a possible approach.

I still don't see the point.  In 30 years, I've gotten by with parsers
that say "syntax error", and had very bad experiences with programs
that do a poor job anticipating where the parse error is.  Of course
there are programs which do a good job of detecting the error, but
generally those don't use yacc...



Re: broadcom netxtreme-c/e driver

2018-07-09 Thread Mark Kettenis
> Date: Mon, 9 Jul 2018 14:23:43 +1000
> From: Jonathan Matthew 
> 
> This started out as a port of freebsd's bnxt driver, but along the way I
> had to rewrite the interesting bits.
> 
> NetXtreme-C/E (BCM573xx and BCM574xx) is a different line of chips to
> NetXtreme II 10G (BCM577xx, which mje was working on), and luckily for me it's
> much easier to talk to.  The reason these ones are interesting to me is that
> they're the only 10G mezzanine option for Dell's Epyc based servers.
> 
> I've got it to the point where I could commit to cvs over it, so I'd
> like to get it into the tree.  Lots of stuff is still missing -
> media types, multicast, jumbos, any of the vast array of offloads,
> and it's not particularly fast yet, only slightly over a gigabit in
> iperf/tcpbench type tests.  Lots of work to do still.
> 
> The current code is below, only I'm not including bnxtreg.h as it's
> fairly large (>30k lines) and it came unmodified from freebsd:
> https://svnweb.freebsd.org/base/head/sys/dev/bnxt/hsi_struct_def.h?revision=323233=markup
> 
> ok?

ok kettenis@

> /*$OpenBSD$   */
> /*-
>  * Broadcom NetXtreme-C/E network driver.
>  *
>  * Copyright (c) 2016 Broadcom, All Rights Reserved.
>  * The term Broadcom refers to Broadcom Limited and/or its subsidiaries
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>  * are met:
>  * 1. Redistributions of source code must retain the above copyright
>  *notice, this list of conditions and the following disclaimer.
>  * 2. Redistributions in binary form must reproduce the above copyright
>  *notice, this list of conditions and the following disclaimer in the
>  *documentation and/or other materials provided with the distribution.
>  *
>  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS'
>  * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>  * ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
>  * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>  * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>  * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>  * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>  * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>  * THE POSSIBILITY OF SUCH DAMAGE.
>  */
> 
> /*
>  * Copyright (c) 2018 Jonathan Matthew 
>  *
>  * Permission to use, copy, modify, and distribute this software for any
>  * purpose with or without fee is hereby granted, provided that the above
>  * copyright notice and this permission notice appear in all copies.
>  *
>  * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>  * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>  * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>  * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>  * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>  */
> 
> 
> #include "bpfilter.h"
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #include 
> 
> #include 
> #include 
> #include 
> 
> #define __FBSDID(x)
> #include 
> 
> #include 
> #include 
> 
> #if NBPFILTER > 0
> #include 
> #endif
> 
> #include 
> #include 
> 
> #define BNXT_HWRM_BAR 0x10
> #define BNXT_DOORBELL_BAR 0x18
> 
> #define BNXT_RX_RING_ID   0
> #define BNXT_AG_RING_ID   1
> #define BNXT_TX_RING_ID   3
> 
> #define BNXT_MAX_QUEUE8
> #define BNXT_MAX_MTU  9000
> 
> #define BNXT_MAX_TX_SEGS  32  /* a bit much? */
> 
> #define BNXT_HWRM_SHORT_REQ_LEN   sizeof(struct hwrm_short_input)
> 
> #define BNXT_HWRM_LOCK_INIT(_sc, _name)   \
>   mtx_init_flags(>sc_lock, IPL_NET, _name, 0)
> #define BNXT_HWRM_LOCK(_sc)   mtx_enter(&_sc->sc_lock)
> #define BNXT_HWRM_UNLOCK(_sc) mtx_leave(&_sc->sc_lock)
> #define BNXT_HWRM_LOCK_DESTROY(_sc)   /* nothing */
> #define BNXT_HWRM_LOCK_ASSERT(_sc)MUTEX_ASSERT_LOCKED(&_sc->sc_lock)
> 
> #define BNXT_FLAG_VF0x0001
> #define BNXT_FLAG_NPAR  0x0002
> #define BNXT_FLAG_WOL_CAP   0x0004
> #define BNXT_FLAG_SHORT_CMD 0x0008
> 
> #define NEXT_CP_CONS_V(_ring, _cons, _v_bit)  \
> do {  \
>   if (++(_cons) == (_ring)->ring_size)\
>   ((_cons) = 0, (_v_bit) = !_v_bit);  \
> } while (0);
> 
> 

Re: Patch for column number in doas error messages

2018-07-09 Thread Otto Moerbeek
On Mon, Jul 09, 2018 at 01:21:25AM -0600, Theo de Raadt wrote:

> Phil Eaton  wrote:
> 
> > Hey,
> > 
> > Doas currently tells you the line but not the column for syntax errors. In
> > the case of a missing newline at the end of a line I was confused. So I
> > added the column number to the message as well.
> > 
> > Also, is there any interest in relaxing the grammar so a trailing rule
> > without a newline is ok?
> > 
> > Let me know what you think.
> > 
> > diff --git parse.y parse.y
> > index fde406bcf5a..f98deb81706 100644
> > --- parse.y
> > +++ parse.y
> > @@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
> >   va_start(va, fmt);
> >   vfprintf(stderr, fmt, va);
> >   va_end(va);
> > - fprintf(stderr, " at line %d\n", yylval.lineno + 1);
> > + fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
> > yylval.colno);
> >   parse_errors++;
> >  }
> 
> I don't see the point of this verboseness, and doubt our yacc and lexer
> cooperate well enough to provide a correct colno.

Agreed. I prefer to have the parser print the unexpected token. This
is on my to-do list, but give it a shot if you want.  You can use
bc/bc.y as an exmaple of a possible approach.

-Otto



Re: ber.c: simplify ber_read()

2018-07-09 Thread Claudio Jeker
On Sun, Jul 08, 2018 at 06:38:16PM +0200, Jeremie Courreges-Anglas wrote:
> 
> When looking at the recent changes in this file I noticed that the
> presence of both ber_read() and ber_readbuf() was due to an incomplete
> cleanup from my part.
> 
>   revision 1.32
>   date: 2018/02/08 18:02:06;  author: jca;  state: Exp;  lines: +6 -22;  
> commitid: YNQMco5lCS8YEifQ;
>   Kill ber.c support for direct fd read/writes
> 
>   This mechanism is already unused and annotated with lots of XXX's, no
>   need to keep it around.  ok claudio@
> 
> I think the recent changes would have been easier if the code had been
> further simplified.  Here's a shot at it:
> - stop looping in ber_read(), there is no fd read to handle any more so
>   calling ber_readbuf() once is enough
> - amend the condition which decides whether to return -1/ECANCELED:
>   - it makes little sense to pass a buffer length size of zero to
>   ber_read(), but then we should probably return 0 and not -1/ECANCELED
>   - I don't think we should perform partial reads: either the caller
>   function has the data it asked for, or nothing.  Allowing partial
>   reads means that we're putting the burden of handling an incomplete
>   buffer on the caller.
> - inline what remains of ber_readbuf() in ber_read()
> 
> regress tests for ldapd and snmpd pass, it would be cool to have test
> reports from people who actually use those programs (ldap, ldapd, snmpd
> and ypldap).
> 
> Looking for reviews and oks.  The diff is not that long but I can split
> it in smaller parts if it helps.
> 

Looks good to me. OK claudio@
 
> Index: usr.bin/ldap/ber.c
> ===
> RCS file: /d/cvs/src/usr.bin/ldap/ber.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 ber.c
> --- usr.bin/ldap/ber.c4 Jul 2018 15:21:24 -   1.11
> +++ usr.bin/ldap/ber.c6 Jul 2018 11:50:24 -
> @@ -31,8 +31,6 @@
>  
>  #include "ber.h"
>  
> -#define MINIMUM(a, b)(((a) < (b)) ? (a) : (b))
> -
>  #define BER_TYPE_CONSTRUCTED 0x20/* otherwise primitive */
>  #define BER_TYPE_SINGLE_MAX  30
>  #define BER_TAG_MASK 0x1f
> @@ -48,7 +46,6 @@ static ssize_t  get_id(struct ber *b, uns
>  int *cstruct);
>  static ssize_t   get_len(struct ber *b, ssize_t *len);
>  static ssize_t   ber_read_element(struct ber *ber, struct ber_element 
> *elm);
> -static ssize_t   ber_readbuf(struct ber *b, void *buf, size_t nbytes);
>  static ssize_t   ber_getc(struct ber *b, u_char *c);
>  static ssize_t   ber_read(struct ber *ber, void *buf, size_t len);
>  
> @@ -1208,28 +1205,6 @@ ber_read_element(struct ber *ber, struct
>   return totlen;
>  }
>  
> -static ssize_t
> -ber_readbuf(struct ber *b, void *buf, size_t nbytes)
> -{
> - size_t   sz, len;
> -
> - if (b->br_rbuf == NULL)
> - return -1;
> -
> - sz = b->br_rend - b->br_rptr;
> - len = MINIMUM(nbytes, sz);
> - if (len == 0) {
> - errno = ECANCELED;
> - return (-1);/* end of buffer and parser wants more data */
> - }
> -
> - bcopy(b->br_rptr, buf, len);
> - b->br_rptr += len;
> - b->br_offs += len;
> -
> - return (len);
> -}
> -
>  void
>  ber_set_readbuf(struct ber *b, void *buf, size_t len)
>  {
> @@ -1269,23 +1244,28 @@ ber_free(struct ber *b)
>  static ssize_t
>  ber_getc(struct ber *b, u_char *c)
>  {
> - return ber_readbuf(b, c, 1);
> + return ber_read(b, c, 1);
>  }
>  
>  static ssize_t
>  ber_read(struct ber *ber, void *buf, size_t len)
>  {
> - u_char *b = buf;
> - ssize_t r, remain = len;
> + size_t  sz;
>  
> - while (remain > 0) {
> - r = ber_readbuf(ber, b, remain);
> - if (r == -1)
> - return -1;
> - b += r;
> - remain -= r;
> + if (ber->br_rbuf == NULL)
> + return -1;
> +
> + sz = ber->br_rend - ber->br_rptr;
> + if (len > sz) {
> + errno = ECANCELED;
> + return -1;  /* parser wants more data than available */
>   }
> - return (b - (u_char *)buf);
> +
> + bcopy(ber->br_rptr, buf, len);
> + ber->br_rptr += len;
> + ber->br_offs += len;
> +
> + return len;
>  }
>  
>  int
> Index: usr.sbin/ldapd/ber.c
> ===
> RCS file: /d/cvs/src/usr.sbin/ldapd/ber.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 ber.c
> --- usr.sbin/ldapd/ber.c  4 Jul 2018 15:21:24 -   1.21
> +++ usr.sbin/ldapd/ber.c  6 Jul 2018 11:47:55 -
> @@ -31,8 +31,6 @@
>  
>  #include "ber.h"
>  
> -#define MINIMUM(a, b)(((a) < (b)) ? (a) : (b))
> -
>  #define BER_TYPE_CONSTRUCTED 0x20/* otherwise primitive */
>  #define BER_TYPE_SINGLE_MAX  30
>  #define BER_TAG_MASK 0x1f
> @@ -48,7 +46,6 @@ static ssize_t  get_id(struct ber *b, uns
>  int *cstruct);
>  static ssize_t   get_len(struct ber *b, ssize_t 

Re: Patch for column number in doas error messages

2018-07-09 Thread Theo de Raadt
Phil Eaton  wrote:

> Hey,
> 
> Doas currently tells you the line but not the column for syntax errors. In
> the case of a missing newline at the end of a line I was confused. So I
> added the column number to the message as well.
> 
> Also, is there any interest in relaxing the grammar so a trailing rule
> without a newline is ok?
> 
> Let me know what you think.
> 
> diff --git parse.y parse.y
> index fde406bcf5a..f98deb81706 100644
> --- parse.y
> +++ parse.y
> @@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
>   va_start(va, fmt);
>   vfprintf(stderr, fmt, va);
>   va_end(va);
> - fprintf(stderr, " at line %d\n", yylval.lineno + 1);
> + fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
> yylval.colno);
>   parse_errors++;
>  }

I don't see the point of this verboseness, and doubt our yacc and lexer
cooperate well enough to provide a correct colno.



Re: Patch for column number in doas error messages

2018-07-09 Thread Martijn van Duren
Hello Phil,

On 07/09/18 02:35, Phil Eaton wrote:
> Hey,
> 
> Doas currently tells you the line but not the column for syntax errors. In
> the case of a missing newline at the end of a line I was confused. So I
> added the column number to the message as well.

I don't care much about the change one way or the other, but your patch
doesn't apply. Also, does this help in other cases that are not POSIX
violations? And if we choose to accept the patch, should this be applied
any or all of our other yyerror implementations?
> 
> Also, is there any interest in relaxing the grammar so a trailing rule
> without a newline is ok?

No, POSIX requires lines to end with a newline.[0]

martijn@
> 
> Let me know what you think.
> 
> diff --git parse.y parse.y
> index fde406bcf5a..f98deb81706 100644
> --- parse.y
> +++ parse.y
> @@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
>   va_start(va, fmt);
>   vfprintf(stderr, fmt, va);
>   va_end(va);
> - fprintf(stderr, " at line %d\n", yylval.lineno + 1);
> + fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
> yylval.colno);
>   parse_errors++;
>  }
> 
> 
> 
[0] 
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206