socket splicing sleeps in task thread

2016-12-20 Thread Alexander Bluhm
Hi, As NET_LOCK() is a read/write lock, it can sleep in sotask(). So the TASKQ_CANTSLEEP flag is no longer valid for the splicing thread. ok? bluhm Index: kern/uipc_socket.c === RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc

Re: if attach/detach netlocks

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 06:47:31PM +0100, Mike Belopuhov wrote: > @@ -1109,11 +1115,16 @@ if_clone_destroy(const char *name) > s = splnet(); > if_down(ifp); > splx(s); > } > > - return ((*ifc->ifc_destroy)(ifp)); > + /* XXXSMP breaks atomici

Re: sleep.1: "while true;" -> "while :;"

2016-12-20 Thread sven falempin
On Tue, Dec 20, 2016 at 5:49 PM, Jason McIntyre wrote: > On Tue, Dec 20, 2016 at 10:58:40PM +0100, Michal Mazurek wrote: > > While there is nothing wrong with "while true", "while :" is better > > and used a lot more often in the source tree. > > > > OK? > > > > i'm not sure why "while :" is bett

Re: sleep.1: "while true;" -> "while :;"

2016-12-20 Thread Jason McIntyre
On Tue, Dec 20, 2016 at 10:58:40PM +0100, Michal Mazurek wrote: > While there is nothing wrong with "while true", "while :" is better > and used a lot more often in the source tree. > > OK? > i'm not sure why "while :" is better in this example, but "while true" is clearer, i think. allowing for

Re: mld6 & splsoftnet()

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 09:42:00PM +0100, Martin Pieuchot wrote: > On 20/12/16(Tue) 20:01, Alexander Bluhm wrote: > > On Tue, Dec 20, 2016 at 12:14:22PM +0100, Martin Pieuchot wrote: > > > mld6_{start,stop}_listeting() are called in ioctl(2) context, thus at > > > IPL_SOFTNET. > > > > These are ca

Re: inet6 ioctl(2) and splsoftnet()

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 11:50:40AM +0100, Martin Pieuchot wrote: > Interface ioctl(2)s are now always executed at IPL_SOFTNET, so remove > recursive splsoftnet() calls. Could you put an splsoftassert into in6_addmulti() and in6_delmulti()? > ok? anyway OK bluhm@ > > Index: netinet6/in6.c > ===

sleep.1: "while true;" -> "while :;"

2016-12-20 Thread Michal Mazurek
While there is nothing wrong with "while true", "while :" is better and used a lot more often in the source tree. OK? Index: bin/sleep/sleep.1 === RCS file: /cvs/src/bin/sleep/sleep.1,v retrieving revision 1.22 diff -u -p -r1.22 slee

Re: loadfirmware.9: mention /mnt/etc/firmware

2016-12-20 Thread Theo de Raadt
I do not think this should be documented. It is a trick. Noone should rely upon it, because later on we might use a different trick. Then this documentation would be come false, and it is another piece for us to worry about. > /mnt/etc/firmware is only mentioned in plus44.html. Mention it in >

loadfirmware.9: mention /mnt/etc/firmware

2016-12-20 Thread Michal Mazurek
/mnt/etc/firmware is only mentioned in plus44.html. Mention it in loadfirmware(9). It was concluded that the FAQ is not a proper place to talk about this directory. Index: ./share/man/man9/loadfirmware.9 === RCS file: /cvs/src/share/

Re: PATCH: Tell what trailing asterisk in sa output means

2016-12-20 Thread Jason McIntyre
On Tue, Dec 20, 2016 at 09:27:14PM +0300, Vadim Zhukov wrote: > 2016-12-20 20:30 GMT+03:00 Theo de Raadt : > >> I had to dig into sa sources to get know what trailing '*' in > >> command names mean. IMHO, that's not how manuals should work. :) > >> > >> Okay? > > > > That seems a little awkward men

Re: More NET_LOCK()

2016-12-20 Thread Martin Pieuchot
On 20/12/16(Tue) 19:51, Alexander Bluhm wrote: > [...] > The sosplice thread is only used for TCP. UDP runs in the softnet > thread. As we are reading 64 bit values, we need some sort of > locking. Now everything is in the kernel lock, but when we start > working to unlock the socket layer, be

Re: mld6 & splsoftnet()

2016-12-20 Thread Martin Pieuchot
On 20/12/16(Tue) 20:01, Alexander Bluhm wrote: > On Tue, Dec 20, 2016 at 12:14:22PM +0100, Martin Pieuchot wrote: > > mld6_{start,stop}_listeting() are called in ioctl(2) context, thus at > > IPL_SOFTNET. > > These are called from in6_addmulti() and in6_delmulti(). They still > have a splsoftnet(

Re: mld6 & splsoftnet()

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 12:14:22PM +0100, Martin Pieuchot wrote: > mld6_{start,stop}_listeting() are called in ioctl(2) context, thus at > IPL_SOFTNET. These are called from in6_addmulti() and in6_delmulti(). They still have a splsoftnet() in them. Please remove this first and put an assert ther

Re: More NET_LOCK()

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 01:55:03PM +0100, Martin Pieuchot wrote: > rzalamena@ found that the IPv4 multicast code have two code paths ending > in ip_output() that are not yet taking the NET_LOCK: sosetopt() and > pffasttimo(). I see this also on my regress test machine. splassert: ip_output: want

Re: if attach/detach netlocks

2016-12-20 Thread Martin Pieuchot
On 20/12/16(Tue) 18:47, Mike Belopuhov wrote: > On Tue, Dec 20, 2016 at 17:06 +0100, Martin Pieuchot wrote: > > > > You'll need to release the lock before calling ifc->ifc_create in > > if_clone_create() and do the same dance in if_clone_destroy(). > > > > Indeed. > > > But I think that's the wa

Re: PATCH: Tell what trailing asterisk in sa output means

2016-12-20 Thread Vadim Zhukov
2016-12-20 20:30 GMT+03:00 Theo de Raadt : >> I had to dig into sa sources to get know what trailing '*' in >> command names mean. IMHO, that's not how manuals should work. :) >> >> Okay? > > That seems a little awkward mentioning fork directly. How about > > Children which have not yet called > .

Re: if attach/detach netlocks

2016-12-20 Thread Mike Belopuhov
On Tue, Dec 20, 2016 at 17:06 +0100, Martin Pieuchot wrote: > > You'll need to release the lock before calling ifc->ifc_create in > if_clone_create() and do the same dance in if_clone_destroy(). > Indeed. > But I think that's the way to go. If you can create/destroy > pseudo-interface without p

NET_LOCK() pr_sysctl

2016-12-20 Thread Alexander Bluhm
Hi, I have seen spl softnet assert failures like this. splassert: sowwakeup: want 1 have 0 Starting stack trace... sowwakeup(1,0,d09d8bd7,d03ba9f9,40) at sowwakeup+0x3f sowwakeup(db549818,d6dc850e,f54d9be8,0,dae674d4) at sowwakeup+0x3f soisdisconnected(db549818,0,db549818,2,f54d9c00) at soisdisco

Re: PATCH: Tell what trailing asterisk in sa output means

2016-12-20 Thread Theo de Raadt
> I had to dig into sa sources to get know what trailing '*' in > command names mean. IMHO, that's not how manuals should work. :) > > Okay? That seems a little awkward mentioning fork directly. How about Children which have not yet called .Xr execve 2 will have .Sq * appended to their command

Re: [patch] Minor corrections to xenocara man pages.

2016-12-20 Thread Ingo Schwarze
Hi Salvador, thanks for noticing defects and providing patches! Salvador Sabaini wrote on Wed, Dec 14, 2016 at 12:55:08PM -0300: > I've just found man references in the SEE ALSO section to xfs(1), > the X font server which was unlinked from base in 5.7, in the > following man pages: > > fslsfon

Re: if attach/detach netlocks

2016-12-20 Thread Martin Pieuchot
On 20/12/16(Tue) 16:33, Mike Belopuhov wrote: > Attach: > > splassert: sowakeup: want 1 have 0 > Starting stack trace... > sowakeup() at sowakeup+0x41 > sorwakeup() at sorwakeup+0xb4 > route_input() at route_input+0x2d7 > if_attach() at if_attach+0x58 > xnf_attach() at xnf_attach+0x4

if attach/detach netlocks

2016-12-20 Thread Mike Belopuhov
Attach: splassert: sowakeup: want 1 have 0 Starting stack trace... sowakeup() at sowakeup+0x41 sorwakeup() at sorwakeup+0xb4 route_input() at route_input+0x2d7 if_attach() at if_attach+0x58 xnf_attach() at xnf_attach+0x45f config_attach() at config_attach+0x1bc xen_attach_device(

Re: bioctl: don't print uninitialized memory

2016-12-20 Thread Mike Belopuhov
On 20 December 2016 at 16:03, Patrick Wildt wrote: > Hi, > > I just realized that bioctl can print its uninitialized memory. The > function bio_status() prints information from a struct basically after > each ioctl(). The BIOCLOCATE ioctl() though never sets this in the > struct. Thus each BIOC

bioctl: don't print uninitialized memory

2016-12-20 Thread Patrick Wildt
Hi, I just realized that bioctl can print its uninitialized memory. The function bio_status() prints information from a struct basically after each ioctl(). The BIOCLOCATE ioctl() though never sets this in the struct. Thus each BIOCLOCATE ioctl() keeps the struct in the state as it has been bef

Re: NET_LOCK() pflow panic

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 03:28:38PM +0100, Martin Pieuchot wrote: > I don't have a solution for the moment and I want to be sure we know all Me neither. That's why I did not send a diff, but only the stack trace. > recursions before trying to write a fix. So here's a diff that mark the > recursi

Re: NET_LOCK() pflow panic

2016-12-20 Thread Martin Pieuchot
On 20/12/16(Tue) 14:50, Alexander Bluhm wrote: > Hi, > > This crash happened during regress/sys/net/pflow on my regression test > machine: > > panic: rw_enter: netlock locking against myself > Stopped at Debugger+0x7: leave >TIDPIDUID PRFLAGS PFLAGS CPU COMMAND > *291

Re: carp(4) fix vs NET_LOCK

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 03:09:11PM +0100, Martin Pieuchot wrote: > No we can't because we're already holding it. So carp_master_down() > must be split in two, diff below does that and only grab the lock in > the timer routine. OK bluhm@ > > Index: netinet/ip_carp.c > ===

Re: carp(4) fix vs NET_LOCK

2016-12-20 Thread Martin Pieuchot
On 20/12/16(Tue) 14:31, Alexander Bluhm wrote: > On Tue, Dec 20, 2016 at 12:08:57PM +0100, Martin Pieuchot wrote: > > I introduced a bug in carp(4), one code path tries to take the NET_LOCK() > > twice: > > > > panic() at panic+0xfe > > rw_enter() at rw_enter+0x1f8 > > carp_send_ad() a

NET_LOCK() pflow panic

2016-12-20 Thread Alexander Bluhm
Hi, This crash happened during regress/sys/net/pflow on my regression test machine: panic: rw_enter: netlock locking against myself Stopped at Debugger+0x7: leave TIDPIDUID PRFLAGS PFLAGS CPU COMMAND *291613 71462 0 0x2 01 ifconfig 96946 6

Re: carp(4) fix vs NET_LOCK

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 12:08:57PM +0100, Martin Pieuchot wrote: > I introduced a bug in carp(4), one code path tries to take the NET_LOCK() > twice: > > panic() at panic+0xfe > rw_enter() at rw_enter+0x1f8 > carp_send_ad() at carp_send_ad+0x4d > carp_vhe_send_ad_all() at c

Re: tcp_ctloutput() and splsoftnet()

2016-12-20 Thread Alexander Bluhm
On Tue, Dec 20, 2016 at 12:11:40PM +0100, Martin Pieuchot wrote: > Kill a recursive splsoftnet()/splx() dance, ``pr_ctloutput'' routines > are now always called at IPL_SOFTNET. > > ok? OK bluhm@ > > Index: netinet/tcp_usrreq.c > ==

More NET_LOCK()

2016-12-20 Thread Martin Pieuchot
rzalamena@ found that the IPv4 multicast code have two code paths ending in ip_output() that are not yet taking the NET_LOCK: sosetopt() and pffasttimo(). Diff below fixes that and include pfslowtimo() as well as sogetopt() for completeness. While here remove a superfluous splsoftnet() which does

PATCH: Tell what trailing asterisk in sa output means

2016-12-20 Thread Vadim Zhukov
Hi all. I had to dig into sa sources to get know what trailing '*' in command names mean. IMHO, that's not how manuals should work. :) Okay? -- WBR, Vadim Zhukov Index: sa.8 === RCS file: /cvs/src/usr.sbin/sa/sa.8,v retrieving r

Re: tcp_ctloutput() and splsoftnet()

2016-12-20 Thread Mike Belopuhov
On 20 December 2016 at 12:11, Martin Pieuchot wrote: > Kill a recursive splsoftnet()/splx() dance, ``pr_ctloutput'' routines > are now always called at IPL_SOFTNET. > > ok? > Yes, please.

Re: mpe(4), mpw(4) and splsoftnet()

2016-12-20 Thread Rafael Zalamena
On Mon, Dec 19, 2016 at 11:48:31AM +0100, Martin Pieuchot wrote: > Interface ioctl(2) are now always run at IPL_SOFTNET, so let's get rid > of recursive splsoftnet()/splx() dances. > > ok? ok rzalamena@

mld6 & splsoftnet()

2016-12-20 Thread Martin Pieuchot
mld6_{start,stop}_listeting() are called in ioctl(2) context, thus at IPL_SOFTNET. mld6_fasttimeo(), like all *fasttimeo() routines is now always called at IPL_SOFTNET. So let's get rid of these recursive splsoftnet(), ok? Index: netinet6/mld6.c ==

tcp_ctloutput() and splsoftnet()

2016-12-20 Thread Martin Pieuchot
Kill a recursive splsoftnet()/splx() dance, ``pr_ctloutput'' routines are now always called at IPL_SOFTNET. ok? Index: netinet/tcp_usrreq.c === RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v retrieving revision 1.137 diff -u -p -r1.13

carp(4) fix vs NET_LOCK

2016-12-20 Thread Martin Pieuchot
I introduced a bug in carp(4), one code path tries to take the NET_LOCK() twice: panic() at panic+0xfe rw_enter() at rw_enter+0x1f8 carp_send_ad() at carp_send_ad+0x4d carp_vhe_send_ad_all() at carp_vhe_send_ad_all+0x4b carp_ioctl() at carp_ioctl+0x491

inet6 ioctl(2) and splsoftnet()

2016-12-20 Thread Martin Pieuchot
Interface ioctl(2)s are now always executed at IPL_SOFTNET, so remove recursive splsoftnet() calls. ok? Index: netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.195 diff -u -p -r1.195 in6.c --- netinet6

cpio fix

2016-12-20 Thread Mark Kettenis
Currently cpio doesn't handle archives that are smaller than 512 bytes very well. The format detection code tries rather hard to read up to 512 bytes, triggering a "next volume" prompt. NetBSD fixed this issue some time ago: revision 1.26 date: 2005-04-24 03:26:03 +; author: christos; stat