Re: if attach/detach netlocks

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 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: More NET_LOCK()

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: mld6 & splsoftnet()

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: if attach/detach netlocks

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

Re: More NET_LOCK()

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: PATCH: Tell what trailing asterisk in sa output means

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: mld6 & splsoftnet()

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: sleep.1: "while true;" -> "while :;"

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: sleep.1: "while true;" -> "while :;"

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: mld6 & splsoftnet()

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

loadfirmware.9: mention /mnt/etc/firmware

/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: loadfirmware.9: mention /mnt/etc/firmware

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 >

Re: if attach/detach netlocks

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

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

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: inet6 ioctl(2) and splsoftnet()

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 >

socket splicing sleeps in task thread

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: [patch] Minor corrections to xenocara man pages.

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: PATCH: Tell what trailing asterisk in sa output means

> 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

NET_LOCK() pr_sysctl

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

mld6 & splsoftnet()

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

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

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

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@

carp(4) fix vs NET_LOCK

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

Re: tcp_ctloutput() and splsoftnet()

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: NET_LOCK() pflow panic

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: bioctl: don't print uninitialized memory

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 >

Re: carp(4) fix vs NET_LOCK

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: NET_LOCK() pflow panic

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 >

bioctl: don't print uninitialized memory

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

More NET_LOCK()

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

Re: tcp_ctloutput() and splsoftnet()

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 >

NET_LOCK() pflow panic

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

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

PATCH: Tell what trailing asterisk in sa output means

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: carp(4) fix vs NET_LOCK

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 >

if attach/detach netlocks

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: if attach/detach netlocks

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