Re: Question about socket timeouts

From: Vitja Makarov <vitja.makarov_at_gmail.com>
Date: Fri, 23 Aug 2013 10:27:58 +0400
2013/8/22 John Baldwin <jhb_at_freebsd.org>:
> On Thursday, August 22, 2013 12:18:48 am Vitja Makarov wrote:
>> 2013/8/21 John Baldwin <jhb_at_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)
>> > _at__at_ -2699,21 +2699,16 _at__at_ 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().

-- 
vitja.
Received on Fri Aug 23 2013 - 04:27:59 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:40 UTC