gfe-relnote: In QUIC v48, support partial write of CRYPTO frames. Protected by existing gfe2_reloadable_flag_quic_enable_version_48. PiperOrigin-RevId: 264373472 Change-Id: I285900280e459bbd70e519ec1e78dc2fa518e4c4
diff --git a/quic/core/congestion_control/bandwidth_sampler.cc b/quic/core/congestion_control/bandwidth_sampler.cc index 1ee1f9d..0311191 100644 --- a/quic/core/congestion_control/bandwidth_sampler.cc +++ b/quic/core/congestion_control/bandwidth_sampler.cc
@@ -166,7 +166,8 @@ // Exit app-limited phase once a packet that was sent while the connection is // not app-limited is acknowledged. - if (is_app_limited_ && packet_number > end_of_app_limited_phase_) { + if (is_app_limited_ && end_of_app_limited_phase_.IsInitialized() && + packet_number > end_of_app_limited_phase_) { is_app_limited_ = false; }
diff --git a/quic/core/quic_crypto_stream.cc b/quic/core/quic_crypto_stream.cc index 395b490..9d70139 100644 --- a/quic/core/quic_crypto_stream.cc +++ b/quic/core/quic_crypto_stream.cc
@@ -154,6 +154,7 @@ QUIC_BUG << "Empty crypto data being written"; return; } + const bool had_buffered_data = HasBufferedCryptoFrames(); // Append |data| to the send buffer for this encryption level. struct iovec iov(QuicUtils::MakeIovec(data)); QuicStreamSendBuffer* send_buffer = &substreams_[level].send_buffer; @@ -167,16 +168,16 @@ CloseConnectionWithDetails(QUIC_STREAM_LENGTH_OVERFLOW, "Writing too much crypto handshake data"); } + if (had_buffered_data) { + // Do not try to write if there is buffered data. + return; + } EncryptionLevel current_level = session()->connection()->encryption_level(); session()->connection()->SetDefaultEncryptionLevel(level); size_t bytes_consumed = session()->connection()->SendCryptoData(level, data.length(), offset); session()->connection()->SetDefaultEncryptionLevel(current_level); - // Since CRYPTO frames aren't flow controlled, SendCryptoData should have sent - // all data we asked it to send. - DCHECK_EQ(bytes_consumed, data.length()); - send_buffer->OnStreamDataConsumed(bytes_consumed); } @@ -412,6 +413,48 @@ session()->connection()->SetDefaultEncryptionLevel(current_encryption_level); } +void QuicCryptoStream::WriteBufferedCryptoFrames() { + QUIC_BUG_IF(!QuicVersionUsesCryptoFrames( + session()->connection()->transport_version())) + << "Versions less than 47 don't use CRYPTO frames"; + EncryptionLevel current_encryption_level = + session()->connection()->encryption_level(); + for (EncryptionLevel level : + {ENCRYPTION_INITIAL, ENCRYPTION_ZERO_RTT, ENCRYPTION_FORWARD_SECURE}) { + QuicStreamSendBuffer* send_buffer = &substreams_[level].send_buffer; + const size_t data_length = + send_buffer->stream_offset() - send_buffer->stream_bytes_written(); + if (data_length == 0) { + // No buffered data for this encryption level. + continue; + } + session()->connection()->SetDefaultEncryptionLevel(level); + size_t bytes_consumed = session()->connection()->SendCryptoData( + level, data_length, send_buffer->stream_bytes_written()); + send_buffer->OnStreamDataConsumed(bytes_consumed); + if (bytes_consumed < data_length) { + // Connection is write blocked. + break; + } + } + session()->connection()->SetDefaultEncryptionLevel(current_encryption_level); +} + +bool QuicCryptoStream::HasBufferedCryptoFrames() const { + QUIC_BUG_IF(!QuicVersionUsesCryptoFrames( + session()->connection()->transport_version())) + << "Versions less than 47 don't use CRYPTO frames"; + for (EncryptionLevel level : + {ENCRYPTION_INITIAL, ENCRYPTION_ZERO_RTT, ENCRYPTION_FORWARD_SECURE}) { + const QuicStreamSendBuffer& send_buffer = substreams_[level].send_buffer; + DCHECK_GE(send_buffer.stream_offset(), send_buffer.stream_bytes_written()); + if (send_buffer.stream_offset() > send_buffer.stream_bytes_written()) { + return true; + } + } + return false; +} + bool QuicCryptoStream::IsFrameOutstanding(EncryptionLevel level, size_t offset, size_t length) const {
diff --git a/quic/core/quic_crypto_stream.h b/quic/core/quic_crypto_stream.h index 1b641b1..01523ee 100644 --- a/quic/core/quic_crypto_stream.h +++ b/quic/core/quic_crypto_stream.h
@@ -130,6 +130,12 @@ // encryption level, offset, and length in |crypto_frame|. void RetransmitData(QuicCryptoFrame* crypto_frame); + // Called to write buffered crypto frames. + void WriteBufferedCryptoFrames(); + + // Returns true if there is buffered crypto frames. + bool HasBufferedCryptoFrames() const; + // Returns true if any portion of the data at encryption level |level| // starting at |offset| for |length| bytes is outstanding. bool IsFrameOutstanding(EncryptionLevel level,
diff --git a/quic/core/quic_crypto_stream_test.cc b/quic/core/quic_crypto_stream_test.cc index 4117786..9d3bda5 100644 --- a/quic/core/quic_crypto_stream_test.cc +++ b/quic/core/quic_crypto_stream_test.cc
@@ -23,6 +23,7 @@ using testing::InSequence; using testing::Invoke; using testing::InvokeWithoutArgs; +using testing::Return; namespace quic { namespace test { @@ -532,6 +533,43 @@ } } +TEST_F(QuicCryptoStreamTest, WriteBufferedCryptoFrames) { + if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { + return; + } + EXPECT_FALSE(stream_->HasBufferedCryptoFrames()); + InSequence s; + // Send [0, 1350) in ENCRYPTION_INITIAL. + EXPECT_EQ(ENCRYPTION_INITIAL, connection_->encryption_level()); + std::string data(1350, 'a'); + // Only consumed 1000 bytes. + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_INITIAL, 1350, 0)) + .WillOnce(Return(1000)); + stream_->WriteCryptoData(ENCRYPTION_INITIAL, data); + EXPECT_TRUE(stream_->HasBufferedCryptoFrames()); + + // Send [1350, 2700) in ENCRYPTION_ZERO_RTT and verify no write is attempted + // because there is buffered data. + EXPECT_CALL(*connection_, SendCryptoData(_, _, _)).Times(0); + connection_->SetDefaultEncryptionLevel(ENCRYPTION_ZERO_RTT); + stream_->WriteCryptoData(ENCRYPTION_ZERO_RTT, data); + EXPECT_EQ(ENCRYPTION_ZERO_RTT, connection_->encryption_level()); + + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_INITIAL, 350, 1000)) + .WillOnce(Return(350)); + // Partial write of ENCRYPTION_ZERO_RTT data. + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_ZERO_RTT, 1350, 0)) + .WillOnce(Return(1000)); + stream_->WriteBufferedCryptoFrames(); + EXPECT_TRUE(stream_->HasBufferedCryptoFrames()); + EXPECT_EQ(ENCRYPTION_ZERO_RTT, connection_->encryption_level()); + + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_ZERO_RTT, 350, 1000)) + .WillOnce(Return(350)); + stream_->WriteBufferedCryptoFrames(); + EXPECT_FALSE(stream_->HasBufferedCryptoFrames()); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index fd3d489..a335804 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -572,11 +572,24 @@ size_t num_writes = flow_controller_.IsBlocked() ? write_blocked_streams_.NumBlockedSpecialStreams() : write_blocked_streams_.NumBlockedStreams(); - if (num_writes == 0 && !control_frame_manager_.WillingToWrite()) { + if (num_writes == 0 && !control_frame_manager_.WillingToWrite() && + (!QuicVersionUsesCryptoFrames(connection_->transport_version()) || + !GetCryptoStream()->HasBufferedCryptoFrames())) { return; } QuicConnection::ScopedPacketFlusher flusher(connection_); + if (QuicVersionUsesCryptoFrames(connection_->transport_version())) { + QuicCryptoStream* crypto_stream = GetMutableCryptoStream(); + if (crypto_stream->HasBufferedCryptoFrames()) { + crypto_stream->WriteBufferedCryptoFrames(); + } + if (crypto_stream->HasBufferedCryptoFrames()) { + // Cannot finish writing buffered crypto frames, connection is write + // blocked. + return; + } + } if (control_frame_manager_.WillingToWrite()) { control_frame_manager_.OnCanWrite(); } @@ -626,6 +639,10 @@ // 3) If the crypto or headers streams are blocked, or // 4) connection is not flow control blocked and there are write blocked // streams. + if (QuicVersionUsesCryptoFrames(connection_->transport_version()) && + HasPendingHandshake()) { + return true; + } return control_frame_manager_.WillingToWrite() || !streams_with_pending_retransmission_.empty() || write_blocked_streams_.HasWriteBlockedSpecialStream() || @@ -635,9 +652,8 @@ bool QuicSession::HasPendingHandshake() const { if (QuicVersionUsesCryptoFrames(connection_->transport_version())) { - // Writing CRYPTO frames is not subject to flow control, so there can't be - // pending data to write, only pending retransmissions. - return GetCryptoStream()->HasPendingCryptoRetransmission(); + return GetCryptoStream()->HasPendingCryptoRetransmission() || + GetCryptoStream()->HasBufferedCryptoFrames(); } return QuicContainsKey( streams_with_pending_retransmission_,
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 3a6db56..09d2bbb 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -2692,6 +2692,31 @@ EXPECT_TRUE(stream->write_side_closed()); } +TEST_P(QuicSessionTestServer, WriteBufferedCryptoFrames) { + if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) { + return; + } + std::string data(1350, 'a'); + TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream(); + // Only consumed 1000 bytes. + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_INITIAL, 1350, 0)) + .WillOnce(Return(1000)); + crypto_stream->WriteCryptoData(ENCRYPTION_INITIAL, data); + EXPECT_TRUE(session_.HasPendingHandshake()); + EXPECT_TRUE(session_.WillingAndAbleToWrite()); + + EXPECT_CALL(*connection_, SendCryptoData(_, _, _)).Times(0); + crypto_stream->WriteCryptoData(ENCRYPTION_ZERO_RTT, data); + + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_INITIAL, 350, 1000)) + .WillOnce(Return(350)); + EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_ZERO_RTT, 1350, 0)) + .WillOnce(Return(1350)); + session_.OnCanWrite(); + EXPECT_FALSE(session_.HasPendingHandshake()); + EXPECT_FALSE(session_.WillingAndAbleToWrite()); +} + } // namespace } // namespace test } // namespace quic