From owner-freebsd-net@FreeBSD.ORG Fri Jun 25 17:29:31 2010 Return-Path: Delivered-To: net@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B7B27106564A for ; Fri, 25 Jun 2010 17:29:31 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail04.syd.optusnet.com.au (mail04.syd.optusnet.com.au [211.29.132.185]) by mx1.freebsd.org (Postfix) with ESMTP id 3C7238FC1E for ; Fri, 25 Jun 2010 17:29:30 +0000 (UTC) Received: from c122-107-125-7.carlnfd1.nsw.optusnet.com.au (c122-107-125-7.carlnfd1.nsw.optusnet.com.au [122.107.125.7]) by mail04.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o5PHTP9Y004881 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 26 Jun 2010 03:29:27 +1000 Date: Sat, 26 Jun 2010 03:29:25 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Randall Stewart In-Reply-To: <9C936FEB-4858-4D8D-89CC-182EA3A80365@lakerest.net> Message-ID: <20100626022258.Y47182@delplex.bde.org> References: <20100622221228.GA93249@onelab2.iet.unipi.it> <20100623232402.X45536@delplex.bde.org> <9C936FEB-4858-4D8D-89CC-182EA3A80365@lakerest.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Luigi Rizzo , net@FreeBSD.org Subject: Re: Observations from an old timer playing with 64 bit numbers... X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Jun 2010 17:29:31 -0000 On Wed, 23 Jun 2010, Randall Stewart wrote: > Bruce: > > Comments (and questions in-line)... (you too Luigi) See Luigi's reply for most details.. > On Jun 23, 2010, at 6:33 AM, Bruce Evans wrote: > >> On Wed, 23 Jun 2010, Luigi Rizzo wrote: >>> strong objection! >>> We should instead use names with exact sizes (16,32,64). > > So please tell me why you object so strongly? We have the 16/32/64 bit names > which > are nice but are not expected so folks seem to not use them. I have > received a few private comments to the effect that "Gee, we would like > that, we rolled our own versions of ntohll() and if you did it we could > remove our version". This tells me that although a nicer name, folks > are rolling their own due to the name. Having a #define that maps > ntohll -> be64toh will make it so that people can actually find > the function. Not really. Such a define would be an obfuscation, and might make it fairly hard to find the actual function. See the corresponding #define for ntohl(). It is far from what is documented and it takes searching through 3-4 layers of include files and/or nested macros (with at least the lowest layer differing across arches) to see what it is: on i386: - man page: no mention of whether it is a function or a macro or both, except for the non-i386 case where it is "null" where it is misdocumented as being null (but it is never null). I think it is actually both a (extern) function and a macro. POSIX may require both, and doesn't prohibit the function, so we implement the function. - sys/param.h: htonl() may be defined at a lower level. It isn't on i386. When it isn't defined at a lower level, it is defined here as __htonl(x). Before this, if it isn't prototyped at a lower level, it is prototyped here, so that #undefing it works right. - i386/include/endian.h: - __htonl(x) is defined as __bswap32(x) - __bswap32(x) is a static inline function taking a uint32_t arg and returns a uint32_t after calling __byte_swap_int(x). The typed arg causes a converion of the 'x' arg in ntohl(x) in cases where it is not already uint32_t. I think this conversion is required by POSIX. On big-endian arches where ntohl(x) is "null", this conversion should be done too, so the macro cannot be null (really the null filter). It is done at the level of endian.h on at least sparc64. - __byte_swap_int(x) is a macro that expands in various ways: - if __OPTIMIZE__ is defined (i.e., for compiling with -O1 or higher), then it expands to the macro __byte_swap_int_var(x) if x is a variable and to the macro __byte_swap_int_const(x) if x is a constant. Otherwise, it always expands to the macro __byte_swap_int_const(x). - __byte_swap_int_var(x) expands to a Gnu C statement-expression with inline asm - __byte_swap_int_const(x) expands to a C expression with shifts and masks. It is expected that this expression is evaluated at compile time iff it is used. Some of this layering is bogus, but fixing it will gives slightly more complicated layering to reduce duplication: - __byte_swap_int_const(x) is MI so it should be defined in a MI file and not duplicated ad nauseum like it is now. Especially since it is only to support a tiny optimization. - __byte_swap_int_const(x) works for the non-constant case too, so its name is bogus. Some arches use it for the non-constant case, and for swapping uint64_t's on i386 there is nothing better than a similar expression. This expression should work on i386, but none of the versions versions of gcc used in FreeBSD optimize it well (to bswap). Someday when they do, ntohl() can be simply this version in all cases. - the ifdef's involving __OPTIMIZE__ can probably be improved. However, just having them gets in the way of letting gcc do the optimization. You don't want even more convoluted __OPTIMIZE__ ifdefs to "fix" thus. > Name changes are a good thing for clarity but if no > one will use your changed name and roll their own.. thats a bad thing. The hand rolled version is as unlikely to be as sophisticated as ntohl() :-). The be/le family is intentionally unsophisticated sinc its speed is believed to be unimportant. Probably the speed of the ntohl() family is similarly unimportant. > By having such a define you might encourage folks (over time) to transition > off to the more clear naming versions.. Nah, that will encourage them to keep using the macro that hides the newer versions. >> Yes, long long should not exist, and there are few places where exact >> sizes should be used, but networking code is one place where they >> should be used. >> >> ntohs() will break semantically or actually on machines with shorts with >> more than 16 bits. > > So two questions here: > > a) What machines that we do support currently have a short larger than > 16bits? None. Even if there were, then this would be moot because ntohs() was fixed some time ago to not take a signed short arg. It takes an unsigned 16-bit arg. (Signed 16-bit shorts would also break on 1's complement and maybe on signed-magnitude machines: 0xFFFF in bits has value -0 on 1's. complement, so ntohs(0xFFFF) might become ntohs(-0) = ntohs(0) = 0). > b) Does anyone that use networking code really expect ntohs() to do anything > different than a 16 bit byte swap? No. It cannot, since it is required to work on 16-bit unsigned values. > The man page is pretty clear on > it i.e. uint16_t is the input and its a "network short" which > if I remember right is defined to be 16 bits... At least most RFC's > are pretty clear and the same is true for UNP Vol 1 ;-) Since ntohs() is now (and maybe always has been) specified specifically enough. But this specification makes the function name bogus -- it never takes a short arg. A similar case that is still broken is fls(). POSIX only standardized historical malpractice for it, so it is still specified to take an int arg, without even any details of what happens when the value is negative. >> ntohl() is already broken semantically on machines with longs with >> more than 32 bits (all 64-bit arches in FreeBSD). It doesn't actually >> handle longs on such arches. Networking code handles a few cases related >> to this with n_long. This is not quite as bad, but its name is nonsense -- >> n_long is very far from being a long -- it is unsigned 32 bits exactly, >> unlike long which is signed >= 32 bits. > > again same two questions only change them to say 32 bits? Like ntohs(), it is specified correcly but doesn't match its name. Renaming it to ntoh32() would break historical practice. >> ntohll() would break similarly on machines with long longs with more >> or less than 64 bits. In practice this and other misuses of long long >> may prevent the size of long long being changed, like it prevented the >> size of long being changed on some systems with historical abuses of >> long. > > Again same first question... the second one I would ask too except the > function > does NOT exist (except where folks go out and roll it themselves). No need to break it from the start. > Basically I don't agree with your assessment since these functions are > designed > to operate on network code which I think defines a short = 16, a long = 32 > (at least > all RFC's I have read do that anyway). shorts and longs are defined by the C standard and the C compiler. Network code cannot change this. > And it appears from feedback I have > received > that ntohll would be defined as a 64bit network quantity in most folks minds > since > that what a lot of folks have implemented... Wouldn't they expect it to have a long long type if it were named ntohll()? Another error (mentioned in a later reply) would be expecting to print a 64 bit network type using %lld. That is as unportable as expecting to print a 32-bit type using %ld. On of the main type errors in practice for 64-bit arches under FreeBSD was printing the result of ntohl() using %ld. When there is a standard for a 64-bit network type, it will necessarily specify the type to be uint64_t (or perhaps a typedefed alias of this, like in_addr_t for uint32_t). Printing of this type will have the usual problems for a typedefed type. Bruce