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
