From dc90f510657875a95d8f82def2c1e79c7284c9dc Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 15 Jun 2020 16:17:55 +0200 Subject: [PATCH] connect: improve happy eyeballs handling For QUIC but also for regular TCP when the second family runs out of IPs with a failure while the first family is still trying to connect. Separated the timeout handling for IPv4 and IPv6 connections when they both have a number of addresses to iterate over. --- lib/connect.c | 79 +++++++++++++++++++++++----------------------- lib/quic.h | 4 ++- lib/urldata.h | 9 ++++-- lib/vquic/ngtcp2.c | 42 +++++++++++++++--------- lib/vquic/quiche.c | 8 +++++ 5 files changed, 84 insertions(+), 58 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index 46c285537..29293f087 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -560,7 +560,7 @@ static bool verifyconnect(curl_socket_t sockfd, int *error) to the correct family */ static struct Curl_addrinfo *ainext(struct connectdata *conn, int tempindex, - bool next) /* use current or next entry */ + bool next) /* use next entry? */ { struct Curl_addrinfo *ai = conn->tempaddr[tempindex]; if(ai && next) @@ -571,7 +571,7 @@ static struct Curl_addrinfo *ainext(struct connectdata *conn, return ai; } -/* Used within the multi interface. Try next IP address, return TRUE if no +/* Used within the multi interface. Try next IP address, returns error if no more address exists or error */ static CURLcode trynextip(struct connectdata *conn, int sockindex, @@ -823,8 +823,8 @@ CURLcode Curl_is_connected(struct connectdata *conn, timediff_t allow; int error = 0; struct curltime now; - int rc; - int i; + int rc = 0; + unsigned int i; DEBUGASSERT(sockindex >= FIRSTSOCKET && sockindex <= SECONDARYSOCKET); @@ -859,49 +859,42 @@ CURLcode Curl_is_connected(struct connectdata *conn, const int other = i ^ 1; if(conn->tempsock[i] == CURL_SOCKET_BAD) continue; - + error = 0; #ifdef ENABLE_QUIC if(conn->transport == TRNSPRT_QUIC) { result = Curl_quic_is_connected(conn, i, connected); - if(result) { - error = SOCKERRNO; - goto error; - } - if(*connected) { + if(!result && *connected) { /* use this socket from now on */ conn->sock[sockindex] = conn->tempsock[i]; conn->ip_addr = conn->tempaddr[i]; conn->tempsock[i] = CURL_SOCKET_BAD; post_SOCKS(conn, sockindex, connected); connkeep(conn, "HTTP/3 default"); - return result; + return CURLE_OK; } - /* should we try another protocol family? */ - if(i == 0 && !conn->bits.parallel_connect && - (Curl_timediff(now, conn->connecttime) >= - data->set.happy_eyeballs_timeout)) { - conn->bits.parallel_connect = TRUE; /* starting now */ - trynextip(conn, sockindex, 1); - } - continue; + if(result) + error = SOCKERRNO; } + else #endif - + { #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); + /* 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 */ - rc = SOCKET_WRITABLE(conn->tempsock[i], 0); + /* check socket for connect */ + rc = SOCKET_WRITABLE(conn->tempsock[i], 0); + } if(rc == 0) { /* no connection yet */ - error = 0; - if(Curl_timediff(now, conn->connecttime) >= conn->timeoutms_per_addr) { + if(Curl_timediff(now, conn->connecttime) >= + conn->timeoutms_per_addr[i]) { infof(data, "After %" CURL_FORMAT_TIMEDIFF_T - "ms connect time, move on!\n", conn->timeoutms_per_addr); + "ms connect time, move on!\n", conn->timeoutms_per_addr[i]); error = ETIMEDOUT; } @@ -946,9 +939,6 @@ CURLcode Curl_is_connected(struct connectdata *conn, else if(rc & CURL_CSELECT_ERR) (void)verifyconnect(conn->tempsock[i], &error); -#ifdef ENABLE_QUIC - error: -#endif /* * The connection failed here, we should attempt to connect to the "next * address" for the given host. But first remember the latest error. @@ -968,7 +958,7 @@ CURLcode Curl_is_connected(struct connectdata *conn, Curl_strerror(error, buffer, sizeof(buffer))); #endif - conn->timeoutms_per_addr = conn->tempaddr[i]->ai_next == NULL ? + conn->timeoutms_per_addr[i] = conn->tempaddr[i]->ai_next == NULL ? allow : allow / 2; ainext(conn, i, TRUE); status = trynextip(conn, sockindex, i); @@ -980,13 +970,15 @@ CURLcode Curl_is_connected(struct connectdata *conn, } } - if(result) { + if(result && + (conn->tempsock[0] == CURL_SOCKET_BAD) && + (conn->tempsock[1] == CURL_SOCKET_BAD)) { /* no more addresses to try */ const char *hostname; char buffer[STRERROR_LEN]; - /* 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 the first address family runs out of addresses to try before the + happy eyeball timeout, go ahead and try the next family now */ result = trynextip(conn, sockindex, 1); if(!result) return result; @@ -1007,6 +999,9 @@ CURLcode Curl_is_connected(struct connectdata *conn, hostname, conn->port, Curl_strerror(error, buffer, sizeof(buffer))); + Curl_quic_disconnect(conn, 0); + Curl_quic_disconnect(conn, 1); + #ifdef WSAETIMEDOUT if(WSAETIMEDOUT == data->state.os_errno) result = CURLE_OPERATION_TIMEDOUT; @@ -1015,6 +1010,8 @@ CURLcode Curl_is_connected(struct connectdata *conn, result = CURLE_OPERATION_TIMEDOUT; #endif } + else + result = CURLE_OK; /* still trying */ return result; } @@ -1206,8 +1203,10 @@ static CURLcode singleipconnect(struct connectdata *conn, (void)curlx_nonblock(sockfd, TRUE); conn->connecttime = Curl_now(); - if(conn->num_addr > 1) - Curl_expire(data, conn->timeoutms_per_addr, EXPIRE_DNS_PER_NAME); + if(conn->num_addr > 1) { + Curl_expire(data, conn->timeoutms_per_addr[0], EXPIRE_DNS_PER_NAME); + Curl_expire(data, conn->timeoutms_per_addr[1], EXPIRE_DNS_PER_NAME2); + } /* Connect TCP and QUIC sockets */ if(!isconnected && (conn->transport != TRNSPRT_UDP)) { @@ -1330,8 +1329,10 @@ CURLcode Curl_connecthost(struct connectdata *conn, /* context */ conn->tempsock[0] = conn->tempsock[1] = CURL_SOCKET_BAD; /* Max time for the next connection attempt */ - conn->timeoutms_per_addr = + conn->timeoutms_per_addr[0] = conn->tempaddr[0]->ai_next == NULL ? timeout_ms : timeout_ms / 2; + conn->timeoutms_per_addr[1] = + conn->tempaddr[1]->ai_next == NULL ? timeout_ms : timeout_ms / 2; conn->tempfamily[0] = conn->tempaddr[0]? conn->tempaddr[0]->ai_family:0; diff --git a/lib/quic.h b/lib/quic.h index 1eb23e976..8e7df90ea 100644 --- a/lib/quic.h +++ b/lib/quic.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -47,11 +47,13 @@ int Curl_quic_ver(char *p, size_t len); CURLcode Curl_quic_done_sending(struct connectdata *conn); void Curl_quic_done(struct Curl_easy *data, bool premature); bool Curl_quic_data_pending(const struct Curl_easy *data); +void Curl_quic_disconnect(struct connectdata *conn, int tempindex); #else /* ENABLE_QUIC */ #define Curl_quic_done_sending(x) #define Curl_quic_done(x,y) #define Curl_quic_data_pending(x) +#define Curl_quic_disconnect(x,y) #endif /* !ENABLE_QUIC */ #endif /* HEADER_CURL_QUIC_H */ diff --git a/lib/urldata.h b/lib/urldata.h index 33ec160ac..74b43abaa 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -981,8 +981,10 @@ struct connectdata { struct curltime connecttime; /* The two fields below get set in Curl_connecthost */ int num_addr; /* number of addresses to try to connect to */ - timediff_t timeoutms_per_addr; /* how long time in milliseconds to spend on - trying to connect to each IP address */ + + /* how long time in milliseconds to spend on trying to connect to each IP + address, per family */ + timediff_t timeoutms_per_addr[2]; const struct Curl_handler *handler; /* Connection's protocol handler */ const struct Curl_handler *given; /* The protocol first given */ @@ -1245,7 +1247,8 @@ typedef enum { EXPIRE_100_TIMEOUT, EXPIRE_ASYNC_NAME, EXPIRE_CONNECTTIMEOUT, - EXPIRE_DNS_PER_NAME, + EXPIRE_DNS_PER_NAME, /* family1 */ + EXPIRE_DNS_PER_NAME2, /* family2 */ EXPIRE_HAPPY_EYEBALLS_DNS, /* See asyn-ares.c */ EXPIRE_HAPPY_EYEBALLS, EXPIRE_MULTI_PENDING, diff --git a/lib/vquic/ngtcp2.c b/lib/vquic/ngtcp2.c index b973a7333..e5528231c 100644 --- a/lib/vquic/ngtcp2.c +++ b/lib/vquic/ngtcp2.c @@ -256,11 +256,11 @@ static int quic_set_encryption_secrets(SSL *ssl, int level = quic_from_ossl_level(ossl_level); if(ngtcp2_crypto_derive_and_install_rx_key( - qs->qconn, NULL, NULL, NULL, level, rx_secret, secretlen) != 0) + qs->qconn, NULL, NULL, NULL, level, rx_secret, secretlen) != 0) return 0; if(ngtcp2_crypto_derive_and_install_tx_key( - qs->qconn, NULL, NULL, NULL, level, tx_secret, secretlen) != 0) + qs->qconn, NULL, NULL, NULL, level, tx_secret, secretlen) != 0) return 0; if(level == NGTCP2_CRYPTO_LEVEL_APP) { @@ -341,9 +341,7 @@ static int quic_init_ssl(struct quicsocket *qs) /* this will need some attention when HTTPS proxy over QUIC get fixed */ const char * const hostname = qs->conn->host.name; - if(qs->ssl) - SSL_free(qs->ssl); - + DEBUGASSERT(!qs->ssl); qs->ssl = SSL_new(qs->sslctx); SSL_set_app_data(qs->ssl, qs); @@ -375,11 +373,11 @@ static int secret_func(gnutls_session_t ssl, if(level != NGTCP2_CRYPTO_LEVEL_EARLY && ngtcp2_crypto_derive_and_install_rx_key( - qs->qconn, NULL, NULL, NULL, level, rx_secret, secretlen) != 0) + qs->qconn, NULL, NULL, NULL, level, rx_secret, secretlen) != 0) return 0; if(ngtcp2_crypto_derive_and_install_tx_key( - qs->qconn, NULL, NULL, NULL, level, tx_secret, secretlen) != 0) + qs->qconn, NULL, NULL, NULL, level, tx_secret, secretlen) != 0) return 0; if(level == NGTCP2_CRYPTO_LEVEL_APP) { @@ -429,8 +427,8 @@ static int tp_recv_func(gnutls_session_t ssl, const uint8_t *data, ngtcp2_transport_params params; if(ngtcp2_decode_transport_params( - ¶ms, NGTCP2_TRANSPORT_PARAMS_TYPE_ENCRYPTED_EXTENSIONS, - data, data_size) != 0) + ¶ms, NGTCP2_TRANSPORT_PARAMS_TYPE_ENCRYPTED_EXTENSIONS, + data, data_size) != 0) return -1; if(ngtcp2_conn_set_remote_transport_params(qs->qconn, ¶ms) != 0) @@ -471,8 +469,7 @@ static int quic_init_ssl(struct quicsocket *qs) const char * const hostname = qs->conn->host.name; int rc; - if(qs->ssl) - gnutls_deinit(qs->ssl); + DEBUGASSERT(!qs->ssl); gnutls_init(&qs->ssl, GNUTLS_CLIENT); gnutls_session_set_ptr(qs->ssl, qs); @@ -782,6 +779,8 @@ CURLcode Curl_quic_connect(struct connectdata *conn, long port; int qfd; + if(qs->conn) + Curl_quic_disconnect(conn, sockindex); qs->conn = conn; /* extract the used address as a string */ @@ -880,11 +879,16 @@ static int ng_perform_getsock(const struct connectdata *conn, return ng_getsock((struct connectdata *)conn, socks); } -static CURLcode qs_disconnect(struct quicsocket *qs) +static void qs_disconnect(struct quicsocket *qs) { int i; - if(qs->qlogfd != -1) + if(!qs->conn) /* already closed */ + return; + qs->conn = NULL; + if(qs->qlogfd != -1) { close(qs->qlogfd); + qs->qlogfd = -1; + } if(qs->ssl) #ifdef USE_OPENSSL SSL_free(qs->ssl); @@ -903,14 +907,22 @@ static CURLcode qs_disconnect(struct quicsocket *qs) #ifdef USE_OPENSSL SSL_CTX_free(qs->sslctx); #endif - return CURLE_OK; +} + +void Curl_quic_disconnect(struct connectdata *conn, + int tempindex) +{ + if(conn->transport == TRNSPRT_QUIC) + qs_disconnect(&conn->hequic[tempindex]); } static CURLcode ng_disconnect(struct connectdata *conn, bool dead_connection) { (void)dead_connection; - return qs_disconnect(&conn->hequic[0]); + Curl_quic_disconnect(conn, 0); + Curl_quic_disconnect(conn, 1); + return CURLE_OK; } static unsigned int ng_conncheck(struct connectdata *conn, diff --git a/lib/vquic/quiche.c b/lib/vquic/quiche.c index 9af2d894f..be6f15c19 100644 --- a/lib/vquic/quiche.c +++ b/lib/vquic/quiche.c @@ -107,6 +107,14 @@ static CURLcode quiche_disconnect(struct connectdata *conn, (void)dead_connection; return qs_disconnect(qs); } + +void Curl_quic_disconnect(struct connectdata *conn, + int tempindex) +{ + if(conn->transport == TRNSPRT_QUIC) + qs_disconnect(&conn->hequic[tempindex]); +} + static unsigned int quiche_conncheck(struct connectdata *conn, unsigned int checks_to_perform) {