Re: 2.1 Listen Broken [Was Re: Darwin and IPv6]
On Sun, Aug 24, 2003 at 03:45:40PM -0700, Justin Erenkrantz wrote: > I just committed a variant of your patch. I had to add some things to get > it to compile on Darwin. > > Please let me know how that works for you. I know it compiles on Linux, > but the Linux box I have partial access to doesn't have IPv6 configured. Works and running for me :) Thanks! -- Colm MacCárthaighPublic Key: [EMAIL PROTECTED] [EMAIL PROTECTED] http://www.stdlib.net/
Re: 2.1 Listen Broken [Was Re: Darwin and IPv6]
--On Sunday, August 24, 2003 4:38 AM +0100 Colm MacCarthaigh <[EMAIL PROTECTED]> wrote: Bah, it's always the way, 2 minutes after testing and then mailing a patch I realise there's a small slip-up. Patch without the stupid-obvious-bug attached :) Heh. I just committed a variant of your patch. I had to add some things to get it to compile on Darwin. Please let me know how that works for you. I know it compiles on Linux, but the Linux box I have partial access to doesn't have IPv6 configured. Thanks! -- justin
2.1 Listen Broken [Was Re: Darwin and IPv6]
On Thu, Aug 14, 2003 at 03:47:09PM -0700, Justin Erenkrantz wrote:
> I've got the patch to do multiple listeners off one Listen directive in my
> tree and tested, but I'm currently swamped with other stuff.
Now that it's been commited, I've been trying it out and it's broken :(
Right now, httpd won't even start for me on Linux. The first problem
is that the order of the getaddrinfo() result set is ignored. This is most
definitely not a good idea, the order is intentional. It's the only
way listening will ever work on an OS that does not support IPv6 only
sockets :)
The next problem is that v6only_setting=0 is now handled incorrectly,
with the result that we generate an addressing conflict. Right now
(after fixing the order) it tries to listen on :: which works,
and then on 0.0.0.0, which doesnt ... on platforms that don't support
V6ONLY sockets (Linux is one, but there are more).
Here's what should happen when we try to listen on all interfaces:
1. call getaddrinfo with PF_UNSPEC and AI_PASSIVE, get
:: and 0.0.0.0 back in that order.
2. try to create a socket on ::, don't scream too loudly if it
doesnt work.
3. try to set it to IPv6 only, don't scream too loudly if it
doesnt work.
4. Try to bind() and listen() to it, don't scream too loudly
if it doesnt work.
5.1 If 2 and 4 were successful, but 3 wasnt; Don't even try to
listen on 0.0.0.0.
5.2 Otherwise try to listen on 0.0.0.0 and bomb out on error.
Now unfortunately listen.c isnt structured to make this logic easy to
implement, the only easy place to remove 0.0.0.0 from the listeners
list is alloc_listener(), but the only place to test part 4 is
make_sock(), bummer. Solution; compromise and put it in ap_listen_open.
So, attachted is a best-effort patch which should solve the
problems. I've tried to make it as readable as possible but
inevitably it's going to be a bit convoluted within the current
design. But that said, I think it strikes a reasonable compromise.
With the patch applied, Linux now works :)
--
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/
Index: server/listen.c
===
RCS file: /home/cvspublic/httpd-2.0/server/listen.c,v
retrieving revision 1.89
diff -u -r1.89 listen.c
--- server/listen.c 15 Aug 2003 02:25:41 - 1.89
+++ server/listen.c 24 Aug 2003 03:17:39 -
@@ -306,8 +306,15 @@
return "Listen setup failed";
}
-new->next = ap_listeners;
-ap_listeners = new;
+/* We need to preserve the order returned by getaddrinfo() */
+if (ap_listeners == NULL ) {
+ap_listeners = new;
+} else {
+while(ap_listeners->next) {
+ap_listeners = ap_listeners->next;
+}
+ap_listeners->next = new;
+}
}
return NULL;
@@ -317,6 +324,7 @@
{
ap_listen_rec *lr;
ap_listen_rec *next;
+ap_listen_rec *previous;
int num_open;
const char *userdata_key = "ap_listen_open";
void *data;
@@ -326,16 +334,61 @@
* config file.
*/
num_open = 0;
-for (lr = ap_listeners; lr; lr = lr->next) {
+previous = NULL;
+for (lr = ap_listeners; lr; previous = lr, lr = lr->next) {
if (lr->active) {
++num_open;
}
else {
+#if APR_HAVE_IPV6
+int v6only_setting;
+/* If we are trying to bind to 0.0.0.0 and the previous listener
+ * was :: on the same port and in turn that socket does not have
+ * the IPV6_V6ONLY flag set; we must skip the current attempt to
+ * listen (which would generate an error). IPv4 will be handled
+ * on the established IPv6 socket.
+ */
+if (previous != NULL &&
+lr->bind_addr->family == APR_INET &&
+*((in_addr_t *)lr->bind_addr->ipaddr_ptr) == INADDR_ANY &&
+lr->bind_addr->port == previous->bind_addr->port &&
+previous->bind_addr->family == APR_INET6 &&
+IN6_IS_ADDR_UNSPECIFIED(previous->bind_addr->ipaddr_ptr) &&
+apr_socket_opt_get(previous->sd, APR_IPV6_V6ONLY,
+ &v6only_setting) == APR_SUCCESS &&
+v6only_setting == 0) {
+
+/* Remove the current listener from the list */
+previous->next = lr->next;
+continue;
+}
+#endif
if (make_sock(pool, lr) == APR_SUCCESS) {
++num_open;
lr->active = 1;
}
else {
+#if APR_HAVE_IPV6
+/* If we tried to bind to ::, and the next listener is
+ * on 0.0.0.0 with the same port, don't give a fatal
+ * error. The user will s
Re: 2.1 Listen Broken [Was Re: Darwin and IPv6]
On Sun, Aug 24, 2003 at 04:31:07AM +0100, Colm MacCarthaigh wrote:
> So, attachted is a best-effort patch which should solve the
> problems.
Bah, it's always the way, 2 minutes after testing and then mailing
a patch I realise there's a small slip-up. Patch without the
stupid-obvious-bug attached :)
--
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/
Index: server/listen.c
===
RCS file: /home/cvspublic/httpd-2.0/server/listen.c,v
retrieving revision 1.89
diff -u -r1.89 listen.c
--- server/listen.c 15 Aug 2003 02:25:41 - 1.89
+++ server/listen.c 24 Aug 2003 03:35:56 -
@@ -306,8 +306,15 @@
return "Listen setup failed";
}
-new->next = ap_listeners;
-ap_listeners = new;
+/* We need to preserve the order returned by getaddrinfo() */
+if (ap_listeners == NULL ) {
+ap_listeners = new;
+} else {
+while(ap_listeners->next) {
+ap_listeners = ap_listeners->next;
+}
+ap_listeners->next = new;
+}
}
return NULL;
@@ -317,6 +324,7 @@
{
ap_listen_rec *lr;
ap_listen_rec *next;
+ap_listen_rec *previous;
int num_open;
const char *userdata_key = "ap_listen_open";
void *data;
@@ -326,16 +334,64 @@
* config file.
*/
num_open = 0;
-for (lr = ap_listeners; lr; lr = lr->next) {
+previous = NULL;
+for (lr = ap_listeners; lr; previous = lr, lr = lr->next) {
if (lr->active) {
++num_open;
}
else {
+#if APR_HAVE_IPV6
+int v6only_setting;
+/* If we are trying to bind to 0.0.0.0 and the previous listener
+ * was :: on the same port and in turn that socket does not have
+ * the IPV6_V6ONLY flag set; we must skip the current attempt to
+ * listen (which would generate an error). IPv4 will be handled
+ * on the established IPv6 socket.
+ */
+if (previous != NULL &&
+lr->bind_addr->family == APR_INET &&
+*((in_addr_t *)lr->bind_addr->ipaddr_ptr) == INADDR_ANY &&
+lr->bind_addr->port == previous->bind_addr->port &&
+previous->bind_addr->family == APR_INET6 &&
+IN6_IS_ADDR_UNSPECIFIED(previous->bind_addr->ipaddr_ptr) &&
+apr_socket_opt_get(previous->sd, APR_IPV6_V6ONLY,
+ &v6only_setting) == APR_SUCCESS &&
+v6only_setting == 0) {
+
+/* Remove the current listener from the list */
+previous->next = lr->next;
+continue;
+}
+#endif
if (make_sock(pool, lr) == APR_SUCCESS) {
++num_open;
lr->active = 1;
}
else {
+#if APR_HAVE_IPV6
+/* If we tried to bind to ::, and the next listener is
+ * on 0.0.0.0 with the same port, don't give a fatal
+ * error. The user will still get a warning from make_sock
+ * though.
+ */
+if (lr->next != NULL && lr->bind_addr->family == APR_INET6 &&
+IN6_IS_ADDR_UNSPECIFIED(previous->bind_addr->ipaddr_ptr) &&
+lr->bind_addr->port == lr->next->bind_addr->port &&
+*((in_addr_t *)lr->next->bind_addr->ipaddr_ptr)
+== INADDR_ANY) {
+
+/* Remove the current listener from the list */
+if (previous)
+previous->next = lr->next;
+else
+ap_listeners = lr->next;
+
+/* So that previous becomes NULL in the next iteration */
+lr = NULL;
+
+continue;
+}
+#endif
/* fatal error */
return -1;
}
