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:

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

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

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

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

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

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:

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

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

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

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

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

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

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

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: > >

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

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

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 >

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

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 >

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 >

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()

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

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

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

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

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

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