Discussion:
[PATCH] Add "Happy Eyeballs" for IPv4/IPv6.
Björn Stenberg
2013-10-26 12:48:51 UTC
Permalink
Add "Happy Eyeballs" for IPv4/IPv6.

This patch invokes two socket connect()s nearly simultaneously, and
the socket that is first connected "wins" and is subsequently used for
the connection. The other is terminated.

There is a very slight IPv4 preference, in that if both sockets connect
simultaneously IPv4 is checked first and thus will win.

Happy Eyeballs is the default behaviour with this patch. I did not add
an option to disable it since it can be disabled in effect by telling
curl to only resolve IPv4 or IPv6 addresses (-4 or -6 on the command-line
or CURL_IPRESOLVE_Vx in the libcurl api).

Björn Stenberg (1):
Add "Happy Eyeballs" for IPv4/IPv6.

lib/connect.c | 298 ++++++++++++++++++++++++++++-----------------------------
lib/connect.h | 2 -
lib/ftp.c | 5 +-
lib/multi.c | 13 ++-
lib/url.c | 8 +-
lib/urldata.h | 2 +
6 files changed, 164 insertions(+), 164 deletions(-)
--
1.7.10.4

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http
Björn Stenberg
2013-10-26 12:48:52 UTC
Permalink
This patch invokes two socket connect()s nearly simultaneously, and
the socket that is first connected "wins" and is subsequently used for
the connection. The other is terminated.

There is a very slight IPv4 preference, in that if both sockets connect
simultaneously IPv4 is checked first and thus will win.

Signed-off-by: Björn Stenberg <***@haxx.se>
---
lib/connect.c | 298 ++++++++++++++++++++++++++++-----------------------------
lib/connect.h | 2 -
lib/ftp.c | 5 +-
lib/multi.c | 13 ++-
lib/url.c | 8 +-
lib/urldata.h | 2 +
6 files changed, 164 insertions(+), 164 deletions(-)

diff --git a/lib/connect.c b/lib/connect.c
index 2b5719d..92f531f 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -233,45 +233,6 @@ long Curl_timeleft(struct SessionHandle *data,
return timeout_ms;
}

-/*
- * checkconnect() checks for a TCP connect on the given socket.
- * It returns:
- */
-
-enum chkconn_t {
- CHKCONN_SELECT_ERROR = -1,
- CHKCONN_CONNECTED = 0,
- CHKCONN_IDLE = 1,
- CHKCONN_FDSET_ERROR = 2
-};
-
-static enum chkconn_t
-checkconnect(curl_socket_t sockfd)
-{
- int rc;
-#ifdef mpeix
- /* Call this function once now, and ignore the results. We do this to
- "clear" the error state on the socket so that we can later read it
- reliably. This is reported necessary on the MPE/iX operating system. */
- (void)verifyconnect(sockfd, NULL);
-#endif
-
- rc = Curl_socket_ready(CURL_SOCKET_BAD, sockfd, 0);
-
- if(-1 == rc)
- /* error, no connect here, try next */
- return CHKCONN_SELECT_ERROR;
-
- else if(rc & CURL_CSELECT_ERR)
- /* error condition caught */
- return CHKCONN_FDSET_ERROR;
-
- else if(rc)
- return CHKCONN_CONNECTED;
-
- return CHKCONN_IDLE;
-}
-
static CURLcode bindlocal(struct connectdata *conn,
curl_socket_t sockfd, int af)
{
@@ -573,17 +534,19 @@ static bool verifyconnect(curl_socket_t sockfd, int *error)
more address exists or error */
static CURLcode trynextip(struct connectdata *conn,
int sockindex,
+ int tempindex,
bool *connected)
{
curl_socket_t sockfd;
Curl_addrinfo *ai;
+ int family = tempindex ? AF_INET6 : AF_INET;

/* First clean up after the failed socket.
Don't close it yet to ensure that the next IP's socket gets a different
file descriptor, which can prevent bugs when the curl_multi_socket_action
interface is used with certain select() replacements such as kqueue. */
- curl_socket_t fd_to_close = conn->sock[sockindex];
- conn->sock[sockindex] = CURL_SOCKET_BAD;
+ curl_socket_t fd_to_close = conn->tempsock[tempindex];
+ conn->tempsock[tempindex] = CURL_SOCKET_BAD;
*connected = FALSE;

if(sockindex != FIRSTSOCKET) {
@@ -591,21 +554,26 @@ static CURLcode trynextip(struct connectdata *conn,
return CURLE_COULDNT_CONNECT; /* no next */
}

- /* try the next address */
- ai = conn->ip_addr->ai_next;
+ /* try the next address with same family */
+ ai = conn->tempaddr[tempindex]->ai_next;
+ while(ai && ai->ai_family != family)
+ ai = ai->ai_next;

- while(ai) {
+ while(ai && ai->ai_family == family) {
CURLcode res = singleipconnect(conn, ai, &sockfd, connected);
if(res)
return res;
if(sockfd != CURL_SOCKET_BAD) {
/* store the new socket descriptor */
- conn->sock[sockindex] = sockfd;
- conn->ip_addr = ai;
+ conn->tempsock[tempindex] = sockfd;
+ conn->tempaddr[tempindex] = ai;
Curl_closesocket(conn, fd_to_close);
return CURLE_OK;
}
- ai = ai->ai_next;
+
+ do {
+ ai = ai->ai_next;
+ } while(ai && ai->ai_family == family);
}
Curl_closesocket(conn, fd_to_close);
return CURLE_COULDNT_CONNECT;
@@ -707,6 +675,7 @@ void Curl_updateconninfo(struct connectdata *conn, curl_socket_t sockfd)
error, Curl_strerror(conn, error));
return;
}
+ memcpy(conn->ip_addr_str, conn->primary_ip, MAX_IPADR_LEN);

if(!getaddressinfo((struct sockaddr*)&ssloc,
conn->local_ip, &conn->local_port)) {
@@ -732,11 +701,11 @@ CURLcode Curl_is_connected(struct connectdata *conn,
{
struct SessionHandle *data = conn->data;
CURLcode code = CURLE_OK;
- curl_socket_t sockfd = conn->sock[sockindex];
long allow = DEFAULT_CONNECT_TIMEOUT;
int error = 0;
struct timeval now;
- enum chkconn_t chk;
+ int result;
+ int i;

DEBUGASSERT(sockindex >= FIRSTSOCKET && sockindex <= SECONDARYSOCKET);

@@ -759,69 +728,94 @@ CURLcode Curl_is_connected(struct connectdata *conn,
return CURLE_OPERATION_TIMEDOUT;
}

- /* check socket for connect */
- chk = checkconnect(sockfd);
- if(CHKCONN_IDLE == chk) {
- if(curlx_tvdiff(now, conn->connecttime) >= conn->timeoutms_per_addr) {
- infof(data, "After %ldms connect time, move on!\n",
- conn->timeoutms_per_addr);
- goto next;
- }
+ for(i=0; i<2; i++) {
+ if(conn->tempsock[i] == CURL_SOCKET_BAD)
+ continue;

- /* not an error, but also no connection yet */
- return code;
- }
+#ifdef mpeix
+ /* Call this function once now, and ignore the results. We do this to
+ "clear" the error state on the socket so that we can later read it
+ reliably. This is reported necessary on the MPE/iX operating system. */
+ (void)verifyconnect(conn->tempsock[i], NULL);
+#endif
+
+ /* check socket for connect */
+ result = Curl_socket_ready(CURL_SOCKET_BAD, conn->tempsock[i], 0);
+
+ switch(result) {
+ case 0: /* no connection yet */
+ if(curlx_tvdiff(now, conn->connecttime) >= conn->timeoutms_per_addr) {
+ infof(data, "After %ldms connect time, move on!\n",
+ conn->timeoutms_per_addr);
+ break;
+ }
+ return CURLE_OK;
+
+ case CURL_CSELECT_OUT:
+ if(verifyconnect(conn->tempsock[i], &error)) {
+ /* we are connected with TCP, awesome! */
+ int other = i ^ 1;
+
+ /* use this socket from now on */
+ conn->sock[sockindex] = conn->tempsock[i];
+ conn->ip_addr = conn->tempaddr[i];
+
+ /* close the other socket, if open */
+ if(conn->tempsock[other] != CURL_SOCKET_BAD) {
+ if(conn->fclosesocket)
+ conn->fclosesocket(conn->closesocket_client,
+ conn->tempsock[other]);
+ else
+ sclose(conn->tempsock[other]);
+ }

- if(CHKCONN_CONNECTED == chk) {
- if(verifyconnect(sockfd, &error)) {
- /* we are connected with TCP, awesome! */
+ /* see if we need to do any proxy magic first once we connected */
+ code = Curl_connected_proxy(conn);
+ if(code)
+ return code;

- /* see if we need to do any proxy magic first once we connected */
- code = Curl_connected_proxy(conn);
- if(code)
- return code;
+ conn->bits.tcpconnect[sockindex] = TRUE;

- conn->bits.tcpconnect[sockindex] = TRUE;
+ *connected = TRUE;
+ if(sockindex == FIRSTSOCKET)
+ Curl_pgrsTime(data, TIMER_CONNECT); /* connect done */
+ Curl_updateconninfo(conn, conn->sock[sockindex]);
+ Curl_verboseconnect(conn);

- *connected = TRUE;
- if(sockindex == FIRSTSOCKET)
- Curl_pgrsTime(data, TIMER_CONNECT); /* connect done */
- Curl_verboseconnect(conn);
- Curl_updateconninfo(conn, sockfd);
+ return CURLE_OK;
+ }
+ else
+ infof(data, "Connection failed\n");
+ break;

+ case CURL_CSELECT_ERR|CURL_CSELECT_OUT:
+ (void)verifyconnect(conn->tempsock[i], &error);
+ break;
+
+ default:
+ infof(data, "Whut?\n");
return CURLE_OK;
}
- /* nope, not connected for real */
- }
- else {
- /* nope, not connected */
- if(CHKCONN_FDSET_ERROR == chk) {
- (void)verifyconnect(sockfd, &error);
- infof(data, "%s\n",Curl_strerror(conn, error));
- }
- else
- infof(data, "Connection failed\n");
- }

- /*
- * The connection failed here, we should attempt to connect to the "next
- * address" for the given host. But first remember the latest error.
- */
- if(error) {
- data->state.os_errno = error;
- SET_SOCKERRNO(error);
- }
- next:
+ /*
+ * The connection failed here, we should attempt to connect to the "next
+ * address" for the given host. But first remember the latest error.
+ */
+ if(error) {
+ data->state.os_errno = error;
+ SET_SOCKERRNO(error);
+ }

- conn->timeoutms_per_addr = conn->ip_addr->ai_next == NULL ?
- allow : allow / 2;
- code = trynextip(conn, sockindex, connected);
+ conn->timeoutms_per_addr = conn->tempaddr[i]->ai_next == NULL ?
+ allow : allow / 2;
+ code = trynextip(conn, sockindex, i, connected);

- if(code) {
- error = SOCKERRNO;
- data->state.os_errno = error;
- failf(data, "Failed connect to %s:%ld; %s",
- conn->host.name, conn->port, Curl_strerror(conn, error));
+ if(code) {
+ error = SOCKERRNO;
+ data->state.os_errno = error;
+ failf(data, "Failed connect to %s:%ld; %s",
+ conn->host.name, conn->port, Curl_strerror(conn, error));
+ }
}

return code;
@@ -948,6 +942,8 @@ singleipconnect(struct connectdata *conn,
struct SessionHandle *data = conn->data;
curl_socket_t sockfd;
CURLcode res = CURLE_OK;
+ char ipaddress[MAX_IPADR_LEN];
+ long port;

*sockp = CURL_SOCKET_BAD;
*connected = FALSE; /* default is not connected */
@@ -961,7 +957,7 @@ singleipconnect(struct connectdata *conn,

/* store remote address and port used in this connection attempt */
if(!getaddressinfo((struct sockaddr*)&addr.sa_addr,
- conn->primary_ip, &conn->primary_port)) {
+ ipaddress, &port)) {
/* malformed address or bug in inet_ntop, try next address */
error = ERRNO;
failf(data, "sa_addr inet_ntop() failed with errno %d: %s",
@@ -969,10 +965,7 @@ singleipconnect(struct connectdata *conn,
Curl_closesocket(conn, sockfd);
return CURLE_OK;
}
- memcpy(conn->ip_addr_str, conn->primary_ip, MAX_IPADR_LEN);
- infof(data, " Trying %s...\n", conn->ip_addr_str);
-
- Curl_persistconninfo(conn);
+ infof(data, " Trying %s...\n", ipaddress);

if(data->set.tcp_nodelay)
tcpnodelay(conn, sockfd);
@@ -1074,24 +1067,18 @@ singleipconnect(struct connectdata *conn,

CURLcode Curl_connecthost(struct connectdata *conn, /* context */
const struct Curl_dns_entry *remotehost,
- curl_socket_t *sockconn, /* the connected socket */
- Curl_addrinfo **addr, /* the one we used */
bool *connected) /* really connected? */
{
struct SessionHandle *data = conn->data;
- curl_socket_t sockfd = CURL_SOCKET_BAD;
- Curl_addrinfo *ai;
- Curl_addrinfo *curr_addr;
-
struct timeval after;
struct timeval before = Curl_tvnow();
+ int i;

/*************************************************************
* Figure out what maximum time we have left
*************************************************************/
long timeout_ms;

- DEBUGASSERT(sockconn);
*connected = FALSE; /* default to not connected */

/* get the timeout left */
@@ -1104,45 +1091,62 @@ CURLcode Curl_connecthost(struct connectdata *conn, /* context */
}

conn->num_addr = Curl_num_addresses(remotehost->addr);
-
- ai = remotehost->addr;
+ conn->tempaddr[0] = remotehost->addr;
+ conn->tempaddr[1] = remotehost->addr;

/* Below is the loop that attempts to connect to all IP-addresses we
- * know for the given host. One by one until one IP succeeds.
+ * know for the given host.
+ * One by one, for each protocol, until one IP succeeds.
*/

- /*
- * Connecting with a Curl_addrinfo chain
- */
- for(curr_addr = ai; curr_addr; curr_addr = curr_addr->ai_next) {
- CURLcode res;
-
- /* Max time for the next address */
- conn->timeoutms_per_addr = curr_addr->ai_next == NULL ?
- timeout_ms : timeout_ms / 2;
-
- /* start connecting to the IP curr_addr points to */
- res = singleipconnect(conn, curr_addr,
- &sockfd, connected);
- if(res)
- return res;
-
- if(sockfd != CURL_SOCKET_BAD)
- break;
+ for(i=0; i<2; i++) {
+ curl_socket_t sockfd = CURL_SOCKET_BAD;
+ Curl_addrinfo *ai = conn->tempaddr[i];
+ int family = i ? AF_INET6 : AF_INET;
+
+ /* find first address for this address family, if any */
+ while(ai && ai->ai_family != family)
+ ai = ai->ai_next;
+
+ /*
+ * Connecting with a Curl_addrinfo chain
+ */
+ while(ai) {
+ CURLcode res;
+
+ /* Max time for the next connection attempt */
+ conn->timeoutms_per_addr = ai->ai_next == NULL ?
+ timeout_ms : timeout_ms / 2;
+
+ /* start connecting to the IP curr_addr points to */
+ res = singleipconnect(conn, ai, &sockfd, connected);
+ if(res)
+ return res;
+
+ if(sockfd != CURL_SOCKET_BAD)
+ break;
+
+ /* get a new timeout for next attempt */
+ after = Curl_tvnow();
+ timeout_ms -= Curl_tvdiff(after, before);
+ if(timeout_ms < 0) {
+ failf(data, "connect() timed out!");
+ return CURLE_OPERATION_TIMEDOUT;
+ }
+ before = after;

- /* get a new timeout for next attempt */
- after = Curl_tvnow();
- timeout_ms -= Curl_tvdiff(after, before);
- if(timeout_ms < 0) {
- failf(data, "connect() timed out!");
- return CURLE_OPERATION_TIMEDOUT;
- }
- before = after;
- } /* end of connect-to-each-address loop */
+ /* next addresses */
+ do {
+ ai = ai->ai_next;
+ } while(ai && ai->ai_family != family);
+ } /* end of connect-to-each-address loop */

- *sockconn = sockfd; /* the socket descriptor we've connected */
+ conn->tempsock[i] = sockfd;
+ conn->tempaddr[i] = ai;
+ }

- if(sockfd == CURL_SOCKET_BAD) {
+ if((conn->tempsock[0] == CURL_SOCKET_BAD) &&
+ (conn->tempsock[1] == CURL_SOCKET_BAD)) {
/* no good connect was made */
failf(data, "couldn't connect to %s at %s:%ld",
conn->bits.proxy?"proxy":"host",
@@ -1152,10 +1156,6 @@ CURLcode Curl_connecthost(struct connectdata *conn, /* context */

/* leave the socket in non-blocking mode */

- /* store the address we use */
- if(addr)
- *addr = curr_addr;
-
data->info.numconnects++; /* to track the number of connections made */

return CURLE_OK;
diff --git a/lib/connect.h b/lib/connect.h
index ab98002..526ce37 100644
--- a/lib/connect.h
+++ b/lib/connect.h
@@ -33,8 +33,6 @@ CURLcode Curl_is_connected(struct connectdata *conn,
CURLcode Curl_connecthost(struct connectdata *conn,
const struct Curl_dns_entry *host, /* connect to
this */
- curl_socket_t *sockconn, /* not set if error */
- Curl_addrinfo **addr, /* the one we used */
bool *connected); /* truly connected? */

/* generic function that returns how much time there's left to run, according
diff --git a/lib/ftp.c b/lib/ftp.c
index e818af6..d35883a 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -1808,7 +1808,6 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
struct ftp_conn *ftpc = &conn->proto.ftpc;
CURLcode result;
struct SessionHandle *data=conn->data;
- Curl_addrinfo *conninfo;
struct Curl_dns_entry *addr=NULL;
int rc;
unsigned short connectport; /* the local port connect() should use! */
@@ -1972,8 +1971,6 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,

result = Curl_connecthost(conn,
addr,
- &conn->sock[SECONDARYSOCKET],
- &conninfo,
&connected);

Curl_resolv_unlock(data, addr); /* we're done using this address */
@@ -1995,7 +1992,7 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,

if(data->set.verbose)
/* this just dumps information about this second connection */
- ftp_pasv_verbose(conn, conninfo, newhost, connectport);
+ ftp_pasv_verbose(conn, conn->ip_addr, newhost, connectport);

switch(conn->proxytype) {
/* FIX: this MUST wait for a proper connect first if 'connected' is
diff --git a/lib/multi.c b/lib/multi.c
index e723a3e..f11ba06 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -621,17 +621,26 @@ static int waitconnect_getsock(struct connectdata *conn,
curl_socket_t *sock,
int numsocks)
{
+ int i;
+ int s=0;
+ int rc=0;
+
if(!numsocks)
return GETSOCK_BLANK;

- sock[0] = conn->sock[FIRSTSOCKET];
+ for(i=0; i<2; i++) {
+ if(conn->tempsock[i] != CURL_SOCKET_BAD) {
+ sock[s] = conn->tempsock[i];
+ rc |= GETSOCK_WRITESOCK(s++);
+ }
+ }

/* when we've sent a CONNECT to a proxy, we should rather wait for the
socket to become readable to be able to get the response headers */
if(conn->tunnel_state[FIRSTSOCKET] == TUNNEL_CONNECT)
return GETSOCK_READSOCK(0);

- return GETSOCK_WRITESOCK(0);
+ return rc;
}

static int domore_getsock(struct connectdata *conn,
diff --git a/lib/url.c b/lib/url.c
index c1672d0..ed4750f 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3257,7 +3257,6 @@ static CURLcode ConnectPlease(struct SessionHandle *data,
bool *connected)
{
CURLcode result;
- Curl_addrinfo *addr;
#ifndef CURL_DISABLE_VERBOSE_STRINGS
char *hostname = conn->bits.proxy?conn->proxy.name:conn->host.name;

@@ -3273,13 +3272,8 @@ static CURLcode ConnectPlease(struct SessionHandle *data,
*************************************************************/
result= Curl_connecthost(conn,
conn->dns_entry,
- &conn->sock[FIRSTSOCKET],
- &addr,
connected);
if(CURLE_OK == result) {
- /* All is cool, we store the current information */
- conn->ip_addr = addr;
-
if(*connected) {
result = Curl_connected_proxy(conn);
if(!result) {
@@ -5640,8 +5634,8 @@ CURLcode Curl_setup_conn(struct connectdata *conn,
Curl_pgrsTime(data, TIMER_APPCONNECT); /* we're connected already */
conn->bits.tcpconnect[FIRSTSOCKET] = TRUE;
*protocol_done = TRUE;
- Curl_verboseconnect(conn);
Curl_updateconninfo(conn, conn->sock[FIRSTSOCKET]);
+ Curl_verboseconnect(conn);
}
/* Stop the loop now */
break;
diff --git a/lib/urldata.h b/lib/urldata.h
index ebaaf6f..98686bb 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -838,6 +838,7 @@ struct connectdata {
within the DNS cache, so this pointer is only valid as long as the DNS
cache entry remains locked. It gets unlocked in Curl_done() */
Curl_addrinfo *ip_addr;
+ Curl_addrinfo *tempaddr[2]; /* for happy eyeballs */

/* 'ip_addr_str' is the ip_addr data as a human readable string.
It remains available as long as the connection does, which is longer than
@@ -889,6 +890,7 @@ struct connectdata {
struct timeval created; /* creation time */
curl_socket_t sock[2]; /* two sockets, the second is used for the data
transfer when doing FTP */
+ curl_socket_t tempsock[2]; /* temporary sockets for happy eyeballs */
bool sock_accepted[2]; /* TRUE if the socket on this index was created with
accept() */
Curl_recv *recv[2];
--
1.7.10.4

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-libr
Björn Stenberg
2013-10-27 09:19:00 UTC
Permalink
Rebased and merged against HEAD.
--
Björn
Daniel Stenberg
2013-10-27 09:31:26 UTC
Permalink
Post by Björn Stenberg
Rebased and merged against HEAD.
Thanks, but now I get a compile error:

ftp.c: In function 'ftp_state_pasv_resp':
ftp.c:2064:28: error: 'conninfo' undeclared (first use in this function)
ftp_pasv_verbose(conn, conninfo, ftpc->newhost, connectport);
^
ftp.c:2064:28: note: each undeclared identifier is reported only once for each
function it appears in

Is that info perhaps now already shown in the general connect function so we
can just remove the ftp_pasv_verbose() one?
--
/ daniel.haxx.se
Björn Stenberg
2013-10-27 10:01:50 UTC
Permalink
Post by Björn Stenberg
Rebased and merged against HEAD.
Argh, git/brain interface malfuntion.

Here it is with the fix actually committed.
--
Björn
Daniel Stenberg
2013-10-27 10:31:30 UTC
Permalink
Post by Björn Stenberg
Here it is with the fix actually committed.
Lovely. I've merged this and pushed on master now. Thanks!

As of commit 7d7df83198, libcurl is using the Happy Eyeballs approach on dual
stack clients.
--
/ daniel.haxx.se
Björn Stenberg
2013-10-27 11:28:55 UTC
Permalink
Post by Daniel Stenberg
Lovely. I've merged this and pushed on master now. Thanks!
Awesome!

I found a mistake. In a special case it will only try the first address in a family. Here is the fix.
--
Björn
Daniel Stenberg
2013-10-27 11:50:53 UTC
Permalink
Post by Björn Stenberg
I found a mistake. In a special case it will only try the first address in a
family. Here is the fix.
Thanks, pushed!
--
/ daniel.haxx.se
Paul Marks
2013-10-27 19:13:56 UTC
Permalink
As an Internet Lorax, I beg you to reconsider this algorithm.

Pretend that it's a few years in the future, and most ISPs are using
Carrier Grade NAT for IPv4 access. We need to ensure that IPv6
remains a practical alternative to building more NAT capacity, and
that requires cooperation from the client-side software.

This patch makes curl send out IPv4 and IPv6 SYNs simultaneously,
which means that even on a fully dual-stack network, every request
creates state in the NAT. Worse, preferring IPv4 requires the NAT to
track the entire session in the majority of cases. In the Prisoner's
Dilemma sense, this algorithm chooses "defect."

Here's an alternative algorithm that's simple and effective; Chrome
does something similar:

- Connect to the first address returned by getaddrinfo().
- After 300ms, connect to an address of the opposite family.
- When one connection succeeds, abort the other.

References:
[1] http://tools.ietf.org/html/rfc6555#section-4.1
[2] https://code.google.com/p/chromium/issues/detail?id=81686#c12
[3] https://labs.ripe.net/Members/emileaben/hampered-eyeballs
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Daniel Stenberg
2013-10-27 19:53:41 UTC
Permalink
Post by Paul Marks
As an Internet Lorax, I beg you to reconsider this algorithm.
I think you're thinking about this a slightly wrong way. It is not
reconsidering the algorithm that we need, it is polishing and improving the
algorithm.

We welcome all contributions that make libcurl better and that includes Happy
Eyeballs.
--
/ daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Paul Marks
2013-10-27 20:35:25 UTC
Permalink
Post by Daniel Stenberg
Post by Paul Marks
As an Internet Lorax, I beg you to reconsider this algorithm.
I think you're thinking about this a slightly wrong way. It is not
reconsidering the algorithm that we need, it is polishing and improving the
algorithm.
We welcome all contributions that make libcurl better and that includes
Happy Eyeballs.
Yes, I'm a fan of Happy Eyeballs in general, provided that the
implementation is consistent with the spirit of the RFC.


I tested the current git code, and this request returned an IPv4
address on every attempt:

$ src/curl -v http://ds.test-ipv6.com/ip/
* About to connect() to ds.test-ipv6.com port 80 (#0)
* Trying 216.218.228.119...
* Trying 2001:470:1:18::119...
* Adding handle: conn: 0x13268e0
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x13268e0) send_pipe: 1, recv_pipe: 0
* Connected to ds.test-ipv6.com (216.218.228.119) port 80 (#0)
...
callback({"ip":"98.248.32.xx","type":"ipv4","subtype":"","via":"","padding":""})
* Connection #0 to host ds.test-ipv6.com left intact


I then ran this on my router:
$ iptables -I FORWARD -i eth1 -s 216.218.228.119 -j DROP


And curl hangs waiting for the IPv4 response:
$ src/curl -v http://ds.test-ipv6.com/ip/
* About to connect() to ds.test-ipv6.com port 80 (#0)
* Trying 216.218.228.119...
* Trying 2001:470:1:18::119...
* Adding handle: conn: 0x15548e0
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x15548e0) send_pipe: 1, recv_pipe: 0
// -- At this point, it hangs for a couple minutes --
* Whut?
* Failed connect to ds.test-ipv6.com:80; Connection timed out
* Failed connect to ds.test-ipv6.com:80; Broken pipe
* Closing connection 0
curl: (7) Failed connect to ds.test-ipv6.com:80; Connection timed out


But IPv6-only mode works fine:
$ src/curl -v -6 http://ds.test-ipv6.com/ip/
* About to connect() to ds.test-ipv6.com port 80 (#0)
* Trying 2001:470:1:18::119...
* Adding handle: conn: 0x21228e0
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x21228e0) send_pipe: 1, recv_pipe: 0
* Connected to ds.test-ipv6.com (2001:470:1:18::119) port 80 (#0)
...
callback({"ip":"2601:9:xxxx:xxx::f61","type":"ipv6","subtype":"","via":"","padding":""})
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Björn Stenberg
2013-10-28 09:20:15 UTC
Permalink
Post by Paul Marks
Post by Paul Marks
As an Internet Lorax, I beg you to reconsider this algorithm.
Yes, the current algorithm is quite naive. I'm working on an update.
Post by Paul Marks
$ iptables -I FORWARD -i eth1 -s 216.218.228.119 -j DROP
$ src/curl -v http://ds.test-ipv6.com/ip/
* About to connect() to ds.test-ipv6.com port 80 (#0)
* Trying 216.218.228.119...
* Trying 2001:470:1:18::119...
* Adding handle: conn: 0x15548e0
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x15548e0) send_pipe: 1, recv_pipe: 0
// -- At this point, it hangs for a couple minutes --
* Whut?
Ah, interesting! Thank you for the bug report.
--
Björn
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Björn Stenberg
2013-10-29 19:18:08 UTC
Permalink
Post by Paul Marks
$ iptables -I FORWARD -i eth1 -s 216.218.228.119 -j DROP
Here is a fix for this.
--
Björn
Daniel Stenberg
2013-10-29 22:01:02 UTC
Permalink
Post by Björn Stenberg
Here is a fix for this.
Merged and pushed, thanks!
--
/ daniel.haxx.se
Paul Marks
2013-10-30 03:54:57 UTC
Permalink
Post by Daniel Stenberg
Post by Björn Stenberg
Here is a fix for this.
Merged and pushed, thanks!
With the latest git, filtering IPv4 now correctly triggers IPv6
fallback. I just hope that you're aiming for something consistent
with RFC6555 before this lands in a stable release.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Kamil Dudka
2013-10-30 15:29:32 UTC
Permalink
Post by Paul Marks
Post by Daniel Stenberg
Post by Björn Stenberg
Here is a fix for this.
Merged and pushed, thanks!
With the latest git, filtering IPv4 now correctly triggers IPv6
fallback. I just hope that you're aiming for something consistent
with RFC6555 before this lands in a stable release.
The latest upstream git HEAD (f6c335d6) still breaks the test-suite on my
RHEL-6 box. The tests exercising non-listening sockets (19 702 703 704 705)
hang consuming 100% CPU and the file tests/log/trace19 consumes several GBs
of disk space very quickly. It contains:

16:21:08.288894 == Info: Rebuilt URL to: 127.0.0.1:60000/
16:21:08.289076 == Info: About to connect() to 127.0.0.1 port 60000 (#0)
16:21:08.289099 == Info: Trying 127.0.0.1...
16:21:08.289159 == Info: Adding handle: conn: 0xc4ed30
16:21:08.289163 == Info: Adding handle: send: 0
16:21:08.289166 == Info: Adding handle: recv: 0
16:21:08.289168 == Info: Curl_addHandleToPipeline: length: 1
16:21:08.289172 == Info: - Conn 0 (0xc4ed30) send_pipe: 1, recv_pipe: 0
16:21:08.289514 == Info: Whut?
16:21:08.289521 == Info: Whut?
16:21:08.289524 == Info: Whut?
16:21:08.289528 == Info: Whut?
16:21:08.289531 == Info: Whut?
16:21:08.289534 == Info: Whut?
16:21:08.289537 == Info: Whut?
16:21:08.289540 == Info: Whut?
16:21:08.289544 == Info: Whut?
16:21:08.289547 == Info: Whut?
16:21:08.289550 == Info: Whut?
16:21:08.289553 == Info: Whut?
16:21:08.289556 == Info: Whut?
16:21:08.289559 == Info: Whut?
16:21:08.289562 == Info: Whut?
16:21:08.289565 == Info: Whut?
...
[[ repeats many many times ]]
...

Kamil

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Björn Stenberg
2013-10-30 15:48:02 UTC
Permalink
Post by Kamil Dudka
The latest upstream git HEAD (f6c335d6) still breaks the test-suite on my
RHEL-6 box. The tests exercising non-listening sockets (19 702 703 704 705)
hang consuming 100% CPU and the file tests/log/trace19 consumes several GBs
16:21:08.288894 == Info: Rebuilt URL to: 127.0.0.1:60000/
16:21:08.289076 == Info: About to connect() to 127.0.0.1 port 60000 (#0)
16:21:08.289099 == Info: Trying 127.0.0.1...
16:21:08.289159 == Info: Adding handle: conn: 0xc4ed30
16:21:08.289163 == Info: Adding handle: send: 0
16:21:08.289166 == Info: Adding handle: recv: 0
16:21:08.289168 == Info: Curl_addHandleToPipeline: length: 1
16:21:08.289172 == Info: - Conn 0 (0xc4ed30) send_pipe: 1, recv_pipe: 0
16:21:08.289514 == Info: Whut?
16:21:08.289521 == Info: Whut?
Ouch. Would you mind changing line lib/connect.c:796 (the "Whut" line) to the following and tell me what it says?

infof(data, "Curl_socket_ready() returned 0x%02x\n", result);
--
Björn
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Kamil Dudka
2013-10-30 16:11:55 UTC
Permalink
Post by Björn Stenberg
Post by Kamil Dudka
The latest upstream git HEAD (f6c335d6) still breaks the test-suite on my
RHEL-6 box. The tests exercising non-listening sockets (19 702 703 704
705) hang consuming 100% CPU and the file tests/log/trace19 consumes
16:21:08.288894 == Info: Rebuilt URL to: 127.0.0.1:60000/
16:21:08.289076 == Info: About to connect() to 127.0.0.1 port 60000 (#0)
16:21:08.289099 == Info: Trying 127.0.0.1...
16:21:08.289159 == Info: Adding handle: conn: 0xc4ed30
16:21:08.289163 == Info: Adding handle: send: 0
16:21:08.289166 == Info: Adding handle: recv: 0
16:21:08.289168 == Info: Curl_addHandleToPipeline: length: 1
16:21:08.289172 == Info: - Conn 0 (0xc4ed30) send_pipe: 1, recv_pipe: 0
16:21:08.289514 == Info: Whut?
16:21:08.289521 == Info: Whut?
Ouch. Would you mind changing line lib/connect.c:796 (the "Whut" line) to
the following and tell me what it says?
infof(data, "Curl_socket_ready() returned 0x%02x\n", result);
Sure. The output follows. Maybe we should commit the change you suggest also
upstream in order to improve the debugging experience of binary releases?

Kamil


=== Start of file trace19
17:07:40.037517 == Info: Rebuilt URL to: 127.0.0.1:60000/
17:07:40.037681 == Info: About to connect() to 127.0.0.1 port 60000 (#0)
17:07:40.037703 == Info: Trying 127.0.0.1...
17:07:40.037762 == Info: Adding handle: conn: 0x1259d30
17:07:40.037767 == Info: Adding handle: send: 0
17:07:40.037770 == Info: Adding handle: recv: 0
17:07:40.037772 == Info: Curl_addHandleToPipeline: length: 1
17:07:40.037776 == Info: - Conn 0 (0x1259d30) send_pipe: 1, recv_pipe: 0
17:07:40.038093 == Info: Curl_socket_ready() returned 0x04
17:07:40.038100 == Info: Curl_socket_ready() returned 0x04
17:07:40.038104 == Info: Curl_socket_ready() returned 0x04
17:07:40.038108 == Info: Curl_socket_ready() returned 0x04
17:07:40.038111 == Info: Curl_socket_ready() returned 0x04
17:07:40.038115 == Info: Curl_socket_ready() returned 0x04
17:07:40.038118 == Info: Curl_socket_ready() returned 0x04
17:07:40.038122 == Info: Curl_socket_ready() returned 0x04
17:07:40.038125 == Info: Curl_socket_ready() returned 0x04
17:07:40.038129 == Info: Curl_socket_ready() returned 0x04
17:07:40.038132 == Info: Curl_socket_ready() returned 0x04
17:07:40.038136 == Info: Curl_socket_ready() returned 0x04
17:07:40.038139 == Info: Curl_socket_ready() returned 0x04
17:07:40.038143 == Info: Curl_socket_ready() returned 0x04
17:07:40.038146 == Info: Curl_socket_ready() returned 0x04
17:07:40.038150 == Info: Curl_socket_ready() returned 0x04
17:07:40.038153 == Info: Curl_socket_ready() returned 0x04
17:07:40.038160 == Info: Curl_socket_ready() returned 0x04

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Björn Stenberg
2013-10-30 20:40:12 UTC
Permalink
Maybe we should commit the change you suggest also upstream in order to
improve the debugging experience of binary releases?
Yes, unless this attached patch fixes it. Please test it at your convenience.
--
Björn
Kamil Dudka
2013-10-30 22:41:46 UTC
Permalink
Post by Björn Stenberg
Maybe we should commit the change you suggest also upstream in order to
improve the debugging experience of binary releases?
Yes, unless this attached patch fixes it. Please test it at your convenience.
This seems to have fixed the issue. Thanks for the quick turnaround!

Kamil

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Kamil Dudka
2013-11-02 21:10:18 UTC
Permalink
Hi Björn,
Post by Björn Stenberg
Maybe we should commit the change you suggest also upstream in order to
improve the debugging experience of binary releases?
Yes, unless this attached patch fixes it. Please test it at your convenience.
is there something that prevents the patch from being pushed upstream?

Kamil

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Daniel Stenberg
2013-11-03 10:08:44 UTC
Permalink
Post by Kamil Dudka
Post by Björn Stenberg
Yes, unless this attached patch fixes it. Please test it at your convenience.
is there something that prevents the patch from being pushed upstream?
Right, I've been a little side-tracked so please tell me again which patches
we should push.
--
/ daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Daniel Stenberg
2013-10-27 20:06:20 UTC
Permalink
Post by Paul Marks
- After 300ms, connect to an address of the opposite family.
BTW, I found this IETF-87 presentation[1] which claims various implementations
use(d) these timer values:

- [RFC 6555] recommends 150-250ms.
- Google Chrome uses 300ms.
- Firefox uses 250ms.
- Happy Eyeballs Erlang Implementation uses 100ms

[1] = http://cnds.eecs.jacobs-university.de/slides/2013-ietf-87-happy.pdf
--
/ daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Loading...