Re: svn commit: r316874 - head/sys/kern
Maxim Sobolev wrote in : so> I've committed another fix for the syslogd code in question which should so> hopefully make it functional again. Peter, please let me know if you still so> having any issues. Thanks! I am sorry for being late and thank you for fixing the breakage in the refactored code. I tried not to change the original code path but it seemed I added some bugs. I will double-check and clean up them. -- Hiroki pgpBiulC5hA_P.pgp Description: PGP signature
Re: svn commit: r316874 - head/sys/kern
Am Sat, 15 Apr 2017 14:22:57 -0500 Larry Rosenman schrieb: > This looks MUCH better, startup was it’s usual speedy self. > > > > > ... same here with FreeBSD 12.0-CURRENT #23 r316999: Sun Apr 16 07:28:14 CEST 2017 amd64. Systems boot as usual again - speedy and reliable. Regards, Oliver -- O. Hartmann Ich widerspreche der Nutzung oder Übermittlung meiner Daten für Werbezwecke oder für die Markt- oder Meinungsforschung (§ 28 Abs. 4 BDSG). pgp8v8lEYzBuu.pgp Description: OpenPGP digital signature
Re: svn commit: r316874 - head/sys/kern
This looks MUCH better, startup was it’s usual speedy self. -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 214-642-9640 E-Mail: l...@lerctr.org US Mail: 17716 Limpia Crk, Round Rock, TX 78664-7281 From: Maksym Sobolyev on behalf of Maxim Sobolev Date: Saturday, April 15, 2017 at 1:22 PM To: "O. Hartmann" Cc: Larry Rosenman , Peter Wemm , src-committers , , Hiroki Sato , Hiren Panchasara , , Ngie Cooper Subject: Re: svn commit: r316874 - head/sys/kern I've committed another fix for the syslogd code in question which should hopefully make it functional again. Peter, please let me know if you still having any issues. Thanks! -Max On Sat, Apr 15, 2017 at 8:46 AM, O. Hartmann wrote: Am Sat, 15 Apr 2017 08:05:23 -0500 Larry Rosenman schrieb: > On 4/15/17, 6:00 AM, "Maxim Sobolev" of > sobo...@freebsd.org> wrote: > > Peter, Ngie, none of this stuff is really directly related to the > shutdown(2) change, so I'll probably let Hiroki to clean it up. > > -Max > > > I’ve backed off to my previous root. Is someone working on this? It’s > PAINFUL. > [...] Processes do not even hang when system is booting/spinning up. On my router project, I've running asterisk13 on a small appliance. Starting asterisk with "service asterisk start" starts the service, but then, stopping the service calling "service asterisk stop" reports Asterisk ending (0). but the process is still running as top reveals: PID USERNAME THR PRI NICE SIZERES STATE C TIMEWCPU COMMAN 1352 asterisk19 520 105M 25284K uwrlck 3 17:40 67.94% asteri [...] This is weird :-( kill -9 1352 works. Finally. Regards, Oliver -- O. Hartmann Ich widerspreche der Nutzung oder Übermittlung meiner Daten für Werbezwecke oder für die Markt- oder Meinungsforschung (§ 28 Abs. 4 BDSG). ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
I've committed another fix for the syslogd code in question which should hopefully make it functional again. Peter, please let me know if you still having any issues. Thanks! -Max On Sat, Apr 15, 2017 at 8:46 AM, O. Hartmann wrote: > Am Sat, 15 Apr 2017 08:05:23 -0500 > Larry Rosenman schrieb: > > > On 4/15/17, 6:00 AM, "Maxim Sobolev" behalf of > > sobo...@freebsd.org> wrote: > > > > Peter, Ngie, none of this stuff is really directly related to the > > shutdown(2) change, so I'll probably let Hiroki to clean it up. > > > > -Max > > > > > > I’ve backed off to my previous root. Is someone working on this? It’s > PAINFUL. > > > [...] > > Processes do not even hang when system is booting/spinning up. > > On my router project, I've running asterisk13 on a small appliance. > Starting asterisk > with "service asterisk start" starts the service, but then, stopping the > service calling > "service asterisk stop" reports > > Asterisk ending (0). > > but the process is still running as top reveals: > > PID USERNAME THR PRI NICE SIZERES STATE C TIMEWCPU > COMMAN > 1352 asterisk19 520 105M 25284K uwrlck 3 17:40 67.94% > asteri > [...] > > This is weird :-( > > kill -9 1352 works. Finally. > > Regards, > Oliver > -- > O. Hartmann > > Ich widerspreche der Nutzung oder Übermittlung meiner Daten für > Werbezwecke oder für die Markt- oder Meinungsforschung (§ 28 Abs. 4 BDSG). > ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
Am Sat, 15 Apr 2017 08:05:23 -0500 Larry Rosenman schrieb: > On 4/15/17, 6:00 AM, "Maxim Sobolev" of > sobo...@freebsd.org> wrote: > > Peter, Ngie, none of this stuff is really directly related to the > shutdown(2) change, so I'll probably let Hiroki to clean it up. > > -Max > > > I’ve backed off to my previous root. Is someone working on this? It’s > PAINFUL. > [...] Processes do not even hang when system is booting/spinning up. On my router project, I've running asterisk13 on a small appliance. Starting asterisk with "service asterisk start" starts the service, but then, stopping the service calling "service asterisk stop" reports Asterisk ending (0). but the process is still running as top reveals: PID USERNAME THR PRI NICE SIZERES STATE C TIMEWCPU COMMAN 1352 asterisk19 520 105M 25284K uwrlck 3 17:40 67.94% asteri [...] This is weird :-( kill -9 1352 works. Finally. Regards, Oliver -- O. Hartmann Ich widerspreche der Nutzung oder Übermittlung meiner Daten für Werbezwecke oder für die Markt- oder Meinungsforschung (§ 28 Abs. 4 BDSG). pgpFSh2qjxy1Z.pgp Description: OpenPGP digital signature
Re: svn commit: r316874 - head/sys/kern
On 4/15/17, 6:00 AM, "Maxim Sobolev" wrote: Peter, Ngie, none of this stuff is really directly related to the shutdown(2) change, so I'll probably let Hiroki to clean it up. -Max I’ve backed off to my previous root. Is someone working on this? It’s PAINFUL. ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
Peter, Ngie, none of this stuff is really directly related to the shutdown(2) change, so I'll probably let Hiroki to clean it up. -Max On Fri, Apr 14, 2017 at 11:58 PM, Peter Wemm wrote: > On Friday, April 14, 2017 08:13:52 PM Ngie Cooper wrote: > > > On Apr 14, 2017, at 20:12, Peter Wemm wrote: > > > > > > On Friday, April 14, 2017 07:36:55 PM Peter Wemm wrote: > > >> On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote: > > On Apr 14, 2017, at 14:10, Maxim Sobolev > wrote: > > > > Peter, Ngie, > > > > Looks like out of that refactoring came a logical bug that is > present > > in > > the head, which causes syslod to first to shutdown the socket for > > reading > > and then try to select/recv on it (which is somewhat stupid). And > that > > issue has been masked by shutdown() on datagram socket becoming > > effectively a NOP in 11 & head 20 months ago. It only affects head > > though, 11-stable still has the old code which does not include that > > half-closed socket into the select list. Attached patch is expected > to > > fix head, Peter, it would be nice if you can give it a try > (restoring > > latest changes into uipc_sockets.c) and let me know if it helps. > > > > Thanks! > > >>> > > >>> CCing hrs@ for input as he did the refactoring. > > >>> Thanks! > > >>> -Ngie > > >>> > > >>> PS LGTM with the change. Will wait for feedback from wemm@. > > >> > > >> This is definitely not working. I get ENOSPC and listen queue > overflows > > >> on /var/run/logpriv now. > > >> > > >> Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of > the > > >> 12.x one worked fine, aside from the include statements. > > > > > > This can't be right: > > > if (SecureMode || res->ai_family == AF_LOCAL) { > > > > > >/* Forbid communication in secure mode. */ > > >if (shutdown(s, SHUT_RD) < 0 && > > > > > >errno != ENOTCONN) { > > > > > >logerror("shutdown"); > > >if (!Debug) > > > > > >die(0); > > > > > >} > > >dprintf("listening on socket\n"); > > >sl_recv = NULL; > > > > > > } > > > > > > This appears to disable unix domain sockets like /var/run/log and > > > /var/run/logpriv. > > > > ACK. This looks like a fun bug. > > > -Ngie > > I suspect it's meant to be "if (SecureMode && res->ai_family != AF_LOCAL) > {" > as a simple logic inversion error of another line earlier. However > there's an > awful lot of strange things in this code. > > 1) listen(s, 5) - on datagram sockets. > 2) dprintf("shutdown") in code regardless of whether the shutdown is going > to > happen. > 3) dprintf("listening on socket") in code that only happens when we're NOT > going to listen. > 4) dprintf("sending on socket") in the code path when we're going to > listen. > 5) shutdown on all unix domain sockets, regardless of securemode.. > > This code block makes my head spin. > > -- > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > KI6FJV > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 > ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 08:13:52 PM Ngie Cooper wrote: > > On Apr 14, 2017, at 20:12, Peter Wemm wrote: > > > > On Friday, April 14, 2017 07:36:55 PM Peter Wemm wrote: > >> On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote: > On Apr 14, 2017, at 14:10, Maxim Sobolev wrote: > > Peter, Ngie, > > Looks like out of that refactoring came a logical bug that is present > in > the head, which causes syslod to first to shutdown the socket for > reading > and then try to select/recv on it (which is somewhat stupid). And that > issue has been masked by shutdown() on datagram socket becoming > effectively a NOP in 11 & head 20 months ago. It only affects head > though, 11-stable still has the old code which does not include that > half-closed socket into the select list. Attached patch is expected to > fix head, Peter, it would be nice if you can give it a try (restoring > latest changes into uipc_sockets.c) and let me know if it helps. > > Thanks! > >>> > >>> CCing hrs@ for input as he did the refactoring. > >>> Thanks! > >>> -Ngie > >>> > >>> PS LGTM with the change. Will wait for feedback from wemm@. > >> > >> This is definitely not working. I get ENOSPC and listen queue overflows > >> on /var/run/logpriv now. > >> > >> Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of the > >> 12.x one worked fine, aside from the include statements. > > > > This can't be right: > > if (SecureMode || res->ai_family == AF_LOCAL) { > > > >/* Forbid communication in secure mode. */ > >if (shutdown(s, SHUT_RD) < 0 && > > > >errno != ENOTCONN) { > > > >logerror("shutdown"); > >if (!Debug) > > > >die(0); > > > >} > >dprintf("listening on socket\n"); > >sl_recv = NULL; > > > > } > > > > This appears to disable unix domain sockets like /var/run/log and > > /var/run/logpriv. > > ACK. This looks like a fun bug. > -Ngie I suspect it's meant to be "if (SecureMode && res->ai_family != AF_LOCAL) {" as a simple logic inversion error of another line earlier. However there's an awful lot of strange things in this code. 1) listen(s, 5) - on datagram sockets. 2) dprintf("shutdown") in code regardless of whether the shutdown is going to happen. 3) dprintf("listening on socket") in code that only happens when we're NOT going to listen. 4) dprintf("sending on socket") in the code path when we're going to listen. 5) shutdown on all unix domain sockets, regardless of securemode.. This code block makes my head spin. -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
> On Apr 14, 2017, at 20:12, Peter Wemm wrote: > > On Friday, April 14, 2017 07:36:55 PM Peter Wemm wrote: >> On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote: On Apr 14, 2017, at 14:10, Maxim Sobolev wrote: Peter, Ngie, Looks like out of that refactoring came a logical bug that is present in the head, which causes syslod to first to shutdown the socket for reading and then try to select/recv on it (which is somewhat stupid). And that issue has been masked by shutdown() on datagram socket becoming effectively a NOP in 11 & head 20 months ago. It only affects head though, 11-stable still has the old code which does not include that half-closed socket into the select list. Attached patch is expected to fix head, Peter, it would be nice if you can give it a try (restoring latest changes into uipc_sockets.c) and let me know if it helps. Thanks! >>> >>> CCing hrs@ for input as he did the refactoring. >>> Thanks! >>> -Ngie >>> >>> PS LGTM with the change. Will wait for feedback from wemm@. >> >> This is definitely not working. I get ENOSPC and listen queue overflows on >> /var/run/logpriv now. >> >> Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of the 12.x >> one worked fine, aside from the include statements. > > This can't be right: > if (SecureMode || res->ai_family == AF_LOCAL) { >/* Forbid communication in secure mode. */ >if (shutdown(s, SHUT_RD) < 0 && >errno != ENOTCONN) { >logerror("shutdown"); >if (!Debug) >die(0); >} >dprintf("listening on socket\n"); >sl_recv = NULL; > } > > This appears to disable unix domain sockets like /var/run/log and > /var/run/logpriv. ACK. This looks like a fun bug. -Ngie signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 07:36:55 PM Peter Wemm wrote: > On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote: > > > On Apr 14, 2017, at 14:10, Maxim Sobolev wrote: > > > > > > Peter, Ngie, > > > > > > Looks like out of that refactoring came a logical bug that is present in > > > the head, which causes syslod to first to shutdown the socket for > > > reading > > > and then try to select/recv on it (which is somewhat stupid). And that > > > issue has been masked by shutdown() on datagram socket becoming > > > effectively a NOP in 11 & head 20 months ago. It only affects head > > > though, 11-stable still has the old code which does not include that > > > half-closed socket into the select list. Attached patch is expected to > > > fix head, Peter, it would be nice if you can give it a try (restoring > > > latest changes into uipc_sockets.c) and let me know if it helps. > > > > > > Thanks! > > > > CCing hrs@ for input as he did the refactoring. > > Thanks! > > -Ngie > > > > PS LGTM with the change. Will wait for feedback from wemm@. > > This is definitely not working. I get ENOSPC and listen queue overflows on > /var/run/logpriv now. > > Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of the 12.x > one worked fine, aside from the include statements. This can't be right: if (SecureMode || res->ai_family == AF_LOCAL) { /* Forbid communication in secure mode. */ if (shutdown(s, SHUT_RD) < 0 && errno != ENOTCONN) { logerror("shutdown"); if (!Debug) die(0); } dprintf("listening on socket\n"); sl_recv = NULL; } This appears to disable unix domain sockets like /var/run/log and /var/run/logpriv. -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 02:14:16 PM Ngie Cooper wrote: > > On Apr 14, 2017, at 14:10, Maxim Sobolev wrote: > > > > Peter, Ngie, > > > > Looks like out of that refactoring came a logical bug that is present in > > the head, which causes syslod to first to shutdown the socket for reading > > and then try to select/recv on it (which is somewhat stupid). And that > > issue has been masked by shutdown() on datagram socket becoming > > effectively a NOP in 11 & head 20 months ago. It only affects head > > though, 11-stable still has the old code which does not include that > > half-closed socket into the select list. Attached patch is expected to > > fix head, Peter, it would be nice if you can give it a try (restoring > > latest changes into uipc_sockets.c) and let me know if it helps. > > > > Thanks! > > CCing hrs@ for input as he did the refactoring. > Thanks! > -Ngie > > PS LGTM with the change. Will wait for feedback from wemm@. This is definitely not working. I get ENOSPC and listen queue overflows on /var/run/logpriv now. Grabbing an old 10.3 /usr/sbin/syslogd and placing it on the top of the 12.x one worked fine, aside from the include statements. -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 02:21:05 PM Maxim Sobolev wrote: > Peter, It is actually the other way around. If you take syslogd code out of > 11-stable and earlier that would work just fine with or without r316874. > But since r285910 syslogd in head had been refactored a lot and I think > that particular bug has introduced that has been masked by the shutdown() > on datagram sockets becoming a NOP after r285910. Then r316874 restored our > historical behavior for the shutdown(2) and bingo, bug in the new syslogd > code is now causing it to spin when shutdown() != NOP. > > -Max Hmm, there's a new problem: 45104 auditd CALL socket(PF_LOCAL,0x1002,0) 45104 auditd RET socket 3 45104 auditd CALL connect(0x3,0x7fffdbd8,0x6a) 45104 auditd STRU struct sockaddr { AF_LOCAL, /var/run/logpriv } 45104 auditd NAMI "/var/run/logpriv" 45104 auditd RET connect 0 45104 auditd CALL sendto(0x3,0x7fffe130,0x2f,0,0,0) 45104 auditd RET sendto -1 errno 55 No buffer space available 45104 auditd CALL open(0x800da5c67,0x15) 45104 auditd NAMI "/dev/console" 45104 auditd RET open 4 .. and it all goes to /dev/console instead. On restarting syslogd: Apr 15 02:17:43 repoman2 syslogd: exiting on signal 15 sonewconn: pcb 0xf80051e72680: Listen queue overflow: 16 already in queue already Umm.. did the patch forget to listen to incoming connections or something? I haven't seen this before anywhere except when syslogd is wedged. > > On Fri, Apr 14, 2017 at 12:46 PM, Peter Wemm wrote: > > On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: > > > Thanks, Peter. I will try to look into this asap. > > > > I don't understand what is going on yet. Presumably there must be other > > changes in play that affect udp/select sometime between the original 2015 > > change and this. The syslogd -s code is Old(TM). I'm also wondering > > whether > > the -s code even works at all since the 2015 / r285910 change... > > > > > -Max > > > > > > On Apr 14, 2017 12:32 PM, "Peter Wemm" wrote: > > > > On Friday, April 14, 2017 11:49:26 AM Peter Wemm wrote: > > > > > On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > > > > > > Author: sobomax > > > > > > Date: Fri Apr 14 17:23:28 2017 > > > > > > New Revision: 316874 > > > > > > URL: https://svnweb.freebsd.org/changeset/base/316874 > > > > > > > > > > > > Log: > > > > > > Restore ability to shutdown DGRAM sockets, still forcing > > > > ENOTCONN to > > > > > > be > > > > > > > > > > returned by the shutdown(2) system call. This ability has been > > > > lost as > > > > > > > > part > > > > > > of the svn revision 285910. > > > > > > > > > > > > Reviewed by: ed, rwatson, glebius, hiren > > > > > > MFC after:2 weeks > > > > > > Differential Revision:https://reviews.freebsd.org/D10351 > > > > > > > > > > This appears to have broken syslogd and had a major change in > > > > behavior > > > > > > with > > > > > > > > > regards to select(2). > > > > > > > > > > If you run syslogd with the -s flag (which is default), it now spins > > > > at > > > > > > 100% > > > > > > > > > cpu as all the shutdown sockets now return readable from select. > > > > > > > > > > Old releases / jails also manifest this behavor. If it wasn't for > > > > > losing > > > > > the ability to run old branch binaries I'd suggest changing syslogd > > > > > instead, but we depend on this in the cluster and I expect others do > > > > > too. > > > > > > > > > > I'm not 100% certain that this change is the culprit but a heads-up > > > > > can't > > > > > hurt. I'll try reverting it on the freebsd cluster next, after > > > > > fixing > > > > > the > > > > > broken auditing changes. > > > > > > > > > > -Peter > > > > > > > > I can confirm that reverting r316874 fixes syslogd and backwards > > > > compatability > > > > with old branches. > > > > > > > > -- > > > > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > > > > KI6FJV > > > > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 > > > > -- > > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > > KI6FJV > > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 02:10:56 PM Maxim Sobolev wrote: > Peter, Ngie, > > Looks like out of that refactoring came a logical bug that is present in > the head, which causes syslod to first to shutdown the socket for reading > and then try to select/recv on it (which is somewhat stupid). And that > issue has been masked by shutdown() on datagram socket becoming effectively > a NOP in 11 & head 20 months ago. It only affects head though, 11-stable > still has the old code which does not include that half-closed socket into > the select list. Attached patch is expected to fix head, Peter, it would be > nice if you can give it a try (restoring latest changes into > uipc_sockets.c) and let me know if it helps. Confirmed - resting uipc_socket.c to HEAD and applying the patch to syslogd does solve the problem we encountered. Thanks! > Thanks! > > On Fri, Apr 14, 2017 at 12:48 PM, Ngie Cooper (yaneurabeya) < > > yaneurab...@gmail.com> wrote: > > > On Apr 14, 2017, at 12:46, Peter Wemm wrote: > > > > > > On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: > > >> Thanks, Peter. I will try to look into this asap. > > > > > > I don't understand what is going on yet. Presumably there must be other > > > changes in play that affect udp/select sometime between the original > > > 2015 > > > change and this. The syslogd -s code is Old(TM). I'm also wondering > > > > whether > > > > > the -s code even works at all since the 2015 / r285910 change... > > > > syslogd has been refactored a lot on ^/head. I don’t think it’s safe to > > say that the ^/head and ^/stable/11 and earlier copies will function the > > same. > > Thanks, > > -Ngie -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 02:21:05 PM Maxim Sobolev wrote: > Peter, It is actually the other way around. If you take syslogd code out of > 11-stable and earlier that would work just fine with or without r316874. > But since r285910 syslogd in head had been refactored a lot and I think > that particular bug has introduced that has been masked by the shutdown() > on datagram sockets becoming a NOP after r285910. Then r316874 restored our > historical behavior for the shutdown(2) and bingo, bug in the new syslogd > code is now causing it to spin when shutdown() != NOP. Ok, this makes sense. Just to be sure I'm on the same page, I should apply the syslogd.diff from a few messages ago and restore the shutdown(2) kernel code and give it a spin. Correct? -Peter > -Max > > On Fri, Apr 14, 2017 at 12:46 PM, Peter Wemm wrote: > > On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: > > > Thanks, Peter. I will try to look into this asap. > > > > I don't understand what is going on yet. Presumably there must be other > > changes in play that affect udp/select sometime between the original 2015 > > change and this. The syslogd -s code is Old(TM). I'm also wondering > > whether > > the -s code even works at all since the 2015 / r285910 change... > > > > > -Max > > > > > > On Apr 14, 2017 12:32 PM, "Peter Wemm" wrote: > > > > On Friday, April 14, 2017 11:49:26 AM Peter Wemm wrote: > > > > > On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > > > > > > Author: sobomax > > > > > > Date: Fri Apr 14 17:23:28 2017 > > > > > > New Revision: 316874 > > > > > > URL: https://svnweb.freebsd.org/changeset/base/316874 > > > > > > > > > > > > Log: > > > > > > Restore ability to shutdown DGRAM sockets, still forcing > > > > ENOTCONN to > > > > > > be > > > > > > > > > > returned by the shutdown(2) system call. This ability has been > > > > lost as > > > > > > > > part > > > > > > of the svn revision 285910. > > > > > > > > > > > > Reviewed by: ed, rwatson, glebius, hiren > > > > > > MFC after:2 weeks > > > > > > Differential Revision:https://reviews.freebsd.org/D10351 > > > > > > > > > > This appears to have broken syslogd and had a major change in > > > > behavior > > > > > > with > > > > > > > > > regards to select(2). > > > > > > > > > > If you run syslogd with the -s flag (which is default), it now spins > > > > at > > > > > > 100% > > > > > > > > > cpu as all the shutdown sockets now return readable from select. > > > > > > > > > > Old releases / jails also manifest this behavor. If it wasn't for > > > > > losing > > > > > the ability to run old branch binaries I'd suggest changing syslogd > > > > > instead, but we depend on this in the cluster and I expect others do > > > > > too. > > > > > > > > > > I'm not 100% certain that this change is the culprit but a heads-up > > > > > can't > > > > > hurt. I'll try reverting it on the freebsd cluster next, after > > > > > fixing > > > > > the > > > > > broken auditing changes. > > > > > > > > > > -Peter > > > > > > > > I can confirm that reverting r316874 fixes syslogd and backwards > > > > compatability > > > > with old branches. > > > > > > > > -- > > > > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > > > > KI6FJV > > > > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 > > > > -- > > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > > KI6FJV > > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
Peter, It is actually the other way around. If you take syslogd code out of 11-stable and earlier that would work just fine with or without r316874. But since r285910 syslogd in head had been refactored a lot and I think that particular bug has introduced that has been masked by the shutdown() on datagram sockets becoming a NOP after r285910. Then r316874 restored our historical behavior for the shutdown(2) and bingo, bug in the new syslogd code is now causing it to spin when shutdown() != NOP. -Max On Fri, Apr 14, 2017 at 12:46 PM, Peter Wemm wrote: > On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: > > Thanks, Peter. I will try to look into this asap. > > I don't understand what is going on yet. Presumably there must be other > changes in play that affect udp/select sometime between the original 2015 > change and this. The syslogd -s code is Old(TM). I'm also wondering > whether > the -s code even works at all since the 2015 / r285910 change... > > > -Max > > > > On Apr 14, 2017 12:32 PM, "Peter Wemm" wrote: > > > On Friday, April 14, 2017 11:49:26 AM Peter Wemm wrote: > > > > On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > > > > > Author: sobomax > > > > > Date: Fri Apr 14 17:23:28 2017 > > > > > New Revision: 316874 > > > > > URL: https://svnweb.freebsd.org/changeset/base/316874 > > > > > > > > > > Log: > > > > > Restore ability to shutdown DGRAM sockets, still forcing > ENOTCONN to > > > > > > be > > > > > > > > returned by the shutdown(2) system call. This ability has been > lost as > > > > > part > > > > > of the svn revision 285910. > > > > > > > > > > Reviewed by: ed, rwatson, glebius, hiren > > > > > MFC after:2 weeks > > > > > Differential Revision:https://reviews.freebsd.org/D10351 > > > > > > > > This appears to have broken syslogd and had a major change in > behavior > > > > > > with > > > > > > > regards to select(2). > > > > > > > > If you run syslogd with the -s flag (which is default), it now spins > at > > > > > > 100% > > > > > > > cpu as all the shutdown sockets now return readable from select. > > > > > > > > Old releases / jails also manifest this behavor. If it wasn't for > > > > losing > > > > the ability to run old branch binaries I'd suggest changing syslogd > > > > instead, but we depend on this in the cluster and I expect others do > > > > too. > > > > > > > > I'm not 100% certain that this change is the culprit but a heads-up > > > > can't > > > > hurt. I'll try reverting it on the freebsd cluster next, after fixing > > > > the > > > > broken auditing changes. > > > > > > > > -Peter > > > > > > I can confirm that reverting r316874 fixes syslogd and backwards > > > compatability > > > with old branches. > > > > > > -- > > > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > > > KI6FJV > > > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 > > -- > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > KI6FJV > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 > ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
> On Apr 14, 2017, at 14:10, Maxim Sobolev wrote: > > Peter, Ngie, > > Looks like out of that refactoring came a logical bug that is present in the > head, which causes syslod to first to shutdown the socket for reading and > then try to select/recv on it (which is somewhat stupid). And that issue has > been masked by shutdown() on datagram socket becoming effectively a NOP in 11 > & head 20 months ago. It only affects head though, 11-stable still has the > old code which does not include that half-closed socket into the select list. > Attached patch is expected to fix head, Peter, it would be nice if you can > give it a try (restoring latest changes into uipc_sockets.c) and let me know > if it helps. > > Thanks! CCing hrs@ for input as he did the refactoring. Thanks! -Ngie PS LGTM with the change. Will wait for feedback from wemm@. signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r316874 - head/sys/kern
Peter, Ngie, Looks like out of that refactoring came a logical bug that is present in the head, which causes syslod to first to shutdown the socket for reading and then try to select/recv on it (which is somewhat stupid). And that issue has been masked by shutdown() on datagram socket becoming effectively a NOP in 11 & head 20 months ago. It only affects head though, 11-stable still has the old code which does not include that half-closed socket into the select list. Attached patch is expected to fix head, Peter, it would be nice if you can give it a try (restoring latest changes into uipc_sockets.c) and let me know if it helps. Thanks! On Fri, Apr 14, 2017 at 12:48 PM, Ngie Cooper (yaneurabeya) < yaneurab...@gmail.com> wrote: > > > On Apr 14, 2017, at 12:46, Peter Wemm wrote: > > > > On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: > >> Thanks, Peter. I will try to look into this asap. > > > > I don't understand what is going on yet. Presumably there must be other > > changes in play that affect udp/select sometime between the original 2015 > > change and this. The syslogd -s code is Old(TM). I'm also wondering > whether > > the -s code even works at all since the 2015 / r285910 change... > > syslogd has been refactored a lot on ^/head. I don’t think it’s safe to > say that the ^/head and ^/stable/11 and earlier copies will function the > same. > Thanks, > -Ngie > -- Maksym Sobolyev Sippy Software, Inc. Internet Telephony (VoIP) Experts Tel (Canada): +1-778-783-0474 Tel (Toll-Free): +1-855-747-7779 Fax: +1-866-857-6942 Web: http://www.sippysoft.com MSN: sa...@sippysoft.com Skype: SippySoft Index: syslogd.c === --- syslogd.c (revision 316854) +++ syslogd.c (working copy) @@ -702,7 +702,7 @@ sizeof(fd_mask)); STAILQ_FOREACH(sl, &shead, next) { - if (sl->sl_socket != -1) + if (sl->sl_socket != -1 && sl->sl_recv != NULL) FD_SET(sl->sl_socket, fdsr); } i = select(fdsrmax + 1, fdsr, NULL, NULL, @@ -2877,6 +2877,7 @@ struct addrinfo hints, *res, *res0; int error; char *cp; + int (*sl_recv)(struct socklist *); /* * We have to handle this case for backwards compatibility: * If there are two (or more) colons but no '[' and ']', @@ -3003,6 +3004,7 @@ } dprintf("new socket fd is %d\n", s); listen(s, 5); + sl_recv = socklist_recv_sock; dprintf("shutdown\n"); if (SecureMode || res->ai_family == AF_LOCAL) { /* Forbid communication in secure mode. */ @@ -3013,6 +3015,7 @@ die(0); } dprintf("listening on socket\n"); + sl_recv = NULL; } else dprintf("sending on socket\n"); addsock(res->ai_addr, res->ai_addrlen, @@ -3019,7 +3022,7 @@ &(struct socklist){ .sl_socket = s, .sl_peer = pe, - .sl_recv = socklist_recv_sock + .sl_recv = sl_recv }); } freeaddrinfo(res0); ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
> On Apr 14, 2017, at 12:46, Peter Wemm wrote: > > On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: >> Thanks, Peter. I will try to look into this asap. > > I don't understand what is going on yet. Presumably there must be other > changes in play that affect udp/select sometime between the original 2015 > change and this. The syslogd -s code is Old(TM). I'm also wondering whether > the -s code even works at all since the 2015 / r285910 change... syslogd has been refactored a lot on ^/head. I don’t think it’s safe to say that the ^/head and ^/stable/11 and earlier copies will function the same. Thanks, -Ngie signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 12:41:52 PM Maxim Sobolev wrote: > Thanks, Peter. I will try to look into this asap. I don't understand what is going on yet. Presumably there must be other changes in play that affect udp/select sometime between the original 2015 change and this. The syslogd -s code is Old(TM). I'm also wondering whether the -s code even works at all since the 2015 / r285910 change... > -Max > > On Apr 14, 2017 12:32 PM, "Peter Wemm" wrote: > > On Friday, April 14, 2017 11:49:26 AM Peter Wemm wrote: > > > On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > > > > Author: sobomax > > > > Date: Fri Apr 14 17:23:28 2017 > > > > New Revision: 316874 > > > > URL: https://svnweb.freebsd.org/changeset/base/316874 > > > > > > > > Log: > > > > Restore ability to shutdown DGRAM sockets, still forcing ENOTCONN to > > > > be > > > > > > returned by the shutdown(2) system call. This ability has been lost as > > > > part > > > > of the svn revision 285910. > > > > > > > > Reviewed by: ed, rwatson, glebius, hiren > > > > MFC after:2 weeks > > > > Differential Revision:https://reviews.freebsd.org/D10351 > > > > > > This appears to have broken syslogd and had a major change in behavior > > > > with > > > > > regards to select(2). > > > > > > If you run syslogd with the -s flag (which is default), it now spins at > > > > 100% > > > > > cpu as all the shutdown sockets now return readable from select. > > > > > > Old releases / jails also manifest this behavor. If it wasn't for > > > losing > > > the ability to run old branch binaries I'd suggest changing syslogd > > > instead, but we depend on this in the cluster and I expect others do > > > too. > > > > > > I'm not 100% certain that this change is the culprit but a heads-up > > > can't > > > hurt. I'll try reverting it on the freebsd cluster next, after fixing > > > the > > > broken auditing changes. > > > > > > -Peter > > > > I can confirm that reverting r316874 fixes syslogd and backwards > > compatability > > with old branches. > > > > -- > > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > > KI6FJV > > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
Thanks, Peter. I will try to look into this asap. -Max On Apr 14, 2017 12:32 PM, "Peter Wemm" wrote: > On Friday, April 14, 2017 11:49:26 AM Peter Wemm wrote: > > On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > > > Author: sobomax > > > Date: Fri Apr 14 17:23:28 2017 > > > New Revision: 316874 > > > URL: https://svnweb.freebsd.org/changeset/base/316874 > > > > > > Log: > > > Restore ability to shutdown DGRAM sockets, still forcing ENOTCONN to > be > > > > > > returned by the shutdown(2) system call. This ability has been lost as > > > part > > > of the svn revision 285910. > > > > > > Reviewed by: ed, rwatson, glebius, hiren > > > MFC after:2 weeks > > > Differential Revision:https://reviews.freebsd.org/D10351 > > > > This appears to have broken syslogd and had a major change in behavior > with > > regards to select(2). > > > > If you run syslogd with the -s flag (which is default), it now spins at > 100% > > cpu as all the shutdown sockets now return readable from select. > > > > Old releases / jails also manifest this behavor. If it wasn't for losing > > the ability to run old branch binaries I'd suggest changing syslogd > > instead, but we depend on this in the cluster and I expect others do too. > > > > I'm not 100% certain that this change is the culprit but a heads-up can't > > hurt. I'll try reverting it on the freebsd cluster next, after fixing the > > broken auditing changes. > > > > -Peter > > I can confirm that reverting r316874 fixes syslogd and backwards > compatability > with old branches. > > -- > Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; > KI6FJV > UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 11:49:26 AM Peter Wemm wrote: > On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > > Author: sobomax > > Date: Fri Apr 14 17:23:28 2017 > > New Revision: 316874 > > URL: https://svnweb.freebsd.org/changeset/base/316874 > > > > Log: > > Restore ability to shutdown DGRAM sockets, still forcing ENOTCONN to be > > > > returned by the shutdown(2) system call. This ability has been lost as > > part > > of the svn revision 285910. > > > > Reviewed by: ed, rwatson, glebius, hiren > > MFC after:2 weeks > > Differential Revision:https://reviews.freebsd.org/D10351 > > This appears to have broken syslogd and had a major change in behavior with > regards to select(2). > > If you run syslogd with the -s flag (which is default), it now spins at 100% > cpu as all the shutdown sockets now return readable from select. > > Old releases / jails also manifest this behavor. If it wasn't for losing > the ability to run old branch binaries I'd suggest changing syslogd > instead, but we depend on this in the cluster and I expect others do too. > > I'm not 100% certain that this change is the culprit but a heads-up can't > hurt. I'll try reverting it on the freebsd cluster next, after fixing the > broken auditing changes. > > -Peter I can confirm that reverting r316874 fixes syslogd and backwards compatability with old branches. -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
Re: svn commit: r316874 - head/sys/kern
On Friday, April 14, 2017 05:23:28 PM Maxim Sobolev wrote: > Author: sobomax > Date: Fri Apr 14 17:23:28 2017 > New Revision: 316874 > URL: https://svnweb.freebsd.org/changeset/base/316874 > > Log: > Restore ability to shutdown DGRAM sockets, still forcing ENOTCONN to be > returned by the shutdown(2) system call. This ability has been lost as part > of the svn revision 285910. > > Reviewed by:ed, rwatson, glebius, hiren > MFC after: 2 weeks > Differential Revision: https://reviews.freebsd.org/D10351 This appears to have broken syslogd and had a major change in behavior with regards to select(2). If you run syslogd with the -s flag (which is default), it now spins at 100% cpu as all the shutdown sockets now return readable from select. Old releases / jails also manifest this behavor. If it wasn't for losing the ability to run old branch binaries I'd suggest changing syslogd instead, but we depend on this in the cluster and I expect others do too. I'm not 100% certain that this change is the culprit but a heads-up can't hurt. I'll try reverting it on the freebsd cluster next, after fixing the broken auditing changes. -Peter -- Peter Wemm - pe...@wemm.org; pe...@freebsd.org; pe...@yahoo-inc.com; KI6FJV UTF-8: for when a ' or ... just won\342\200\231t do\342\200\246 signature.asc Description: This is a digitally signed message part.
svn commit: r316874 - head/sys/kern
Author: sobomax Date: Fri Apr 14 17:23:28 2017 New Revision: 316874 URL: https://svnweb.freebsd.org/changeset/base/316874 Log: Restore ability to shutdown DGRAM sockets, still forcing ENOTCONN to be returned by the shutdown(2) system call. This ability has been lost as part of the svn revision 285910. Reviewed by: ed, rwatson, glebius, hiren MFC after:2 weeks Differential Revision:https://reviews.freebsd.org/D10351 Modified: head/sys/kern/uipc_socket.c Modified: head/sys/kern/uipc_socket.c == --- head/sys/kern/uipc_socket.c Fri Apr 14 17:22:54 2017(r316873) +++ head/sys/kern/uipc_socket.c Fri Apr 14 17:23:28 2017(r316874) @@ -2343,13 +2343,27 @@ int soshutdown(struct socket *so, int how) { struct protosw *pr = so->so_proto; - int error; + int error, soerror_enotconn; if (!(how == SHUT_RD || how == SHUT_WR || how == SHUT_RDWR)) return (EINVAL); + + soerror_enotconn = 0; if ((so->so_state & - (SS_ISCONNECTED | SS_ISCONNECTING | SS_ISDISCONNECTING)) == 0) - return (ENOTCONN); + (SS_ISCONNECTED | SS_ISCONNECTING | SS_ISDISCONNECTING)) == 0) { + /* +* POSIX mandates us to return ENOTCONN when shutdown(2) is +* invoked on a datagram sockets, however historically we would +* actually tear socket down. This is known to be leveraged by +* some applications to unblock process waiting in recvXXX(2) +* by other process that it shares that socket with. Try to meet +* both backward-compatibility and POSIX requirements by forcing +* ENOTCONN but still asking protocol to perform pru_shutdown(). +*/ + if (so->so_type != SOCK_DGRAM) + return (ENOTCONN); + soerror_enotconn = 1; + } CURVNET_SET(so->so_vnet); if (pr->pr_usrreqs->pru_flush != NULL) @@ -2360,11 +2374,12 @@ soshutdown(struct socket *so, int how) error = (*pr->pr_usrreqs->pru_shutdown)(so); wakeup(&so->so_timeo); CURVNET_RESTORE(); - return (error); + return ((error == 0 && soerror_enotconn) ? ENOTCONN : error); } wakeup(&so->so_timeo); CURVNET_RESTORE(); - return (0); + + return (soerror_enotconn ? ENOTCONN : 0); } void ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"