From 3b84147f45a12b81c977b417b10157882d2172c1 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 6 Apr 2022 18:50:31 +0900 Subject: [PATCH] nghttpx, h2load: Fix QUIC performance regression --- src/h2load.h | 11 ++++-- src/h2load_quic.cc | 76 +++++++++++++++++++++++++++---------- src/shrpx_http3_upstream.cc | 32 ++++++++-------- src/shrpx_http3_upstream.h | 5 +-- 4 files changed, 81 insertions(+), 43 deletions(-) diff --git a/src/h2load.h b/src/h2load.h index df0d06a1..36756897 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -345,11 +345,14 @@ struct Client { struct { bool send_blocked; + size_t num_blocked; + size_t num_blocked_sent; struct { Address remote_addr; + const uint8_t *data; size_t datalen; - size_t max_udp_payload_size; - } blocked; + size_t gso_size; + } blocked[2]; std::unique_ptr data; } tx; } quic; @@ -475,8 +478,8 @@ struct Client { int write_quic(); int write_udp(const sockaddr *addr, socklen_t addrlen, const uint8_t *data, size_t datalen, size_t gso_size); - void on_send_blocked(const ngtcp2_addr &remote_addr, size_t datalen, - size_t max_udp_payload_size); + void on_send_blocked(const ngtcp2_addr &remote_addr, const uint8_t *data, + size_t datalen, size_t gso_size); int send_blocked_packet(); void quic_close_connection(); diff --git a/src/h2load_quic.cc b/src/h2load_quic.cc index 8c22f1ce..e4f31e1d 100644 --- a/src/h2load_quic.cc +++ b/src/h2load_quic.cc @@ -465,6 +465,7 @@ int Client::quic_init(const sockaddr *local_addr, socklen_t local_addrlen, } if (config->max_udp_payload_size) { settings.max_udp_payload_size = config->max_udp_payload_size; + settings.no_udp_payload_size_shaping = 1; } ngtcp2_transport_params params; @@ -704,6 +705,7 @@ int Client::write_quic() { #endif // !UDP_SEGMENT uint8_t *bufpos = quic.tx.data.get(); ngtcp2_path_storage ps; + size_t gso_size = 0; ngtcp2_path_storage_zero(&ps); @@ -767,11 +769,12 @@ int Client::write_quic() { if (nwrite == 0) { if (bufpos - quic.tx.data.get()) { + auto data = quic.tx.data.get(); auto datalen = bufpos - quic.tx.data.get(); - rv = write_udp(ps.path.remote.addr, ps.path.remote.addrlen, - quic.tx.data.get(), datalen, max_udp_payload_size); + rv = write_udp(ps.path.remote.addr, ps.path.remote.addrlen, data, + datalen, gso_size); if (rv == 1) { - on_send_blocked(ps.path.remote, datalen, max_udp_payload_size); + on_send_blocked(ps.path.remote, data, datalen, gso_size); signal_write(); return 0; } @@ -781,14 +784,37 @@ int Client::write_quic() { bufpos += nwrite; - // Assume that the path does not change. - if (++pktcnt == max_pktcnt || - static_cast(nwrite) < max_udp_payload_size) { - auto datalen = bufpos - quic.tx.data.get(); - rv = write_udp(ps.path.remote.addr, ps.path.remote.addrlen, - quic.tx.data.get(), datalen, max_udp_payload_size); + if (pktcnt == 0) { + gso_size = nwrite; + } 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, + gso_size); if (rv == 1) { - on_send_blocked(ps.path.remote, datalen, max_udp_payload_size); + on_send_blocked(ps.path.remote, data, datalen, gso_size); + on_send_blocked(ps.path.remote, bufpos - nwrite, nwrite, 0); + } else { + auto data = bufpos - nwrite; + rv = write_udp(ps.path.remote.addr, ps.path.remote.addrlen, data, + nwrite, 0); + if (rv == 1) { + on_send_blocked(ps.path.remote, data, nwrite, 0); + } + } + + signal_write(); + return 0; + } + + // Assume that the path does not change. + if (++pktcnt == max_pktcnt || static_cast(nwrite) < gso_size) { + 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, + gso_size); + if (rv == 1) { + on_send_blocked(ps.path.remote, data, datalen, gso_size); } signal_write(); return 0; @@ -796,19 +822,22 @@ int Client::write_quic() { } } -void Client::on_send_blocked(const ngtcp2_addr &remote_addr, size_t datalen, - size_t max_udp_payload_size) { - assert(!quic.tx.send_blocked); +void Client::on_send_blocked(const ngtcp2_addr &remote_addr, + const uint8_t *data, size_t datalen, + size_t gso_size) { + assert(quic.tx.num_blocked || !quic.tx.send_blocked); + assert(quic.tx.num_blocked < 2); quic.tx.send_blocked = true; - auto &p = quic.tx.blocked; + auto &p = quic.tx.blocked[quic.tx.num_blocked++]; memcpy(&p.remote_addr.su, remote_addr.addr, remote_addr.addrlen); p.remote_addr.len = remote_addr.addrlen; + p.data = data; p.datalen = datalen; - p.max_udp_payload_size = max_udp_payload_size; + p.gso_size = gso_size; } int Client::send_blocked_packet() { @@ -816,17 +845,22 @@ int Client::send_blocked_packet() { assert(quic.tx.send_blocked); - auto &p = quic.tx.blocked; + for (; quic.tx.num_blocked_sent < quic.tx.num_blocked; + ++quic.tx.num_blocked_sent) { + auto &p = quic.tx.blocked[quic.tx.num_blocked_sent]; - rv = write_udp(&p.remote_addr.su.sa, p.remote_addr.len, quic.tx.data.get(), - p.datalen, p.max_udp_payload_size); - if (rv == 1) { - signal_write(); + rv = write_udp(&p.remote_addr.su.sa, p.remote_addr.len, p.data, p.datalen, + p.gso_size); + if (rv == 1) { + signal_write(); - return 0; + return 0; + } } quic.tx.send_blocked = false; + quic.tx.num_blocked = 0; + quic.tx.num_blocked_sent = 0; return 0; } diff --git a/src/shrpx_http3_upstream.cc b/src/shrpx_http3_upstream.cc index a15662b2..0caabb4a 100644 --- a/src/shrpx_http3_upstream.cc +++ b/src/shrpx_http3_upstream.cc @@ -739,6 +739,7 @@ int Http3Upstream::write_streams() { ngtcp2_path_storage ps, prev_ps; size_t pktcnt = 0; int rv; + size_t gso_size = 0; auto ts = quic_timestamp(); ngtcp2_path_storage_zero(&ps); @@ -855,10 +856,10 @@ int Http3Upstream::write_streams() { rv = send_packet(faddr, prev_ps.path.remote.addr, prev_ps.path.remote.addrlen, prev_ps.path.local.addr, prev_ps.path.local.addrlen, prev_pi, data, datalen, - max_udp_payload_size); + gso_size); if (rv == SHRPX_ERR_SEND_BLOCKED) { on_send_blocked(faddr, prev_ps.path.remote, prev_ps.path.local, - prev_pi, data, datalen, max_udp_payload_size); + prev_pi, data, datalen, gso_size); ngtcp2_conn_update_pkt_tx_time(conn_, ts); reset_idle_timer(); @@ -884,8 +885,10 @@ int Http3Upstream::write_streams() { if (pktcnt == 0) { ngtcp2_path_copy(&prev_ps.path, &ps.path); prev_pi = pi; + gso_size = nwrite; } else if (!ngtcp2_path_eq(&prev_ps.path, &ps.path) || - prev_pi.ecn != pi.ecn) { + prev_pi.ecn != pi.ecn || + static_cast(nwrite) > gso_size) { auto faddr = static_cast(prev_ps.path.user_data); auto data = tx_.data.get(); auto datalen = bufpos - data - nwrite; @@ -893,15 +896,15 @@ int Http3Upstream::write_streams() { rv = send_packet(faddr, prev_ps.path.remote.addr, prev_ps.path.remote.addrlen, prev_ps.path.local.addr, prev_ps.path.local.addrlen, prev_pi, data, datalen, - max_udp_payload_size); + gso_size); switch (rv) { case SHRPX_ERR_SEND_BLOCKED: on_send_blocked(faddr, prev_ps.path.remote, prev_ps.path.local, prev_pi, - data, datalen, max_udp_payload_size); + data, datalen, gso_size); on_send_blocked(static_cast(ps.path.user_data), ps.path.remote, ps.path.local, pi, bufpos - nwrite, - nwrite, max_udp_payload_size); + nwrite, 0); signal_write_upstream_addr(faddr); @@ -912,10 +915,10 @@ int Http3Upstream::write_streams() { rv = send_packet(faddr, ps.path.remote.addr, ps.path.remote.addrlen, ps.path.local.addr, ps.path.local.addrlen, pi, data, - nwrite, max_udp_payload_size); + nwrite, 0); if (rv == SHRPX_ERR_SEND_BLOCKED) { on_send_blocked(faddr, ps.path.remote, ps.path.local, pi, data, - nwrite, max_udp_payload_size); + nwrite, 0); } signal_write_upstream_addr(faddr); @@ -928,18 +931,17 @@ int Http3Upstream::write_streams() { return 0; } - if (++pktcnt == max_pktcnt || - static_cast(nwrite) < max_udp_payload_size) { + if (++pktcnt == max_pktcnt || static_cast(nwrite) < gso_size) { auto faddr = static_cast(ps.path.user_data); auto data = tx_.data.get(); auto datalen = bufpos - data; rv = send_packet(faddr, ps.path.remote.addr, ps.path.remote.addrlen, ps.path.local.addr, ps.path.local.addrlen, pi, data, - datalen, max_udp_payload_size); + datalen, gso_size); if (rv == SHRPX_ERR_SEND_BLOCKED) { on_send_blocked(faddr, ps.path.remote, ps.path.local, pi, data, datalen, - max_udp_payload_size); + gso_size); } ngtcp2_conn_update_pkt_tx_time(conn_, ts); @@ -1875,7 +1877,7 @@ void Http3Upstream::on_send_blocked(const UpstreamAddr *faddr, const ngtcp2_addr &local_addr, const ngtcp2_pkt_info &pi, const uint8_t *data, size_t datalen, - size_t max_udp_payload_size) { + size_t gso_size) { assert(tx_.num_blocked || !tx_.send_blocked); assert(tx_.num_blocked < 2); @@ -1892,7 +1894,7 @@ void Http3Upstream::on_send_blocked(const UpstreamAddr *faddr, p.pi = pi; p.data = data; p.datalen = datalen; - p.max_udp_payload_size = max_udp_payload_size; + p.gso_size = gso_size; } int Http3Upstream::send_blocked_packet() { @@ -1905,7 +1907,7 @@ int Http3Upstream::send_blocked_packet() { rv = send_packet(p.faddr, &p.remote_addr.su.sa, p.remote_addr.len, &p.local_addr.su.sa, p.local_addr.len, p.pi, p.data, - p.datalen, p.max_udp_payload_size); + p.datalen, p.gso_size); if (rv == SHRPX_ERR_SEND_BLOCKED) { signal_write_upstream_addr(p.faddr); diff --git a/src/shrpx_http3_upstream.h b/src/shrpx_http3_upstream.h index 253dea5e..89a47538 100644 --- a/src/shrpx_http3_upstream.h +++ b/src/shrpx_http3_upstream.h @@ -160,8 +160,7 @@ public: void on_send_blocked(const UpstreamAddr *faddr, const ngtcp2_addr &remote_addr, const ngtcp2_addr &local_addr, const ngtcp2_pkt_info &pi, - const uint8_t *data, size_t datalen, - size_t max_udp_payload_size); + const uint8_t *data, size_t datalen, size_t gso_size); int send_blocked_packet(); void signal_write_upstream_addr(const UpstreamAddr *faddr); @@ -194,7 +193,7 @@ private: ngtcp2_pkt_info pi; const uint8_t *data; size_t datalen; - size_t max_udp_payload_size; + size_t gso_size; } blocked[2]; std::unique_ptr data; } tx_;