From 6b33fa3417d891b7588f3b4f653804b8f3654b71 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 20 Apr 2022 21:28:23 +0900 Subject: [PATCH] Bump ngtcp2 and nghttp3 --- .github/workflows/build.yml | 4 +- README.rst | 8 +-- configure.ac | 6 +-- src/h2load_quic.cc | 4 +- src/shrpx_http3_upstream.cc | 76 ++++++++-------------------- src/shrpx_http3_upstream.h | 4 -- src/shrpx_quic.cc | 18 +++---- src/shrpx_quic.h | 14 ++--- src/shrpx_quic_connection_handler.cc | 6 +-- 9 files changed, 50 insertions(+), 90 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b9090ddc..cf0a5575 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -116,7 +116,7 @@ jobs: - name: Build nghttp3 if: matrix.http3 == 'http3' run: | - git clone --depth 1 -b v0.3.0 https://github.com/ngtcp2/nghttp3 + git clone --depth 1 -b v0.4.0 https://github.com/ngtcp2/nghttp3 cd nghttp3 autoreconf -i ./configure --prefix=$PWD/build --enable-lib-only @@ -125,7 +125,7 @@ jobs: - name: Build ngtcp2 if: matrix.http3 == 'http3' run: | - git clone --depth 1 -b v0.3.0 https://github.com/ngtcp2/ngtcp2 + git clone --depth 1 -b v0.4.0 https://github.com/ngtcp2/ngtcp2 cd ngtcp2 autoreconf -i ./configure --prefix=$PWD/build --enable-lib-only PKG_CONFIG_PATH="../openssl/build/lib/pkgconfig" diff --git a/README.rst b/README.rst index 110bd2fd..9a123097 100644 --- a/README.rst +++ b/README.rst @@ -154,8 +154,8 @@ following libraries are required: `_; or `BoringSSL `_ (commit 36a41bf0bf2dd3176f8780e09c03585351f29963) -* `ngtcp2 `_ >= 0.3.0 -* `nghttp3 `_ >= 0.2.0 +* `ngtcp2 `_ >= 0.4.0 +* `nghttp3 `_ >= 0.4.0 Use ``--enable-http3`` configure option to enable HTTP/3 feature for h2load and nghttpx. @@ -366,7 +366,7 @@ Build nghttp3: .. code-block:: text - $ git clone --depth 1 -b v0.2.0 https://github.com/ngtcp2/nghttp3 + $ git clone --depth 1 -b v0.4.0 https://github.com/ngtcp2/nghttp3 $ cd nghttp3 $ autoreconf -i $ ./configure --prefix=$PWD/build --enable-lib-only @@ -378,7 +378,7 @@ Build ngtcp2: .. code-block:: text - $ git clone --depth 1 -b v0.3.0 https://github.com/ngtcp2/ngtcp2 + $ git clone --depth 1 -b v0.4.0 https://github.com/ngtcp2/ngtcp2 $ cd ngtcp2 $ autoreconf -i $ ./configure --prefix=$PWD/build --enable-lib-only \ diff --git a/configure.ac b/configure.ac index e387f728..45eedbaf 100644 --- a/configure.ac +++ b/configure.ac @@ -540,7 +540,7 @@ fi # ngtcp2 (for src) have_libngtcp2=no if test "x${request_libngtcp2}" != "xno"; then - PKG_CHECK_MODULES([LIBNGTCP2], [libngtcp2 >= 0.3.0], [have_libngtcp2=yes], + PKG_CHECK_MODULES([LIBNGTCP2], [libngtcp2 >= 0.4.0], [have_libngtcp2=yes], [have_libngtcp2=no]) if test "x${have_libngtcp2}" = "xno"; then AC_MSG_NOTICE($LIBNGTCP2_PKG_ERRORS) @@ -557,7 +557,7 @@ have_libngtcp2_crypto_openssl=no if test "x${have_ssl_is_quic}" = "xyes" && test "x${request_libngtcp2}" != "xno"; then PKG_CHECK_MODULES([LIBNGTCP2_CRYPTO_OPENSSL], - [libngtcp2_crypto_openssl >= 0.3.0], + [libngtcp2_crypto_openssl >= 0.4.0], [have_libngtcp2_crypto_openssl=yes], [have_libngtcp2_crypto_openssl=no]) if test "x${have_libngtcp2_crypto_openssl}" = "xno"; then @@ -599,7 +599,7 @@ fi # nghttp3 (for src) have_libnghttp3=no if test "x${request_libnghttp3}" != "xno"; then - PKG_CHECK_MODULES([LIBNGHTTP3], [libnghttp3 >= 0.2.0], [have_libnghttp3=yes], + PKG_CHECK_MODULES([LIBNGHTTP3], [libnghttp3 >= 0.4.0], [have_libnghttp3=yes], [have_libnghttp3=no]) if test "x${have_libnghttp3}" = "xno"; then AC_MSG_NOTICE($LIBNGHTTP3_PKG_ERRORS) diff --git a/src/h2load_quic.cc b/src/h2load_quic.cc index e4f31e1d..4e234e8b 100644 --- a/src/h2load_quic.cc +++ b/src/h2load_quic.cc @@ -786,7 +786,7 @@ int Client::write_quic() { if (pktcnt == 0) { gso_size = nwrite; - } else if (static_cast(nwrite) > gso_size) { + } else if (static_cast(nwrite) != gso_size) { auto data = quic.tx.data.get(); auto datalen = bufpos - quic.tx.data.get() - nwrite; rv = write_udp(ps.path.remote.addr, ps.path.remote.addrlen, data, datalen, @@ -808,7 +808,7 @@ int Client::write_quic() { } // Assume that the path does not change. - if (++pktcnt == max_pktcnt || static_cast(nwrite) < gso_size) { + if (++pktcnt == max_pktcnt) { auto data = quic.tx.data.get(); auto datalen = bufpos - quic.tx.data.get(); rv = write_udp(ps.path.remote.addr, ps.path.remote.addrlen, data, datalen, diff --git a/src/shrpx_http3_upstream.cc b/src/shrpx_http3_upstream.cc index 0caabb4a..8ebecbf5 100644 --- a/src/shrpx_http3_upstream.cc +++ b/src/shrpx_http3_upstream.cc @@ -49,22 +49,6 @@ namespace shrpx { -namespace { -void idle_timeoutcb(struct ev_loop *loop, ev_timer *w, int revents) { - auto upstream = static_cast(w->data); - - if (LOG_ENABLED(INFO)) { - ULOG(INFO, upstream) << "QUIC idle timeout"; - } - - upstream->idle_close(); - - auto handler = upstream->get_client_handler(); - - delete handler; -} -} // namespace - namespace { void timeoutcb(struct ev_loop *loop, ev_timer *w, int revents) { auto upstream = static_cast(w->data); @@ -125,7 +109,6 @@ Http3Upstream::Http3Upstream(ClientHandler *handler) httpconn_{nullptr}, downstream_queue_{downstream_queue_size(handler->get_worker()), !get_config()->http2_proxy}, - idle_close_{false}, retry_close_{false}, tx_{ .data = std::unique_ptr(new uint8_t[64_k]), @@ -135,13 +118,6 @@ Http3Upstream::Http3Upstream(ClientHandler *handler) ngtcp2_connection_close_error_default(&last_error_); - auto config = get_config(); - auto &quicconf = config->quic; - - ev_timer_init(&idle_timer_, idle_timeoutcb, 0., - quicconf.upstream.timeout.idle); - idle_timer_.data = this; - ev_timer_init(&shutdown_timer_, shutdown_timeout_cb, 0., 0.); shutdown_timer_.data = this; @@ -155,7 +131,6 @@ Http3Upstream::~Http3Upstream() { ev_prepare_stop(loop, &prep_); ev_timer_stop(loop, &shutdown_timer_); - ev_timer_stop(loop, &idle_timer_); ev_timer_stop(loop, &timer_); nghttp3_conn_del(httpconn_); @@ -862,14 +837,11 @@ int Http3Upstream::write_streams() { prev_pi, data, datalen, gso_size); ngtcp2_conn_update_pkt_tx_time(conn_, ts); - reset_idle_timer(); signal_write_upstream_addr(faddr); return 0; } - - reset_idle_timer(); } ngtcp2_conn_update_pkt_tx_time(conn_, ts); @@ -888,7 +860,7 @@ int Http3Upstream::write_streams() { gso_size = nwrite; } else if (!ngtcp2_path_eq(&prev_ps.path, &ps.path) || prev_pi.ecn != pi.ecn || - static_cast(nwrite) > gso_size) { + static_cast(nwrite) != gso_size) { auto faddr = static_cast(prev_ps.path.user_data); auto data = tx_.data.get(); auto datalen = bufpos - data - nwrite; @@ -926,12 +898,11 @@ int Http3Upstream::write_streams() { } ngtcp2_conn_update_pkt_tx_time(conn_, ts); - reset_idle_timer(); return 0; } - if (++pktcnt == max_pktcnt || static_cast(nwrite) < gso_size) { + if (++pktcnt == max_pktcnt) { auto faddr = static_cast(ps.path.user_data); auto data = tx_.data.get(); auto datalen = bufpos - data; @@ -945,7 +916,6 @@ int Http3Upstream::write_streams() { } ngtcp2_conn_update_pkt_tx_time(conn_, ts); - reset_idle_timer(); signal_write_upstream_addr(faddr); @@ -964,7 +934,6 @@ int Http3Upstream::write_streams() { 0); ngtcp2_conn_update_pkt_tx_time(conn_, ts); - reset_idle_timer(); signal_write_upstream_addr(faddr); @@ -973,7 +942,6 @@ int Http3Upstream::write_streams() { if (++pktcnt == max_pktcnt) { ngtcp2_conn_update_pkt_tx_time(conn_, ts); - reset_idle_timer(); signal_write_upstream_addr(faddr); @@ -1508,7 +1476,9 @@ void Http3Upstream::on_handler_delete() { quic_conn_handler->remove_connection_id(cid); } - if (idle_close_ || retry_close_) { + if (retry_close_ || + last_error_.type == + NGTCP2_CONNECTION_CLOSE_ERROR_CODE_TYPE_TRANSPORT_IDLE_CLOSE) { return; } @@ -1839,8 +1809,6 @@ int Http3Upstream::on_read(const UpstreamAddr *faddr, return handle_error(); } - reset_idle_timer(); - return 0; } @@ -1937,7 +1905,8 @@ void Http3Upstream::signal_write_upstream_addr(const UpstreamAddr *faddr) { } int Http3Upstream::handle_error() { - if (ngtcp2_conn_is_in_closing_period(conn_)) { + if (ngtcp2_conn_is_in_closing_period(conn_) || + ngtcp2_conn_is_in_draining_period(conn_)) { return -1; } @@ -2020,7 +1989,11 @@ int Http3Upstream::handle_expiry() { rv = ngtcp2_conn_handle_expiry(conn_, ts); if (rv != 0) { - ULOG(ERROR, this) << "ngtcp2_conn_handle_expiry: " << ngtcp2_strerror(rv); + if (rv == NGTCP2_ERR_IDLE_CLOSE) { + ULOG(INFO, this) << "Idle connection timeout"; + } else { + ULOG(ERROR, this) << "ngtcp2_conn_handle_expiry: " << ngtcp2_strerror(rv); + } ngtcp2_connection_close_error_set_transport_error_liberr(&last_error_, rv, nullptr, 0); return handle_error(); @@ -2029,26 +2002,19 @@ int Http3Upstream::handle_expiry() { return 0; } -void Http3Upstream::reset_idle_timer() { - auto ts = quic_timestamp(); - auto idle_ts = ngtcp2_conn_get_idle_expiry(conn_); - - idle_timer_.repeat = - idle_ts > ts ? static_cast(idle_ts - ts) / NGTCP2_SECONDS - : 1e-9; - - ev_timer_again(handler_->get_loop(), &idle_timer_); -} - void Http3Upstream::reset_timer() { auto ts = quic_timestamp(); auto expiry_ts = ngtcp2_conn_get_expiry(conn_); + auto loop = handler_->get_loop(); - timer_.repeat = expiry_ts > ts - ? static_cast(expiry_ts - ts) / NGTCP2_SECONDS - : 1e-9; + if (expiry_ts <= ts) { + ev_feed_event(loop, &timer_, EV_TIMER); + return; + } - ev_timer_again(handler_->get_loop(), &timer_); + timer_.repeat = static_cast(expiry_ts - ts) / NGTCP2_SECONDS; + + ev_timer_again(loop, &timer_); } namespace { @@ -2922,8 +2888,6 @@ int Http3Upstream::submit_goaway() { return 0; } -void Http3Upstream::idle_close() { idle_close_ = true; } - int Http3Upstream::open_qlog_file(const StringRef &dir, const ngtcp2_cid &scid) const { std::array buf; diff --git a/src/shrpx_http3_upstream.h b/src/shrpx_http3_upstream.h index 89a47538..4b190d6e 100644 --- a/src/shrpx_http3_upstream.h +++ b/src/shrpx_http3_upstream.h @@ -110,7 +110,6 @@ public: int handle_error(); int handle_expiry(); - void reset_idle_timer(); void reset_timer(); int setup_httpconn(); @@ -148,7 +147,6 @@ public: int check_shutdown(); int start_graceful_shutdown(); int submit_goaway(); - void idle_close(); int send_packet(const UpstreamAddr *faddr, const sockaddr *remote_sa, size_t remote_salen, const sockaddr *local_sa, size_t local_salen, const ngtcp2_pkt_info &pi, @@ -167,7 +165,6 @@ public: private: ClientHandler *handler_; ev_timer timer_; - ev_timer idle_timer_; ev_timer shutdown_timer_; ev_prepare prep_; int qlog_fd_; @@ -177,7 +174,6 @@ private: uint8_t tls_alert_; nghttp3_conn *httpconn_; DownstreamQueue downstream_queue_; - bool idle_close_; bool retry_close_; std::vector conn_close_; diff --git a/src/shrpx_quic.cc b/src/shrpx_quic.cc index fcb2cf51..d49c1744 100644 --- a/src/shrpx_quic.cc +++ b/src/shrpx_quic.cc @@ -270,16 +270,16 @@ int generate_quic_stateless_reset_token(uint8_t *token, const ngtcp2_cid &cid, return 0; } -int generate_retry_token(uint8_t *token, size_t &tokenlen, const sockaddr *sa, - socklen_t salen, const ngtcp2_cid &retry_scid, - const ngtcp2_cid &odcid, const uint8_t *secret, - size_t secretlen) { +int generate_retry_token(uint8_t *token, size_t &tokenlen, uint32_t version, + const sockaddr *sa, socklen_t salen, + const ngtcp2_cid &retry_scid, const ngtcp2_cid &odcid, + const uint8_t *secret, size_t secretlen) { auto t = std::chrono::duration_cast( std::chrono::system_clock::now().time_since_epoch()) .count(); auto stokenlen = ngtcp2_crypto_generate_retry_token( - token, secret, secretlen, sa, salen, &retry_scid, &odcid, t); + token, secret, secretlen, version, sa, salen, &retry_scid, &odcid, t); if (stokenlen < 0) { return -1; } @@ -290,16 +290,16 @@ int generate_retry_token(uint8_t *token, size_t &tokenlen, const sockaddr *sa, } int verify_retry_token(ngtcp2_cid &odcid, const uint8_t *token, size_t tokenlen, - const ngtcp2_cid &dcid, const sockaddr *sa, - socklen_t salen, const uint8_t *secret, - size_t secretlen) { + uint32_t version, const ngtcp2_cid &dcid, + const sockaddr *sa, socklen_t salen, + const uint8_t *secret, size_t secretlen) { auto t = std::chrono::duration_cast( std::chrono::system_clock::now().time_since_epoch()) .count(); if (ngtcp2_crypto_verify_retry_token(&odcid, token, tokenlen, secret, - secretlen, sa, salen, &dcid, + secretlen, version, sa, salen, &dcid, 10 * NGTCP2_SECONDS, t) != 0) { return -1; } diff --git a/src/shrpx_quic.h b/src/shrpx_quic.h index cf1e0c1b..a1533a60 100644 --- a/src/shrpx_quic.h +++ b/src/shrpx_quic.h @@ -107,15 +107,15 @@ int generate_quic_stateless_reset_token(uint8_t *token, const ngtcp2_cid &cid, const uint8_t *secret, size_t secretlen); -int generate_retry_token(uint8_t *token, size_t &tokenlen, const sockaddr *sa, - socklen_t salen, const ngtcp2_cid &retry_scid, - const ngtcp2_cid &odcid, const uint8_t *secret, - size_t secretlen); +int generate_retry_token(uint8_t *token, size_t &tokenlen, uint32_t version, + const sockaddr *sa, socklen_t salen, + const ngtcp2_cid &retry_scid, const ngtcp2_cid &odcid, + const uint8_t *secret, size_t secretlen); int verify_retry_token(ngtcp2_cid &odcid, const uint8_t *token, size_t tokenlen, - const ngtcp2_cid &dcid, const sockaddr *sa, - socklen_t salen, const uint8_t *secret, - size_t secretlen); + uint32_t version, const ngtcp2_cid &dcid, + const sockaddr *sa, socklen_t salen, + const uint8_t *secret, size_t secretlen); int generate_token(uint8_t *token, size_t &tokenlen, const sockaddr *sa, size_t salen, const uint8_t *secret, size_t secretlen); diff --git a/src/shrpx_quic_connection_handler.cc b/src/shrpx_quic_connection_handler.cc index 7a131b60..a1d78e94 100644 --- a/src/shrpx_quic_connection_handler.cc +++ b/src/shrpx_quic_connection_handler.cc @@ -225,8 +225,8 @@ int QUICConnectionHandler::handle_packet(const UpstreamAddr *faddr, switch (hd.token.base[0]) { case NGTCP2_CRYPTO_TOKEN_MAGIC_RETRY: - if (verify_retry_token(odcid, hd.token.base, hd.token.len, hd.dcid, - &remote_addr.su.sa, remote_addr.len, + if (verify_retry_token(odcid, hd.token.base, hd.token.len, hd.version, + hd.dcid, &remote_addr.su.sa, remote_addr.len, qkm->secret.data(), qkm->secret.size()) != 0) { if (LOG_ENABLED(INFO)) { LOG(INFO) << "Failed to validate Retry token from remote=" @@ -473,7 +473,7 @@ int QUICConnectionHandler::send_retry( ngtcp2_cid_init(&idcid, ini_dcid, ini_dcidlen); ngtcp2_cid_init(&iscid, ini_scid, ini_scidlen); - if (generate_retry_token(token.data(), tokenlen, &remote_addr.su.sa, + if (generate_retry_token(token.data(), tokenlen, version, &remote_addr.su.sa, remote_addr.len, retry_scid, idcid, qkm.secret.data(), qkm.secret.size()) != 0) { return -1;