From 8b06168dbf75d0beda0c70542d1f21d176f63e73 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 14 Mar 2026 09:56:01 +0900 Subject: [PATCH] nghttpx: Rework close-wait packet generation for h3 --- src/shrpx_http3_upstream.cc | 62 +++++++++++----------------- src/shrpx_http3_upstream.h | 4 +- src/shrpx_quic.h | 1 - src/shrpx_quic_connection_handler.cc | 37 ++++++++++------- src/shrpx_quic_connection_handler.h | 6 ++- 5 files changed, 54 insertions(+), 56 deletions(-) diff --git a/src/shrpx_http3_upstream.cc b/src/shrpx_http3_upstream.cc index 05053491..c02bf8b8 100644 --- a/src/shrpx_http3_upstream.cc +++ b/src/shrpx_http3_upstream.cc @@ -1497,12 +1497,6 @@ void Http3Upstream::on_handler_delete() { // If this is not idle close, send CONNECTION_CLOSE. if (!ngtcp2_conn_in_closing_period(conn_) && !ngtcp2_conn_in_draining_period(conn_)) { - ngtcp2_path_storage ps; - ngtcp2_pkt_info pi; - conn_close_.resize(SHRPX_QUIC_CONN_CLOSE_PKTLEN); - - ngtcp2_path_storage_zero(&ps); - ngtcp2_ccerr ccerr; ngtcp2_ccerr_default(&ccerr); @@ -1511,23 +1505,8 @@ void Http3Upstream::on_handler_delete() { ccerr.error_code = NGTCP2_CONNECTION_REFUSED; } - auto nwrite = ngtcp2_conn_write_connection_close( - conn_, &ps.path, &pi, conn_close_.data(), conn_close_.size(), &ccerr, - quic_timestamp()); - if (nwrite < 0) { - if (nwrite != NGTCP2_ERR_INVALID_STATE) { - ULOG(ERROR, this) << "ngtcp2_conn_write_connection_close: " - << ngtcp2_strerror(static_cast(nwrite)); - } - - return; - } - - conn_close_.resize(as_unsigned(nwrite)); - - send_packet(static_cast(ps.path.user_data), - ps.path.remote.addr, ps.path.remote.addrlen, ps.path.local.addr, - ps.path.local.addrlen, pi, conn_close_, conn_close_.size()); + // Ignore return value. We always enter into close-wait. + send_connection_close(ccerr); } auto d = @@ -1535,11 +1514,11 @@ void Http3Upstream::on_handler_delete() { if (LOG_ENABLED(INFO)) { ULOG(INFO, this) << "Enter close-wait period " << d << "s with " - << conn_close_.size() << " bytes sentinel packet"; + << conn_closelen_ << " bytes sentinel packet"; } - auto cw = std::make_unique(worker, std::move(scids), - std::move(conn_close_), d); + auto cw = std::make_unique( + worker, std::move(scids), std::move(conn_close_), conn_closelen_, d); quic_conn_handler->add_close_wait(cw.release()); } @@ -1938,33 +1917,42 @@ int Http3Upstream::handle_error() { return -1; } + return send_connection_close(last_error_); +} + +int Http3Upstream::send_connection_close(const ngtcp2_ccerr &ccerr) { ngtcp2_path_storage ps; ngtcp2_pkt_info pi; ngtcp2_path_storage_zero(&ps); - auto ts = quic_timestamp(); + std::array buf; - conn_close_.resize(SHRPX_QUIC_CONN_CLOSE_PKTLEN); - - auto nwrite = - ngtcp2_conn_write_connection_close(conn_, &ps.path, &pi, conn_close_.data(), - conn_close_.size(), &last_error_, ts); + auto nwrite = ngtcp2_conn_write_connection_close( + conn_, &ps.path, &pi, buf.data(), buf.size(), &ccerr, quic_timestamp()); if (nwrite < 0) { - ULOG(ERROR, this) << "ngtcp2_conn_write_connection_close: " - << ngtcp2_strerror(static_cast(nwrite)); + if (nwrite != NGTCP2_ERR_INVALID_STATE) { + ULOG(ERROR, this) << "ngtcp2_conn_write_connection_close: " + << ngtcp2_strerror(static_cast(nwrite)); + } + return -1; } - conn_close_.resize(static_cast(nwrite)); - if (nwrite == 0) { return -1; } + conn_closelen_ = as_unsigned(nwrite); + conn_close_ = std::make_unique_for_overwrite(conn_closelen_); + + std::ranges::copy_n(std::ranges::begin(buf), as_signed(conn_closelen_), + conn_close_.get()); + send_packet(static_cast(ps.path.user_data), ps.path.remote.addr, ps.path.remote.addrlen, ps.path.local.addr, - ps.path.local.addrlen, pi, conn_close_, conn_close_.size()); + ps.path.local.addrlen, pi, {conn_close_.get(), conn_closelen_}, + conn_closelen_); return -1; } diff --git a/src/shrpx_http3_upstream.h b/src/shrpx_http3_upstream.h index 7dc565d2..40ef6c04 100644 --- a/src/shrpx_http3_upstream.h +++ b/src/shrpx_http3_upstream.h @@ -106,6 +106,7 @@ public: size_t destlen, ngtcp2_tstamp ts); int handle_error(); + int send_connection_close(const ngtcp2_ccerr &ccerr); int handle_expiry(); void reset_timer(); @@ -177,7 +178,8 @@ private: #endif // OPENSSL_3_5_0_API nghttp3_conn *httpconn_; DownstreamQueue downstream_queue_; - std::vector conn_close_; + std::unique_ptr conn_close_; + size_t conn_closelen_{}; struct { bool send_blocked; diff --git a/src/shrpx_quic.h b/src/shrpx_quic.h index 5aa0a761..931382fb 100644 --- a/src/shrpx_quic.h +++ b/src/shrpx_quic.h @@ -89,7 +89,6 @@ inline constexpr size_t SHRPX_QUIC_DECRYPTED_DCIDLEN = inline constexpr size_t SHRPX_QUIC_SCIDLEN = SHRPX_QUIC_CID_WORKER_ID_OFFSET + SHRPX_QUIC_DECRYPTED_DCIDLEN; inline constexpr size_t SHRPX_QUIC_CID_ENCRYPTION_KEYLEN = 16; -inline constexpr size_t SHRPX_QUIC_CONN_CLOSE_PKTLEN = 256; inline constexpr size_t SHRPX_QUIC_STATELESS_RESET_BURST = 100; inline constexpr size_t SHRPX_QUIC_SECRET_RESERVEDLEN = 4; inline constexpr size_t SHRPX_QUIC_SECRETLEN = 32; diff --git a/src/shrpx_quic_connection_handler.cc b/src/shrpx_quic_connection_handler.cc index da8294e1..2b56246b 100644 --- a/src/shrpx_quic_connection_handler.cc +++ b/src/shrpx_quic_connection_handler.cc @@ -507,23 +507,29 @@ int QUICConnectionHandler::send_retry( return -1; } - std::vector buf; - buf.resize(std::min(max_pktlen, static_cast(256))); + std::array buf; + auto buflen = std::min(max_pktlen, buf.size()); - auto nwrite = ngtcp2_crypto_write_retry(buf.data(), buf.size(), version, - &iscid, &retry_scid, &idcid, - token->data(), token->size()); + auto nwrite = + ngtcp2_crypto_write_retry(buf.data(), buflen, version, &iscid, &retry_scid, + &idcid, token->data(), token->size()); if (nwrite < 0) { LOG(ERROR) << "ngtcp2_crypto_write_retry: " << ngtcp2_strerror(static_cast(nwrite)); return -1; } - buf.resize(as_unsigned(nwrite)); + assert(nwrite); + + auto retrylen = as_unsigned(nwrite); + auto retry = std::make_unique_for_overwrite(retrylen); + + std::ranges::copy_n(std::ranges::begin(buf), as_signed(retrylen), + retry.get()); quic_send_packet(faddr, remote_sockaddr, remote_sockaddrlen, local_addr.as_sockaddr(), local_addr.size(), - ngtcp2_pkt_info{}, buf, buf.size()); + ngtcp2_pkt_info{}, {retry.get(), retrylen}, retrylen); if (generate_quic_hashed_connection_id(idcid, remote_addr, local_addr, idcid) != 0) { @@ -534,12 +540,12 @@ int QUICConnectionHandler::send_retry( static_cast(NGTCP2_DEFAULT_INITIAL_RTT * 3) / NGTCP2_SECONDS; if (LOG_ENABLED(INFO)) { - LOG(INFO) << "Enter close-wait period " << d << "s with " << buf.size() + LOG(INFO) << "Enter close-wait period " << d << "s with " << retrylen << " bytes sentinel packet"; } auto cw = std::make_unique(worker_, std::vector{idcid}, - std::move(buf), d); + std::move(retry), retrylen, d); add_close_wait(cw.release()); @@ -728,10 +734,12 @@ static void close_wait_timeoutcb(struct ev_loop *loop, ev_timer *w, } CloseWait::CloseWait(Worker *worker, std::vector scids, - std::vector pkt, ev_tstamp period) + std::unique_ptr pkt, size_t pktlen, + ev_tstamp period) : worker{worker}, scids{std::move(scids)}, pkt{std::move(pkt)}, + pktlen{pktlen}, bytes_recv{0}, bytes_sent{0}, num_pkts_recv{0}, @@ -763,28 +771,27 @@ int CloseWait::handle_packet(const UpstreamAddr *faddr, const Address &local_addr, const ngtcp2_pkt_info &pi, std::span data) { - if (pkt.empty()) { + if (pktlen == 0) { return 0; } ++num_pkts_recv; bytes_recv += data.size(); - if (bytes_sent + pkt.size() > 3 * bytes_recv || - next_pkts_recv > num_pkts_recv) { + if (bytes_sent + pktlen > 3 * bytes_recv || next_pkts_recv > num_pkts_recv) { return 0; } auto rv = quic_send_packet(faddr, remote_addr.as_sockaddr(), remote_addr.size(), local_addr.as_sockaddr(), local_addr.size(), - ngtcp2_pkt_info{}, pkt, pkt.size()); + ngtcp2_pkt_info{}, {pkt.get(), pktlen}, pktlen); if (rv != 0) { return -1; } next_pkts_recv *= 2; - bytes_sent += pkt.size(); + bytes_sent += pktlen; return 0; } diff --git a/src/shrpx_quic_connection_handler.h b/src/shrpx_quic_connection_handler.h index 35afb43b..208e846b 100644 --- a/src/shrpx_quic_connection_handler.h +++ b/src/shrpx_quic_connection_handler.h @@ -51,7 +51,7 @@ class Worker; // closing period). struct CloseWait { CloseWait(Worker *worker, std::vector scids, - std::vector pkt, ev_tstamp period); + std::unique_ptr pkt, size_t pktlen, ev_tstamp period); ~CloseWait(); int handle_packet(const UpstreamAddr *faddr, const Address &remote_addr, @@ -63,7 +63,9 @@ struct CloseWait { std::vector scids; // QUIC packet which is sent in response to the incoming packet. It // might be empty. - std::vector pkt; + std::unique_ptr pkt; + // The length of pkt. + size_t pktlen; // Close-wait (draining or closing period) timer. ev_timer timer; // The number of bytes received during close-wait period.