myx_intr without a mutex for the sts block

2015-08-31 Thread David Gwynne
myx_intr should be able to run without a lock, except in teh situation where 
are bringing an interface down.

this interlock was done by serialising the changes myx_down and myx_intr needed 
to do with a mutex, but for the many millions and billions of interrupts you 
get when you're actually using the interface, this is an annoying overhead.

this plays tricks to avoid the mutex.

it works for me, but im throwing it out so someone else can try and hopefully 
break it.

Index: if_myx.c
===
RCS file: /cvs/src/sys/dev/pci/if_myx.c,v
retrieving revision 1.82
diff -u -p -r1.82 if_myx.c
--- if_myx.c15 Aug 2015 01:17:01 -  1.82
+++ if_myx.c1 Sep 2015 05:09:34 -
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -123,7 +124,6 @@ struct myx_softc {
 
struct myx_dmamemsc_sts_dma;
volatile struct myx_status  *sc_sts;
-   struct mutex sc_sts_mtx;
 
int  sc_intx;
void*sc_irqh;
@@ -226,26 +226,6 @@ void   myx_tx_free(struct myx_softc *);
 
 void   myx_refill(void *);
 
-static inline void
-myx_sts_enter(struct myx_softc *sc)
-{
-   bus_dmamap_t map = sc->sc_sts_dma.mxm_map;
-
-   mtx_enter(&sc->sc_sts_mtx);
-   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
-   BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
-}
-
-static inline void
-myx_sts_leave(struct myx_softc *sc)
-{
-   bus_dmamap_t map = sc->sc_sts_dma.mxm_map;
-
-   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
-   BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
-   mtx_leave(&sc->sc_sts_mtx);
-}
-
 struct cfdriver myx_cd = {
NULL, "myx", DV_IFNET
 };
@@ -285,8 +265,6 @@ myx_attach(struct device *parent, struct
timeout_set(&sc->sc_rx_ring[MYX_RXBIG].mrr_refill, myx_refill,
&sc->sc_rx_ring[MYX_RXBIG]);
 
-   mtx_init(&sc->sc_sts_mtx, IPL_NET);
-
/* Map the PCI memory space */
memtype = pci_mapreg_type(sc->sc_pc, sc->sc_tag, MYXBAR0);
if (pci_mapreg_map(pa, MYXBAR0, memtype, BUS_SPACE_MAP_PREFETCHABLE,
@@ -889,6 +867,7 @@ void
 myx_media_status(struct ifnet *ifp, struct ifmediareq *imr)
 {
struct myx_softc*sc = (struct myx_softc *)ifp->if_softc;
+   bus_dmamap_t map = sc->sc_sts_dma.mxm_map;
u_int32_tsts;
 
imr->ifm_active = IFM_ETHER | IFM_AUTO;
@@ -897,9 +876,11 @@ myx_media_status(struct ifnet *ifp, stru
return;
}
 
-   myx_sts_enter(sc);
+   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
+   BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
sts = sc->sc_sts->ms_linkstate;
-   myx_sts_leave(sc);
+   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
+   BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
 
myx_link_state(sc, sts);
 
@@ -1220,9 +1201,7 @@ myx_up(struct myx_softc *sc)
goto empty_rx_ring_big;
}
 
-   mtx_enter(&sc->sc_sts_mtx);
sc->sc_state = MYX_S_RUNNING;
-   mtx_leave(&sc->sc_sts_mtx);
 
if (myx_cmd(sc, MYXCMD_SET_IFUP, &mc, NULL) != 0) {
printf("%s: failed to start the device\n", DEVNAME(sc));
@@ -1349,24 +1328,28 @@ myx_down(struct myx_softc *sc)
struct ifnet*ifp = &sc->sc_ac.ac_if;
volatile struct myx_status *sts = sc->sc_sts;
bus_dmamap_t map = sc->sc_sts_dma.mxm_map;
+   struct sleep_state   sls;
struct myx_cmd   mc;
int  s;
int  ring;
 
-   myx_sts_enter(sc);
+   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
+   BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
sc->sc_linkdown = sts->ms_linkdown;
+   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
+   BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
+
sc->sc_state = MYX_S_DOWN;
+   membar_producer();
 
memset(&mc, 0, sizeof(mc));
(void)myx_cmd(sc, MYXCMD_SET_IFDOWN, &mc, NULL);
 
-   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
-   BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
-   while (sc->sc_state != MYX_S_OFF)
-   msleep(sts, &sc->sc_sts_mtx, 0, "myxdown", 0);
-   bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
-   BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
-   mtx_leave(&sc->sc_sts_mtx);
+   while (sc->sc_state != MYX_S_OFF) {
+   sleep_setup(&sls, sts, PWAIT, "myxdown");
+   membar_consumer();
+   sleep_finish(&sls, sc->sc_state != MYX_S_OFF);
+   }
 
s = splnet();
if (ifp->if_link_state != LINK_STATE_UNKNOWN) {
@@ -1604,23 +1587,22 @@ myx_intr(void *arg)
struct myx_softc*sc = (struct myx

Re: escape minus signs in sndio manpages

2015-08-31 Thread Peter Piwowarski

Ingo Schwarze wrote:

Until somebody shows me a specific list of roff implementations
that do so, i challenge that statement as urban legend, and the
advice to use \- for flags as cargo cult programming.


The reason lintian complains about this is that Debian had problems with 
- rendering as a unicode hyphen in their groff at one time[1]. As far as 
I can tell this no longer applies to current groff (which does indeed 
weaken the rationale here).



Can you tell me who the author and who the maintainer of that
tool is?  I'd like to question them about the reasons for their
recommendation, if possible.


lintian is maintained by a team who can be contacted at 
mailto:lintian-ma...@debian.org.


...however, digging further, they also don't care about this anymore[2]. 
In this light I'm sorry to have wasted everybody's time with this patch.


[1] 
https://lists.debian.org/debian-devel/2003/debian-devel-200303/msg01481.html

[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=785353



syslogd reuseaddr *:514

2015-08-31 Thread Alexander Bluhm
Hi,

I had some problems running syslogd on a machine where another
process had a 514 socket bound to a specific address.  As this is
not a real conflict, I think syslogd should bind *:514 with
SO_REUSEADDR.  This is already the case with all other sockets.

ok?

bluhm

Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.179
diff -u -p -r1.179 syslogd.c
--- usr.sbin/syslogd/syslogd.c  27 Aug 2015 17:53:35 -  1.179
+++ usr.sbin/syslogd/syslogd.c  31 Aug 2015 00:10:02 -
@@ -334,7 +334,7 @@ voidusage(void);
 void   wallmsg(struct filed *, struct iovec *);
 intloghost_parse(char *, char **, char **, char **);
 intgetmsgbufsize(void);
-intsocket_bind(const char *, const char *, const char *, int, int,
+intsocket_bind(const char *, const char *, const char *, int,
 int *, int *);
 intunix_socket(char *, int, mode_t);
 void   double_rbuf(int);
@@ -450,7 +450,7 @@ main(int argc, char *argv[])
die(0);
}
 
-   if (socket_bind("udp", NULL, "syslog", 0, SecureMode,
+   if (socket_bind("udp", NULL, "syslog", SecureMode,
&fd_udp, &fd_udp6) == -1) {
errno = 0;
logerror("socket bind *");
@@ -458,7 +458,7 @@ main(int argc, char *argv[])
die(0);
}
fd_bind = -1;
-   if (bind_host && socket_bind("udp", bind_host, bind_port, 1, 0,
+   if (bind_host && socket_bind("udp", bind_host, bind_port, 0,
&fd_bind, &fd_bind) == -1) {
errno = 0;
logerror("socket bind udp");
@@ -466,7 +466,7 @@ main(int argc, char *argv[])
die(0);
}
fd_listen = -1;
-   if (listen_host && socket_bind("tcp", listen_host, listen_port, 1, 0,
+   if (listen_host && socket_bind("tcp", listen_host, listen_port, 0,
&fd_listen, &fd_listen) == -1) {
errno = 0;
logerror("socket listen tcp");
@@ -695,12 +695,12 @@ main(int argc, char *argv[])
 
 int
 socket_bind(const char *proto, const char *host, const char *port,
-int reuseaddr, int shutread, int *fd, int *fd6)
+int shutread, int *fd, int *fd6)
 {
struct addrinfo  hints, *res, *res0;
char hostname[NI_MAXHOST], servname[NI_MAXSERV];
char ebuf[ERRBUFSIZE];
-   int *fdp, error;
+   int *fdp, error, reuseaddr;
 
*fd = *fd6 = -1;
if (proto == NULL)
@@ -761,6 +761,7 @@ socket_bind(const char *proto, const cha
*fdp = -1;
continue;
}
+   reuseaddr = 1;
if (setsockopt(*fdp, SOL_SOCKET, SO_REUSEADDR, &reuseaddr,
sizeof(reuseaddr)) == -1) {
snprintf(ebuf, sizeof(ebuf), "setsockopt SO_REUSEADDR "



Re: ksh sanatizing argv redundant

2015-08-31 Thread Theo de Raadt
> Martijn van Duren wrote:
> > Hello tech@,
> > 
> > I took a quick glance at ksh and one of the first things I noticed was 
> > that it uses some sanatizing code on argv. When looking at execve(2) I 
> > see that EINVAL or EFAULT are returned when argv isn't properly 
> > formatted. I've also verified this quickly by a small PoC and in 
> > sys/kern/kern_exec.c.
> > 
> > Would it make sense to remove the check all together?
> 
> I think this is ok. You used to have to worry about it, because the kernel let
> you exec something with empty argv. And there's still perhaps a portability
> concern. But old workarounds need to die sometime. I support removing this,
> but I'd like some one else to comment.

with fire

(or else  i wonder if doas needs this checking...)



Re: ksh sanatizing argv redundant

2015-08-31 Thread Ted Unangst
Martijn van Duren wrote:
> Hello tech@,
> 
> I took a quick glance at ksh and one of the first things I noticed was 
> that it uses some sanatizing code on argv. When looking at execve(2) I 
> see that EINVAL or EFAULT are returned when argv isn't properly 
> formatted. I've also verified this quickly by a small PoC and in 
> sys/kern/kern_exec.c.
> 
> Would it make sense to remove the check all together?

I think this is ok. You used to have to worry about it, because the kernel let
you exec something with empty argv. And there's still perhaps a portability
concern. But old workarounds need to die sometime. I support removing this,
but I'd like some one else to comment.



Re: Like free(), ksh's afree() is NULL-safe

2015-08-31 Thread Ted Unangst
Michael McConville wrote:
> Ted Unangst wrote:
> > Michael McConville wrote:
> > > Also, why is fs on line 390 cast to char* when afree() takes void*?
> > 
> > this code is older than void. and NULL apparently. please remove the casts
> > too.
> 
> Does this look good? I had to leave one cast because the freed pointer
> is a const.

yup. only binary change is in two places, where the null check was removed.



Re: Like free(), ksh's afree() is NULL-safe

2015-08-31 Thread Michael McConville
Ted Unangst wrote:
> Michael McConville wrote:
> > Also, why is fs on line 390 cast to char* when afree() takes void*?
> 
> this code is older than void. and NULL apparently. please remove the casts
> too.

Does this look good? I had to leave one cast because the freed pointer
is a const.


Index: c_ksh.c
===
RCS file: /cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.34
diff -u -p -r1.34 c_ksh.c
--- c_ksh.c 17 Dec 2013 16:37:05 -  1.34
+++ c_ksh.c 1 Sep 2015 00:22:03 -
@@ -943,7 +943,7 @@ c_alias(char **wp)
if ((val && !tflag) || (!val && tflag && !Uflag)) {
if (ap->flag&ALLOC) {
ap->flag &= ~(ALLOC|ISSET);
-   afree((void*)ap->val.s, APERM);
+   afree(ap->val.s, APERM);
}
/* ignore values for -t (at&t ksh does this) */
newval = tflag ? search(alias, path, X_OK, (int *) 0) :
@@ -998,7 +998,7 @@ c_unalias(char **wp)
}
if (ap->flag&ALLOC) {
ap->flag &= ~(ALLOC|ISSET);
-   afree((void*)ap->val.s, APERM);
+   afree(ap->val.s, APERM);
}
ap->flag &= ~(DEFINED|ISSET|EXPORT);
}
@@ -1009,7 +1009,7 @@ c_unalias(char **wp)
for (ktwalk(&ts, t); (ap = ktnext(&ts)); ) {
if (ap->flag&ALLOC) {
ap->flag &= ~(ALLOC|ISSET);
-   afree((void*)ap->val.s, APERM);
+   afree(ap->val.s, APERM);
}
ap->flag &= ~(DEFINED|ISSET|EXPORT);
}
Index: edit.c
===
RCS file: /cvs/src/bin/ksh/edit.c,v
retrieving revision 1.40
diff -u -p -r1.40 edit.c
--- edit.c  12 Mar 2015 10:20:30 -  1.40
+++ edit.c  1 Sep 2015 00:22:04 -
@@ -489,7 +489,7 @@ x_command_glob(int flags, const char *st
path_order_cmp);
for (i = 0; i < nwords; i++)
words[i] = info[i].word;
-   afree((void *) info, ATEMP);
+   afree(info, ATEMP);
} else {
/* Sort and remove duplicate entries */
char **words = (char **) XPptrv(w);
Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.50
diff -u -p -r1.50 emacs.c
--- emacs.c 25 Mar 2015 12:10:52 -  1.50
+++ emacs.c 1 Sep 2015 00:22:04 -
@@ -1149,7 +1149,7 @@ x_push(int nchars)
 {
char*cp = str_nsave(xcp, nchars, AEDIT);
if (killstack[killsp])
-   afree((void *)killstack[killsp], AEDIT);
+   afree(killstack[killsp], AEDIT);
killstack[killsp] = cp;
killsp = (killsp + 1) % KILLSIZE;
 }
@@ -1307,8 +1307,7 @@ static void
 kb_del(struct kb_entry *k)
 {
TAILQ_REMOVE(&kblist, k, entry);
-   if (k->args)
-   free(k->args);
+   free(k->args);
afree(k, AEDIT);
 }
 
Index: expand.h
===
RCS file: /cvs/src/bin/ksh/expand.h,v
retrieving revision 1.6
diff -u -p -r1.6 expand.h
--- expand.h30 Mar 2005 17:16:37 -  1.6
+++ expand.h1 Sep 2015 00:22:04 -
@@ -55,7 +55,7 @@ typedef char * XStringP;
 #define Xcheck(xs, xp) XcheckN(xs, xp, 1)
 
 /* free string */
-#defineXfree(xs, xp)   afree((void*) (xs).beg, (xs).areap)
+#defineXfree(xs, xp)   afree((xs).beg, (xs).areap)
 
 /* close, return string */
 #defineXclose(xs, xp)  (char*) aresize((void*)(xs).beg, \
@@ -104,4 +104,4 @@ typedef struct XPtrV {
 #defineXPclose(x)  (void**) aresize((void*)(x).beg, \
 sizeofN(void*, XPsize(x)), ATEMP)
 
-#defineXPfree(x)   afree((void*) (x).beg, ATEMP)
+#defineXPfree(x)   afree((x).beg, ATEMP)
Index: history.c
===
RCS file: /cvs/src/bin/ksh/history.c,v
retrieving revision 1.40
diff -u -p -r1.40 history.c
--- history.c   20 Nov 2014 15:22:39 -  1.40
+++ history.c   1 Sep 2015 00:22:04 -
@@ -417,7 +417,7 @@ histbackup(void)
 
if (histptr >= history && last_line != hist_source->line) {
hist_source->line--;
-   afree((void*)*histptr, APERM);
+   afree(*histptr, APERM);
histptr--;
last_line = hist_source->line;
}
@@ -589,7 +589,7 @@ histsave(int lno, const char *cmd, int d
hp = histptr;
 
if (++hp >= history + histsize) { /* remove oldest command */
-   afree((void*)*history, APERM);
+  

doas path

2015-08-31 Thread Ted Unangst
I think we can relax the path restriction if there's no restriction on
command.


Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.39
diff -u -p -r1.39 doas.c
--- doas.c  27 Aug 2015 16:31:02 -  1.39
+++ doas.c  31 Aug 2015 19:03:27 -
@@ -433,8 +433,10 @@ main(int argc, char **argv, char **envp)
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
myname, cmdline, pw->pw_name, cwd);
 
-   if (setenv("PATH", safepath, 1) == -1)
-   err(1, "failed to set PATH '%s'", safepath);
+   if (rule->cmd) {
+   if (setenv("PATH", safepath, 1) == -1)
+   err(1, "failed to set PATH '%s'", safepath);
+   }
execvpe(cmd, argv, envp);
if (errno == ENOENT)
errx(1, "%s: command not found", cmd);
Index: doas.conf.5
===
RCS file: /cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.14
diff -u -p -r1.14 doas.conf.5
--- doas.conf.5 30 Jul 2015 14:02:04 -  1.14
+++ doas.conf.5 31 Aug 2015 19:05:58 -
@@ -73,6 +73,9 @@ The default is all users.
 The command the user is allowed or denied to run.
 The default is all commands.
 Be advised that it's best to specify absolute paths.
+If a cmd is specified, only a restricted
+.Ev PATH
+will be searched.
 .It Ic args ...
 Arguments to command.
 If specified, the command arguments provided by the user



Re: Introducing rtvalid(9)

2015-08-31 Thread Alexander Bluhm
On Fri, Aug 28, 2015 at 12:47:51PM +0200, Martin Pieuchot wrote:
>  The rtvalid() function checks if the route entry rt is still valid and
>  can be used.  Cached entries that are no longer valid should be released
>  by calling rtfree().

I like it.  As it does some checks and returns a boolian value, I
wonder wether rtisvalid() would be a better name.

> + if (rtvalid(rt) == 0) {
...
As the function  returns a boolean value I perfer the logical check with !.
if (rtvalid(rt)) { route is valid }
if (!rtvalid(rt)) { route is not valid }


> @@ -1239,8 +1233,10 @@ ip_rtaddr(struct in_addr dst, u_int rtab
>   ipforward_rt.ro_rt = rtalloc(&ipforward_rt.ro_dst,
>   RT_REPORT|RT_RESOLVE, rtableid);
>   }
> - if (ipforward_rt.ro_rt == 0)
> + if (rtvalid(ipforward_rt.ro_rt) == 0) {
> + rtfree(ipforward_rt.ro_rt);
>   return (NULL);

If you free the global variable ipforward_rt.ro_rt you have to reset
the pointer with ipforward_rt.ro_rt = NULL.  Otherwise you will run
into a use after free.

> @@ -1417,12 +1412,13 @@ ip_forward(struct mbuf *m, struct ifnet 
>  
>   ipforward_rt.ro_rt = rtalloc_mpath(&ipforward_rt.ro_dst,
>   &ip->ip_src.s_addr, ipforward_rt.ro_tableid);
> - if (ipforward_rt.ro_rt == 0) {
> + if (rtvalid(ipforward_rt.ro_rt) == 0) {
> + rtfree(ipforward_rt.ro_rt);
>   icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0);
>   return;

Same here, ipforward_rt.ro_rt = NULL

> +++ sys/netinet6/ip6_forward.c28 Aug 2015 09:40:40 -
> @@ -240,24 +238,23 @@ reroute:
>   ip6_forward_rt.ro_tableid);
>   }
>  
> - if (ip6_forward_rt.ro_rt == NULL) {
> + if (rtvalid(ip6_forward_rt.ro_rt) == 0) {
>   ip6stat.ip6s_noroute++;
>   /* XXX in6_ifstat_inc(rt->rt_ifp, ifs6_in_noroute) */
>   if (mcopy) {
>   icmp6_error(mcopy, ICMP6_DST_UNREACH,
>   ICMP6_DST_UNREACH_NOROUTE, 0);
>   }
> + rtfree(rt);
>   m_freem(m);
>   return;

rt is uninitialized.  You want to free ip6_forward_rt.ro_rt and set
it to NULL.

> @@ -268,13 +265,14 @@ reroute:
>   &ip6->ip6_src.s6_addr32[0],
>   ip6_forward_rt.ro_tableid);
>  
> - if (ip6_forward_rt.ro_rt == NULL) {
> + if (rtvalid(ip6_forward_rt.ro_rt) == 0) {
>   ip6stat.ip6s_noroute++;
>   /* XXX in6_ifstat_inc(rt->rt_ifp, ifs6_in_noroute) */
>   if (mcopy) {
>   icmp6_error(mcopy, ICMP6_DST_UNREACH,
>   ICMP6_DST_UNREACH_NOROUTE, 0);
>   }
> + rtfree(rt);
>   m_freem(m);
>   return;

Same here.

> @@ -429,10 +429,9 @@ ip6_input(struct mbuf *m)
>   /*
>*  Unicast check
>*/
> - if (ip6_forward_rt.ro_rt != NULL &&
> - (ip6_forward_rt.ro_rt->rt_flags & RTF_UP) != 0 &&
> + if (rtvalid(ip6_forward_rt.ro_rt) &&
>   IN6_ARE_ADDR_EQUAL(&ip6->ip6_dst,
> -&ip6_forward_rt.ro_dst.sin6_addr) &&
> +&ip6_forward_rt.ro_dst.sin6_addr) &&

Strange indentation.

All other conversions look correct to me.

bluhm



Re: escape minus signs in sndio manpages

2015-08-31 Thread Alexandre Ratchov
On Sun, Aug 30, 2015 at 03:35:21PM -0400, Peter Piwowarski wrote:
> This patch replaces some literal '-' with '\-' in examples for sndio-
> related manpages. Certain roff implementations may render '-' as a
> hyphen (unicode U+2010) rather than a minus sign (unicode U+002D or ascii 
> 0x2D); while mandoc renders them as intended with or without
> the escape, it should be more portable to use it. Found with Debian's
> lintian tool while packaging portable sndio for Debian; if there is
> interest I can fix any other instances in the tree that I can find.

replacing - by \- in literals produces identical files; tried
mandoc and latest groff (1.22.3) with utf8, ps and html outputs, so
I don't see how this could hurt.

schwarze, jmc, opinions?



Re: [patch] return instead of exit(3) in src/bin/

2015-08-31 Thread Fritjof Bornebusch
On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote:
> As suggested by deraadt@ and tobias@ it might be better to use the *return* 
> statement instead of exit(3) 
> inside the *main* function, to let the stack protector do its work.
> 
> This diff removes such calls in all *src/bin/* tools, except those who 
> already use it.
> I think I didn't miss a call and didn't introduce any bugs.
>

New diff with help from tobias@, as theo@ pointed me to a downside.

> --F.
> 



Index: cat/cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.21
diff -u -p -r1.21 cat.c
--- cat/cat.c   16 Jan 2015 06:39:28 -  1.21
+++ cat/cat.c   31 Aug 2015 20:44:20 -
@@ -103,8 +103,7 @@ main(int argc, char *argv[])
raw_args(argv);
if (fclose(stdout))
err(1, "stdout");
-   exit(rval);
-   /* NOTREACHED */
+   return (rval);
 }
 
 void
Index: chio/chio.c
===
RCS file: /cvs/src/bin/chio/chio.c,v
retrieving revision 1.25
diff -u -p -r1.25 chio.c
--- chio/chio.c 16 Mar 2014 18:38:30 -  1.25
+++ chio/chio.c 31 Aug 2015 20:44:21 -
@@ -148,7 +148,7 @@ main(int argc, char *argv[])
if (commands[i].cc_name == NULL)
errx(1, "unknown command: %s", *argv);
 
-   exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
+   return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
 }
 
 static int
Index: chmod/chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.34
diff -u -p -r1.34 chmod.c
--- chmod/chmod.c   25 Jun 2015 02:04:08 -  1.34
+++ chmod/chmod.c   31 Aug 2015 20:44:22 -
@@ -279,7 +279,7 @@ done:
if (errno)
err(1, "fts_read");
fts_close(ftsp);
-   exit(rval);
+   return (rval);
 }
 
 /*
Index: cp/cp.c
===
RCS file: /cvs/src/bin/cp/cp.c,v
retrieving revision 1.38
diff -u -p -r1.38 cp.c
--- cp/cp.c 7 May 2015 17:32:20 -   1.38
+++ cp/cp.c 31 Aug 2015 20:44:22 -
@@ -224,7 +224,7 @@ main(int argc, char *argv[])
type = FILE_TO_DIR;
}
 
-   exit(copy(argv, type, fts_options));
+   return (copy(argv, type, fts_options));
 }
 
 char *
Index: date/date.c
===
RCS file: /cvs/src/bin/date/date.c,v
retrieving revision 1.47
diff -u -p -r1.47 date.c
--- date/date.c 17 Apr 2015 16:47:47 -  1.47
+++ date/date.c 31 Aug 2015 20:44:22 -
@@ -143,7 +143,7 @@ main(int argc, char *argv[])
errx(1, "conversion error");
(void)strftime(buf, sizeof(buf), format, tp);
(void)printf("%s\n", buf);
-   exit(0);
+   return (0);
 }
 
 #defineATOI2(ar)   ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - 
'0'))
Index: dd/dd.c
===
RCS file: /cvs/src/bin/dd/dd.c,v
retrieving revision 1.21
diff -u -p -r1.21 dd.c
--- dd/dd.c 16 Jan 2015 06:39:31 -  1.21
+++ dd/dd.c 31 Aug 2015 20:44:22 -
@@ -85,7 +85,7 @@ main(int argc, char *argv[])
}
 
dd_close();
-   exit(0);
+   return (0);
 }
 
 static void
Index: df/df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.52
diff -u -p -r1.52 df.c
--- df/df.c 16 Jan 2015 06:39:31 -  1.52
+++ df/df.c 31 Aug 2015 20:44:23 -
@@ -175,7 +175,7 @@ main(int argc, char *argv[])
bsdprint(mntbuf, mntsize, maxwidth);
}
 
-   exit(mntsize ? 0 : 1);
+   return (mntsize ? 0 : 1);
 }
 
 char *
Index: domainname/domainname.c
===
RCS file: /cvs/src/bin/domainname/domainname.c,v
retrieving revision 1.9
diff -u -p -r1.9 domainname.c
--- domainname/domainname.c 16 Jan 2015 06:39:31 -  1.9
+++ domainname/domainname.c 31 Aug 2015 20:44:23 -
@@ -66,7 +66,7 @@ main(int argc, char *argv[])
err(1, "getdomainname");
(void)printf("%s\n", domainname);
}
-   exit(0);
+   return (0);
 }
 
 void
Index: expr/expr.c
===
RCS file: /cvs/src/bin/expr/expr.c,v
retrieving revision 1.20
diff -u -p -r1.20 expr.c
--- expr/expr.c 11 Aug 2015 17:15:46 -  1.20
+++ expr/expr.c 31 Aug 2015 20:44:23 -
@@ -518,5 +518,5 @@ main(int argc, char *argv[])
else
printf("%s\n", vp->u.s);
 
-   exit(is_zero_or_null(vp));
+   return (is_zero_or_null(vp));
 }
Index: hostname/hostname.c
===
RCS file:

PF ignores block action when rule contains route-to/dup-to action

2015-08-31 Thread Alexandr Nedvedicky
Hello,

Dilli Paudel in Oracle was playing with PF enough to find funny glitch.
He used rule as follows:

block in on vnic4 from 192.168.1.0/24 to any route-to 172.16.1.1@vnic5

Many people expect the route-to action is somewhat futile as 'block' action
takes precedence here, so packet gets always dropped. Well the reality is
very different (and still makes a sort of sense) from PF point of view.
The snippet comes from pf_test():

6586 switch (action) {
6587 case PF_SYNPROXY_DROP:

6593 case PF_DIVERT:
6594 switch (pd.af) {
6595 case AF_INET:

6610 case PF_AFRT:
6611 if (pf_translate_af(&pd)) {

6624 #endif /* INET6 */
6625 default:
6626 /* pf_route can free the mbuf causing *m0 to become 
NULL */
6627 if (r->rt) {
6628 switch (pd.af) {
6629 case AF_INET:
6630 pf_route(m0, r, pd.dir, 
pd.kif->pfik_ifp, s);
6631 break;

the action comes from matching rule. It's PF_DROP in case of Dilli's rule.  As
you can see there is no case-branch for PF_DROP in switch statement at line
6586, so a default: is executed. For route-to action the r->rt is set and PF
executes the route_to*().

Dilli  suggests to introduce PF_DROP case branch to switch() at line 6586 Issue
has been also discussed on Friday over icb, where Mr. bluhm further suggested
we should try to add some sanity check to parse.y.

As a side effect the patch breaks block rules with dup-to action. dup-to
action as a part of block rule might make some sense... So if there is
someone, who really needs block ... dup-to he should opt for equivalent
rule using pass ... route-to 

Also there is one more question:

shall we implement similar sanity checks for nat-to/rdr-to/... actions?

no one should expect those in block rule, so making pfctl to refuse such rules
loudly sounds like a right thing to do...

regards
sasha

8<---8<---8<--8<

Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.648
diff -u -p -r1.648 parse.y
--- sbin/pfctl/parse.y  21 Apr 2015 16:34:59 -  1.648
+++ sbin/pfctl/parse.y  31 Aug 2015 20:30:56 -
@@ -3997,8 +3997,11 @@ rule_consistent(struct pf_rule *r, int a
problems++;
}
 
-   /* match rules rules */
-   if (r->action == PF_MATCH) {
+   /* 
+* Basic rule sanity check.
+*/
+   switch (r->action) {
+   case PF_MATCH:
if (r->divert.port) {
yyerror("divert is not supported on match rules");
problems++;
@@ -4016,6 +4019,15 @@ rule_consistent(struct pf_rule *r, int a
yyerror("af-to is not supported on match rules");
problems++;
}
+   break;
+   case PF_DROP:
+   if (r->rt) {
+   yyerror("route-to, reply-to and dup-to "
+  "must not be used on block rules");
+   problems++;
+   }
+   break;
+   default:;
}
return (-problems);
 }
Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.936
diff -u -p -r1.936 pf.c
--- sys/net/pf.c19 Aug 2015 21:22:41 -  1.936
+++ sys/net/pf.c31 Aug 2015 20:31:02 -
@@ -6622,6 +6622,10 @@ done:
action = PF_PASS;
break;
 #endif /* INET6 */
+   case PF_DROP:
+   m_freem(*m0);
+   *m0 = NULL;
+   break;
default:
/* pf_route can free the mbuf causing *m0 to become NULL */
if (r->rt) {



[WIP PATCH] SR RAID1 checksumming support

2015-08-31 Thread Karel Gardas
Hello,

attached my work in progress on checksumming support for softraid
RAID1. Currently it does just:
- computation of checksums (crc32)
- verification of checksums
- signal bad checksum to console and to sensors

E.g.:
$ sysctl hw.sensors.softraid0
hw.sensors.softraid0.raw0=0 (sd0f), OK
hw.sensors.softraid0.raw1=0 (sd0g), OK
hw.sensors.softraid0.drive0=online (sd1), OK


Next TODO items:
- hang-over to another chunk (restart wu) in case of checksum error
- properly handle errors hapenning on all chunks
- "self-healing" of bad sector

Note: checksums are computed per sector basis, saved in the area
allocated at the end of the drive. Due to this design,
LBA collision detection in softraid.c was enhanced/fixed to support
also this case of application
and currently it may not be compatible with RAID5/6 usage.

Any comments welcome!

Thanks!
Karel

Index: sbin/bioctl/bioctl.8
===
RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
retrieving revision 1.96
diff -u -p -u -r1.96 bioctl.8
--- sbin/bioctl/bioctl.829 May 2015 00:33:37 -1.96
+++ sbin/bioctl/bioctl.831 Aug 2015 20:02:47 -
@@ -199,6 +199,11 @@ for example, force the creation of volum
 with unclean data in the metadata areas.
 .It Ar noauto
 Do not automatically assemble this volume at boot time.
+.It Ar chksum
+Enforce usage of checksums on the device blocks. The checksum area is
+located at the end of the device data area and since it accupies some
+space it makes actual usable device size smaller. We need exactly 8
+bytes of checksum per device data block.
 .El
 .It Fl c Ar raidlevel
 Create a
Index: sbin/bioctl/bioctl.c
===
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.129
diff -u -p -u -r1.129 bioctl.c
--- sbin/bioctl/bioctl.c18 Jul 2015 23:23:20 -1.129
+++ sbin/bioctl/bioctl.c31 Aug 2015 20:02:47 -
@@ -1053,6 +1053,9 @@ bio_createflags(char *lst)
 case 'n':
 flags |= BIOC_SCNOAUTOASSEMBLE;
 break;
+case 'c':
+flags |= BIOC_SCCHKSUM;
+break;
 default:
 strlcpy(fs, s, sz + 1);
 errx(1, "invalid flag %s", fs);
Index: sys/dev/biovar.h
===
RCS file: /cvs/src/sys/dev/biovar.h,v
retrieving revision 1.44
diff -u -p -u -r1.44 biovar.h
--- sys/dev/biovar.h29 May 2015 00:33:37 -1.44
+++ sys/dev/biovar.h31 Aug 2015 20:02:49 -
@@ -213,6 +213,7 @@ struct bioc_createraid {
 #define BIOC_SCDEVT0x02/* dev_t array or string in dev_list */
 #define BIOC_SCNOAUTOASSEMBLE0x04/* do not assemble during autoconf */
 #define BIOC_SCBOOTABLE0x08/* device is bootable */
+#define BIOC_SCCHKSUM0x10/* device provides chksum capability */
 u_int32_tbc_opaque_size;
 u_int32_tbc_opaque_flags;
 #defineBIOC_SOINVALID0x00/* no opaque pointer */
Index: sys/dev/softraid.c
===
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.364
diff -u -p -u -r1.364 softraid.c
--- sys/dev/softraid.c19 Aug 2015 19:05:24 -1.364
+++ sys/dev/softraid.c31 Aug 2015 20:02:50 -
@@ -71,6 +71,7 @@ uint32_tsr_debug = 0
 /* | SR_D_DIS */
 /* | SR_D_STATE */
 /* | SR_D_REBUILD */
+/* | SR_D_CHKSUM  */
 ;
 #endif

@@ -144,6 +145,8 @@ intsr_chunk_in_use(struct sr_softc *,
 intsr_rw(struct sr_softc *, dev_t, char *, size_t,
 daddr_t, long);
 voidsr_wu_done_callback(void *);
+intsr_wu_collision(struct sr_workunit *,
+struct sr_workunit *);

 /* don't include these on RAMDISK */
 #ifndef SMALL_KERNEL
@@ -2264,6 +2267,9 @@ sr_wu_done_callback(void *xwu)

 s = splbio();

+DNPRINTF(SR_D_WU, "%s: sr_wu_done: %p\n",
+ DEVNAME(sd->sd_sc), wu);
+
 if (xs != NULL) {
 if (wu->swu_ios_failed)
 xs->error = XS_DRIVER_STUFFUP;
@@ -2286,11 +2292,54 @@ sr_wu_done_callback(void *xwu)
 TAILQ_REMOVE(&sd->sd_wu_pendq, wu, swu_link);

 if (wu->swu_collider) {
-if (wu->swu_ios_failed)
-sr_raid_recreate_wu(wu->swu_collider);
+DNPRINTF(SR_D_WU, "%s: sr_wu_done, searching for collider: %p\n",
+ DEVNAME(sd->sd_sc), wu->swu_collider);
+if (wu->swu_ios_failed) {
+  DNPRINTF(SR_D_WU, "%s: sr_wu_done, recreate collider?: %p WHY???\n",
+   DEVNAME(sd->sd_sc), wu->swu_collider);
+  sr_raid_recreate_wu(wu->swu_collider);
+}
+/*
+ * We're searching for wu which do have the same collider
+ * like current wu. If we find such wu we can continue
+ * without starting the collider. If we do not find su

ksh sanatizing argv redundant

2015-08-31 Thread Martijn van Duren

Hello tech@,

I took a quick glance at ksh and one of the first things I noticed was 
that it uses some sanatizing code on argv. When looking at execve(2) I 
see that EINVAL or EFAULT are returned when argv isn't properly 
formatted. I've also verified this quickly by a small PoC and in 
sys/kern/kern_exec.c.


Would it make sense to remove the check all together?

Furthermore I see that kshname is based on directly dereferencing argv. 
Although it's semantics I personally reckon it's cleaner to do it by 
array index.


Sincerely,

Martijn van Duren
Index: main.c
===
RCS file: /cvs/src/bin/ksh/main.c,v
retrieving revision 1.55
diff -u -p -r1.55 main.c
--- main.c	9 Feb 2015 09:09:30 -	1.55
+++ main.c	31 Aug 2015 19:51:25 -
@@ -100,16 +100,7 @@ main(int argc, char *argv[])
 	struct env env;
 	pid_t ppid;
 
-	/* make sure argv[] is sane */
-	if (!*argv) {
-		static const char *empty_argv[] = {
-			"ksh", (char *) 0
-		};
-
-		argv = (char **) empty_argv;
-		argc = 1;
-	}
-	kshname = *argv;
+	kshname = argv[0];
 
 	ainit(&aperm);		/* initialize permanent Area */
 


[patch] in usb(4) check for dangerous requests

2015-08-31 Thread Grant Czajkowski
Hi all,

This patch adds the usb control request validity checks
already present in ugen(4) to usb(4).

Grant

Index: usb.c
===
RCS file: /cvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.107
diff -u -p -r1.107 usb.c
--- usb.c   14 Mar 2015 03:38:50 -  1.107
+++ usb.c   31 Aug 2015 19:37:22 -
@@ -622,7 +622,15 @@ usbioctl(dev_t devt, u_long cmd, caddr_t
return (EBADF);
 
DPRINTF(("usbioctl: USB_REQUEST addr=%d len=%d\n", addr, len));
-   if (len < 0 || len > 32768)
+   /* Avoid requests that would damage the bus integrity. */
+   if ((ur->ucr_request.bmRequestType == UT_WRITE_DEVICE &&
+ur->ucr_request.bRequest == UR_SET_ADDRESS) ||
+   (ur->ucr_request.bmRequestType == UT_WRITE_DEVICE &&
+ur->ucr_request.bRequest == UR_SET_CONFIG) ||
+   (ur->ucr_request.bmRequestType == UT_WRITE_INTERFACE &&
+ur->ucr_request.bRequest == UR_SET_INTERFACE))
+   return (EINVAL);
+   if (len < 0 || len > 32767)
return (EINVAL);
if (addr < 0 || addr >= USB_MAX_DEVICES)
return (EINVAL);



Re: rt_ifa_del() tweak

2015-08-31 Thread Alexander Bluhm
On Tue, Aug 25, 2015 at 01:01:38PM +0200, Martin Pieuchot wrote:
> I want to remove this chunked introduce in r7.19 in 1991 by sklower@
> because it no longer makes any sense, it is a layer violation and
> does no play well with rt refcounting.
> 
> When this chunk was introduced rtrequest1(RTM_DELETE...) was *not* doing
> a route lookup.  So this check is now (and since a long time) redundant
> with the rtable_lookup()+rtable_mpath_match() done inside rtrequest1(9).
> 
> Remember that when this code was introduced ``ifa'' where linked to
> ``rt'' in 1:1 fashion and rtrequest() started by rtfree(9)ing route
> entries. That's why you wanted to be sure that the following line was
> correct:
> 
>   error = rtrequest(cmd, dst, ifa->ifa_addr, ifa->ifa_netmask,
>   flags | ifa->ifa_flags, &ifa->ifa_rt)
> 
> Ok?

It looks that the error changes from EHOSTUNREACH/ENETUNREACH to
ESRCH.  That is good, when you try to remove an non existing
address/route.

The check in the old code and in rtable_mpath_match() is not
equivalent.  The ifa was used to compare it with every route.  In
rtable_mpath_match() the RTAX_GATEWAY is used for that purpose.
But this field is only set, if RTF_LLINFO is set in flags.

So at least the old check does not look completely redundant.  But
it still looks wrong to search for a route by comparing rt_ifa with
ifa and, after that has been found, delete another one.

So I would say, delete that chunk.

bluhm

> Index: net/route.c
> ===
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.225
> diff -u -p -r1.225 route.c
> --- net/route.c   24 Aug 2015 22:11:33 -  1.225
> +++ net/route.c   25 Aug 2015 10:43:32 -
> @@ -1230,21 +1230,6 @@ rt_ifa_del(struct ifaddr *ifa, int flags
>   rt_maskedcopy(dst, deldst, ifa->ifa_netmask);
>   dst = deldst;
>   }
> - if ((rt = rtalloc(dst, 0, rtableid)) != NULL) {
> - rt->rt_refcnt--;
> -#ifndef ART
> - /* try to find the right route */
> - while (rt && rt->rt_ifa != ifa)
> - rt = (struct rtentry *)
> - ((struct radix_node *)rt)->rn_dupedkey;
> - if (!rt) {
> - if (m != NULL)
> - (void) m_free(m);
> - return (flags & RTF_HOST ? EHOSTUNREACH
> - : ENETUNREACH);
> - }
> -#endif
> - }
>  
>   memset(&info, 0, sizeof(info));
>   info.rti_ifa = ifa;



Re: Like free(), ksh's afree() is NULL-safe

2015-08-31 Thread Ted Unangst
Michael McConville wrote:
> Also, why is fs on line 390 cast to char* when afree() takes void*?

this code is older than void. and NULL apparently. please remove the casts
too.



Re: ntpd(8): Make -n quieter

2015-08-31 Thread Michael Reed
On 08/31/15 07:36, Sebastian Benoit wrote:
> Michael Reed(m.r...@mykolab.com) on 2015.08.30 14:58:35 -0400:
>> Hi all,
>>
>> If ntpd is run with the -n flag, and /etc/ntpd.conf is parsed without
>> error, then "Configuration OK" is printed.  I don't think this is
>> particularly useful, as both a lack of an error message and an exit
>> value of 0 already indicate success in this case.  This seems to be the
>> case for most (many?) programs in the base system, such as doas(1).
> 
> I like the message. Why is it a problem?
> 
> /Benno
> 

It's admittedly not much of a problem, more just to follow the Unix
principle of saying nothing if there's nothing wrong.

Regards,
Michael



Re: syslogd host matches ip

2015-08-31 Thread Sebastien Marie
On Sun, Aug 30, 2015 at 07:18:34PM +0200, Alexander Bluhm wrote:
> 
> I reconsidered it.  It does not make sense to truncate the hostname
> in the config at some character from a list.  Just take whatever
> the user specified as progname or hostname.
> 
> ok?

visual ok + slightly tested

OK semarie@

> bluhm
> 
> Index: usr.sbin/syslogd/syslogd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.179
> diff -u -p -r1.179 syslogd.c
> --- usr.sbin/syslogd/syslogd.c27 Aug 2015 17:53:35 -  1.179
> +++ usr.sbin/syslogd/syslogd.c30 Aug 2015 17:06:13 -
> @@ -1937,7 +1937,7 @@ die(int signo)
>  void
>  init(void)
>  {
> - char progblock[NAME_MAX+1], hostblock[NAME_MAX+1], *cline, *p;
> + char progblock[NAME_MAX+1], hostblock[NAME_MAX+1], *cline, *p, *q;
>   struct filed_list mb;
>   struct filed *f, *m;
>   FILE *cf;
> @@ -2025,40 +2025,22 @@ init(void)
>   continue;
>   if (*p == '\0' || *p == '#')
>   continue;
> - if (*p == '!') {
> + if (*p == '!' || *p == '+') {
> + q = (*p == '!') ? progblock : hostblock;
>   p++;
>   while (isspace((unsigned char)*p))
>   p++;
> - if (!*p || (*p == '*' && (!p[1] ||
> + if (*p == '\0' || (*p == '*' && (p[1] == '\0' ||
>   isspace((unsigned char)p[1] {
> - strlcpy(progblock, "*", sizeof(progblock));
> + strlcpy(q, "*", NAME_MAX+1);
>   continue;
>   }
>   for (i = 0; i < NAME_MAX; i++) {
> - if (!isalnum((unsigned char)p[i]) &&
> - p[i] != '-' && p[i] != '!')
> + if (*p == '\0' || isspace((unsigned char)*p))
>   break;
> - progblock[i] = p[i];
> + *q++ = *p++;
>   }
> - progblock[i] = 0;
> - continue;
> - }
> - if (*p == '+') {
> - p++;
> - while (isspace((unsigned char)*p))
> - p++;
> - if (!*p || (*p == '*' && (!p[1] ||
> - isspace((unsigned char)p[1] {
> - strlcpy(hostblock, "*", sizeof(hostblock));
> - continue;
> - }
> - for (i = 0; i < NAME_MAX; i++) {
> - if (!isalnum((unsigned char)p[i]) &&
> - p[i] != '-' && p[i] != '+')
> - break;
> - hostblock[i] = p[i];
> - }
> - hostblock[i] = 0;
> + *q = '\0';
>   continue;
>   }
>  

-- 
Sebastien Marie



virtualization support

2015-08-31 Thread Mike Larkin
TL;DR - a native hypervisor is coming. stay tuned.

For the last few months, I've been working on a hypervisor for OpenBSD.
The idea for this started a few years ago, and after playing around with
it from time to time, things really started to take shape around the
time of the Brisbane hackathon earlier this year. As development
accelerated, the OpenBSD Foundation generously offered to fund the
project so that I could focus on it in more earnest.

At this point, I think I've made sufficient progress that a public
announcement is in order. I've also reached the point where I think
other developers can step in and help out as much of the gooey bits
in the core of the vmm are functioning the way I want.

Presently, the vmm code I've built is capable of launching a kernel
and asking for the root filesystem; it doesn't do much more than that
for now. However, getting to this point invovled building most of the
scaffolding needed to finish the rest, and like I stated before, more
people can now more easily help with what's left (basically the virtio(4)
emulation for disks and network interfaces).

One might ask - why not port one of the other hypervisors out there
instead of rolling your own from scratch? Fair question. However, for
various technical reasons, choosing to port an existing vmm just didn't
make a whole lot of sense. For example, I've been baking in support
for things that the other implementations don't care about (namely
i386 support, shadow paging, nested virtualization, support for legacy
peripherals, etc) and trying to backfit support for those things into
another hypervisor would probably have been just as hard as building
it from the ground up.

The inevitable questions:

Q. What OSes can I run?
A. To start, OSes that support virtio-based devices (most of the CPU
goo is done and is basically the same across most OSes once you get
that part finished). Later, we'll see if we can expose a place for
qemu to attach (maybe mimic KVM) to run legacy OSes or OSes that
require BIOS/UEFI boot support. There is no legacy-free mandate in this
vmm.

Q. When will it be ready?
A. Hard to say. I hope by the end of October, but no promises. A lot
also depends on how much help I get writing some of the virtio backends.

Q. What will be the CPU requirements?
A. Any AMD or Intel CPU that supports hardware virtualization (SVM for
AMD, and VMX for Intel). For those CPUs that don't support RVI or EPT,
we'll use shadow paging.

Q. What will be the OS requirements?
A. The core tools (vmd(8) and vmmctl(8)) and the main vmm (vmm(4)) will
be built in to base, on i386 and amd64. This should be sufficient to run
virtio based guests without any additional software requirements.

Q. Yuck, i386?
A. Yes. It helps find bugs. And I urge you to review my last dozen or so
commits to i386 for another reason.

And now, the most important question:

Q. How can I help?
A. I would have likely had no time to work on this project for the past
few months had it not been for the generous sponsorship of the OpenBSD
Foundation. Your donations made that possible, and other projects like
this. So, you can help by donating. If you think a hypervisor in OpenBSD
is something you'd like to take advantage of, I urge you to go make a
donation right now.

http://www.openbsdfoundation.org/donations.html

---


And finally, a sample boot:

Enable vmm mode (ala apm/apmd):
# vmmctl -e
cpu0: entered VMM mode

Start a VM:
# vmmctl -S -c openbsd_amd64.vmrc
/dev/ttyp0

Attach to VM console:
# cu -l /dev/ttyp0
exit save va/pa  0xf000bcff000 0xbcff000
exit load va/pa  0xf000bd1e000 0xbd1e000
entry load va/pa 0xff000bd2c000 0xbd2c000
vmd: new vm with id 1 created
loading 64 bit kernel
warning: bcopy during ELF kernel load not supported
warning: bcopy during ELF kernel load not supported
mark[start] = 0x100
mark[entry] = 0x1000160
mark[nsym] = 0x1
mark[sym] = 0x1939000
mark[end] = 0x1a1de00
mark[random] = 0x18a4000
mark[erandom] = 0x18a6408
vmd: all vcpu run threads for vm 3 launched
vcpu_run_vmx: yielding the cpu
vmd: waiting on thread 0
vcpu_run_vmx: yielding the cpu
vcpu_run_vmx: yielding the cpu
loading 0x1a44000-0x400 (0x1a44-0x4000)
loading 0x10-0x100
avail_start = 0x26000
avail_end = 0x400
First_avail = 0x1a44000
[ bsd ELF symbol table not valid: bad magic ]
[ no symbol table formats found ]
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2015 OpenBSD. All rights reserved.  http://www.OpenBSD.org

OpenBSD 5.8-current (GENERIC) #204: Mon Aug 31 01:13:40 PDT 2015

mlar...@miskatonic.azathoth.net:/export/bin/src/OpenBSD/vmm/src/sys/arch/amd64/compile/GENERIC
vmd: i8253 PIT: 16 bit counter I/O not supported
RTC BIOS diagnostic error ff
real mem = 50331648 (48MB)
avail mem = 45101056 (43MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0
acpi at bios0 not configured
cpu0 at mainbus0: (uniprocessor)
cpu0:

Re: synaptics touchpads: w mode and resolution

2015-08-31 Thread Alexandr Shadchin
On Sat, Aug 29, 2015 at 6:10 PM, Ulf Brosziewski <
ulf.brosziew...@t-online.de> wrote:

> On 08/29/2015 01:13 PM, Alexandr Shadchin wrote:
>
>> On Fri, Aug 28, 2015 at 10:04:51PM +0200, Ulf Brosziewski wrote:
>>
>>>
>>> Some weeks ago a change was made in pms to support Synaptics touchpads
>>> that don't provide W mode. I assume that only fairly old models are
>>> concerned, and that the variant in the patch below is more accurate. It
>>> only fakes W values where necessary.
>>>
>>> The patch contains a second change, a check whether the resolution query
>>> was successful. According to the "Synaptics PS/2 Interfacing Guide", bit
>>> 7 of the second response byte will be set in case of success. It seems
>>> that if it isn't set, the other bytes are garbage or have an unknown
>>> meaning.
>>>
>>> Unfortunately I only have that old model - with firmware 4.1 - for
>>> testing. Could someone confirm that newer models aren't affected by
>>> these changes?
>>>
>>>
>>>
>> Tested on X201 (Synaptics touchpad, firmware 7.4), no regress.
>>
>> See comment below.
>>
>> Index: dev/pckbc/pms.c
>>> ===
>>> RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
>>> retrieving revision 1.65
>>> diff -u -p -r1.65 pms.c
>>> --- dev/pckbc/pms.c 23 Aug 2015 04:45:23 -  1.65
>>> +++ dev/pckbc/pms.c 28 Aug 2015 18:04:45 -
>>> @@ -949,8 +949,10 @@ synaptics_get_hwinfo(struct pms_softc *s
>>> return (-1);
>>> }
>>>
>>> -   syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution);
>>> -   syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution);
>>> +   if (syn->resolution&  0x8000) {
>>>
>>
>> I think it makes sense to define SYNAPTICS_RESOLUTION_VALID
>> instead of magic numbers.
>>
>> +   syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution);
>>> +   syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution);
>>> +   }
>>> [...]
>>>
>>
>
> Thanks a lot. I have added the constant to pmsreg.h, this is the
> updated diff:
>
>
ok shadchin@


> Index: dev/pckbc/pms.c
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 pms.c
> --- dev/pckbc/pms.c 23 Aug 2015 04:45:23 -  1.65
> +++ dev/pckbc/pms.c 29 Aug 2015 12:33:57 -
> @@ -949,8 +949,10 @@ synaptics_get_hwinfo(struct pms_softc *s
> return (-1);
> }
>
> -   syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution);
> -   syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution);
> +   if (syn->resolution & SYNAPTICS_RESOLUTION_VALID) {
>
> +   syn->res_x = SYNAPTICS_RESOLUTION_X(syn->resolution);
> +   syn->res_y = SYNAPTICS_RESOLUTION_Y(syn->resolution);
> +   }
> syn->min_x = SYNAPTICS_XMIN_BEZEL;
> syn->min_y = SYNAPTICS_YMIN_BEZEL;
> syn->max_x = (syn->dimension) ?
> @@ -1163,30 +1165,21 @@ pms_proc_synaptics(struct pms_softc *sc)
>
> w = ((sc->packet[0] & 0x30) >> 2) | ((sc->packet[0] & 0x04) >> 1) |
> ((sc->packet[3] & 0x04) >> 2);
> +   z = sc->packet[2];
>
> -   /*
> -* Conform to the encoding understood by
> -* /usr/xenocara/driver/xf86-input-synaptics/src/wsconscomm.c
> -*/
> -   switch (w) {
> -   case 0:
> -   /* fingerwidth 5, numfingers 2 */
> -   break;
> -   case 1:
> -   /* fingerwidth 5, numfingers 3 */
> -   break;
> -   case 5:
> -   /* fingerwidth 5, numfingers 1 */
> -   break;
> -   case 4:
> -   case 8:
> -   /* fingerwidth 4, numfingers 1 */
> -   w = 4;
> -   break;
> -   default:
> -   break;
> +   if ((syn->capabilities & SYNAPTICS_CAP_EXTENDED) == 0) {
> +   /*
> +* Emulate W mode for models that don't provide it. Bit 3
> +* of the w-input signals a touch ("finger"), Bit 2 and
> +* the "gesture" bits 1-0 can be ignored.
> +*/
> +   if (w & 8)
> +   w = 4;
> +   else
> +   z = w = 0;
> }
>
> +
> if ((syn->capabilities & SYNAPTICS_CAP_PASSTHROUGH) && w == 3) {
> synaptics_sec_proc(sc);
> return;
> @@ -1203,7 +1196,6 @@ pms_proc_synaptics(struct pms_softc *sc)
> sc->packet[4];
> y = ((sc->packet[3] & 0x20) << 7) | ((sc->packet[1] & 0xf0) << 4) |
> sc->packet[5];
> -   z = sc->packet[2];
>
> buttons = ((sc->packet[0] & sc->packet[3]) & 0x01) ?
> WSMOUSE_BUTTON(1) : 0;
> Index: dev/pckbc/pmsreg.h
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pmsreg.h,v
> retrieving revision 1.11
> diff -u -p -r1.11 pmsreg.h
> --- dev/pckbc/pmsre

Re: Call for Testing: rtalloc(9) change

2015-08-31 Thread Martin Pieuchot
On 25/08/15(Tue) 12:27, Martin Pieuchot wrote:
> On 12/08/15(Wed) 17:03, Martin Pieuchot wrote:
> > I'm currently working on the routing table interface to make is safe
> > to use by multiple CPUs at the same time.  The diff below is a big
> > step in this direction and I'd really appreciate if people could test
> > it with their usual network setup and report back.
> 
> Updated version to match recent changes.  I'm still looking for test
> reports and reviews.

Thanks to all the testers.  Here's a third version that should fix a
regression reported by phessler@.  The idea is to use rtvalid(9) to
check early if the gateway route is still valid or not.  If it isn't
then call rtalloc(9) again to perform the ARP/NDP resolution.

This diff includes rtvalid(9) and makes use of it in ip{,6}_output(),
more conversions will be done afterward, but these should hopefully
be all the necessary bits to fix the regression.

Tests & reviews are still welcome!

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.226
diff -u -p -r1.226 route.c
--- net/route.c 30 Aug 2015 10:39:16 -  1.226
+++ net/route.c 31 Aug 2015 09:33:27 -
@@ -153,6 +153,7 @@ int rtable_alloc(void ***, u_int);
 intrtflushclone1(struct rtentry *, void *, u_int);
 void   rtflushclone(unsigned int, struct rtentry *);
 intrt_if_remove_rtdelete(struct rtentry *, void *, u_int);
+struct rtentry *rt_match(struct sockaddr *, int, unsigned int);
 
 struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *,
u_int);
@@ -300,19 +301,68 @@ rtable_exists(u_int id)   /* verify table 
return (1);
 }
 
+
+/*
+ * Do the actual checks for rtvalid(9), do not use directly!
+ */
+static inline int
+rt_is_valid(struct rtentry *rt)
+{
+   if (rt == NULL)
+   return (0);
+
+   if ((rt->rt_flags & RTF_UP) == 0)
+   return (0);
+
+   /* Routes attached to stall ifas should be freed. */
+   if (rt->rt_ifa == NULL || rt->rt_ifa->ifa_ifp == NULL)
+   return (0);
+
+   return (1);
+}
+
+/*
+ * Returns 1 if the cached ``rt'' entry is still valid, 0 otherwise.
+ */
+int
+rtvalid(struct rtentry *rt)
+{
+   if (rt_is_valid(rt) == 0)
+   return (0);
+
+   /* Next hop must also be valid. */
+   if ((rt->rt_flags & RTF_GATEWAY) && rt_is_valid(rt->rt_gwroute) == 0)
+   return (0);
+
+   return (1);
+}
+
+/*
+ * Do the actual lookup for rtalloc(9), do not use directly!
+ *
+ * Return the best matching entry for the destination ``dst''.
+ *
+ * "RT_RESOLVE" means that a corresponding L2 entry should
+ *   be added to the routing table and resolved (via ARP or
+ *   NDP), if it does not exist.
+ *
+ * "RT_REPORT" indicates that a message should be sent to
+ *   userland if no matching route has been found or if an
+ *   error occured while adding a L2 entry.
+ */
 struct rtentry *
-rtalloc(struct sockaddr *dst, int flags, unsigned int tableid)
+rt_match(struct sockaddr *dst, int flags, unsigned int tableid)
 {
struct rtentry  *rt;
struct rtentry  *newrt = NULL;
struct rt_addrinfo   info;
-   int  s, error = 0, msgtype = RTM_MISS;
+   int  s, error = 0;
 
-   s = splsoftnet();
 
bzero(&info, sizeof(info));
info.rti_info[RTAX_DST] = dst;
 
+   s = splsoftnet();
rt = rtable_match(tableid, dst);
if (rt != NULL) {
newrt = rt;
@@ -325,10 +375,6 @@ rtalloc(struct sockaddr *dst, int flags,
goto miss;
}
rt = newrt;
-   if (rt->rt_flags & RTF_XRESOLVE) {
-   msgtype = RTM_RESOLVE;
-   goto miss;
-   }
/* Inform listeners of the new route */
rt_sendmsg(rt, RTM_ADD, tableid);
} else
@@ -336,11 +382,8 @@ rtalloc(struct sockaddr *dst, int flags,
} else {
rtstat.rts_unreach++;
 miss:
-   if (ISSET(flags, RT_REPORT)) {
-   bzero((caddr_t)&info, sizeof(info));
-   info.rti_info[RTAX_DST] = dst;
-   rt_missmsg(msgtype, &info, 0, NULL, error, tableid);
-   }
+   if (ISSET(flags, RT_REPORT))
+   rt_missmsg(RTM_MISS, &info, 0, NULL, error, tableid);
}
splx(s);
return (newrt);
@@ -374,6 +417,75 @@ rtalloc_mpath(struct sockaddr *dst, uint
 }
 #endif /* SMALL_KERNEL */
 
+/*
+ * Look in the routing table for the best matching entry for
+ * ``dst''.
+ *
+ * If a route with a gateway is found and its next hop is no
+ * longer valid, try to cache it.
+ */
+struct rtentry *
+rtalloc(struct sockaddr *dst, int flags, unsigned int rta

Re: tap-and-drag on ALPS touchpads

2015-08-31 Thread Alexandr Shadchin
On Sat, Aug 15, 2015 at 4:32 AM, Ulf Brosziewski <
ulf.brosziew...@t-online.de> wrote:

>
> The tap-and-drag gesture doesn't work reliably with the ALPS touchpads
> supported by pms. To make it work, it is necessary to start dragging
> immediately with the second touch, which doesn't always succeed.
> Increasing the tap timeout helps, but doesn't change the principle. The
> patch below solves that problem. Tests and comments would be welcome.
>
> In case someone is interested in the background: I have observed the
> same problem with a Linux installation on a machine with an ALPS
> touchpad; however, increasing the tap timeout to a sufficiently high
> value (>= 260ms) makes it work normally. The somewhat special behaviour
> of those ALPS models is described in this LKML posting:
> https://lkml.org/lkml/2004/7/28/210
> The approach put forward there - which is partially reproduced in the
> current version of pms - is improved by the patch as follows: When the
> hardware signals a tap, the handler does nothing. The missing events
> will be emulated when the next packet arrives, which either signals the
> end of the gesture, or the start of a drag action. No timeout will occur
> even if the hardware introduces a long delay between the first and the
> second packet (which just happens when the finger is resting after the
> second contact).
>
>
>
Now is a good time to commit it :-)
ok shadchin@


> Index: dev/pckbc/pms.c
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 pms.c
> --- dev/pckbc/pms.c 20 Jul 2015 00:55:06 -  1.64
> +++ dev/pckbc/pms.c 14 Aug 2015 18:27:57 -
> @@ -120,7 +120,8 @@ struct alps_softc {
>
> int min_x, min_y;
> int max_x, max_y;
> -   int old_fin;
> +
> +   u_int gesture;
>
> u_int sec_buttons;  /* trackpoint */
>
> @@ -1521,8 +1522,7 @@ pms_proc_alps(struct pms_softc *sc)
>  {
> struct alps_softc *alps = sc->alps;
> int x, y, z, w, dx, dy;
> -   u_int buttons;
> -   int fin, ges;
> +   u_int buttons, gesture;
>
> if ((alps->model & ALPS_DUALPOINT) && alps_sec_proc(sc))
> return;
> @@ -1557,28 +1557,44 @@ pms_proc_alps(struct pms_softc *sc)
> y = ALPS_YMAX_BEZEL - y + ALPS_YMIN_BEZEL;
>
> if (alps->wsmode == WSMOUSE_NATIVE) {
> -   ges = sc->packet[2] & 0x01;
> -   fin = sc->packet[2] & 0x02;
> +   if (alps->gesture == ALPS_TAP) {
> +   /* Report a touch with the tap coordinates. */
> +   wsmouse_input(sc->sc_wsmousedev, buttons,
> +   alps->old_x, alps->old_y, ALPS_PRESSURE, 4,
> +   WSMOUSE_INPUT_ABSOLUTE_X
> +   | WSMOUSE_INPUT_ABSOLUTE_Y
> +   | WSMOUSE_INPUT_ABSOLUTE_Z
> +   | WSMOUSE_INPUT_ABSOLUTE_W);
> +   if (z > 0) {
> +   /*
> +* The hardware doesn't send a null
> pressure
> +* event when dragging starts.
> +*/
> +   wsmouse_input(sc->sc_wsmousedev, buttons,
> +   alps->old_x, alps->old_y, 0, 0,
> +   WSMOUSE_INPUT_ABSOLUTE_X
> +   | WSMOUSE_INPUT_ABSOLUTE_Y
> +   | WSMOUSE_INPUT_ABSOLUTE_Z
> +   | WSMOUSE_INPUT_ABSOLUTE_W);
> +   }
> +   }
>
> -   /* Simulate click (tap) */
> -   if (ges && !fin)
> -   z = 35;
> -
> -   /* Generate a null pressure event (needed for tap & drag)
> */
> -   if (ges && fin && !alps->old_fin)
> -   z = 0;
> -
> -   /* Generate a width value corresponding to one finger */
> -   if (z > 0)
> -   w = 4;
> -   else
> -   w = 0;
> +   gesture = sc->packet[2] & 0x03;
> +   if (gesture != ALPS_TAP) {
> +   w = z ? 4 : 0;
> +   wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z,
> w,
> +   WSMOUSE_INPUT_ABSOLUTE_X
> +   | WSMOUSE_INPUT_ABSOLUTE_Y
> +   | WSMOUSE_INPUT_ABSOLUTE_Z
> +   | WSMOUSE_INPUT_ABSOLUTE_W);
> +   }
>
> -   wsmouse_input(sc->sc_wsmousedev, buttons, x, y, z, w,
> -   WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y |
> -   WSMOUSE_INPUT_ABSOLUTE_Z | WSMOUSE_INPUT_ABSOLUTE_W);
> +   if (alps->gesture != ALPS_DRAG || gesture != ALPS_TAP)
> +   alps

Re: rt_ifa_del() tweak

2015-08-31 Thread Martin Pieuchot
On 25/08/15(Tue) 13:01, Martin Pieuchot wrote:
> I want to remove this chunked introduce in r7.19 in 1991 by sklower@
> because it no longer makes any sense, it is a layer violation and
> does no play well with rt refcounting.
> 
> When this chunk was introduced rtrequest1(RTM_DELETE...) was *not* doing
> a route lookup.  So this check is now (and since a long time) redundant
> with the rtable_lookup()+rtable_mpath_match() done inside rtrequest1(9).
> 
> Remember that when this code was introduced ``ifa'' where linked to
> ``rt'' in 1:1 fashion and rtrequest() started by rtfree(9)ing route
> entries. That's why you wanted to be sure that the following line was
> correct:
> 
>   error = rtrequest(cmd, dst, ifa->ifa_addr, ifa->ifa_netmask,
>   flags | ifa->ifa_flags, &ifa->ifa_rt)
> 
> Ok?

Anyone?

> Index: net/route.c
> ===
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.225
> diff -u -p -r1.225 route.c
> --- net/route.c   24 Aug 2015 22:11:33 -  1.225
> +++ net/route.c   25 Aug 2015 10:43:32 -
> @@ -1230,21 +1230,6 @@ rt_ifa_del(struct ifaddr *ifa, int flags
>   rt_maskedcopy(dst, deldst, ifa->ifa_netmask);
>   dst = deldst;
>   }
> - if ((rt = rtalloc(dst, 0, rtableid)) != NULL) {
> - rt->rt_refcnt--;
> -#ifndef ART
> - /* try to find the right route */
> - while (rt && rt->rt_ifa != ifa)
> - rt = (struct rtentry *)
> - ((struct radix_node *)rt)->rn_dupedkey;
> - if (!rt) {
> - if (m != NULL)
> - (void) m_free(m);
> - return (flags & RTF_HOST ? EHOSTUNREACH
> - : ENETUNREACH);
> - }
> -#endif
> - }
>  
>   memset(&info, 0, sizeof(info));
>   info.rti_ifa = ifa;
> 



Re: uow patch

2015-08-31 Thread Martin Pieuchot
On 30/08/15(Sun) 09:31, John L. Scarfone wrote:
> On Sun, Aug 30, 2015 at 12:32:24PM +0200, Martin Pieuchot stated:
> > That's good but the original design of allocating an xfer for every
> > usbd_transfer(9) call does not make much sense.  
> > 
> > You could allocate two xfers (one for read and one for write) at the
> > same time the pipes are opened and free them when they're closed.
> > 
> > You could even allocate the DMA buffer with usb_alloc_buffer() and pass
> > the USBD_NO_COPY flag to usbd_transfer(9) to avoid a supplementary copy.
> 
> Thanks for the feedback, Martin. I've been running with your first
> suggestion for a bit with no issues. I wonder though if the xfer can't
> just be shared for rx and tx?

You cannot easily do that because the CPU executing usbd_transfer(9)
with a USBD_SYNCHRONOUS flag might sleep.  So if you try to submit
another transfer at this moment using the same "xfer" descriptor bad
things will happen ;)

>   I also NULLed out the pipes on failure
> since it looks like otherwise they'd be re-closed on detach. I'll look
> into the usbd_alloc_buffer() idea.

That'd great, thanks.

I've just committed your diff, good work.



Re: [PATCH] PF: cksum modification & refactor [0/24]

2015-08-31 Thread Alexandr Nedvedicky
Hello,

I'm fine with this change. It certainly posses no thread to SMP branch...

> * Initialise pd->pcksum for icmp6 
> 
>   - ensures pcksum is set for all known checksummed protocols   
>   - later patches rely on this 
may be this patch/change should be folded to patch, which
really expects pd->pcksum to be set to address of icmp6_cksum field.
just to keep things sane in CVS tree.

regards
sasha



Re: remove RH0 support from ping6(8)

2015-08-31 Thread Mike Belopuhov
On 30 August 2015 at 15:44, Florian Obser  wrote:
> RH0 has been deprecated for quite some time now in RFC 5095. It's
> quite useless on OpenBSD since our stack unconditionally drops packets
> with a RH0 header so you can't get the packet out anyway.
> And last but not least it might get in the way if I ever manage to
> unify ping(8) and ping6(8).
>
> OK?
>

Aye! Kill it with fire.



Re: pool allocator names

2015-08-31 Thread Ted Unangst
Mark Kettenis wrote:
> > From: "Ted Unangst" 
> > Date: Sat, 29 Aug 2015 18:38:45 -0400
> > 
> > Mark Kettenis wrote:
> > > This diff is purely mechanical.  This means that it also changes some
> > > pool_allocator_nointr into pool_allocator_single where the intention
> > > was to signal that the pool would never be used in interrupt context.
> > > However, using pool_allocator_single in those cases isn't a big deal
> > > as there is no downside.  But we could consider changing those into
> > > passing NULL and setting the PR_WAITOK flag to the pool_init calls in
> > > question.
> > 
> > I would prefer we do this (NULL and WAITOK). I've done some before as I came
> > across them, but didn't scan the whole tree.
> 
> This converts sys/isofs.  These pools are clearly not used in
> interrupt handlers since all pool_get calls specify PR_WAITOK already.
> 
> ok?

ok



Re: ntpd(8): Make -n quieter

2015-08-31 Thread Sebastian Benoit
Michael Reed(m.r...@mykolab.com) on 2015.08.30 14:58:35 -0400:
> Hi all,
> 
> If ntpd is run with the -n flag, and /etc/ntpd.conf is parsed without
> error, then "Configuration OK" is printed.  I don't think this is
> particularly useful, as both a lack of an error message and an exit
> value of 0 already indicate success in this case.  This seems to be the
> case for most (many?) programs in the base system, such as doas(1).

I like the message. Why is it a problem?

/Benno



Like free(), ksh's afree() is NULL-safe

2015-08-31 Thread Michael McConville
Also, why is fs on line 390 cast to char* when afree() takes void*?


Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.50
diff -u -p -r1.50 emacs.c
--- emacs.c 25 Mar 2015 12:10:52 -  1.50
+++ emacs.c 31 Aug 2015 04:02:29 -
@@ -1307,8 +1307,7 @@ static void
 kb_del(struct kb_entry *k)
 {
TAILQ_REMOVE(&kblist, k, entry);
-   if (k->args)
-   free(k->args);
+   free(k->args);
afree(k, AEDIT);
 }
 
Index: var.c
===
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.42
diff -u -p -r1.42 var.c
--- var.c   19 Aug 2015 16:05:46 -  1.42
+++ var.c   31 Aug 2015 04:02:29 -
@@ -385,8 +385,7 @@ setstr(struct tbl *vq, const char *s, in
vq->flag |= ISSET;
if ((vq->flag&SPECIAL))
setspec(vq);
-   if (fs)
-   afree((char *)fs, ATEMP);
+   afree((char *)fs, ATEMP);
return 1;
 }
 
@@ -576,8 +575,7 @@ export(struct tbl *vp, const char *val)
*xp++ = '=';
vp->type = xp - vp->val.s; /* offset to value */
memcpy(xp, val, vallen);
-   if (op != NULL)
-   afree((void*)op, vp->areap);
+   afree((void*)op, vp->areap);
 }
 
 /*
@@ -704,8 +702,7 @@ typeset(const char *var, Tflag set, Tfla
t->type = 0;
}
}
-   if (free_me)
-   afree((void *) free_me, t->areap);
+   afree((void *) free_me, t->areap);
}
}
if (!ok)
@@ -952,8 +949,7 @@ setspec(struct tbl *vp)
 
switch (special(vp->name)) {
case V_PATH:
-   if (path)
-   afree(path, APERM);
+   afree(path, APERM);
path = str_save(str_val(vp), APERM);
flushcom(1);/* clear tracked aliases */
break;
@@ -1059,8 +1055,7 @@ unsetspec(struct tbl *vp)
 {
switch (special(vp->name)) {
case V_PATH:
-   if (path)
-   afree(path, APERM);
+   afree(path, APERM);
path = str_save(def_path, APERM);
flushcom(1);/* clear tracked aliases */
break;



Re: [PATCH] PF: cksum modification & refactor [3/24]

2015-08-31 Thread Alexandr Nedvedicky
Hello Richard,

the code in patch looks good for the first glance. However it seems to me
the newly introduced pf_cksum_fixup*() are not called yet.  Do you think you
can reshuffle changes between your set of patches a bit, so the newly
introduced functions will become alive (get called)?

Also I think your patch 0/24, you've sent earlier, can be fold here (setting
pd->pcksum to point to icmp6 header chksum field). 

thanks a lot
regards
sasha