From owner-freebsd-net@FreeBSD.ORG Wed Jul 16 18:02:45 2014 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 32223E99 for ; Wed, 16 Jul 2014 18:02:45 +0000 (UTC) Received: from mail-qg0-x22f.google.com (mail-qg0-x22f.google.com [IPv6:2607:f8b0:400d:c04::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E714A2278 for ; Wed, 16 Jul 2014 18:02:44 +0000 (UTC) Received: by mail-qg0-f47.google.com with SMTP id i50so1058637qgf.6 for ; Wed, 16 Jul 2014 11:02:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=9ZDt8s0yjbvacPJWvZRgTfUF+59NoBVEleC/GH18NAU=; b=tehMPb9p9QefcSaQz/ORdv8esShZQGe3HkKTYNC2Js3RZ6gGeIA7a/uzKrNX7RxGyy FedTJDO625IQUhzBhVERIvxkR/EI+3kECI7a3nIRZLGCFUEbVYGhQ8BSRmx0T0y+EOa2 qf9dwHgX1RDV/fHFfS6hPx0GeadmA+FEiqorMUbYaIK1yfsuYFUBMDREQ+JR4whOszOY udtUUCdjl8DrThi96HmxQK5VTwYf5ZyO7t5TuglF5RqHuzYXa+TP6oQBEzzGGSjM6UHX ZFUggfT3KJewj11klmT1P61KoXRWzKDsDVbIzJksZpWZUgvqVDQaYFO4swSAmWZdnrzm pSHQ== MIME-Version: 1.0 X-Received: by 10.140.47.80 with SMTP id l74mr24579835qga.24.1405533763742; Wed, 16 Jul 2014 11:02:43 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.224.202.193 with HTTP; Wed, 16 Jul 2014 11:02:43 -0700 (PDT) In-Reply-To: <20140716223814.O2597@besplex.bde.org> References: <20140716223814.O2597@besplex.bde.org> Date: Wed, 16 Jul 2014 11:02:43 -0700 X-Google-Sender-Auth: 8rJuvwkkdW8bZ05aQRZB4BOYhxk Message-ID: Subject: Re: does setsockopt(SO_RCVTIMEO) work? From: Adrian Chadd To: Bruce Evans Content-Type: text/plain; charset=UTF-8 Cc: FreeBSD Net , Dmitry Sivachenko X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Jul 2014 18:02:45 -0000 I'm about to bump into this - would you be able to put together a patch to address these issues? That way I can also test it out with the UDP stuff I'm working on and get it into the tree. Thanks, -a On 16 July 2014 06:24, Bruce Evans wrote: > On Wed, 16 Jul 2014, Dmitry Sivachenko wrote: > >> I am having trouble using {g,s}etsockopt(SO_RCVTIMEO). Consider the >> following small test program. >> I expect to retrieve the value of 1 second via getsockopt call, I expect >> the following output: >> tv_sec=1, tv_usec=0 >> But I actually get >> tv_sec=0, tv_usec=0 >> >> What am I missing? >> >> Thanks! >> >> PS: on Linux it works as I expect. > > > On old versions of FreeBSD it works as expected. > > This was broken last year. I pointed out the bug and many others in my > review, but it is still there. From the review: > > @ On Sun, 1 Sep 2013, Davide Italiano wrote: > @ @ > Log: > @ > Fix socket buffer timeouts precision using the new sbintime_t KPI > instead > @ > of relying on the tvtohz() workaround. The latter has been introduced > @ > lately by jhb@ (r254699) in order to have a fix that can be backported > @ > to STABLE. > @ > > @ > Reported by: Vitja Makarov > @ > Reviewed by: jhb (earlier version) > @ @ This reintroduces overflow bugs that were fixed by using tvtohz(). > tvtohz() > @ clamps the value to INT_MAX instead of overflowing, but tvtosbt() just > @ overflows. > > These bugs are small (they are are only for timevals larger than INT_MAX > seconds, which can only occur on systems with bloated 64-bit time_t's), > but they were fixed long ago. > > @ @ This gives much larger overflow bugs in getsockopt(). > > This bug is still there. So in your test program, the timeout is set > correctly, but it is returned as 0 after overflow of a large value > gives 0. All values larger than 1 second are truncated to less than > 1 second. > > I expected more garbage in the values for timevals between 0.5 seconds > (inclusive) and 1.0 seconds (not inclusive). I think they would happen > for the corresonding bug in setsockopt(), but in getsockopt() there is > only a small (but annoying) rounding error. I tried a timeval of > 0 seconds, 500000 microseconds. This was returned as 0 seconds, > 499999 microseconds. This rounding error happens because almost no values > in the power-of-10 time scale for timevals can be represented in the > power-of-2 time scale for sbintimes. The rounding is always down, so > conversion back and forth drops 1 ULP in almost all cases. bintime > gives similar errors. sys/time.h has a large comment defending this > bug. > > @ @ > ... > @ > Modified: head/sys/kern/uipc_socket.c > @ > > ============================================================================== > @ > --- head/sys/kern/uipc_socket.c Sun Sep 1 23:06:28 2013 > (r255137) > @ > +++ head/sys/kern/uipc_socket.c Sun Sep 1 23:34:53 2013 > (r255138) > @ > @@ -2541,7 +2541,7 @@ sosetopt(struct socket *so, struct socko > @ > int error, optval; > @ > struct linger l; > @ > struct timeval tv; > @ > - u_long val; > @ > + sbintime_t val; > @ > uint32_t val32; > @ > #ifdef MAC > @ > struct mac extmac; > @ @ This fixes a style bug (type error) that should have been fixed in the > @ previous commit. 'int optval' is used for most options, but the old > @ code needed `u_long val' for its home made conversion. u_long became > @ unnecessary and the extra variable became bogus when tvtohz() was used, > @ since optval can hold tvtohz(). > @ @ > @@ -2703,7 +2703,7 @@ sosetopt(struct socket *so, struct socko > @ > error = EDOM; > @ > goto bad; > @ > } > @ @ There used to be more range checking above here. Some was moved into > @ tvtohz(). Changing to tvtosbt() moves it to /dev/null. > @ @ > - val = tvtohz(&tv); > @ > + val = tvtosbt(tv); > @ > > @ > switch (sopt->sopt_name) { > @ > case SO_SNDTIMEO: > @ @ optval can't hold tvtosbt(), so an extra variable is not a large style > @ bug now. But it can be avoided here by doing 2 assignments of tvtosbt() > @ to so_snd/rcv.sb_timeout. > > 'val' _can_ hold tvtosbt() (it was enlarged to do this), so there is only > a far-off overflow bug here (when tvtosbt() overflows internally)). > > @ @ > @@ -2857,8 +2857,7 @@ integer: > @ > optval = (sopt->sopt_name == SO_SNDTIMEO ? > @ > so->so_snd.sb_timeo : > so->so_rcv.sb_timeo); > @ @ This is now very broken. optval is only int, so it can't hold the > 64-bit > @ sbintime_t. I think it can hold times less than 0.5 seconds. Overflow > @ starts at 0.5 seconds by giving sign extension bugs on 2's complement > machines. > @ @ Style bug: non-KNF indentation. > > This still has all its bugs and style bugs. > > @ @ > > @ @ Style bug: extra blank line separates related code. > @ @ > - tv.tv_sec = optval / hz; > @ > - tv.tv_usec = (optval % hz) * tick; > @ > + tv = sbttotv(optval); > @ @ This together with the style can be fixed by 2 assignments also (1 > @ assignment in the conditional operater also fixes verbosenes): > @ @ tv = sbttotv(sopt->sopt_name == SO_SNDTIMEO ? > @ so->so_snd.sb_timeo : so->so_rcv.sb_timeo); > > Fix for the main bug some style bugs. > > [... 200 more lines with further duscussion of style and overflow bugs] > > Bruce > > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"