commit 18cdf130f3ab402a16145807512f70a05ebc152c
author dschinazi <dschinazi@google.com> Wed Oct 09 16:08:18 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Oct 09 16:08:50 2019 -0700
tree 440aa9a67886056adb298ece56f83961849d49fa
parent aab6818eb3658653fc48f374b93343e80d91f2b8

Support local vs remote flow control with TLS, and fix initial limits

This CL fixes a bug found during interop testing where we were failing to communicate with clients that do not send initial_max_stream_data_bidi_local. (When sent by clients, this is the transport parameter that governs the server-initiated streams which are not used by HTTP/3.) All of our tests sent the same value for _local and _remote so we did not notice that we had them backwards. This CL fixes the issue and adds tests to prevent regressions. This CL also fixes an issue where the initial limits were incorrectly set for versions that AllowsLowFlowControlLimits().

gfe-relnote: change QUIC flow control transport parameters, protected by disabled TLS flag.
PiperOrigin-RevId: 273843552
Change-Id: Id8bb3a59029ee9442ebf163fd658f38c47024950

quic/core/quic_config.cc

diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc
index 65e2c58..1b529fa 100644
--- a/quic/core/quic_config.cc
+++ b/quic/core/quic_config.cc
@@ -624,6 +624,19 @@
     window_bytes = kMinimumFlowControlSendWindow;
   }
   initial_stream_flow_control_window_bytes_.SetSendValue(window_bytes);
+
+  // If the IETF flow control configs have not been set yet, set them.
+  if (!initial_max_stream_data_bytes_incoming_bidirectional_.HasSendValue()) {
+    initial_max_stream_data_bytes_incoming_bidirectional_.SetSendValue(
+        window_bytes);
+  }
+  if (!initial_max_stream_data_bytes_outgoing_bidirectional_.HasSendValue()) {
+    initial_max_stream_data_bytes_outgoing_bidirectional_.SetSendValue(
+        window_bytes);
+  }
+  if (!initial_max_stream_data_bytes_unidirectional_.HasSendValue()) {
+    initial_max_stream_data_bytes_unidirectional_.SetSendValue(window_bytes);
+  }
 }
 
 uint32_t QuicConfig::GetInitialStreamFlowControlWindowToSend() const {
@@ -640,12 +653,6 @@
 
 void QuicConfig::SetInitialMaxStreamDataBytesIncomingBidirectionalToSend(
     uint32_t window_bytes) {
-  if (window_bytes < kMinimumFlowControlSendWindow) {
-    QUIC_BUG << "Initial stream flow control receive window (" << window_bytes
-             << ") cannot be set lower than minimum ("
-             << kMinimumFlowControlSendWindow << ").";
-    window_bytes = kMinimumFlowControlSendWindow;
-  }
   initial_max_stream_data_bytes_incoming_bidirectional_.SetSendValue(
       window_bytes);
 }
@@ -669,12 +676,6 @@
 
 void QuicConfig::SetInitialMaxStreamDataBytesOutgoingBidirectionalToSend(
     uint32_t window_bytes) {
-  if (window_bytes < kMinimumFlowControlSendWindow) {
-    QUIC_BUG << "Initial stream flow control receive window (" << window_bytes
-             << ") cannot be set lower than minimum ("
-             << kMinimumFlowControlSendWindow << ").";
-    window_bytes = kMinimumFlowControlSendWindow;
-  }
   initial_max_stream_data_bytes_outgoing_bidirectional_.SetSendValue(
       window_bytes);
 }
@@ -698,12 +699,6 @@
 
 void QuicConfig::SetInitialMaxStreamDataBytesUnidirectionalToSend(
     uint32_t window_bytes) {
-  if (window_bytes < kMinimumFlowControlSendWindow) {
-    QUIC_BUG << "Initial stream flow control receive window (" << window_bytes
-             << ") cannot be set lower than minimum ("
-             << kMinimumFlowControlSendWindow << ").";
-    window_bytes = kMinimumFlowControlSendWindow;
-  }
   initial_max_stream_data_bytes_unidirectional_.SetSendValue(window_bytes);
 }
 
@@ -939,10 +934,16 @@
   params->max_packet_size.set_value(kMaxIncomingPacketSize);
   params->initial_max_data.set_value(
       initial_session_flow_control_window_bytes_.GetSendValue());
+  // The max stream data bidirectional transport parameters can be either local
+  // or remote. A stream is local iff it is initiated by the endpoint that sent
+  // the transport parameter (see the Transport Parameter Definitions section of
+  // draft-ietf-quic-transport). In this function we are sending transport
+  // parameters, so a local stream is one we initiated, which means an outgoing
+  // stream.
   params->initial_max_stream_data_bidi_local.set_value(
-      initial_max_stream_data_bytes_incoming_bidirectional_.GetSendValue());
-  params->initial_max_stream_data_bidi_remote.set_value(
       initial_max_stream_data_bytes_outgoing_bidirectional_.GetSendValue());
+  params->initial_max_stream_data_bidi_remote.set_value(
+      initial_max_stream_data_bytes_incoming_bidirectional_.GetSendValue());
   params->initial_max_stream_data_uni.set_value(
       initial_max_stream_data_bytes_unidirectional_.GetSendValue());
   params->initial_max_streams_bidi.set_value(
@@ -1033,6 +1034,12 @@
       std::min<uint64_t>(params.initial_max_streams_uni.value(),
                          std::numeric_limits<uint32_t>::max()));
 
+  // The max stream data bidirectional transport parameters can be either local
+  // or remote. A stream is local iff it is initiated by the endpoint that sent
+  // the transport parameter (see the Transport Parameter Definitions section of
+  // draft-ietf-quic-transport). However in this function we are processing
+  // received transport parameters, so a local stream is one initiated by our
+  // peer, which means an incoming stream.
   initial_max_stream_data_bytes_incoming_bidirectional_.SetReceivedValue(
       std::min<uint64_t>(params.initial_max_stream_data_bidi_local.value(),
                          std::numeric_limits<uint32_t>::max()));