Date: Fri, 23 Aug 2013 11:29:03 -0400 From: John Baldwin <jhb@freebsd.org> To: Davide Italiano <davide@freebsd.org> Cc: Vitja Makarov <vitja.makarov@gmail.com>, freebsd-current <freebsd-current@freebsd.org> Subject: Re: Question about socket timeouts Message-ID: <201308231129.03990.jhb@freebsd.org> In-Reply-To: <CACYV=-FKAn3Mot2F%2Bko%2BNRfLiRC0ZCJcFc5W%2BFwOghngJY88EA@mail.gmail.com> References: <CAKGHGPS=HCYfXxPXuUz5G83j5sGieejPU-QHmi9TrmMhmweHLw@mail.gmail.com> <201308230945.28701.jhb@freebsd.org> <CACYV=-FKAn3Mot2F%2Bko%2BNRfLiRC0ZCJcFc5W%2BFwOghngJY88EA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, August 23, 2013 9:53:12 am Davide Italiano wrote: > On Fri, Aug 23, 2013 at 3:45 PM, John Baldwin <jhb@freebsd.org> wrote: > > On Friday, August 23, 2013 2:27:58 am Vitja Makarov wrote: > >> 2013/8/22 John Baldwin <jhb@freebsd.org>: > >> > On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote: > >> >> 2013/8/21 John Baldwin <jhb@freebsd.org>: > >> >> > On Monday, August 19, 2013 11:13:02 pm Daniel Eischen wrote: > >> >> >> On Mon, 19 Aug 2013, Adrian Chadd wrote: > >> >> >> > >> >> >> > Yes! Please file a PR! > >> >> >> > >> >> >> This sorta implies that both are acceptable (although, > >> >> >> the Linux behavior seems more desirable). > >> >> >> > >> >> >> http://austingroupbugs.net/view.php?id=369 > >> >> > > >> >> > No, that says "round up", so it does mean that the requested timeout > >> >> > should be the minimum amount slept. tvtohz() does this. Really odd > >> >> > that the socket code is using its own version of this rather than > >> >> > tvtohz(). > >> >> > > >> >> > Oh, I bet this just predates tvtohz(). Interesting that it keeps getting > >> >> > bug fixes in its history that simply using tvtohz() would have solved. > >> >> > > >> >> > Try this: > >> >> > > >> >> > Index: uipc_socket.c > >> >> > =================================================================== > >> >> > --- uipc_socket.c (revision 254570) > >> >> > +++ uipc_socket.c (working copy) > >> >> > @@ -2699,21 +2699,16 @@ sosetopt(struct socket *so, struct sockopt *sopt) > >> >> > if (error) > >> >> > goto bad; > >> >> > > >> >> > - /* assert(hz > 0); */ > >> >> > if (tv.tv_sec < 0 || tv.tv_sec > INT_MAX / hz || > >> >> > tv.tv_usec < 0 || tv.tv_usec >= 1000000) { > >> >> > error = EDOM; > >> >> > goto bad; > >> >> > } > >> >> > - /* assert(tick > 0); */ > >> >> > - /* assert(ULONG_MAX - INT_MAX >= 1000000); */ > >> >> > - val = (u_long)(tv.tv_sec * hz) + tv.tv_usec / tick; > >> >> > - if (val > INT_MAX) { > >> >> > + val = tvtohz(&tv); > >> >> > + if (val == INT_MAX) { > >> >> > error = EDOM; > >> >> > goto bad; > >> >> > } > >> >> > - if (val == 0 && tv.tv_usec != 0) > >> >> > - val = 1; > >> >> > > >> >> > switch (sopt->sopt_name) { > >> >> > case SO_SNDTIMEO: > >> >> > > >> >> > >> >> That must help. But I want to see the issue solved in the next > >> >> release. I can't apply patch to the production system. Btw in > >> >> production environment we have kern.hz set to 1000 so it's not a > >> >> problem there. > >> > > >> > Can you test this in some way in a test environment? > >> > > >> > >> Ok, sorry for posting out of the list. > >> > >> Simple test program is attached. Without your patch timeout expires in > >> about 20ms. With it it's ~40ms. > >> > >> 40 instead of 30 is beacuse of odd tick added by tvtohz(). > > > > Ok, thanks. tvtohz() will be good to MFC (and I will do that), but for > > HEAD I think we can fix this to use a precise timeout. I've cc'd davide@ > > so he can take a look at that. > > > > -- > > John Baldwin > > Hi, > I think I can switch this code to new timeout KPI, but this will > require the timeout field of 'struct sockbuf' to be changed from 'int' > to 'sbintime_t' which breaks binary compatibility. Do you have any > strong objections about this? If any, I would like this to happen ABI > freeze, so it looks like this is the right moment. This should be fine to change now, it just can't be MFC'd (which we can't do anyway). -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308231129.03990.jhb>