From dbd16c3e256c6ce872829d1654785485361a0a78 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 12 Mar 2020 14:03:26 +0100 Subject: connect: happy eyeballs cleanup Make sure each separate index in connn->tempaddr[] is used for a fixed family (and only that family) during the connection process. If family one takes a long time and family two fails immediately, the previous logic could misbehave and retry the same family two address repeatedly. Reported-by: Paul Vixie Reported-by: Jay Satiro Fixes #5083 Fixes #4954 Closes #5089 --- lib/connect.c | 101 ++++++++++++++++++++++++++++------------------------------ lib/urldata.h | 3 ++ 2 files changed, 51 insertions(+), 53 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index 0a7475cb6..5e631baa9 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -167,7 +167,7 @@ tcpkeepalive(struct Curl_easy *data, static CURLcode singleipconnect(struct connectdata *conn, const Curl_addrinfo *ai, /* start connecting to this */ - int sockindex); /* 0 or 1 among the temp ones */ + int tempindex); /* 0 or 1 among the temp ones */ /* * Curl_timeleft() returns the amount of milliseconds left allowed for the @@ -555,13 +555,27 @@ static bool verifyconnect(curl_socket_t sockfd, int *error) return rc; } +/* update tempaddr[tempindex] (to the next entry), makes sure to stick + to the correct family */ +static Curl_addrinfo *ainext(struct connectdata *conn, + int tempindex, + bool next) /* use current or next entry */ +{ + Curl_addrinfo *ai = conn->tempaddr[tempindex]; + if(ai && next) + ai = ai->ai_next; + while(ai && (ai->ai_family != conn->tempfamily[tempindex])) + ai = ai->ai_next; + conn->tempaddr[tempindex] = ai; + return ai; +} + /* Used within the multi interface. Try next IP address, return TRUE if no more address exists or error */ static CURLcode trynextip(struct connectdata *conn, int sockindex, int tempindex) { - const int other = tempindex ^ 1; CURLcode result = CURLE_COULDNT_CONNECT; /* First clean up after the failed socket. @@ -572,38 +586,15 @@ static CURLcode trynextip(struct connectdata *conn, conn->tempsock[tempindex] = CURL_SOCKET_BAD; if(sockindex == FIRSTSOCKET) { - Curl_addrinfo *ai = NULL; - int family = AF_UNSPEC; - - if(conn->tempaddr[tempindex]) { - /* find next address in the same protocol family */ - family = conn->tempaddr[tempindex]->ai_family; - ai = conn->tempaddr[tempindex]->ai_next; - } -#ifdef ENABLE_IPV6 - else if(conn->tempaddr[0]) { - /* happy eyeballs - try the other protocol family */ - int firstfamily = conn->tempaddr[0]->ai_family; - family = (firstfamily == AF_INET) ? AF_INET6 : AF_INET; - ai = conn->tempaddr[0]->ai_next; - } -#endif + Curl_addrinfo *ai = conn->tempaddr[tempindex]; while(ai) { - if(conn->tempaddr[other]) { - /* we can safely skip addresses of the other protocol family */ - while(ai && ai->ai_family != family) - ai = ai->ai_next; - } - if(ai) { result = singleipconnect(conn, ai, tempindex); if(result == CURLE_COULDNT_CONNECT) { - ai = ai->ai_next; + ai = ainext(conn, tempindex, TRUE); continue; } - - conn->tempaddr[tempindex] = ai; } break; } @@ -905,9 +896,10 @@ CURLcode Curl_is_connected(struct connectdata *conn, } /* should we try another protocol family? */ - if(i == 0 && conn->tempaddr[1] == NULL && + if(i == 0 && !conn->parallel_connect && (Curl_timediff(now, conn->connecttime) >= data->set.happy_eyeballs_timeout)) { + conn->parallel_connect = TRUE; /* starting now */ trynextip(conn, sockindex, 1); } } @@ -967,7 +959,7 @@ CURLcode Curl_is_connected(struct connectdata *conn, conn->timeoutms_per_addr = conn->tempaddr[i]->ai_next == NULL ? allow : allow / 2; - + ainext(conn, i, TRUE); status = trynextip(conn, sockindex, i); if((status != CURLE_COULDNT_CONNECT) || conn->tempsock[other] == CURL_SOCKET_BAD) @@ -984,7 +976,7 @@ CURLcode Curl_is_connected(struct connectdata *conn, /* if the first address family runs out of addresses to try before the happy eyeball timeout, go ahead and try the next family now */ - if(conn->tempaddr[1] == NULL) { + { result = trynextip(conn, sockindex, 1); if(!result) return result; @@ -1113,7 +1105,7 @@ void Curl_sndbufset(curl_socket_t sockfd) */ static CURLcode singleipconnect(struct connectdata *conn, const Curl_addrinfo *ai, - int sockindex) + int tempindex) { struct Curl_sockaddr_ex addr; int rc = -1; @@ -1129,15 +1121,12 @@ static CURLcode singleipconnect(struct connectdata *conn, int optval = 1; #endif char buffer[STRERROR_LEN]; - curl_socket_t *sockp = &conn->tempsock[sockindex]; + curl_socket_t *sockp = &conn->tempsock[tempindex]; *sockp = CURL_SOCKET_BAD; result = Curl_socket(conn, ai, &addr, &sockfd); if(result) - /* Failed to create the socket, but still return OK since we signal the - lack of socket as well. This allows the parent function to keep looping - over alternative addresses/socket families etc. */ - return CURLE_OK; + return result; /* store remote address and port used in this connection attempt */ if(!Curl_addr2string((struct sockaddr*)&addr.sa_addr, addr.addrlen, @@ -1257,7 +1246,7 @@ static CURLcode singleipconnect(struct connectdata *conn, else if(conn->transport == TRNSPRT_QUIC) { /* pass in 'sockfd' separately since it hasn't been put into the tempsock array at this point */ - result = Curl_quic_connect(conn, sockfd, sockindex, + result = Curl_quic_connect(conn, sockfd, tempindex, &addr.sa_addr, addr.addrlen); if(result) error = SOCKERRNO; @@ -1315,7 +1304,7 @@ CURLcode Curl_connecthost(struct connectdata *conn, /* context */ struct Curl_easy *data = conn->data; struct curltime before = Curl_now(); CURLcode result = CURLE_COULDNT_CONNECT; - + int i; timediff_t timeout_ms = Curl_timeleft(data, &before, TRUE); if(timeout_ms < 0) { @@ -1325,28 +1314,34 @@ CURLcode Curl_connecthost(struct connectdata *conn, /* context */ } conn->num_addr = Curl_num_addresses(remotehost->addr); - conn->tempaddr[0] = remotehost->addr; - conn->tempaddr[1] = NULL; - conn->tempsock[0] = CURL_SOCKET_BAD; - conn->tempsock[1] = CURL_SOCKET_BAD; + conn->tempaddr[0] = conn->tempaddr[1] = remotehost->addr; + conn->tempsock[0] = conn->tempsock[1] = CURL_SOCKET_BAD; /* Max time for the next connection attempt */ conn->timeoutms_per_addr = conn->tempaddr[0]->ai_next == NULL ? timeout_ms : timeout_ms / 2; - /* start connecting to first IP */ - while(conn->tempaddr[0]) { - result = singleipconnect(conn, conn->tempaddr[0], 0); - if(!result) - break; - conn->tempaddr[0] = conn->tempaddr[0]->ai_next; - } + conn->tempfamily[0] = conn->tempaddr[0]? + conn->tempaddr[0]->ai_family:0; + conn->tempfamily[1] = conn->tempfamily[0] == AF_INET6 ? + AF_INET : AF_INET6; + ainext(conn, 1, FALSE); /* assigns conn->tempaddr[1] accordingly */ - if(conn->tempsock[0] == CURL_SOCKET_BAD) { - if(!result) - result = CURLE_COULDNT_CONNECT; - return result; + DEBUGF(infof(data, "family0 == %s, family1 == %s\n", + conn->tempfamily[0] == AF_INET ? "v4" : "v6", + conn->tempfamily[1] == AF_INET ? "v4" : "v6")); + + /* get through the list in family order in case of quick failures */ + for(i = 0; (i < 2) && result; i++) { + while(conn->tempaddr[i]) { + result = singleipconnect(conn, conn->tempaddr[i], i); + if(!result) + break; + ainext(conn, i, TRUE); + } } + if(result) + return result; data->info.numconnects++; /* to track the number of connections made */ Curl_expire(conn->data, data->set.happy_eyeballs_timeout, diff --git a/lib/urldata.h b/lib/urldata.h index fbb8b645e..4ee568fd6 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -957,6 +957,7 @@ struct connectdata { 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 */ + int tempfamily[2]; /* family used for the temp sockets */ Curl_recv *recv[2]; Curl_send *send[2]; @@ -1113,6 +1114,8 @@ struct connectdata { handle */ BIT(sock_accepted); /* TRUE if the SECONDARYSOCKET was created with accept() */ + BIT(parallel_connect); /* set TRUE when a parallel connect attempt has + started (happy eyeballs) */ }; /* The end of connectdata. */ -- cgit v1.2.3