Subtract QUIC overhead from the bandwidth estimate returned by the WebTransport API. The specification defines the estimate as having overhead excluded, and this should improve our bitrate adjustment in MoQT. PiperOrigin-RevId: 781546844
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc index f0cf2ca..91f8175 100644 --- a/quiche/quic/core/http/quic_spdy_session.cc +++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -609,6 +609,10 @@ // Limit HPACK buffering to 2x header list size limit. h2_deframer_.GetHpackDecoder().set_max_decode_buffer_size_bytes( 2 * max_inbound_header_list_size_); + + if (ShouldNegotiateWebTransport()) { + connection()->sent_packet_manager().EnableOverheadMeasurement(); + } } void QuicSpdySession::FillSettingsFrame() {
diff --git a/quiche/quic/core/quic_generic_session.cc b/quiche/quic/core/quic_generic_session.cc index f142190..f0edb3f 100644 --- a/quiche/quic/core/quic_generic_session.cc +++ b/quiche/quic/core/quic_generic_session.cc
@@ -97,6 +97,11 @@ } } +void QuicGenericSessionBase::Initialize() { + QuicSession::Initialize(); + connection()->sent_packet_manager().EnableOverheadMeasurement(); +} + QuicStream* QuicGenericSessionBase::CreateIncomingStream(QuicStreamId id) { QUIC_DVLOG(1) << "Creating incoming QuicGenricStream " << id; QuicGenericStream* stream = CreateStream(id);
diff --git a/quiche/quic/core/quic_generic_session.h b/quiche/quic/core/quic_generic_session.h index d8c220a..a3e7a0c 100644 --- a/quiche/quic/core/quic_generic_session.h +++ b/quiche/quic/core/quic_generic_session.h
@@ -55,6 +55,7 @@ ~QuicGenericSessionBase(); // QuicSession implementation. + void Initialize() override; std::vector<std::string> GetAlpnsToOffer() const override { return std::vector<std::string>({alpn_}); }
diff --git a/quiche/quic/core/quic_generic_session_test.cc b/quiche/quic/core/quic_generic_session_test.cc index a1b9d97..fcc4ce8 100644 --- a/quiche/quic/core/quic_generic_session_test.cc +++ b/quiche/quic/core/quic_generic_session_test.cc
@@ -7,6 +7,7 @@ #include "quiche/quic/core/quic_generic_session.h" +#include <algorithm> #include <cstddef> #include <cstring> #include <memory> @@ -14,16 +15,21 @@ #include <string> #include <vector> +#include "absl/status/status.h" +#include "absl/strings/str_format.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "quiche/quic/core/crypto/quic_compressed_certs_cache.h" #include "quiche/quic/core/crypto/quic_crypto_client_config.h" #include "quiche/quic/core/crypto/quic_crypto_server_config.h" #include "quiche/quic/core/crypto/quic_random.h" +#include "quiche/quic/core/quic_bandwidth.h" #include "quiche/quic/core/quic_connection.h" #include "quiche/quic/core/quic_constants.h" #include "quiche/quic/core/quic_datagram_queue.h" #include "quiche/quic/core/quic_error_codes.h" #include "quiche/quic/core/quic_stream.h" +#include "quiche/quic/core/quic_time.h" #include "quiche/quic/core/quic_types.h" #include "quiche/quic/core/web_transport_interface.h" #include "quiche/quic/platform/api/quic_test.h" @@ -34,6 +40,8 @@ #include "quiche/quic/test_tools/simulator/test_harness.h" #include "quiche/quic/test_tools/web_transport_test_tools.h" #include "quiche/quic/tools/web_transport_test_visitors.h" +#include "quiche/common/platform/api/quiche_logging.h" +#include "quiche/common/quiche_mem_slice.h" #include "quiche/common/quiche_stream.h" #include "quiche/common/test_tools/quiche_test_utils.h" #include "quiche/web_transport/web_transport.h" @@ -43,6 +51,24 @@ enum ServerType { kDiscardServer, kEchoServer }; +constexpr char kZeroes[8192] = {0}; + +absl::Status SendZeroes(quiche::WriteStream& stream, int num_of_zeroes, + bool fin = false) { + std::vector<quiche::QuicheMemSlice> slices; + slices.reserve(num_of_zeroes / sizeof(kZeroes) + 1); + int remaining = num_of_zeroes; + while (remaining > 0) { + size_t chunk_size = std::min<size_t>(remaining, sizeof(kZeroes)); + slices.push_back( + quiche::QuicheMemSlice(kZeroes, chunk_size, +[](absl::string_view) {})); + remaining -= chunk_size; + } + quiche::StreamWriteOptions options; + options.set_send_fin(fin); + return stream.Writev(absl::MakeSpan(slices), options); +} + using quiche::test::StatusIs; using simulator::Simulator; using testing::_; @@ -508,5 +534,59 @@ EXPECT_EQ(total_received, 128u * 1024u + 2); } +// Disable bandwidth measurement test in non-opt builds since it's slow. +#if defined(NDEBUG) +#define BANDWIDTH_MEASUREMENT_TEST BandwidthMeasurement +#else +#define BANDWIDTH_MEASUREMENT_TEST DISABLED_BandwidthMeasurement +#endif + +// Verifies that the bandwidth estimation in the WebTransport stats API works, +// and excludes framing overhead as expected. +TEST_F(QuicGenericSessionTest, BANDWIDTH_MEASUREMENT_TEST) { + CreateDefaultEndpoints(kDiscardServer); + WireUpEndpoints(); + RunHandshake(); + + // Warm up the connection. + webtransport::Stream* stream = + client_->session()->OpenOutgoingBidirectionalStream(); + ASSERT_TRUE(stream != nullptr); + QUICHE_ASSERT_OK(SendZeroes(*stream, 1024 * 1024, /*fin=*/true)); + bool finished = test_harness_.RunUntilWithDefaultTimeout( + [&]() { return stream->PeekNextReadableRegion().fin_next; }); + ASSERT_TRUE(finished); + + // Send the actual probe. + constexpr QuicByteCount kTestSize = 32 * 1024 * 1024; + stream = client_->session()->OpenOutgoingBidirectionalStream(); + ASSERT_TRUE(stream != nullptr); + QUICHE_ASSERT_OK(SendZeroes(*stream, kTestSize, /*fin=*/true)); + + // Measure the simulated time it takes to finish a transfer. + const QuicTimeDelta timeout = + 2 * simulator::TestHarness::kServerBandwidth.TransferTime(kTestSize); + const QuicTime start = test_harness_.simulator().GetClock()->Now(); + finished = test_harness_.simulator().RunUntilOrTimeout( + [&]() { return stream->PeekNextReadableRegion().fin_next; }, timeout); + const QuicTime end = test_harness_.simulator().GetClock()->Now(); + const QuicBandwidth effective_bandwidth = + QuicBandwidth::FromBytesAndTimeDelta(kTestSize, end - start); + const QuicBandwidth stats_bandwidth = QuicBandwidth::FromBitsPerSecond( + client_->session()->GetSessionStats().estimated_send_rate_bps); + + // Allow up to 1.25% error (server link capacity is 4Mbps). + constexpr QuicBandwidth kMaxError = QuicBandwidth::FromKBitsPerSecond(50); + EXPECT_NEAR(effective_bandwidth.ToBitsPerSecond(), + stats_bandwidth.ToBitsPerSecond(), kMaxError.ToBitsPerSecond()); + QUICHE_LOG(INFO) << "Stats-reported bandwidth estimate: " << stats_bandwidth; + QUICHE_LOG(INFO) << "Application-observed bandwidth: " << effective_bandwidth; + const QuicBandwidth error = stats_bandwidth - effective_bandwidth; + const float ratio = static_cast<float>(error.ToBitsPerSecond()) / + static_cast<float>(effective_bandwidth.ToBitsPerSecond()); + QUICHE_LOG(INFO) << "Error: " << error + << absl::StrFormat(" (%.2f%%)", ratio * 100.0f); +} + } // namespace } // namespace quic::test
diff --git a/quiche/quic/core/quic_sent_packet_manager.cc b/quiche/quic/core/quic_sent_packet_manager.cc index 62e879c..ef8b7a9 100644 --- a/quiche/quic/core/quic_sent_packet_manager.cc +++ b/quiche/quic/core/quic_sent_packet_manager.cc
@@ -72,6 +72,15 @@ // The default number of PTOs to trigger path degrading. static const uint32_t kNumProbeTimeoutsForPathDegradingDelay = 4; +// QUIC overhead returned if there is not enough data to provide an estimate. 5% +// is roughly the number one can get from a simulated unit test. +constexpr float kDefaultOverhead = 0.05f; + +// Minimum number of data bytes sent before the packet manager can provide an +// estimate of the QUIC overhead. +constexpr QuicByteCount kMinBytesForOverheadMeasurement = + kDefaultMaxPacketSize * 20; + } // namespace #define ENDPOINT \ @@ -106,7 +115,8 @@ one_rtt_packet_acked_(false), num_ptos_for_path_degrading_(kNumProbeTimeoutsForPathDegradingDelay), ignore_pings_(false), - ignore_ack_delay_(false) { + ignore_ack_delay_(false), + measure_overhead_(false) { SetSendAlgorithm(congestion_control_type); } @@ -720,6 +730,8 @@ --pending_timer_transmission_count_; } + UpdateOverheadMeasurements(packet); + bool in_flight = has_retransmittable_data == HAS_RETRANSMITTABLE_DATA; if (ignore_pings_ && mutable_packet->retransmittable_frames.size() == 1 && mutable_packet->retransmittable_frames[0].type == PING_FRAME) { @@ -1667,5 +1679,37 @@ ->first; } +float QuicSentPacketManager::GetOverheadEstimate() const { + // `overhead_total_bytes_` check is a defense-in-depth again divide-by-zero. + if (overhead_total_bytes_ < kMinBytesForOverheadMeasurement || + overhead_good_bytes_ < kMinBytesForOverheadMeasurement) { + return kDefaultOverhead; + } + + return 1.0f - static_cast<float>(overhead_good_bytes_) / + static_cast<float>(overhead_total_bytes_); +} + +void QuicSentPacketManager::UpdateOverheadMeasurements( + const SerializedPacket& packet) { + if (!measure_overhead_) { + return; + } + // Ignore packets with the long header. + if (packet.encryption_level != ENCRYPTION_FORWARD_SECURE) { + return; + } + + overhead_total_bytes_ += packet.encrypted_length; + for (const QuicFrame& frame : packet.retransmittable_frames) { + if (frame.type == QuicFrameType::STREAM_FRAME) { + overhead_good_bytes_ += frame.stream_frame.data_length; + } + if (frame.type == QuicFrameType::MESSAGE_FRAME) { + overhead_good_bytes_ += frame.message_frame->message_length; + } + } +} + #undef ENDPOINT // undef for jumbo builds } // namespace quic
diff --git a/quiche/quic/core/quic_sent_packet_manager.h b/quiche/quic/core/quic_sent_packet_manager.h index 4b0dfe6..f17c386 100644 --- a/quiche/quic/core/quic_sent_packet_manager.h +++ b/quiche/quic/core/quic_sent_packet_manager.h
@@ -512,6 +512,14 @@ // kMinUntrustedInitialRoundTripTimeUs if not |trusted|. void SetInitialRtt(QuicTime::Delta rtt, bool trusted); + // Enables QUIC overhead measurement. + void EnableOverheadMeasurement() { measure_overhead_ = true; } + + // Returns an estimate of overhead added by QUIC as a fraction of application + // payload sent to total data sent (total data includes everything inside UDP + // packets sent by QUIC, but excludes UDP headers and above). + float GetOverheadEstimate() const; + private: friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; @@ -623,6 +631,9 @@ void RecordEcnMarkingSent(QuicEcnCodepoint ecn_codepoint, EncryptionLevel level); + // Updates the QUIC overhead measurements if those are enabled. + void UpdateOverheadMeasurements(const SerializedPacket& packet); + // Newly serialized retransmittable packets are added to this map, which // contains owning pointers to any contained frames. If a packet is // retransmitted, this map will contain entries for both the old and the new @@ -738,6 +749,9 @@ // Whether to ignore the ack_delay in received ACKs. bool ignore_ack_delay_; + // Whether to record stats necessary for the QUIC overhead estimation. + bool measure_overhead_; + // The total number of packets sent with ECT(0) or ECT(1) in each packet // number space over the life of the connection. QuicPacketCount ect0_packets_sent_[NUM_PACKET_NUMBER_SPACES] = {0, 0, 0}; @@ -746,6 +760,11 @@ // Most recent ECN codepoint counts received in an ACK frame sent by the peer. QuicEcnCounts peer_ack_ecn_counts_[NUM_PACKET_NUMBER_SPACES]; + // The numerator and denominator used for overhead measurements. Only recorded + // if `measure_overhead_` is true. + QuicByteCount overhead_good_bytes_ = 0; + QuicByteCount overhead_total_bytes_ = 0; + std::optional<QuicTime::Delta> deferred_send_alarm_delay_; // If true, QuicConnection has called EnableECT0() or EnableECT1(). This is
diff --git a/quiche/quic/core/quic_sent_packet_manager_test.cc b/quiche/quic/core/quic_sent_packet_manager_test.cc index 925c41b..48c9b59 100644 --- a/quiche/quic/core/quic_sent_packet_manager_test.cc +++ b/quiche/quic/core/quic_sent_packet_manager_test.cc
@@ -9,6 +9,7 @@ #include <cstdint> #include <memory> #include <optional> +#include <string> #include <utility> #include <vector> @@ -3655,6 +3656,61 @@ } } +static constexpr float kDefaultOverhead = 0.05f; + +TEST_F(QuicSentPacketManagerTest, DefaultOverhead) { + manager_.EnableOverheadMeasurement(); + EXPECT_NEAR(manager_.GetOverheadEstimate(), kDefaultOverhead, 1e-6); +} + +TEST_F(QuicSentPacketManagerTest, OverheadFromStreamFrames) { + manager_.EnableOverheadMeasurement(); + EXPECT_CALL(*send_algorithm_, OnPacketSent).Times(AnyNumber()); + std::string buffer(kDefaultLength / 2, '\0'); + for (int i = 1; i < 1000; ++i) { + SerializedPacket packet(QuicPacketNumber(i), PACKET_4BYTE_PACKET_NUMBER, + nullptr, kDefaultLength, false, false); + packet.encryption_level = ENCRYPTION_FORWARD_SECURE; + packet.retransmittable_frames.push_back( + QuicFrame(QuicStreamFrame(kStreamId, false, 0, buffer))); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA, true, ECN_NOT_ECT); + } + EXPECT_NEAR(manager_.GetOverheadEstimate(), 0.5, 0.01); +} + +TEST_F(QuicSentPacketManagerTest, OverheadFromDatagramFrames) { + manager_.EnableOverheadMeasurement(); + EXPECT_CALL(*send_algorithm_, OnPacketSent).Times(AnyNumber()); + std::string buffer(kDefaultLength / 2, '\0'); + for (int i = 1; i < 1000; ++i) { + SerializedPacket packet(QuicPacketNumber(i), PACKET_4BYTE_PACKET_NUMBER, + nullptr, kDefaultLength, false, false); + packet.encryption_level = ENCRYPTION_FORWARD_SECURE; + packet.retransmittable_frames.push_back(QuicFrame( + new QuicMessageFrame(i, quiche::QuicheMemSlice::Copy(buffer)))); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA, true, ECN_NOT_ECT); + } + EXPECT_NEAR(manager_.GetOverheadEstimate(), 0.5, 0.01); +} + +TEST_F(QuicSentPacketManagerTest, IgnoreNon1RttFrames) { + manager_.EnableOverheadMeasurement(); + EXPECT_CALL(*send_algorithm_, OnPacketSent).Times(AnyNumber()); + std::string buffer(kDefaultLength / 2, '\0'); + for (int i = 1; i < 1000; ++i) { + SerializedPacket packet(QuicPacketNumber(i), PACKET_4BYTE_PACKET_NUMBER, + nullptr, kDefaultLength, false, false); + packet.encryption_level = ENCRYPTION_INITIAL; + packet.retransmittable_frames.push_back( + QuicFrame(QuicStreamFrame(kStreamId, false, 0, buffer))); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA, true, ECN_NOT_ECT); + } + EXPECT_NEAR(manager_.GetOverheadEstimate(), kDefaultOverhead, 1e-6); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/web_transport_stats.cc b/quiche/quic/core/web_transport_stats.cc index 15797b2..302da2e 100644 --- a/quiche/quic/core/web_transport_stats.cc +++ b/quiche/quic/core/web_transport_stats.cc
@@ -27,11 +27,18 @@ result.min_rtt = rtt_stats->min_rtt().ToAbsl(); result.smoothed_rtt = rtt_stats->smoothed_rtt().ToAbsl(); result.rtt_variation = rtt_stats->mean_deviation().ToAbsl(); - result.estimated_send_rate_bps = session.connection() - ->sent_packet_manager() - .BandwidthEstimate() - .ToBitsPerSecond(); result.datagram_stats = WebTransportDatagramStatsForQuicSession(session); + + // "This estimate excludes any framing overhead and represents the rate at + // which an application payload might be sent." + // https://w3c.github.io/webtransport/#web-transport-connection-stats + float adjustment = + 1.0f - session.connection()->sent_packet_manager().GetOverheadEstimate(); + result.estimated_send_rate_bps = adjustment * session.connection() + ->sent_packet_manager() + .BandwidthEstimate() + .ToBitsPerSecond(); + return result; }
diff --git a/quiche/quic/tools/web_transport_test_visitors.h b/quiche/quic/tools/web_transport_test_visitors.h index 685a892..e33eedd 100644 --- a/quiche/quic/tools/web_transport_test_visitors.h +++ b/quiche/quic/tools/web_transport_test_visitors.h
@@ -22,7 +22,8 @@ // Discards any incoming data. class WebTransportDiscardVisitor : public WebTransportStreamVisitor { public: - WebTransportDiscardVisitor(WebTransportStream* stream) : stream_(stream) {} + WebTransportDiscardVisitor(WebTransportStream* stream, bool bidi) + : stream_(stream), bidi_(bidi) {} void OnCanRead() override { std::string buffer; @@ -30,6 +31,10 @@ QUIC_DVLOG(2) << "Read " << result.bytes_read << " bytes from WebTransport stream " << stream_->GetStreamId() << ", fin: " << result.fin; + if (bidi_ && result.fin) { + absl::Status status = quiche::SendFinOnStream(*stream_); + QUICHE_DCHECK(status.ok()) << status; + } } void OnCanWrite() override {} @@ -40,6 +45,7 @@ private: WebTransportStream* stream_; + bool bidi_; }; class DiscardWebTransportSessionVisitor : public WebTransportVisitor { @@ -58,7 +64,8 @@ if (stream == nullptr) { return; } - stream->SetVisitor(std::make_unique<WebTransportDiscardVisitor>(stream)); + stream->SetVisitor( + std::make_unique<WebTransportDiscardVisitor>(stream, /*bidi=*/true)); stream->visitor()->OnCanRead(); } } @@ -70,7 +77,8 @@ if (stream == nullptr) { return; } - stream->SetVisitor(std::make_unique<WebTransportDiscardVisitor>(stream)); + stream->SetVisitor( + std::make_unique<WebTransportDiscardVisitor>(stream, /*bidi=*/false)); stream->visitor()->OnCanRead(); } }