From: Michal K. <mi...@mk...> - 2014-11-03 23:20:55
|
Hello, I'm happy to announce that attached series finally passed my tests for all three modes (superserver, standaloneclassic and classic) including events passing via aux connection. I only need to write a README documenting extensions to the connection string and configuration. First eight patches implement IPv6. These are divided into logical parts and are quite well tested (but only on Linux, AFAIK). Last patch allows server to listen on multiple ports. It's quite big, less tested (as it was only completely finished last week). As Beta 1 is already out, I'm reluctant to commit it without consulting first. Now, I suppose, there are three options (not counting the obvious one to leave all of it after 3.0): 1. Playing it safe, taking the IPv6 implementation with change to make server only listen on (PF_INET) 0.0.0.0 by default. That way backward compatibility is preserved and who wants to serve IPv6 requests, will need to explicitely enable it. 2. If all supported platforms allow PF_INET6 socket bound to :: to accept IPv4 connections (IPV6_V6ONLY = 0), we can take the IPv6 implementation as is, i.e. listening on :: (with IPV6_V6ONLY disabled) by default. 3. Take the whole series including the last patch which allows server to listen on multiple sockets (RemoteBindAddress being a semicolon separated list). Ideas? Suggestions? Michal Kubecek |
From: Michal K. <mi...@mk...> - 2014-11-04 05:51:21
|
On Tue, Nov 04, 2014 at 12:20:44AM +0100, Michal Kubecek wrote: > > I'm happy to announce that attached series finally passed my tests for Forgot to attach the patch series. It is available at http://www.mk-sys.cz/tmp/fb3-ipv6/ Michal Kubecek |
From: Michal K. <mi...@mk...> - 2014-11-10 15:40:54
|
On Tue, Nov 04, 2014 at 12:20:44AM +0100, Michal Kubecek wrote: > > 1. Playing it safe, taking the IPv6 implementation with change to make > server only listen on (PF_INET) 0.0.0.0 by default. That way backward > compatibility is preserved and who wants to serve IPv6 requests, will > need to explicitely enable it. > > 2. If all supported platforms allow PF_INET6 socket bound to :: to > accept IPv4 connections (IPV6_V6ONLY = 0), we can take the IPv6 > implementation as is, i.e. listening on :: (with IPV6_V6ONLY disabled) > by default. > > 3. Take the whole series including the last patch which allows server to > listen on multiple sockets (RemoteBindAddress being a semicolon > separated list). > > Ideas? Suggestions? No response so far, not sure if it's a good sign or a bad one. Unless someone objects, I'm going to commit option 2 (all patches except the last plus adding README.IPv6) on Wednesday. That way it will be easy to move to 1 or 3 if there are problems. Michal Kubecek |
From: Adriano d. S. F. <adr...@gm...> - 2014-11-10 16:11:49
|
On 10/11/2014 13:40, Michal Kubecek wrote: > On Tue, Nov 04, 2014 at 12:20:44AM +0100, Michal Kubecek wrote: >> 1. Playing it safe, taking the IPv6 implementation with change to make >> server only listen on (PF_INET) 0.0.0.0 by default. That way backward >> compatibility is preserved and who wants to serve IPv6 requests, will >> need to explicitely enable it. >> >> 2. If all supported platforms allow PF_INET6 socket bound to :: to >> accept IPv4 connections (IPV6_V6ONLY = 0), we can take the IPv6 >> implementation as is, i.e. listening on :: (with IPV6_V6ONLY disabled) >> by default. >> >> 3. Take the whole series including the last patch which allows server to >> listen on multiple sockets (RemoteBindAddress being a semicolon >> separated list). >> >> Ideas? Suggestions? > No response so far, not sure if it's a good sign or a bad one. Unless > someone objects, I'm going to commit option 2 (all patches except the > last plus adding README.IPv6) on Wednesday. That way it will be easy to > move to 1 or 3 if there are problems. > > I just had a basic look, but I never used ipv6, so I can only say about the basic stuff: - methodNames in camel case, please - do not start variables with underline - SockAddr.h is copyright by Dmitry in 2006?? (you borrow the header from another file or is this based on existing code?) Adriano |
From: Michal K. <mi...@mk...> - 2014-11-10 19:46:41
|
On Mon, Nov 10, 2014 at 02:12:25PM -0200, Adriano dos Santos Fernandes wrote: > > - methodNames in camel case, please > - do not start variables with underline > - SockAddr.h is copyright by Dmitry in 2006?? (you borrow the header > from another file or is this based on existing code?) Yes, I copied the header from another file and forgot to edit it. Thank you for your comments. I updated the patch series at http://www.mk-sys.cz/tmp/fb3-ipv6/ to address them. Michal Kubecek |
From: Adriano d. S. F. <adr...@gm...> - 2014-11-16 02:45:19
|
In firebird.conf it says: # IPv4 and IPv6. Default is false on POSIX systems, true on Windows. # On Windows, "false" value can only be used since Windows Vista. Why it's different in POSIX and Windows? And why the default in Windows is something not accepted after Vista? But I then see in the code that the default is always false? In SockAddr.h: static const unsigned maxLen = sizeof(struct sockaddr_in6); char data[maxLen]; Please change maxLen (static const) to MAX_LEN. And you declare a char array and cast to different structs. It will cause alignment problems in some architectures. It's better to use an union in this case, which will make avoid the casts too. Adriano |
From: Michal K. <mi...@mk...> - 2014-11-16 18:12:17
|
On Sun, Nov 16, 2014 at 12:45:10AM -0200, Adriano dos Santos Fernandes wrote: > In firebird.conf it says: > > # IPv4 and IPv6. Default is false on POSIX systems, true on Windows. > # On Windows, "false" value can only be used since Windows Vista. > > Why it's different in POSIX and Windows? And why the default in Windows > is something not accepted after Vista? > > But I then see in the code that the default is always false? Originally the default was different but then I thought about it and realized that if IPV6_V6ONLY can't be set, the default doesn't matter, and if it can be set, the default can be always false (or rather should be, unless listening on multiple sockets is also implemented). I forgot to change the comment in firebird.conf, I'll fix it. > In SockAddr.h: > static const unsigned maxLen = sizeof(struct sockaddr_in6); > char data[maxLen]; > > Please change maxLen (static const) to MAX_LEN. > > And you declare a char array and cast to different structs. It will > cause alignment problems in some architectures. It's better to use an > union in this case, which will make avoid the casts too. I wasn't sure if the union would guarantee that af_family, sin_family and sin6_family members will be always at the same offset. But it seems so and assumptions like this are quite frequent. Moreover, I tried to rewrite the file now and there is only one place where char array is more convenient and several where a union would be easier to handle. So I'm going to change it (I'll need to run a few tests to make sure nothing breaks). Michal Kubecek |